This paves the way for a follow-up CL that will standardize all
diagnostic names to `lower_snake_case` conventions.
For now, the case differences between the entries in
`error_fix_status.yaml` and the actual diagnostic code names are
accounted for by adding some calls to `.toLowerCase()` to
`verify_error_fix_status.dart`.
Change-Id: I6a6a6964b0eacf50b11f302ab511fba149b1ac07
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/465981
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Fixes https://github.com/dart-lang/sdk/issues/61684
This caching is based largely on the `--depfile` feature offered by
`dart compile`, which is based on a Ninja depfile concept
(https://ninja-build.org/manual.html#_depfile), which spits out a file
(`depfile.txt` here) which lists all of the input files which were
required to build an AOT snapshot.
The process is essentially:
1. If an AOT snapshot is found, maybe use it as a cached snapshot!
a. If the `pubspec.yaml` modification timestamp is newer, re-compile!
b. If the `.dart_tool/package_config.json` modification timestamp is
newer, re-compile!
c. If the `bin/plugin.dart` modification timestamp is newer,
re-compile!
d. If the `bin/depfile.txt` file is missing or malformed, re-compile!
e. If any files mentioned in `bin/depfile.txt` have a newer
modification timestamp, or don't exist, or are an otherwise bad
path, re-compile!
f. Otherwise, save a dozen seconds and use the cached snapshot.
Change-Id: Icc747198f8af76d256ac915685473d6f529a3cef
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/464602
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Work towards https://github.com/dart-lang/sdk/issues/61684
PluginManager needs to be more testable before implementing the caching feature. This CL lays the groundwork.
We add ProcessManager.runSync, and a handler for that in MockProcessHandler. Then we can add tests which call `PluginManager.filesFor` with `isLegacy: false`, and `dart pub upgrade` will not be run on the real filesystem.
Change-Id: I7b8877d985296741e51543c10a7cb87c8a43c116
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/465662
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Introduce explicit origin flags on ConstructorElement and its fragment
(isOriginDeclaration, isOriginImplicitDefault, isOriginMixinApplication)
and deprecate isSynthetic in favor of these. Define isSynthetic as the
inverse of isOriginDeclaration to preserve the legacy meaning while
encouraging clients to use the more precise origin predicates.
Eventually `Element.isSynthetic` also will be deprecated and removed.
This CL is a step toward this goal, migrating what is possible with new
flags.
Update analyzer internals to rely on the new origin flags when checking
for non-factory generative constructors, building synthetic constructors
for mixin applications, and walking constructor chains in index/search
logic. Only constructors with an origin declaration are now treated as
declarations, and nonSynthetic is defined in terms of origin
declarations rather than synthetic-ness. Add corresponding origin
descriptors to the manifest enum and bump AnalysisDriver.DATA_VERSION.
Adjust analysis server refactorings and fixes to distinguish implicit
default constructors from other synthetic constructors. Code paths that
previously checked isSynthetic for default constructors now check
isOriginImplicitDefault, and mixin-application traversal uses
isOriginMixinApplication.
Overall, this change removes the overloaded semantics of isSynthetic,
makes constructor provenance explicit, and prepares the element model
for future DeCo and primary-constructor scenarios without relying on
brittle synthetic heuristics.
Change-Id: I8568bdfe478867af313a4d13afe1f2859394831b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/465201
Reviewed-by: Paul Berry <paulberry@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Modifies the code generation logic that populates the
`DiagnosticCode.uniqueName` field so that it doesn't include the
diagnostic's class name.
This paves the way for removing the last remenants of the diagnostic
classes from the analyzer.
Change-Id: I6a6a696453fe1f2bd8bd3cea00a9a496392fbf98
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/464240
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Sam noted that AOT snapshots might return `dartaotruntime` for `Platform.resolvedExecutable`. It doesn't seem to happen if you invoke server with `dart language_server` because `dart` is still the executable, but if you invoked the snapshot directly with `dartaotruntime` then this would fail.
To avoid any possibly issues, this changes it to use the same `sdk.dart` getter that some other code uses that handles this difference by constructed the path to the `dart` executable.
Change-Id: I099335a255a792b6754d8d2a1f0bde491c09699e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/464681
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Setting the command-line arguments separately from the protocol used to
be necessary, but it no long is, so it's cleaner to just set the command
line arguments when creating the server and moving the handling of the
protocol option into the server. This resulted in a cleaner API and
implementation.
Change-Id: I9456e2a43310bb855b54c010af64ce0328a08ff7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/464785
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
This adds error recovery for explicitly naming a constructor 'new' with the new constructor syntax. This is not allowed so an error is explicitly emitted while handling this.
Change-Id: Iac3045f04c9c75779ffb2d9e866817ac43ab5d26
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/463281
Reviewed-by: Jens Johansen <jensj@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
We previously just showed "<unnamed extension>" for extensions without names, but in some files I noticed we have a lot of these, and the outline/symbols list looks awful (just "<unnamed extension>" repeated many times).
This changes it to instead show "extension on FooClass" instead (if there is a valid type name). I added a new field to the protocol to support this because the LSP classes convert from those classes (something we've discussed changing, but might be easier later).
Screenshots of before/after are in https://github.com/Dart-Code/Dart-Code/issues/5818
Fixes https://github.com/Dart-Code/Dart-Code/issues/5818
Change-Id: I3885a722443291bfa2419514841469c862b74450
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/464560
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Adds `type:` entries to the analyzer-style `messages.yaml` files.
Currently the only function of these entries is that they are checked
against the diagnostic types implied by the diagnostic's class name
(diagnostics under the heading `CompileTimeErrorCode` must have type
`compileTimeError`, those under `StaticWarningCode` must have type
`staticWarning`, etc).
In a follow-up CL I will change the code generation logic to these
`type:` entries instead of inferring the type from the diagnostic
code's class. This will pave the way for removing the notion of
diagnostic code class entirely.
This change was produced automatically by running the script
`pkg/analyzer_utilities/tool/messages/add_types_to_yaml.dart`.
Change-Id: I6a6a69646958a34e24ccbf69cb96ead9d7e7865b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/464282
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This `socketError` callback doesn't record exceptions to the instrumentation service because they were expected to be things like the socket closing. However it was also used for unhandled errors in the zone used for handling errors, which meant they also were not logged. This meant the user would see the error text, but the stack trace would never be logged anyway.
With this change, unhandled exceptions from the zone will be logged through the instrumentation service too.
Fixes https://github.com/dart-lang/sdk/issues/62082
Change-Id: I71d56a8b3c9241744eb614d74de46c190d61f2e3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/464381
Commit-Queue: Keerti Parthasarathy <keertip@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Changes the null check in `if (stackTrace != null && exception is!
CaughtException)` from `!=` to `==`. The test was clearly intended to
supply a stack trace if (a) there isn't one already, and (b) one can't
be obtained from `CaughtException`. But with the accidental use of
`!=`, what it was actually doing was destroying the stack trace
supplied by the caller in the circumstance where `exception` was not a
`CaughtException`.
This should make it easier to debug some trybot failures that are
occurring in https://dart-review.googlesource.com/c/sdk/+/464245.
Change-Id: I6a6a6964072bf58db0bcddb22492b0a70203d43a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/464460
Auto-Submit: Paul Berry <paulberry@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Previously, the process for removing a lint from `pkg/linter` was the
following:
- Remove the lint class's override of
`AbstractAnalysisRule.registerNodeProcessors` (effectively changing
the rule into a no-op).
- Change its override of `AnalysisRule.diagnosticCode` to return the
pseudo-diagnostic code `removedLint`.
- Modify its constructor to use `RuleState.removed`, so that the lint
would be marked as being in the "removed" state.
This change introduces a new class, `RemovedAnalysisRule`, as the
standard way to represent an analysis rule that has been removed. So
the new process for removing a lint from `pkg/linter` will be to
remove its class entirely and instead register an instance of
`RemovedAnalysisRule`.
This avoids the need for the pseudo-diagnostic code `removedLint` to
exist at all, and also makes the representation of a removed lint much
more compact.
To help encourage clients to use the new `RemovedAnalysisRule` class,
the `RuleState.removed` constructor has been deprecated. It will be
removed in a future version of the analyzer.
Change-Id: I6a6a6964726595b7bb32664846cf4e4722bbb4f1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/463463
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This reverts commit 19b345cadc.
Reason for revert: Breaks analysis server / VSCode integration.
Original change's description:
> Do not rebuild contexts on Linux if there is a error indicating the watcher limit has been reached. This prevents the hang for the cli.
>
> In the IDE, tested on both VS Code and IntelliJ, a message is shown when there are no watchers.
>
> For the cli, there is no message shown now. To do so we would need to plumb through the messaging, as this exception happens when we set roots, and there is no exception handling there.
>
> Like to land this before looking into that.
>
> https://github.com/dart-lang/sdk/issues/61931.
>
> Change-Id: Iaae9a85e646dfed4015e130076265be39b932f1c
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/463062
> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
> Commit-Queue: Keerti Parthasarathy <keertip@google.com>
Change-Id: Ief7cab1974535c526030803ab7624f6a9fc06844
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/463700
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Paul Berry <paulberry@google.com>
When generating the mock packages from the real packages, I found this
discrepancy; we're missing this deprecation. To keep things aligned as
close as possible, we should add it.
Change-Id: Ibbfd9164be5b344c358a4546542723f7deaff73d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/463162
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Removes all the generated classes derived from `DiagnosticCode` that
are specific to a type of diagnostic, except for those associated with
lints (`LinterLintCode`, `LinterLintWithoutArguments`, and
`LinterLintTemplate`). These exceptions are needed because the base
class for lint codes, `LintCode`, is part of the anlyzer public API,
and so it's necessary for lint codes to all implement it.
The generated static constants in these classes are removed too
(including the ones in `LinterLintCode`), since they are no longer
used; the analyzer and related packages have all been transitioned
over to refer to top level diagnostic constants instead.
Note that the class `ParserErrorCode` could not be completely removed,
because it is dependend upon by `package:dart_style`. So a stub
version of it is added to
`package:analyzer/src/dart/scanner/scanner.dart` (the file that
`package:dart_style` imports it from) as a temporary workaround.
Change-Id: I6a6a69648acac350e4e2249efe50ae9c652772b2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/461880
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
In the IDE, tested on both VS Code and IntelliJ, a message is shown when there are no watchers.
For the cli, there is no message shown now. To do so we would need to plumb through the messaging, as this exception happens when we set roots, and there is no exception handling there.
Like to land this before looking into that.
https://github.com/dart-lang/sdk/issues/61931.
Change-Id: Iaae9a85e646dfed4015e130076265be39b932f1c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/463062
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Keerti Parthasarathy <keertip@google.com>
This improves the error recovery for syntax like `new C.named();`. Previously, the parser would derail with messages like "A method declaration needs an explicit list of parameters" and a cascade of other errors.
With this change the qualified name is recognized as an attemp to write the name of the constructor, reporting that qualified names are not allowed in this case.
Part of https://github.com/dart-lang/sdk/issues/61699
Change-Id: Id1062aac3b90db8d2346013cf8a0bd9a541b9a3c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/463020
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Adds logic to `verify_error_fix_status.dart` to validate that every
entry is a YAML map and contains a valid status code.
The error reporting logic is rewritten to make use of the
`LocatedError` class so that if validation fails, it's not necessary
to describe where to find the erroneous entry; instead, the offending
path, line number, and column is printed as part of the exception
message.
Change-Id: I6a6a6964c8cc3094f81ad2b28dd444bfcb6c15c4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/463084
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Prior to this CL, the analyzer's set of unignorable diagnostic code
names (`AnalysisOptionsImpl.unignorableDiagnosticCodeNames`) was
constructed in the following way:
- For each entry in the `cannot-ignore` section of the
`analysis_options.yaml` file that matches the name of a severity
code, the names of all diagnostic codes with matching severities
were added (1).
- All other entries in the `cannot-ignore` section were converted to
upper case and then added (2).
However, diagnostic codes associated with lints are named using
`lower_snake_case`, while analyzer diagnonstic codes are named using
`UPPER_SNAKE_CASE`.
Some of the logic that consumed
`AnalysisOptionsImpl.unignorableDiagnosticCodeNames` didn't account
for this, resulting in some subtle bugs:
- When the resolved correction producer base class
`_BaseIgnoreDiagnostic` attempted to figure out if a diagnostic was
unignorable, it compared elements of
`unignorableDiagnosticCodeNames` to `DiagnosticCode.name`, which
meant that it would successfully recognize non-lint codes as
unignorable, but it would only recognize that a lint code was
unignorable if it was included in `unignorableDiagnosticCodeNames`
by putting the name of a severity in the `cannot-ignore` section of
the `analysis_options.yaml` file.
- When `LibraryAnalyzer._filterIgnoredDiagnostics` attempted to block
unignorable diagnostics from being ignored, it compared elements of
`unignorableDiagnosticCodeNames` to `DiagnosticCode.name`,
`DiagnosticCode.uniqueName`, and
`DiagnosticCode.name.toUpperCase()`. This worked, however the
comparison to `DiagnosticCode.uniqueName` had no effect. Note that
values of `DiagnosticCode.uniqueName` always take the form
`ClassName.snake_case_diagnostic_code` or
`ClassName.SNAKE_CASE_DIAGNOSTIC_CODE`. It's impossible for any of
the values added to
`AnalysisOptionsImpl.unignorableDiagnosticCodeNames` to ever match
this, because (1) always adds values of `DiagnosticCode.name` (which
never contains a `.`), and (2) always adds strings that have been
converted to upper case.
- When `IgnoreValidator.reportErrors` attempted to report unignorable
and duplicate entries, it compared elements of
`unignorableDiagnosticCodeNames` to `IgnoredDiagnosticName.name`,
which is always lower case. That meant that it would only recognize
that a diagnostic code was unignorable if the diagnostic code was
associated with a lint and was included in
`unignorableDiagnosticCodeNames` by putting the name of a severity
in the `cannot-ignore` section of the `analysis_options.yaml`
file. (Note, however, that this bug was unobservable because the
reporting of the `unignorable_ignore` diagnostic is currently
disabled; I will address this in a follow-up CL.)
- Additionally, the logic to populate
`AnalysisOptionsImpl.unignorableDiagnosticCodeNames` based on a
severity code had a bug in its handling of error processors: if one
or more error processors were used to change the severity of a
diagnostic, then entries would be added to
`AnalysisOptionsImpl.unignorableDiagnosticCodeNames` corresponding
to both the original and the new severity.
These buggy behaviors have been fixed by:
- Streamlining and simplifying the logic that builds
`AnalysisOptionsImpl.unignorableDiagnosticCodeNames`, and ensuring
that all strings added to it are all lower case.
- Changing all logic that checks whether a string is contained in
`AnalysisOptionsImpl.unignorableDiagnosticCodeNames` so that it
first converts that string to lower case.
- Removing the ineffective logic in
`LibraryAnalyzer._filterIgnoredDiagnostics` that attempted to
compare elements of `unignorableDiagnosticCodeNames` to
`DiagnosticCode.uniqueName`.
Change-Id: I6a6a6964d89c139492dbb11d8ba3b2d33c0e2ee8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/462864
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Currently, diagnostic codes associated with lints are named using
`lower_snake_case`, while analyzer diagnonstic codes are named using
`UPPER_SNAKE_CASE`.
However, when the analyzer builds instances of the `ErrorProcessor`
class, it always uses `UPPER_SNAKE_CASE` names.
Some pieces of logic that matched up `ErrorProcessor`s to diagnostic
codes accounted for this difference; others didn't.
This led to a some buggy behaviors:
- If an instance of `ErrorProcessor` got constructed outside of the
analyzer (by an analyzer client using the analyzer public API), and
it supplied a `lower_snake_case` name, then
`ErrorProcessor.appliesTo` would only successfully match if the name
referred to a lint.
- The resolved correction producer base class `_BaseIgnoreDiagnostic`
(which forms the basis for the quick fixes "Ignore '...' in
`analysis_options.yaml`", "Ignore '...' for this line", and "Ignore
'...' for the whole file") would only notice that a diagnostic was
unignorable if the case matched exactly. In practice, this meant
that when operating on instances of `ErrorProcessor` created by the
analyzer, it wouldn't properly handle lints.
These buggy behaviors have been fixed by:
- Changing the `ErrorProcessor` constructor to always convert the
`code` to lower case.
- Changing all references to `ErrorProcessor.code` to assume lower
case.
Change-Id: I6a6a69645284f646e0c070fc2b55c4a90203d74a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/462863
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Paul Berry