-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[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
Comments
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]>
Not sure if here is the place but we could add a lint to tell the user to use this feature where possible 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 If these points should be somewhere else please say so and I can move them. If I missed anything please let me know! |
We are planning to add that, yes! I've added it onto the analysis server work issue (under quickfixes for now) #59836
Yep, prefixed imports with shorthands should work as well. There are some language tests covering this behaviour.
Good point. Added this, thank you! |
…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]>
… 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]>
…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]>
…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]>
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]>
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]>
…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]>
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]>
…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]>
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]>
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]>
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]>
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]>
…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]>
…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]>
`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]>
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]>
…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]>
…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]>
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]>
…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]>
…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]>
`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]>
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
AST enhancements (AstBuilder)
Summary support
Resolution
Constant evaluation
Index and search
Warnings (annotation-based, unused*, strict-mode-based, a few others)
Ambiguous Import
to make sure it triggers correctly for this case too.NodeLintRegistry
The text was updated successfully, but these errors were encountered: