Work towards https://github.com/dart-lang/sdk/issues/55660
The comment for `MemoryResourceProvider.convertPath` includes:
> This is a utility method for testing; paths passed in to other
> methods in this class are never converted automatically.
and indeed, the actual impl of this method is found in an extension,
in analyzer's test_utilities/ directory. It seems to me better to
leave a testing utility as a testing utility, and not expose it in
the public API of MemoryResourceProvider.
Additionally, the extension method is used by `ResourceProviderMixin`,
which is moving to the public analyzer_testing package. It is illegal
to have a circular non-dev dependency between the analyzer package
and the analyzer_testing package.
The migration for the ~half dozen test files that use this method is
to call the extension method directly. Sometimes with an extension
override, and sometimes without.
Change-Id: I9c2e18600461134bfd91c082b3af0d5079600f0c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/426984
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This change means that during `dart analyze`, an exception caught in a plugin isolate will be printed to the terminal, and the process will
exit (similar to the support for when the isolate has static errors).
Change-Id: I31b1ebe7a71a331274d4f1dc1ea1b94f33e2329b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/415981
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@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 change is the final piece in enabling plugins in the new style to
be launched from a specification in analysis options.
Work towards https://github.com/dart-lang/sdk/issues/53402
* We support both one legacy analyzer plugin (the current max), and
a set of new analyzer plugins, which are combined and launched in one
shared plugin isolate.
* Make PluginLocator.pluginMap private.
* Add a parameter to PluginManager.addPluginToContextRoot:
isLegacyPlugin. This method is used for both legacy and new plugins,
but has slightly different behavior, finding where the plugin files
are.
Change-Id: I6644aecd4283eea22586ffd051a01b0ec8987fc5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/395360
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
* Update comments to be modern and reference "package config files"
rather than "packages files"
* Remove commented out code that we aren't going to uncomment.
* Rename PluginFiles.packages.
* Simplify `PluginManager._computeFiles` to use less nesting, and use
early `throw` statements, rather than storing data in local
and choosing late whether an exception should be thrown with the
stored data.
Change-Id: I7d5da0e299962d8887f2222137478a2fd96671b3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/392960
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
In many cases, AnalysisOptions is sufficient.
* ResolvedCorrectionProducer will be public API for someone writing an
analyzer plugin; it should not expose an AnalysisOptionsImpl. Luckily
the only need for the Impl, today, is in the "ignore diagnostic" fixes
so we can cast in there. (We could also expose the `file` and the
`unignorableNames` fields.)
* Some other spots only cast in order to access one of the 'strict'
fields, but all of those have been made public.
* AnalysisOptionsImpl.enabledLegacyPluginNames can be made final.
* Many other users don't need AnalysisOptionsImpl, or only need it to
pass it to other code that _does_ need it. In many of those cases it
makes sense to look at an object as an AnalysisOptions, and let the
code that needs a field from Impl to do a cast.
Change-Id: I3640934fb9d93c9b95f15a22457af604e420c7a9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/392240
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
+ make the implementation of ResourceProviderMixin methods not depend on MemoryResourceProvider.
This is a step towards being able to share more code between tests that use different kinds of ResourceProviders. Although ResourceProviderMixin currently still has a MemoryResourceProvider, all of the methods themselves can work against any ResourceProvider.
convertPath was moved to an extension in test_utilities (rather than to the base ResourceProvider) because it seems quite test-specific (it assumes any absolute paths are relative to drive C - something we'll need to update to use for non-memory tests.
Change-Id: Ibb3cfb31ebbdac6410868f0395bd19f9ce7b0e18
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/350380
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Maybe related to https://github.com/dart-lang/sdk/issues/38629. These tests have been skipped for so long, enabling them took some work, to migrate them from '.packages' files to package config files.
Some other tidying in the test file:
* inline `byteStorePath`, only used once.
* simplify `_packagesFileContent` and `_getPackagesFileContent`
into a static getter.
* simplify `_defaultPluginContent` into a const String, so it can
be used as a function parameter default value
The diff is way bigger than the functional changes, because we sort
elements.
This reverts commit aa6b6470e3.
Change-Id: I9dc533710255534ec27454712a1a64facf5dd12d
Cq-Include-Trybots: luci.dart.try:analyzer-win-release-try,flutter-analyze-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/345367
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
This reverts commit 7784cf3f94.
Reason for revert: broke windows bot
Original change's description:
> analyzer: Do not overwrite an original exception when a plugin crashes
>
> Maybe related to https://github.com/dart-lang/sdk/issues/38629. These tests have been skipped for so long, enabling them took some work, to migrate them from '.packages' files to package config files.
>
> Some other tidying in the test file:
>
> * inline `byteStorePath`, only used once.
> * simplify `_packagesFileContent` and `_getPackagesFileContent`
> into a static getter.
> * simplify `_defaultPluginContent` into a const String, so it can
> be used as a function parameter default value
>
> The diff is way bigger than the functional changes, because we sort
> elements.
>
> Change-Id: I193316316750e80268b684fdc1abe558a77994fe
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/344601
> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
> Commit-Queue: Samuel Rawlins <srawlins@google.com>
Change-Id: Ibeb761afebad4fb4166cec756743dbb35d323e7d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/345143
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Maybe related to https://github.com/dart-lang/sdk/issues/38629. These tests have been skipped for so long, enabling them took some work, to migrate them from '.packages' files to package config files.
Some other tidying in the test file:
* inline `byteStorePath`, only used once.
* simplify `_packagesFileContent` and `_getPackagesFileContent`
into a static getter.
* simplify `_defaultPluginContent` into a const String, so it can
be used as a function parameter default value
The diff is way bigger than the functional changes, because we sort
elements.
Change-Id: I193316316750e80268b684fdc1abe558a77994fe
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/344601
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
In https://dart-review.googlesource.com/c/sdk/+/191862 we added two new
required fields to `Location`. Unfortunately this was a breaking change
because plugins using an older version of the `analyzer_plugin` produce
location objects without those fields, leading to deserialization failures.
This CL makes those fields optional in order to fix the deserialization
issue.
Unfortunately, the `analyzer_plugin` package was published after the
required fields were added. Making them optional is a breaking change
because the constructor parameters go from being positional to being
named parameters.
We also neglected to update the version number of the protocol as part
of the previous CL. Technically this is also a breaking change for clients
of the analysis server, but given that they had no way to test to see
whether these fields existed they would need to have been written as if
the fields were optional in order to reference them at all, so I think
that from a practical standpoint it isn't a breaking change. That does,
however, raise the question of whether we should increment the version
numbers as part of this CL.
Change-Id: I35fc1f8e950669a3d8dd33cee6b81890261b5c47
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/206942
Reviewed-by: Danny Tuppeny <danny@tuppeny.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
This should allow doing partial migration, specifically protocol files,
which are imported by other libraries, but are a small library cycle
that does not import much outside of it.
Change-Id: I904c05d6d5b444ee9a9dbd1f7ada12aabdcc5165
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/193583
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Sam Rawlins