Skip to content

[Dot Shorthands] Analyzer Implementation #59835

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
6 of 8 tasks
Tracked by #57036
kallentu opened this issue Jan 2, 2025 · 2 comments
Open
6 of 8 tasks
Tracked by #57036

[Dot Shorthands] Analyzer Implementation #59835

kallentu opened this issue Jan 2, 2025 · 2 comments
Assignees
Labels
area-dart-model For issues related to conformance to the language spec in the parser, compilers or the CLI analyzer. feature-dot-shorthands Implementation of the dot shorthands feature. P2 A bug or feature request we're likely to work on

Comments

@kallentu
Copy link
Member

kallentu commented Jan 2, 2025

This meta issue tracks all the analyzer implementation work items. Referenced from: https://github.com/dart-lang/sdk/blob/main/pkg/analyzer/doc/process/new_language_feature.md

Estimated work

  • Parser

    • Need changes to identify when we have a shorthand, add all selectors
    • Recovery/error when . followed by a keyword?, some random symbol
    • Expressions can’t start with .
  • AST enhancements (AstBuilder)

    • New AST type to represent the enum shorthand
    • . operator
    • And then the rest of the expression
    • Based on Paul’s prototype we need to represent a non-existent target (which he does with a “MAGIC” SimpleIdentifier)
  • Summary support

    • Summaries record constants so I might need to do work here? Check with Konstantin
  • Resolution

    • ResolverVisitor (type-based resolution)
      • Paul’s prototype used a stack of type schemas (named _enumShorthands). “When the ResolverVisitor enters the enum shorthand context node, it pushes the context onto this stack; when it leaves it, it pops it back off”
      • Work needed, tests needed.
      • == work in the TypeAnalyzer as well.
  • Constant evaluation

    • We allow consts for enum shorthands.
    • Use the new AST.
    • Tests
  • Index and search

    • Not entirely sure?
    • We can refer to a class without naming it. Might need updating?
  • Warnings (annotation-based, unused*, strict-mode-based, a few others)

    • ErrorVerifier (report other errors and warnings)
      • Error when we are missing the context type
      • Error with asymmetric == and != (These errors might just belong in the ResolverVisitor?)
      • From @FMorschel : Check Ambiguous Import to make sure it triggers correctly for this case too.
  • NodeLintRegistry

    • If we make a new AST node representing enum shorthands, should add it to this.
@kallentu kallentu added dart-model-analyzer-spec Issues with the analyzer's implementation of the language spec legacy-area-analyzer Use area-devexp instead. feature-dot-shorthands Implementation of the dot shorthands feature. P2 A bug or feature request we're likely to work on labels Jan 2, 2025
@kallentu kallentu self-assigned this Jan 2, 2025
@kallentu kallentu changed the title [Enum Shorthands] Analyzer Implementation [Dot Shorthands] Analyzer Implementation Feb 1, 2025
copybara-service bot pushed a commit that referenced this issue Feb 18, 2025
Doing some initial work for parsing dot shorthands.

The listeners in the CFE and analyzer will report an extra error if the experiment is not turned on. The compilation should still fail and produce errors, but it won't crash.

If you turn on the experiment, the parsing will crash, but I'd like to get this in so I can modularly work on the CFE and analyzer separately.

Bug: #59758, #59835
Change-Id: I262b0bd5cffc8e5e04ac79c76454b6e355779ade
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/409540
Reviewed-by: Jens Johansen <[email protected]>
Reviewed-by: Paul Berry <[email protected]>
Commit-Queue: Kallen Tu <[email protected]>
Reviewed-by: Chloe Stefantsova <[email protected]>
@bwilkerson bwilkerson added area-dart-model For issues related to conformance to the language spec in the parser, compilers or the CLI analyzer. and removed dart-model-analyzer-spec Issues with the analyzer's implementation of the language spec legacy-area-analyzer Use area-devexp instead. labels Feb 21, 2025
@FMorschel
Copy link
Contributor

Not sure if here is the place but we could add a lint to tell the user to use this feature where possible prefer_dot_shorthands or something, with a quick-fix (server) to remove the initial part.

Also, one question to have in mind here would be: what about prefixed imports that expose that element? Will this feature work with it?

Another error to look at would be Ambiguous Import to make sure it triggers correctly for this case too.


If these points should be somewhere else please say so and I can move them. If I missed anything please let me know!

@kallentu
Copy link
Member Author

Not sure if here is the place but we could add a lint to tell the user to use this feature where possible prefer_dot_shorthands or something, with a quick-fix (server) to remove the initial part.

We are planning to add that, yes! I've added it onto the analysis server work issue (under quickfixes for now) #59836

