An attempt to capture some of the conclusions from recent discussions.
Please let me know if this doesn't sound reasonable or if there are
pieces I've missed (such as other exceptions to the general rule).
Change-Id: I2520a768de116be5d9a86f9cb4494dcad7d88f5c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/497780
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
I changed the parsing of const dot shorthands to parse the entire selector chain in `parsePrecedenceExpression` rather than parsing them in `parseConstExpression`. We were originally parsing the initial expression before the `.` and only flagging that as a dot shorthand which prevented any chaining on const constructors. Const expressions with chained methods/property accesses were not being flagged as a dot shorthand so we weren’t saving the context type properly which led to producing the “unknown context type” error.
Before:
```
class C {
const new someConstCtor();
C method() => this;
}
C c = const .someConstCtor.method();
// (const .someConstCtor) is parsed and handled as a const constructor
// (const .someConstCtor) is handled as a dot shorthand, context is
// saved on only this AST and not the outer chain.
// Then we parse the rest of the selector chain (.method())
//
// Error: No context type for (const .someConstCtor.method())
```
After:
```
class C {
const new someConstCtor();
C method() => this;
}
C c = const .someConstCtor.method();
// (const .someConstCtor) is parsed and handled as a const constructor.
// Then we parse the rest of the selector chain (.method())
// (const .someConstCtor.method()) is handled as a dot shorthand, context is saved for this outer node.
//
// OK. No error because we are able to retrieve the type for the
// entire dot shorthand chain.
```
Fixes: https://github.com/dart-lang/sdk/issues/63119
Change-Id: I3308d8eb7ce101466be257aba6b5448921bff136
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/495560
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Kallen Tu <kallentu@google.com>
This adds a new lint that was requested in order to support the primary
constructors feature.
This also enables the existing fix to remove the keyword and adds some
tests for the new use of the fix.
Change-Id: Iae3c86ca87a2af6abb82488e218d839a06699778
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/496741
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
This CL is a bit different from many previous CLs in a couple of ways.
The smallest difference is that I realized that some of the logic could
be shared and that we have a utility method that just need to be
enhanced a bit to support primary constructors. At some point we should
go back to the previous implementations and update them to use the
utility method.
The larger difference is the use of a pair of wrapper classes to
encapsulate some of the differences between primary and secondary
constructors. I'm not sure how I feel about the two approaches. On the
one hand, the wrapper classes feel a bit heavy-handed, but on the other
hand they could make a lot of code more DRY.
Your thoughts would be appreciated.
Closes https://github.com/dart-lang/sdk/issues/63047
Change-Id: I93af6d2a529bbc0491cf64ff03f7a58e6b874d81
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/495461
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
There's one class left to convert, as far as I know, and I have it
mostly converted, but adding `awaits` to the code causes an inconsistent
analysis exception and I don't want to wait until I've solved that
problem to land the rest.
Change-Id: Ic5f159f0703fc708119f8bfa249270cb8a853678
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/495782
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Permit `abstract` on top-level variable declarations when the
augmentations feature is enabled. Previously the parser always reported
`abstract` as extraneous at the top level, which rejected valid syntax
and dropped the modifier before later stages could see it.
Thread the abstract token through top-level field parsing, record it on
`TopLevelVariableDeclaration`, and pass it through the front-end
builders. Reorder the field callback arguments so `augment` precedes
`abstract`, matching the augmentation grammar for incomplete top-level
variables and keeping the parser, listener, and outline plumbing
consistent.
Change-Id: I680414a746b707d483e485702685c95f28a9c9ac
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/494564
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Add `InterfaceType.interfaceMembers` and
`InterfaceType.inheritedConcreteMembers`, and implement them by applying
the type's substitution to the underlying element members.
This moves member instantiation to the `InterfaceType` API instead of
requiring callers to read raw element maps and wrap the results
manually. With this change, code that works with an instantiated type
can ask the type itself for the corresponding instantiated members.
Update completion, inheritance, extension resolution, and summary
reading code to use the new API and `ExecutableElement.substitute()`
instead of `SubstitutedExecutableElementImpl.from()`. This removes the
old helper, simplifies callers, and makes substitution behavior more
uniform across the analyzer.
Change-Id: I2349546d65e9745c2cdeda19ca6ff9283a5fd001
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/495100
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Refactor `applicable_extensions.dart` so its public entry points accept
`DartType` and `ExtensionElement`, and perform implementation casts
internally at the analyzer boundary.
Remove the `strictCasts` parameter as well. Extension applicability
always ignores casts, so the parameter only added noise to the API and
to every call site.
Update completion and fix code in analysis_server to pass public types
directly and drop imports of internal analyzer type classes. Also
centralize the cast from `ExtensionElement` to `ExtensionElementImpl`
inside the helper instead of repeating it in multiple loops.
This makes the API cleaner, keeps the public/internal split in one
place, and reduces accidental coupling to analyzer implementation types.
Change-Id: I770ee5d8788f4c4a7004a0ad18a00cc52552e3b5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/494802
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
The lint rule was sometimes reported in the same positions as the
warning.
In 'remove_lexeme_test', there were tests for removing `final` in the
lint rule case, which just needed to be moved to testing removing
`final` in the warning case. Otherwise the test cases are unchanged.
Change-Id: I3f600b9bc7846069e793e0902dcb2456e7106ab6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/494163
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Report dedicated diagnostics for mixin classes that use a `with` clause
and for mixin application classes that use multiple mixins, instead of
folding both cases into `mixinClassDeclarationExtendsNotObject`.
This makes the reported error match the actual invalid construct. A
`mixin class` with a `with` clause does not have the same problem as a
`mixin class` that extends a non-`Object` superclass, and a mixin
application with multiple mixins is a separate restriction again.
Update the error verifier, diagnostic definitions, messages, and fix
status entries to use the new codes. Also guard `RemoveExtendsClause` so
it only offers a fix when an actual extends clause is present, avoiding
a bogus fix for diagnostics that are now reported on `with` clauses
instead.
Consolidate the diagnostic tests under a single mixin class declaration
test file and update expectations to use the more precise error codes.
The original impetus for this change was a crash in RemoveExtendsClause
quick fix.
Change-Id: I937276f37deb293ca1fecab3ff838ace45af2ec4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/493865
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Brian Wilkerson