It would incorrectly put the primary constructor parameter list before
the type's type parameter list, like:
```dart
// Before:
class C<T> {
C();
}
// After:
class C()<T> {
}
```
This fixes it to follow the type parameter list if there is one.
Change-Id: Ib49c7df7923e9feed11f87579fefc8300c56c4a6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/506760
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Auto-Submit: Bob Nystrom <rnystrom@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Bob Nystrom <rnystrom@google.com>
Work towards https://github.com/dart-lang/sdk/issues/62686
`dart analyze` uses an 'analysis.setAnalysisRoots' "command" to let
the analysis server know that the user requested, for example,
`dart analyze foo` or `dart analyze foo/bar.dart`. This was not conveyed
to analyzer plugins, so analyzer plugins will always analyze the entire
context collection.
(For some reason, only files in the "analysis roots" were reported in
stdout; maybe DAS or dartdev does its own filtering on diagnostics.)
This change forwards the `setAnalysisRoots` request to DAS plugins using
a new protocol message that mirrors the one for the server.
We should not change the behavior of `analysis.setContextRoots` for
legacy plugins, so we keep sending that request as per usual. Then
we send `analysis.setAnalysisRoots` at the same time.
If we receive an `analysis.setAnalysisRoots` message, dispose and
delete references to the existing context collection, and instantiate
a new one, with the specified `included` and `excluded` paths.
Change-Id: I53627da1c30351a22b5e5410a557bf486620a7aa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/505041
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
I recently introduced this deletion logic but was deleting the wrong
file; this was deleting the `plugin.dart` file that `plugin.aot` is
compiled from. Over the weekend I found this issue with manual testing.
I've also verified this fix with manual testing.
Change-Id: I3f570368068117e6bb952ada0707a09ee97b765c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/506320
Auto-Submit: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Extension type augmentations do not have representation declarations, so
an `ExtensionTypeDeclaration` can no longer always expose its name
through a primary constructor. Add `ExtensionTypeDeclaration.namePart`
as the canonical API for the declared name and type parameters, and keep
`primaryConstructor` as a deprecated compatibility API for introductory
declarations.
Report `extensionTypeAugmentationHasRepresentation` when an augmentation
writes representation syntax. This keeps the parser recovery explicit:
the augmentation still gets a plain `namePart`, while the invalid
representation is diagnosed instead of being modeled as the
declaration's primary constructor.
Synthesize recovery representation and primary constructor fragments
only for the element model when an extension type has no introductory
declaration.
Migrate analyzer, analysis server, analyzer plugin, and linter clients
to read extension type names and type parameters from `namePart`.
Change-Id: I59dd957ac38f087c861b993caf246986dcdac713
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/505067
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Add a syntactic diagnostic for extension type augmentations that declare
representation fields. Extension type augmentations may augment the
declaration, but they must not redeclare the representation.
Thread an explicit parser option through primary constructor parsing so
extension type augmentations can omit a representation without producing
the usual missing-primary-constructor diagnostics, while still reporting
an error when a representation is present.
Register the new diagnostic in the shared and analyzer generated
diagnostic tables and add it to fix status tracking.
Change-Id: I84815bb0669a76126564e590760e93096a3af046
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/505860
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
SLSA-Policy-Verified: SLSA Policy Verification Service <devtools-gerritcodereview-exitgate@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
The element matcher threw a null check exception when getting a matcher
in the argument list of a constructor redirect in a secondary
constructor declaration with no explicit type name. This fixes it so
that no exception will be thrown.
That said, it isn't clear to me that this code could ever be reached
outside of the tests. There might be a lot of unreachable code in
ElementMatcher. I just can't prove that this particular bug would never
have occurred.
Change-Id: I5adf9ab5d4f79686110099054902a28535483f4d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/505921
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
This CL is to resolve a TODO. I couldn't find a way to trigger any bugs
in the code, but I did add a test. I believe that the reason I couldn't
trigger a bug is because any constructor that has a null `typeName`
will not have a simple identifier in a place that would allow
`_isConstructorDeclarationReturnType` to be invoked.
I realized, while looking at the code, that the type check is
unnecessary. If the type name is null, then the test will somply return
`false`, which is what we'd expect it to do. Given that it isn't needed,
I decided to remove it.
Change-Id: Ib36aaf16475db0db5817e65a8b8bfbb8aa834dbd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/505501
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
This adds some classes for processing interactive forms (I'm splitting this from the CL that updates refactorings to simplify reviewing).
The `InteractiveForm` class managers a whole form (all fields), and processes a set of fields/answers that come back from the client (which may be the full set, or a subset). It produces a list of the outstanding fields (which need to be go back to the client) as well as maintaining the full set of answers for the master field list (to use as arguments for a command, for example).
I've also commented out enums/lists from the protocol code for now, because I made the field kinds `sealed` so we get exhaustiveness checking in the validation, but want to delay fully implementing those for now.
See https://github.com/dart-lang/sdk/issues/63371
Change-Id: Icc4dcea8c9eab20ffbdd19ebbc69e1285464f7ec
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/505683
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
(Part of https://github.com/dart-lang/sdk/issues/63288)
Updates the `CodeGenerator` mixin so that it outputs `ignore_for_file`
comments to ignore the following lints:
- unnecessary_type_name_in_constructor
- unnecessary_ignore
- duplicate_ignore
This mixin is used by the code generators that produce the Dart
wrappers for the analysis server and analyzer plugin wire protocols.
This is a first step towards migrating the packages `analysis_server`,
`analysis_server_client`, and `analyzer_plugin` packages to use the
new constructor declaration syntax, since it will allow the
`unnecessary_type_name_in_constructor` lint to be enabled without
breaking generated code.
Once all the packages have had their SDK constraints bumped to a
language version that supports the new syntax, I'll update the code
generator to use the new syntax, and remove the ignores.
For more information about the new constructor declaration syntax, see
https://github.com/dart-lang/language/blob/main/accepted/future-releases/primary-constructors/feature-specification.md#abbreviations-of-in-body-constructor-declarations.
Change-Id: Ied17e3ea772546675aad48efc324f6f16a6a6964
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/505521
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
This fixes an exeception when code completing before the `new` or
`factory` keyword in a constructor and the type name is not explicitly
used. The exception prevents any suggestions from being returned, so
it's fairly serious.
Change-Id: I2f5028812ee1945ac9635f55953fb800b19a48c5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/505620
Auto-Submit: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
I couldn't find a way to trigger an exception, so there's no test.
I think this is ok because this code isn't actually used. IntelliJ
doesn't register for the notifications built by this code. And if
there's another client somewhere that does, the effect of the change
is to not generate an occurrence in a place where it would be
invalid because the legacy protocol doesn't support occurrences where
the name length is different.
Change-Id: I328d7f605b97e7570a9e6a5385dca4b0c4aa64af
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/505502
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Auto-Submit: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
In the new interactive forms, some of the enums can be combined (eg. `FileExistence.New | FileExisting.Existing`). This adds a flag that produces some helper functions for `hasFlag()` and `combine()`.
These helpers are not used outside of tests in this CL but will be used in a future CL (which I'm trying to avoid getting too big to simplify reviewing).
Change-Id: I1b5f06f05d96781c4bc6a246a0a9309a786c1987
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/505320
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Keerti Parthasarathy <keertip@google.com>
Codes specified on the command-line are force-enabled in all of the
analysis options for all of the files. This is done in a brute force
way, but it should be safe because the code is creating an analysis
context collection just for this one run, so the options objects
should be short lived.
Closes https://github.com/dart-lang/sdk/issues/63345
Change-Id: I8925da426d6cb5b9ce686725e3b95696b552cae0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/505045
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
These are just some minor changes (logically, but a bit noisy mixed in with upcoming functional changes) from a future change I'm working on to handle command resolution for the new refactorings. In particular:
- Make it a bit clearer which classes refer to which refactoring system
- Rename `RefactorCommandHandler` -> `RefactorCommandExecutor` since there will also be a Resolver soon
- Extract some code from `RefactorCommandExecutor` into `RefactorCommandHandlerMixin` so it can be reused by the upcoming resolve handler
- Fix some type args that used `Object` when some implementations are `void` but we don't care about the type anyway
- Create a `ParameterizedRefactoringProducer` abstract class so that all refactors with no parameters don't have to implement stubs for `parameters` (and soon `resolve()`)
Change-Id: I865da1c5dac9dc5834564b5a4a47199c20b64979
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/504601
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Commit-Queue: Keerti Parthasarathy <keertip@google.com>
No functional changes, this just converts the tests from group/test to test_reflective_loader to match other tests.
Changing the group()/test() calls to methods results in the tests being re-sorted by member name.
To possibly simplify reviewing, I've pushed this to Gerrit as two patch sets:
- PS1: does the conversion but includes index numbers in each test to preserve the order
- PS2: removes the index numbers and re-orders the tests
Change-Id: I72ebe3d3066b181a77052bec5082e4bd34066939
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/504580
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Uses the pubspec util `computeVersionBumpEdit` that bumps the pubspec
file's `sdk:` one minor version up.
Very basic iteration through the given pubspec files to bump all of
them. We'll very likely have to refactor this as we get the pre/post
migrations involved, but I wanted to get there in small (reviewable)
steps.
Bug: https://github.com/dart-lang/sdk/issues/63268
Change-Id: I2a7abeacdc76b114dfef7888cd6db6bc8973d01a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/504381
Commit-Queue: Kallen Tu <kallentu@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Report a dedicated diagnostic when an augmenting field or top-level
variable induces a getter whose return type differs from the getter
being augmented.
Previously, augmentation return type checking only covered executable
declarations with explicit return type annotations. Variable
augmentations that induced getters could therefore miss this mismatch,
or would not describe the induced getter case directly.
Add the new diagnostic code and use it for getter fragments induced by
variables. Keep the existing augmentation return type mismatch
diagnostic for methods and explicitly declared getters, and report the
induced getter diagnostic on the variable name so that mismatches in
multi-variable declarations are attributed to the specific variable.
Change-Id: Ifb421112d659815070110834702f68616e360a10
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/504181
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Add diagnostics for augmenting fields and top-level variables when the
augmentation induces an accessor that has no corresponding declaration
to augment.
Report these errors on each variable name rather than on the augment
keyword, so multi-variable declarations can report missing augmentation
targets independently. Include context pointing to the matching declared
or induced accessor when only one side of the getter/setter pair exists.
Register the getter- and setter-specific diagnostics in the generated
diagnostic tables and correction status metadata.
Change-Id: I78393a6e8bffe8748eb0fbc5182a0d01770771d8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/504180
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
It is disabled when augmentations feature is enabled.
This is not a breaking change, because CFE did report this error.
The relevant reason is indirect, not a single sentence saying “on A
implements A is forbidden”.
Spec chain:
1. In dartLangSpec.tex:6019, a mixin declaration is modeled as:
`mixin N<T...> on T1, ..., Tn implements I1, ..., Ik { ... }`
2. In dartLangSpec.tex:6065, the mixin interface M_I is defined as if
by:
`abstract class N<T...> implements T1, ..., Tn, I1, ..., Ik { ... }`
and dartLangSpec.tex:6078 says it is a compile-time error for the mixin
if that synthetic class declaration would be a compile-time error.
3. In dartLangSpec.tex:5189, class superinterfaces say:
It is a compile-time error if two elements in the type list of the
IMPLEMENTS clause of a class C specifies the same type T.
So:
`mixin M on A implements A {}`
is checked like:
`abstract class M implements A, A {}`
which violates the duplicate implements type rule.
Change-Id: I29e5fa17c47d111346ef6a6e321ab5dbab985ad1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/503501
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
This will allow use of Dart 3.13 features (such as primary
constructors) within the SDK itself.
Note that some generated files required benign updates due to
formatter behavior changes.
Change-Id: I595554e9cff3dbaa3f11da407d6c26036a6a6964
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/503662
Auto-Submit: Paul Berry <paulberry@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Report new diagnostics when an introductory function, member, or factory
constructor is still incomplete after applying all augmentations.
Keep the existing missing-body diagnostics for declarations that have no
augmentations, but report augmentation-specific diagnostics when an
augmentation chain exists and none of the fragments provides a body or
factory redirection.
Move the checks into error verification so that extension and extension
type members can participate in augmentation completion before reporting
the existing abstract-member diagnostics. Also suppress the
corresponding shared parser diagnostic when it is reported by the
verifier.
Consolidate body-related tests by declaration shape instead of by
individual diagnostic. This keeps missing bodies, external bodies,
augmentation completion, and already-complete checks side by side,
making the interaction between these rules easier to review and extend.
Move the constructorAlreadyComplete coverage from its dedicated test
file into constructor_body_test.dart, and add the factory body
completeness cases there as well. Add executable_body_test.dart for
function and member body coverage, including top-level declarations,
static members, extension members, and extension type members.
Add diagnostic definitions and fix-status entries for the new
diagnostics.
Change-Id: I9ee803c1e767ff47a608c86413ef7ffc71518cfd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/503540
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
The correction producer implements a fix. Pending discussion we might
choose to extend it to produce an assist, either in addition to the fix
(if we keep the lint) or instead of a fix (if we drop the lint).
Whatever we choose, I'd like to do that in a separate CL because I want
this to be available for testing purposes as soon as possible.
Change-Id: Icdd17ba0130149fc785c196be69e9ef990529800
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/503520
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Robert Nystrom