Also, one question to have in mind here would be: what about prefixed imports that expose that element? Will this feature work with it?

Yep, prefixed imports with shorthands should work as well. There are some language tests covering this behaviour.

Another error to look at would be Ambiguous Import to make sure it triggers correctly for this case too.

Good point. Added this, thank you!

copybara-service bot pushed a commit that referenced this issue Mar 28, 2025
…andPropertyAccess.

This CL adds new nodes for the dot shorthands feature. These new nodes will be used, alongside a context type, to resolve to a method/constructor invocation or a static field/getter or tearoff.

Design decision for this CL made here: https://docs.google.com/document/d/1rJuwytXFyG9Ir9key146_hAa7uhEXk4Otqd_3RpSg8A/edit?usp=sharing&resourcekey=0-hRydkMfiTwDEwsX3fN4taQ

Bug: #59835
Change-Id: Iab78bd9e55e488656d1138b1d5d6fc5c9ed64bde
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/418201
Reviewed-by: Brian Wilkerson <[email protected]>
Reviewed-by: Paul Berry <[email protected]>
Commit-Queue: Kallen Tu <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 2, 2025
… SimpleIdentifier.

This CL changes `propertyName` to be a `SimpleIdentifier` instead of `Token` because we need to resolve and store the resolved element in the `SimpleIdentifier`.

Note that the corresponding class `PropertyAccess` represents its
`propertyName` as a `SimpleIdentifier` for the same reason.

Bug: #59835
Change-Id: I934f66f914b061b8593399c79a0f7b590b84cc12
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/419584
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Kallen Tu <[email protected]>
Reviewed-by: Paul Berry <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 9, 2025
…ndle dot shorthands.

This is the base preliminary work for resolving dot shorthand invocations. I'll be calling `resolveDotShorthand` from the `Resolver` in a future CL.

Dot shorthand invocation resolution is very similar to normal MethodInvocation resolution, with a few exceptions (like no `target` expression), so it made sense to re-use the logic we had in the `MethodInvocationResolver`.

I refactored `_rewriteAsFunctionExpressionInvocation` heavily to support both dot shorthands and method invocations, which involved pulling out all of the members into parameters.

Most `_report<some error>` methods were refactored to support dot shorthands as well. Half of them called `_setInvalidTypeResolution` and that ended up just being invoked at the call site rather than in the report helper itself.

Dot shorthands has a very basic `DotShorthandInvocationInferrer` in this CL. None of the overrides for a `MethodInvocationInferrer` seemed to apply since we don't have a target.

No tests yet. Tests will be added once I make the next CL that calls this `resolveDotShorthand` entry point.

Bug: #59835
Change-Id: I8ebeb772c30c88b85ae68d805e062accb1d6d17d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/421560
Commit-Queue: Kallen Tu <[email protected]>
Reviewed-by: Chloe Stefantsova <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 11, 2025
…andle method invocations.

With this CL, we build the `DotShorthandInvocation` AST as soon as we find a `MethodInvocation` when parsing a dot shorthand head. The context is saved as we encounter ASTs with the `isDotShorthand` flag enabled (which currently is just the invocation and property get head, more will be added later). We pop the context after resolving the dot shorthand head and using the context for resolution.

The resolution of dot shorthand invocations is handled by the `MethodInvocationInferrer` where most of the logic was added in https://dart-review.googlesource.com/c/sdk/+/421560. In this CL, we're calling the `resolveDotShorthand` entry point for the very basic resolving. This is the groundwork that we'll build off of for constructor invocations, extension type invocations and other cases.

Some co19 tests are crashing, as per expected, but I added resolution tests for the .shorthand invocations and ast building tests.

Bug: #59835
Change-Id: I62bcb5fecb3c4fdfcf29d6c079d5d77547c4a21a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/419780
Reviewed-by: Chloe Stefantsova <[email protected]>
Reviewed-by: Paul Berry <[email protected]>
Commit-Queue: Kallen Tu <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 15, 2025
Allows the resolution of `.new` constructor references and static property accesses, using the existing `PropertyElementResolver` resolution logic.

Testing includes an AST builder test, and the `DotShorthandPropertyAccess` resolution tests. Some property access language tests are now passing.

Bug: #59835
Change-Id: Idb576ef7fa0866bc4b2d155bbf867886ae2b4df6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/421964
Commit-Queue: Kallen Tu <[email protected]>
Reviewed-by: Chloe Stefantsova <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 15, 2025
Finds the namespace of `S` for a context type of `FutureOr<S>` and uses that to resolve the shorthand.

Unit tests written and language tests cover this behavior.

