These files had comments indicating that running
`pkg/analysis_server/tool/spec/generate_files` would regenerate them,
but that was not the case.
Change-Id: I6ceb6352edf6eab5e746276a0a2f33b16a6a6964
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/507521
Reviewed-by: Brian Wilkerson <brianwilkerson@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>
This avoids a second Map access for retrieving every value in every
message from a plugin.
This also extracts out the `if (json is Map)` case outside of a block,
by short-circuiting when `json` is not a Map.
Change-Id: I962511d44ea797a0105901510a9182b5811b0230
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/487080
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>
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>
When hitting a protocol error, I saw some errors that were confusing
for a moment because of the lack of quotes, for example:
> Invalid parameter 'params'. Expected to be files; found \"...
This CL doesn't actually improve the situation _much_, but it at least
puts quotes around `files`, to indicate it is a property or something
similar.
Change-Id: Ic12d063b427661025cd4418951c58abf3394df3f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/449220
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@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>
Most calls to `jsonDecoder.mismatch` include the 3rd argument. I add
more here, so that the error can say, "I expected this field in some
JSON, but instead the JSON looks like this: ..."
Change-Id: I7d1944624e1fe758d9732d71db6571a579dc4690
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/447360
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
This fixes a bootstrapping problem: previously we could get into a
situation where the generated files for analyzer diagnostic messages
were out of date, but the code generator for those messages couldn't
be run, because it imported the analyzer itself, which contained
compiled errors due to the generated files being out of date.
There were two problematic dependencies:
- The `GeneratedContent.check` and `GeneratedContent.checkAll` methods
relied on invoking the Dart formatter as a library (rather than
invoking `dart format` as a subprocess); the Dart formatter library
imports the analyzer. Fortunately, this functionality is not needed
by the code generators themselves. It's only needed by the _tests_
that verify that code generation has run. So I was able to break the
dependency by moving this functionality into a new file,
`package:analyzer_utilities/generated_content_check.dart`, which is
imported only by the code generation tests. The new API is slightly
different: `GeneratedContent.checkAll` has been replaced by an
extension method `check` on `Iterable<GeneratedContent>`.
- The diagnostic message code generator used to have a post-processing
step that made use of the scanner to find error messages used by the
parser that needed to be translated, and issued a warning if it
found any. This check was useful a long time ago, when we were
unifying the analyzer and CFE parsers, and is no longer useful, so
I've removed it.
Change-Id: I509b741faca3b9ae21c23936001105ded731b490
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/442241
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Modifies `GeneratedDirectory.outputDirPath` and
`GeneratedFile.outputPath` to be relative to the SDK's `pkg` directory
rather than relative to the containing package. Accordingly, modifies
the `GeneratedContent` methods `check`, `checkAll`, `generate`,
`generateAll`, `output`, as well as the `DirectoryContentsComputer`
and `FileContentsComputer` callbacks, so that their first parameter is
the path to the `pkg` directory rather than the path to the containing
package.
Also modifies the `readApi` functions in `pkg/analysis_server` and
`pkg/analyzer_plugin` to accept a path to the `pkg` directory rather
than a path to the containing package, since these functions are
called by code generation callbacks.
These changes should make code generation logic easier to reason
about. They also will make it easier to move the outputs of code
generation from one package to another, which will pave the way for
some follow-up work in which I intend to start sharing error message
representations belonging to `pkg/analyzer`, `pkg/front_end`, and
`pkg/_fe_analyzer_shared`.
Change-Id: Ia9b369b16f2df931c8a472f91400f2c5a0b8be9d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/438480
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
All code generation logic that uses the analyzer_utilities package's
`GeneratedContent` class hierarchy is modified to use
`package:analyzer_testing/package_root.dart` to locate the `pkg`
directory in the root of the SDK repo.
This paves the way for some follow-up work in which I intend to modify
the `GeneratedContent` class hierarchy so that it tracks the locations
of all code generation targets relative to the `pkg` directory rather
than the individual package directories. That in turn should make it
easier for code generation logic to be shared between error messages
belonging to `pkg/analyzer`, `pkg/front_end`, and
`pkg/_fe_analyzer_shared`.
Change-Id: Iee59e977e844261f46fb62ed4e222db79db45a25
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/438061
Auto-Submit: Paul Berry <paulberry@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Luckily, the two cases which were reported could both be remedied by
making more things private.
* `DartFileEditBuilderImpl.librariesToImport` exposed `_LibraryImport`,
but could be made private anyhow. Also, it's package-private code,
so this is not a breaking change.
* `CodegenVisitor.generateConstant` accepted a private type as an
argument, but the whole class can be made private.
Change-Id: I8613812385ed0e9a7e36922d86889dad4cedf3cd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/428924
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Adding edits (e.g. via `dart fix --apply` are often done with
`List.insert(0, whatnot)` which takes O(n) time.
Here QueueList that can insert fast at both ends is used instead
(and we use `addFirst` instead of `insert(0)`.
On the example from
https://github.com/feinstein/google-i18n-address-dart.git we go from:
```
$ time dart fix --use-aot-snapshot --apply
[...]
249517 fixes made in 255 files.
real 3m55.810s
user 4m1.209s
sys 0m3.714s
(resetting)
$ time dart fix --use-aot-snapshot --apply
[...]
249517 fixes made in 255 files.
real 3m33.966s
user 3m37.588s
sys 0m2.058s
(resetting)
$ time dart fix --use-aot-snapshot --apply
[...]
249517 fixes made in 255 files.
real 3m36.525s
user 3m40.083s
sys 0m1.907s
```
to:
```
$ time dart fix --use-aot-snapshot --apply
[...]
249517 fixes made in 255 files.
real 0m9.970s
user 0m12.676s
sys 0m2.100s
(resetting)
$ time dart fix --use-aot-snapshot --apply
[...]
249517 fixes made in 255 files.
real 0m9.862s
user 0m12.926s
sys 0m1.797s
(resetting)
$ time dart fix --use-aot-snapshot --apply
[...]
249517 fixes made in 255 files.
real 0m9.612s
user 0m12.712s
sys 0m1.834s
```
Statistics on the `real` runtime:
```
N Min Max Median Avg Stddev
x 3 213.966 235.81 216.525 222.10033 11.941664
+ 3 9.612 9.97 9.862 9.8146667 0.18363369
Difference at 95.0% confidence
-212.286 +/- 19.1415
-95.581% +/- 8.61838%
(Student's t, pooled s = 8.44503)
```
For `lsp_many_prefer_single_quotes_violations_benchmark.dart --sizes=3200`:
Before from something like:
```
Initial analysis: 0.115654
First code action call: 0.835152
Subsequent action call 1: 0.538592
Subsequent action call 2: 0.561636
Select all code action call: 1.564402
```
After to something like:
```
Initial analysis: 0.086985
First code action call: 0.411660
Subsequent action call 1: 0.171566
Subsequent action call 2: 0.193708
Select all code action call: 1.107339
```
Statistics on 5 runs gives:
First code action call:
```
Difference at 95.0% confidence
-0.44381 +/- 0.0261597
-52.4602% +/- 3.09218%
(Student's t, pooled s = 0.0179367)
```
Subsequent action call 1:
```
Difference at 95.0% confidence
-0.381012 +/- 0.0195618
-69.9139% +/- 3.5895%
(Student's t, pooled s = 0.0134128)
```
Subsequent action call 2:
```
Difference at 95.0% confidence
-0.360077 +/- 0.0265277
-64.634% +/- 4.76173%
(Student's t, pooled s = 0.0181891)
```
Select all code action call:
```
Difference at 95.0% confidence
-0.405855 +/- 0.027662
-26.7277% +/- 1.82169%
(Student's t, pooled s = 0.0189668)
```
Change-Id: I3868afaa8c32a24c01c3a52bd8a53d5e8e4e3afe
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/427401
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This adds an optional version number to AddContentOverlay and UpdateContentOverlay in the legacy protocol.
It also moves some code (such as the `Map` that stores the current document versions) from the LSP server into the base class, and updates the overlay handlers to update it.
And finally, it removes the FailingTest() annotations on the versioned EditArguments test since they now work.
Change-Id: Icf2a2825eb6227f5faa3e21ae6c3a7d15997ef5a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/404821
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
I wrote a test that generates 10,000 lines of code, each containing one
violation of `prefer_single_quotes`, computes the fixes for all of the
diagnostics, and applies the 20,000 edits, measuring how long the
application takes.
Before this change the average was 3355.0 ms.
After this change the average is 3.8 ms.
The change was inspired by Jens.
Change-Id: Icd54a81ca3848fd5f0168934f713a7be8caec359
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/425601
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
There is no code in this CL that _uses_ the new AnalysisStatus
Notification or type, but landing it is important, so that I can test
plugins which will use the analyzer_plugin package from pub.
Change-Id: I31f835d5f7b8bda3781a6962d37f1f769b22852e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/404702
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Previously, we only had descriptions of changes on a SourceChange. In order to support annotated edits over LSP for "Fix All in Workspace" we need to be able to track descriptions on each change within a file so that if we're fixing multiple different lints with a single SourceFileEdit they can be annotated/grouped in the UI.
In order to avoid every fix providing a description, ChangeBuilder has a field that allows recording the current change description and all builders created from it will use that description when creating their changes. This allows the BulkFixProcessor to set the description on behalf of each fix producer ensuring they all get descriptions (which is done in a follow-up CL along with Fix All in Workspace functionality).
Change-Id: Ie1c508056898bd4000a5505cf59a1e6d35d28da0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/356160
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This change shouldn't change any current behaviour, but means all "FilePath" types in the legacy protocol spec will go through a (currently no-op) conversion. The server will be able to replace this conversion based on client capabilities in a future CL.
Because a lot of the generated classes are in analyzer_plugin, this also moves the ClientUriConverter class there.
`pkg\analysis_server\test\src\utilities\json_test.dart` contains tests that the toJson/fromJson methods go through the converter recursively (inc. map keys/values/etc.).
Change-Id: If5aec884070128eea594540fd25a9017ada86079
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/349060
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
With the parser code in the same library as the DOM classes, the
`pkg/analyzer/tool/messages/generate.dart` program, which generates all
of the diagnostic classes and diagnostics, _depends_ on a library
with a `parse` method which depends on these diagnostic classes (in
order to report errors while parsing HTML). This means that if
there is any existing error (like an unknown identifier) in the
existing error codes, it is impossible to generate the error codes.
Since we don't need the `parse` method to generate diagnostics, we
split up the library and remove the indirect dependency.
Separating the parser code out removes the loop.
Change-Id: Ifb9e9fd979e341ae64a1ed49aaf3758d3dcf1dee
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/287220
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
I don't think that there are any other highlight kinds we're going to
want to add, but if you can think of any please let me know.
I'll add code to actually associate the kind with record literals in the
next CL.
Change-Id: I15922984d96c2d0c7d97ea3f3832a95a68172968
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/256524
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Paul Berry