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>
Fixes https://github.com/dart-lang/sdk/issues/62649
This change overhauls what the PluginServer does when it receives requests from DAS like watch events, overlay changes, setting context roots, and setting priority files, to use the AnalysisDriver APIs. These APIs provide a much smarter, more fine-grained mechanism for determining which files need to be re-analyzed.
This requires a lot of changes that really don't seem obvious (or just,
to me, they are not intuitive), because they relate to Streams being
processed, drained, read, etc. So I'll summarize as much as I can:
* In order to track that results coming from AnalysisDriver are
definitely for the same AnalysisSession we are concerned with, at
any given time, we add `_filesBeingAnalyzed` and
`_filesBeingResolved`. These two maps are cleared any time the plugin
isolate is notified of changes, so there is not a memory concern.
* We add a `waitForIdle` method which is just used by tests
occasionally, in order to ensure analysis is complete.
* We tackle a related bug mentioned in the issue thread regarding
adding or removing overlays without changing the source contents.
There are now a few checks that old-content is not equal to
new-content.
* Due to the more async nature of reading events off of the analysis
driver, we have to be more particular in tests about sending
`AnalysisSetContextRootsParams` and reading from the notification
stream, so there are a lot of changes in tests to re-position these
calls.
This results in massive savings in re-analysis time. Using the example in https://github.com/dart-lang/sdk/issues/62649, typing in the
`build_runner/lib/src/logging/build_log_logger.dart` file, I see the
following:
* Before this fix, every keystroke results in 183 files being
re-analyzed by the plugin (always 183).
* After this fix:
* keystrokes that do not result in a summary change (or maybe
fine-grained deps analysis), like whitespace, results in 1 file
being re-analyzed (the file I'm editing).
* A keystroke that results in slightly changed summary, like changing
an import to something invalid, results 2 or 4 files being
re-analyzed.
* A keystroke that changes the name of class BuildLogLogger (large
change to summary) results in 9-10 files being re-analyzed.
Change-Id: Ie35053b0d90457b5b4b53bc8803188def0308d2a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/503040
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Work towards https://github.com/dart-lang/sdk/issues/61868
This adds a new notification type, 'PluginPrint'. There are several
fields and variables then named 'pluginPrint' or 'print', and I am
definitely open to changing these names, but this is the best one that
I thought of.
PluginPrint has three fields: The name of the plugin that printed, the
message that was printed, and the timestamp.
We wrap each plugin's AnalysisRule invocations with a zone, so that
the `print` handler can know the name of the plugin. The prints are
caught and sent to the server isolate as Notifications. The
PluginIsolate then stores the collected prints. The Plugins Insights
page can then retrieve them and display them.
Manual testing, with 1000 libraries that get new lint reported once per
file, I did not observe a negative performance impact.
Change-Id: Id800ae41781c15ce89d06b563878c2b0edae43d3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/486827
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Changes the analyzer and related packages so that when they refer to
diagnostic constants, they do so via the import prefix `diag`, which
refers to the appropriate `diagnostic.dart` file containing the top
level diagnostic constant declarations, rather than the static
declarations inside `DiagnosticCode`-derived classes (which will soon
be removed).
This CL was created by the following steps:
- Run the script
`pkg/analyzer_utilities/tool/messages/switch_to_toplevel_diagnostics.dart`.
- Execute `dart fix --apply --code=unused_import,unnecessary_import`
on the following directories (this removes imports that are no
longer necessary due to the change):
- `pkg/analysis_server`
- `pkg/analyzer`
- `pkg/linter`
- `pkg/analysis_server_plugin`
- `pkg/analyzer_plugin`
- `pkg/analyzer_testing`
- `pkg/front_end`
- `pkg/analyzer_cli`
- Execute `dart format` on the following files and directories:
- `pkg/analysis_server`
- `pkg/analyzer`
- `pkg/linter`
- `pkg/analysis_server_plugin`
- `pkg/analyzer_plugin`
- `pkg/analyzer_testing`
- `pkg/front_end/test/scanner_test.dart`
(Note that `pkg/front_end` and `pkg/analyzer_cli` are not
re-formatted as whole directories because they contain `.dart` files
that are test cases rather than source code, and reformatting those
files might change test expectations.)
- Manually add `diag` to
pkg/front_end/test/spell_checking_list_tests.txt.
- Manually fix the ignore comment in
`pkg/analyzer_testing/lib/src/analysis_rule/pub_package_resolution.dart`. (The
script `switch_to_toplevel_diagnostics.dart` automatically adds it
after `import 'package:analyzer/src/diagnostic/diagnostic.dart' as
diag;`, but then executing `dart format` bumps the ignore comment to
the following line, where it has no effect.)
Change-Id: I6a6a69643022aab2b5a6224fb4124eead243260d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/461521
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Removes nearly all of the special handling of `uniqueName` in
constructors in the `DiagnosticCode` class hierarchy.
With three exceptions, the value passed for the named parameter
`uniqueName` is the same as the value that will eventually be stored
in the `DiagnosticCode.uniqueName` field. The three exceptions are the
constructors for `LintCode` and `SecurityLintCode`, and the unnamed
constructor for `LinterLintCode`. These constructors allow
`uniqueName` to be `null`, and if `null` is supplied, they replace it
with `LintCode.$name`.
These exceptions are needed for analyzer API compatibility in the case
of `LintCode`, and for compatibility with google3 code in the case of
`SecurityLintCode` and `LinterLintCode`.
In follow-up CLs, I will be updating google3 code to make the
exceptions in `SecurityLintCode` and `LinterLintCode` unnecessary.
Change-Id: I6a6a6964d828c3a8db6563d47347d5ca8f550ac3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/459001
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Adds the constructor parameter `uniqueNameCheck` to `DiagnosticCode`
and derived classes. If a non-null value is supplied for this
parameter, an assertion checks that it's equal to
`uniqueName`. (Passing a `null` value disables the assertion; this is
necessary to preserve analyzer API compatibility).
The reason for this change is that the value supplied for `uniqueName`
when calling constructors in the `DiagnosticCode` class hierarchy is
not necessarily the same as the value that is eventually stored in the
`uniqueName` field; it goes through a sequence of manipulations while
being passed up the superclass chain that make its actual behavior
difficult to follow. The `uniqueNameCheck` field allows me to test the
theory that the actual values that wind up in `uniqueName` are simply
the analyzer codes, with the exception that `LinterLintCode.` is
replaced with `LintCode.`.
In a follow-up CL, I plan to eliminate nearly all the manipulations of
`uniqueName` that occur while being passed up the superclass chain,
which will simplify the code generation logic for computing
`uniqueName` values. Adding `uniqueNameCheck` now allows me to test
out the new, simpler logic, and verify that it will produce the same
result as today's more complex logic.
The `uniqueNameCheck` parameter is marked as `@deprecated` to prevent
clients from trying to use it; this ensures that when I remove it in a
follow-up CL, it won't be necessary to bump the analyzer major
version.
Change-Id: I6a6a696440bce62a31b988d12064244e54e37b6b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/458820
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
I had not run the first fix through manual testing. Manual testing
revealed a second location (!!) where the file path is stored, for
diagnostics. This updates the code to send a notification for each file
path, including parts.
Change-Id: Iee60330c579563f31c8d9a2c65a22c9cde4b64d8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/448881
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Fixes https://github.com/dart-lang/sdk/issues/61449
The previous code, sure enough, only created a DiagnosticReporter for
the defining unit, and only visited the main unit.
The fix is to create a DiagnosticListener for each unit, set the
DiagnosticReporters correctly and the "current unit" as we go, and then
pull the reported diagnostics from each listener.
Change-Id: I6b645284c58e5e35ccc5b89f30f6ffe97f48296c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/448644
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
This renames the old screen, "Legacy plugins" and adds a new screen,
"Plugins". On the new plugins screen, we ask the plugins isolate(s) for
their plugins details, and print the following, for eadh plugin:
* the plugin's name
* the names of the registered lint rules
* the names of the registered warning rules
* the IDs and "messages" of the registered assists
* the IDs and "messages" and associated diagnostic codes of the
registered quick fixes
More to come in follow ups:
* The resolved versions of plugin packages (coming from package_config.json)
Change-Id: Ic3dc4c5bffa64fd4da4097c042a847cc064e41ce
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/447763
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This change was generated by the following process:
- The script `pkg/analyzer/tool/messages/rename_error_constants.dart`
was run. This generated the vast majority of the diffs.
- Then all modified files were reformatted using
`tools/sdk/dart-sdk/bin/dart/format`.
- Finally, the script `pkg/analyzer/tool/messages/generate.dart` was
run, to rebuild generated code.
Change-Id: I6a6a69644ed8740ad6269d98cb169076151824ed
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/444921
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Until recently, everything in rule_context.dart was found in
linter.dart. That file was broken up [1]. That file previously had a
lot of exports; it was a sort of barrel file for lint rules. In order
to make that change "minimal," (it was still huge), I retained some of
the exports. Now we're in the process of removing those exports.
In this change, I remove the exports for `rule_state.dart` and `linter_visitor.dart`. This doesn't affect too many files.
[1]: https://github.com/dart-lang/sdk/commit/438b560e44659cebf73bccba3ecf968806477c5c
Change-Id: I83110b22a07186560f6c853f033e070ac050ecd1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/445123
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
This library mostly exports _public_ API, and the to-be-deprecated
NodeLintRegistry class. So we just move private RuleContext classes
into a new library, and remove the exports.
The lint rules then are adjusted, mostly just renaming
NodeLintRegistry -> RuleVisitorRegistry. But also, this removes a lot
of implementation imports; most rules now have _no_ private imports!
Change-Id: I023f7efa979f94d208a6ae7421902145e8761c9a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/443364
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
This change adds a `@Deprecated` annotation to each of the analyzer's
generated error code files, with a deprecation message that says the
corresponding non-code-generated file should be imported instead. For
example, `package:analyzer/src/error/codes.g.dart` now has a
deprecation message saying that
`package:analyzer/src/error/codes.dart` should be used instead.
All existing imports of these libraries are switched to the
corresponding non-code-generated file, where possible. (The only
circumstance in which it's not possible is when the corresponding
non-code-generated file imports the code-generated file---e.g., when
`package:analyzer/src/error/codes.dart` imports
`package:analyzer/src/error/codes.g.dart`. In these cases, the
deprecation warning is suppressed with an "ignore" comment.)
This change is part of a long term effort to unify the analyzer and
CFE representations of error messages. It paves the way for a
follow-up CL in which I intend to move some of the generated code from
`package:analyzer` to `package:_fe_analyzer_shared`, to allow it to be
shared with the CFE.
Change-Id: Ie91dc8fc9357defa8974cbe6836d582979d87d81
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/437840
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This is a big CL; no code is really "changed." We basically move 3
components into the public API, which can all be reviewed concisely
in the `api.txt` file.
* The AnalysisRule classes: `AbstractAnalysisRule` (which is good to
make public as a lot of the public API is specified and documented
here), `AnalysisRule`, `MultiAnalysisRule`.
* The Pubspec classes, available for linting pubspec files:
`PubspecDependency`, `PubspecDependencyList`, `PubspecEntry`,
`PubspecEnvironment`, `PubspecGitRepo`, `PubspecHost`, `PubspecNode`,
`PubspecNodeList`, `PubspecVisitor`.
* The `RuleVisitorRegistry` class. This class is needed by analysis
rule authors, and is part of the public API of AnalysisRule.
Change-Id: Ib1803180de9469f4ff39cf1778f96787f5f74b14
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/432363
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
To support this feature:
* Remove `DartAssistContextImpl` as it has no differences from
`DartAssistContext`. Thus the getters `DartAssistContext` are made
into fields, and all callers now call `DartAssistContext()`.
* Add a top level `computeAssists` function, similar to the top-level
`computeFixes` function.
Change-Id: I3ef5fd9ee9430150004b29c11ab972939b85b214
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/417360
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
This change allows the LegacyAnalysisServer to understand when the
plugin isolate (if there is one) is analyzing or not. There are a few
primary concepts:
* The plugin isolate (PluginServer) notifies the analysis server, when
analyzing all files in a context collection, and analyzing changed
files, that it is analyzing, and later that it isn't.
* The NotificationManager tracks whether the plugin isolate is analyzing
or not, based on the last status.
* The PluginManager tracks whether new plugins are initialized or not.
This is determined by the work done by the PluginWatcher. If no
plugins are configured, then plugins are declared to be "initialized".
Otherwise, the AnalysisServer sets their status to be "initialized"
after receiving the first status notification from the plugin isolate.
* The LegacyAnalysisServer now uses the additional "are plugins
analyzing" signal, held in NotificationManager, and the "are plugins
initializing" signal, held in PluginManager, to determine whether to
notify the client that analysis is complete.
Change-Id: Ie2b6a6048f074d7a26d7d5d07622a17c30fcab96
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405444
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This enables plugin sources to be specified in one of three ways:
* as a YamlScalar, like `plugin_name: ^1.2.3`
* with a 'version' key, like `plugin_name:\n version: ^1.2.3`, which
allows diagnostic configurations to be alongside.
* with a 'path' key, like `plugin_name:\n path: foo/bar`, which
allows diagnostic configurations to be alongside.
We introduce a PluginSource with a `toYaml` method to convert the
values back into YAML for the generates pubspec. We update
PluginPackageGenerator to use PluginSource.toYaml.
Change-Id: Ic5f372f0339edd6aa638dd9b568a83f41a023b08
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/393900
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Also allow warning diagnostics to be disabled.
In many places, the RuleConfig is unchanged because currently the
`linter/rules` section parsing is shared with the `plugin/<plugin-name>/diagnostics` section parsing.
They will probably diverge at some point, but as long as they are the
same, we can use the same Registry class to identify enabled and
disabled rules/diagnostics.
Change-Id: I9c00770af14d3b753532bcf1bc6bff62624d99ff
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/393460
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
* PluginConfiguration.ruleConfigs is now a Map instead of a List,
mapping analysis rule names to each RuleConfig. This makes it more
straightforward (and theoretically more performant) for the Registry
to determine the set of enabled analysis rules. The primary parsing
code, `parseLinterSection`, also returns a mapping now.
* This merges seamlessly into `AnalysisOptionsImpl`'s call to
`parseLinterSection`.
This is most of the refactoring work found in
https://dart-review.googlesource.com/c/sdk/+/392981, but without the
change to allow warnings to be disabled.
Change-Id: I4d1e6791da83ad370c18a15e9e3bb85b1daed58d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/393120
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Sam Rawlins