Bug: #59835
Change-Id: Iec7bfeaef2c0ab20ee2031c4dc0b3ef0b9e1fc66
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/422363
Commit-Queue: Kallen Tu <[email protected]>
Reviewed-by: Chloe Stefantsova <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 16, 2025
…xins.

The code for https://dart-review.googlesource.com/c/sdk/+/419780 and https://dart-review.googlesource.com/c/sdk/+/421964 luckily also added the right behavior for extension types and mixins (for the most part).

So this CL adds a couple of analyzer tests for the two.

Bug: #59835
Change-Id: Ia177d5ab9811fb6c742ba649628e435295a13441
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/422543
Reviewed-by: Chloe Stefantsova <[email protected]>
Commit-Queue: Kallen Tu <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 17, 2025
This CL adds the ability to evaluate constant DotShorthandPropertyAccesses.

Added a test to test const enums, class static properties, extension types. Language tests are also passing.

Bug: #59835
Change-Id: I01432b7aa17d08d4a1dc005f134b7c17cd223d75
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/422844
Reviewed-by: Paul Berry <[email protected]>
Commit-Queue: Kallen Tu <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 18, 2025
…and resolve constructors.

When we resolve a DotShorthandInvocation, it can either be a method invocation or a constructor invocation. This CL adds a `DotShorthandConstructorInvocation` node to represent the shorthand equivalent of an `InstanceCreationExpression` (without the extra bulk of needing a synthetic `NamedType`), and adds the resolution logic.

Test added in the form of unit test and existing language tests.

Bug: #59835
Change-Id: I6966ce549f2b079efc0b002fba41bc31a90871c1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/422544
Reviewed-by: Paul Berry <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Kallen Tu <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 18, 2025
Testing - Language tests stop crashing, some passing (the failures are from other unimplemented things). Added a unit test for a top level dot shorthand.

Bug: #59835
Change-Id: I885cfc5c33b0b206aeae73438c765f73f4c9175f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/422845
Commit-Queue: Kallen Tu <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 22, 2025
When there's exactly a dot shorthand of the RHS of `==`, we use the namespace of the LHS to resolve the shorthand on the RHS.

There's existing CFE work done for the `case ==` case in the type analyzer, so we just have to make sure `isDotShorthand` is properly set. This will take care of the relational pattern `==` behaviour.

Tested using language tests and resolution unit tests.

Bug: #59835
Change-Id: I3823641d1872bca68e344bc323209b58fb5098db
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/422941
Reviewed-by: Paul Berry <[email protected]>
Commit-Queue: Kallen Tu <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 23, 2025
Missed one refactor in the binary_expression_resolver.

Following up to comments on:
https://dart-review.googlesource.com/c/sdk/+/422941/12/pkg/analyzer/lib/src/dart/resolver/binary_expression_resolver.dart#111

Bug: #59835
Change-Id: Ia210ddc5141222e13bda163681e1ae2b05dc55bb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/423960
Commit-Queue: Kallen Tu <[email protected]>
Reviewed-by: Paul Berry <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 23, 2025
Follow up to comment: https://dart-review.googlesource.com/c/sdk/+/422544/comment/36fb49ba_f9f3c908/

Invocations are rewritten to `FunctionExpressionInvocation`s with a `DotShorthandPropertyAccess` as its `function`. Any actual static method invocations are kept as is.

Added some special handling of dot shorthands in the `method_invocation_resolver` to rewrite an invocation to a property access with a function expression invocation as a parent. The static type is set for later usage in the `function_expression_invocation_resolver`.

`language/dot_shorthands/simple/call_test` passing and unit tests added.

Bug: #59835
Change-Id: I42649cefab54f80dc96357a03e208ad374183d09
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/423211
Reviewed-by: Paul Berry <[email protected]>
Reviewed-by: Chloe Stefantsova <[email protected]>
Commit-Queue: Kallen Tu <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 24, 2025
…ss and MethodInvocation.

For supporting selector chains, I've added the DotShorthandMixin to PropertyAccess and MethodInvocation, and marked the `isDotShorthand` flag when it's the outer most selector whose context type we care about saving.

Multiple language tests passing + added some unit tests for some simple selector chain cases.

Bug: #59835
Change-Id: I7571eb22b4213be895a756b89ecf74f65c4cced5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/423382
Commit-Queue: Kallen Tu <[email protected]>
Reviewed-by: Paul Berry <[email protected]>
Reviewed-by: Chloe Stefantsova <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 28, 2025
…pertyAccesses.

When we test for potentially-constant constants (ie. in asserts), we need to also test whether the identifier of a `DotShorthandPropertyAccess` is constant or not.

This CL adds one bit of logic and a few unit tests.
The language test `language/dot_shorthands/equality/equality_test` also covers this behavior and is passing.

