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>
In the recent change,
https://dart-review.googlesource.com/c/sdk/+/503040, the asynchronous
nature of the change required adding _new_ hooks to track that the
plugin isolate is analyzing or not. I missed removing the old hooks.
The result is that an invocation of `dart analyze` will get a notification from the plugin isolate that it is analyzing, and then
_immediately_ a notification indicating that it is not.
So this change removes the old hooks.
Change-Id: Idc871d25c043ee1be94030729a1fd8226bdc753f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/504140
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@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>
Move the analyzer-only Diagnostic, DiagnosticMessage, Severity, and
locatable diagnostic helper types out of _fe_analyzer_shared and into
package:analyzer.
I paln to make changes outlined in
https://github.com/dart-lang/sdk/issues/63311 and chat discussion.
Keeping these classes in the analyzer simplifies the migration and
avoids introducing a shared abstraction before there is a concrete need
for one.
If we decide later need to have a shared abstraction, we can always
extract one at that point. With coding agents internal code motion is
cheap.
Update analyzer, analysis server plugin, analyzer plugin, linter, and
scanner call sites to import the moved APIs from analyzer libraries, and
refresh API baselines to reflect the new public owner.
Change-Id: Ie0ef0f01c6e4be7ebaac25619ac3e3fe991a44d9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/501000
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Replace the positional Scanner constructors with a single constructor
that takes the input text and error reporting callback as named required
arguments.
Remove the unused fasta forwarding constructor, reader offset handling,
and stored first token. The scanner now keeps only the input text and
returns the token stream directly from tokenize.
Update analyzer, analysis server, plugin, CLI, scrape, and scanner test
call sites to use the named constructor arguments.
Change-Id: Ieabb7b28570d4a963563fb2d1c5ef5ac0c89032a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/500340
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Paul Berry <paulberry@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>
The RegEx engine in the VM was updated in
e443b89f23 which caused the analyzer
analyzing the CFE to use ~150 mio instructions more.
Part of this was an increased cost in ignore comment processing which
relied on regex. Using regex before the updated engine made
`processPrecedingComments` have a cost of ~240 mio instructions,
updating the regex engine took that to ~264 mio instructions.
This CL gets rid of the regex and takes the cost of
`processPrecedingComments` to ~74.4 mio instructions a saving of about
189 mio instructions (all then analyzing the CFE and looking at output
from `valgrind --tool=callgrind`).
Benchmarking with `perf stat` with normal GC gives:
```
task-clock:u: -2.1760% +/- 1.6063% (-265608341.80 +/- 196062087.98) (12206056036.20 -> 11940447694.40)
page-faults:u: 0.2301% +/- 0.0313% (448.20 +/- 60.88) (194764.60 -> 195212.80)
cycles:u: -2.2906% +/- 1.6090% (-1180119481.60 +/- 828994273.75) (51521138476.00 -> 50341018994.40)
instructions:u: -0.3325% +/- 0.0032% (-196547942.60 +/- 1874215.18) (59120651337.60 -> 58924103395.00)
seconds time elapsed: -2.1715% +/- 1.6011% (-0.27 +/- 0.20) (12.21 -> 11.95)
seconds user: -2.2487% +/- 1.7816% (-0.27 +/- 0.21) (11.87 -> 11.60)
Comparing GC data:
'No' GC change.
```
Note that it must push the GC - the savings isn't really 2% in time.
And with GC disabled:
```
instructions:u: -0.4562% +/- 0.0029% (-185499444.00 +/- 1189012.77) (40663084597.80 -> 40477585153.80)
```
So here a saving of ~185 mio which fits okay with the data from
valgrind.
Change-Id: Ib203baeac6a93f5e37c737080fed342dbd0740a7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/487021
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Adds the modifiers `abstract`, `base`, `final`, and `interface` to the
API summary output.
This information is an important part of the public API of a package,
because it determines whether a client can:
- Construct an instance of the class,
- Extend the class, or
- Implement the class.
Change-Id: I6a6a6964ba07db1714bc2fcb549cc15230e87058
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/482362
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Fixes two minor bugs with the sorting of members in the API summary
tool:
- The technique for placing getters next to their corresponding
setters was to sort them lexicographically based on
`Element.apiName`, which in the case of setters appends `=`. This
mostly worked, but due to the fact that `=` is between `9` and `A`
in ASCII, it was wrong in a few corner cases. For example, it would
sort `x`, `x=`, `x1`, and `x1=` in the order `x`, `x1`, `x1=`,
`x=`. Fixing this didn't affect any `api.txt` files in practice.
- The technique for sorting constructors also used `Element.apiName`,
which in the case of an unnamed constructor is `new`. This meant
that if a class had both named and unnamed constructors, the unnamed
constructor would not always be sorted before the other
constructors.
The fix for both bugs is to sort by `Element.name` (which does not add
`=` for setters and is the empty string for unnamed constructors), and
then to break ties by explicitly checking whether the element is a
setter.
Change-Id: I6a6a69648fb5915266a9111c5d884531bba4405d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/482361
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
`YamlEditor.update` should never throw `YamlException`, it should also
never throw `AssertionError`, but we know that it has bugs, and in those
cases it does throw `AssertionError`.
Change-Id: I941d88063effc8c46ba270da8531978a72e7a806
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/472562
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Auto-Submit: Jonas Jensen <jonasfj@google.com>
Commit-Queue: Jonas Jensen <jonasfj@google.com>
Added support for the `hosted` field in plugin definition,
so that analyzer plugins can be sourced from pub servers other
than pub.dev.
TEST=pkg/analysis_server/test/src/plugin2/generator_test.dart
TEST=pkg/analyzer/test/src/options/analysis_options_test.dart
Change-Id: Ie99693ab39c77b3d814843963613798f72e27f62
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/475580
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Auto-Submit: Albert Moravec <albert.moravec@gmail.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Remove the analyzer-only `scanner/reader.dart` re-export and refactor
`Scanner` to accept source text directly.
* Delete `pkg/analyzer/src/dart/scanner/reader.dart`, which only re-exported
`_fe_analyzer_shared` reader types.
* Replace the `Scanner(CharacterReader, ...)` factory with
`Scanner(String contents, ...)`, aligning the API with the underlying
`scanString` implementation.
* Update analyzer, analysis_server, analyzer_cli, analyzer_plugin, and scrape
call sites to pass the source string directly instead of constructing
`CharSequenceReader`.
This removes an unnecessary abstraction layer, reduces
imports/indirection, and makes scanner usage simpler and more uniform
across the repo.
Change-Id: I380b365676c8a62971a3700deadb695d1519d653
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/477600
Reviewed-by: Paul Berry <paulberry@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
This release is so we can publish a version of analysis_server_plugin,
and then start depending on that published version in DAS.
In addition, we get a highly requested fix out to analyzer_testing
users.
Change-Id: I694f47aeee59367c1fc066bc8f7a865406b09917
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/476620
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Konstantin Shcheglov