Bug: #59835
Change-Id: I4e3daeb008c7dbadbb71d988012bf2e521df2e84
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/424661
Reviewed-by: Chloe Stefantsova <[email protected]>
Commit-Queue: Kallen Tu <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 29, 2025
`IndexExpressionImpl`s now have the `DotShorthandMixin` applied. We save the context at the point of resolving an index expression to be used for resolving a dot shorthand head later.

Unit tests added. There's multiple co19 tests that will start passing, but rely on https://dart-review.googlesource.com/c/sdk/+/425181.

Bug: #59835
Change-Id: I08076bb437bad1955d353a24fc119466b7dfad61
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/425360
Reviewed-by: Paul Berry <[email protected]>
Commit-Queue: Kallen Tu <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 30, 2025
Added `isConst` and `constKeyword` to `DotShorthandConstructorInvocation`.

`DotShorthandInvocation`s are rewritten to `DotShorthandConstructorInvocation` immediately once we parse the `const` keyword. If we don't resolve to a constructor, we'll produce the `CONST_WITH_UNDEFINED_CONSTRUCTOR` error.

Summary changes in a different CL that follows this one: https://dart-review.googlesource.com/c/sdk/+/424340

co19 tests passing and unit tests added.

Bug: #59835
Change-Id: Ic12cd98cf08009187b8bf32afb62aa026cb7c8c7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/424240
Reviewed-by: Paul Berry <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Kallen Tu <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 30, 2025
…property access.

Added `DOT_SHORTHAND_MISSING_CONTEXT` error code for when we don't find a valid context type to resolve the dot shorthand head with. `DOT_SHORTHAND_UNDEFINED_GETTER` is used for being unable to find a static getter or field with a certain context type and name.

Unit tests added and a few language error + co19 tests passing (eg `simple_identifier_extends_error_test`).

Bug: #59835
Change-Id: Ic515b57dd0f4243b049e02bd58b694a500d57975
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/425181
Reviewed-by: Brian Wilkerson <[email protected]>
Reviewed-by: Paul Berry <[email protected]>
Commit-Queue: Kallen Tu <[email protected]>
copybara-service bot pushed a commit that referenced this issue Apr 30, 2025
…n summaries.

Updates the summary reading and writing to handle constructor invocations.

Once this was updated, it uncovered some errors in the constant evaluation so I added some logic to the potential-constant checker to handle `DotShorthandsConstructorInvocation`s. Tests cover this new behavior.

`equality_ctor_test` and `constructor_future_or_test` language tests passing. Added unit tests.

Bug: #59835
Change-Id: Ia1926d5efc3bbf0207c07c43baaa4d43351c32ff
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/424340
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Kallen Tu <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 1, 2025
Updating some `??` tests to make them emit the correct warnings.
Removed an expected error where there wasn't one.

Bug: #59835
Change-Id: I8a1282e01ece6f3fd776eb06bcbb2cd7a1f8ed2a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/425587
Reviewed-by: Erik Ernst <[email protected]>
Commit-Queue: Kallen Tu <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 1, 2025
…ation.

Added error code `DOT_SHORTHAND_UNDEFINED_INVOCATION` for when we are unable to resolve a dot shorthand invocation with a given context type and member name.

New unit tests and multiple co19/language tests passing.

Bug: #59835
Change-Id: I74fc053d668496539d88bee7d6eeee12acd9b710
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/425145
Reviewed-by: Paul Berry <[email protected]>
Commit-Queue: Kallen Tu <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 2, 2025
…ssignable.

This is a bug fix of the current implementation of dot shorthands type argument resolution. We want to make sure we're doing inference with the proper context type given at that point (with the element based on the dot shorthand context type we've saved).

Then I made sure there was proper argument checking, not just for `FunctionExpressionInvocation`s, but with `DotShorthandInvocation`s as well.

Tested with unit tests and language tests.

Bug: #59835
Change-Id: I795046502214628389c6471e9424aca7152657e2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/425780
Reviewed-by: Chloe Stefantsova <[email protected]>
Commit-Queue: Kallen Tu <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 2, 2025
`FunctionReference`s like `.foo<T>` can also be dot shorthands. This CL adds the flag onto that AST.

Unit test added and co19 tests that have function references are now passing.

Bug: #59835
Change-Id: I8c85e7915b665de5c4de2b249b22c6e8d8fc364d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/426001
Reviewed-by: Paul Berry <[email protected]>
Commit-Queue: Kallen Tu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-dart-model For issues related to conformance to the language spec in the parser, compilers or the CLI analyzer. feature-dot-shorthands Implementation of the dot shorthands feature. P2 A bug or feature request we're likely to work on
Projects
None yet
Development

No branches or pull requests

3 participants