-
Notifications
You must be signed in to change notification settings - Fork 1.7k
☂️ New analyzer plugin system #53402
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
This is really exciting Sam! If you could use any help trying out prototypes I'd volunteer some time! |
@srawlins Would you mind creating go links for your design docs on https://github.com/flutter/website/blob/main/firebase.json? Then you can add them to the top of your docs making them a bit easier to share and discuss publicly. Community members often follow new go links on flutter.dev as well. |
In order for CorrectionProducers to be used in analyzer plugins, we need to access DartFixContributor which needs to compute fixes via FixProcessor. So both DartFixContributor and FixProcessor need to be moved into analyzer_plugin (eventually). For now, to support prototyping, I am moving FixProcessor to its own library, and separating all of the built in mappings (from diagnostic to producer generators). I think this separation actually stands on its own as being tidier, simplifying fix_internal.dart, and separating code from data, as it were. Work towards #53402 Cq-Include-Trybots: luci.dart.try:flutter-analyze-try,analyzer-win-release-try Change-Id: I3b4fadc7ed94c23597d72bef7dcd832380d34e9a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/345561 Commit-Queue: Samuel Rawlins <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]>
With a simple move of two classes, Fix and FixContent. The class moves are not exactly no-ops. I moved each class into its own library and made the following changes: * Start doc comments with third person verbs [1]. * Make `Fix` class **final**. * Make `FixContext` class **interface**. [1]: https://dart.dev/effective-dart/documentation#prefer-starting-function-or-method-comments-with-third-person-verbs Work towards #53402 Cq-Include-Trybots: dart-internal/g3.dart-internal.try:g3-cbuild-try Change-Id: Ic046b236d634543832825db8746e0abdca5191fe Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/355889 Reviewed-by: Alexander Thomas <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
* Combine DartFixContext and DartFixContextImpl into one class; separation seems unnecessary. * Change constructor (which was on DartFixContextImpl) to use all named parameters. * Change `resolveResult` field to `resolvedResult`, since the type is called `ResolvedUnitResult`. * Start doc comments with third person verbs [1]. [1]: https://dart.dev/effective-dart/documentation#prefer-starting-function-or-method-comments-with-third-person-verbs Work towards #53402 Change-Id: Idb3c6776f899d5bc9b634c1d9c4b998de2603d04 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358382 Reviewed-by: Konstantin Shcheglov <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
`_getInsertionLocationTop` and `addLibraryImports` are each moved to extract_method.dart, as the only location where they were used. We get to delete the `_InsertionLocation` class. The tests are also moved, unchanged, and CorrectionUtilsTest is very much simplified. Bug: #53402 Change-Id: Ia410f04a837d85a0e06ec523d156c6c6c8bf6a3b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/363402 Reviewed-by: Brian Wilkerson <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
…sites Cq-Include-Trybots: luci.dart.try:flutter-analyze-try,analyzer-win-release-try,pkg-win-release-try Bug: #53402 Change-Id: Id62b06f3419820d4a74b34b51a6022792bcef7ab Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/363404 Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
…nto extract_method Bug: #53402 Change-Id: I4fca3482c27520328339d1d1f0b6b541399ed612 Cq-Include-Trybots: luci.dart.try:flutter-analyze-try,analyzer-win-release-try,pkg-win-release-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/363501 Reviewed-by: Brian Wilkerson <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
We have chosen a new name for this package. So before we write more code inside, we need to move everything from 'server_plugin' to 'analysis_server_plugin'. There are some steps do doing so, to not break various infra: 1. Land this change, introducing the new package in the SDK, but no dependencies on it. 2. Land this change independently in google3 (the package is not yet "unbundled.") 3. Mark the package as "unbundled" in google3. 4. Move all code from server_plugin to analysis_server_plugin; update imports; and delete server_plugin. 5. Remove server_plugin as an "unbundled" package; and remove server_plugin from google3. Bug: #53402 Change-Id: I70197fdf61dd5862c2220d8ed5dd0880a1593ead Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/363600 Reviewed-by: Brian Wilkerson <[email protected]> Reviewed-by: Alexander Thomas <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
Work towards #53402 None of the classes which were moved are changed in any way. Cq-Include-Trybots: luci.dart.try:flutter-analyze-try,analyzer-win-release-try,pkg-win-release-try Change-Id: If81098971de044e2f69c1039ec23eff07b108af6 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/368066 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
I'm not good about referencing this issue in my CLs, as all the work is sort of just preparation and grooming the code. So I'll write down some of the under-the-hood changes I've been making. They are largely to support two areas: lint rule APIs and correction producer (mostly quick fix) APIs: lint rule APIs:
correction producer APIs:
Plugin server API:
Analysis Options
I'll update this comment as I make more preparatory changes. |
FixProcessor has to be part of the analysis_server_plugin package, so it cannot have any dependencies on the analysis_server package. This removes one. Work towards #53402 Cq-Include-Trybots: luci.dart.try:flutter-analyze-try,analyzer-win-release-try,pkg-win-release-try Change-Id: I935712bf75837ba95438ac9ed5f7ba5d8f941a7a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/376126 Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
Work towards #53402 I move them all out of static fields on FixProcessor to be instance fields on a singleton class in a new file, fix_generators.dart. For each one that ended in the word 'Map', I removed that. I also move `_bulkFixableErrorCodes` and `canBulkFix` to BulkFixProcessor, as it is the only caller, and holds onto the data. I also correct some outdated doc comments. I also make `_isFixableError` static. Cq-Include-Trybots: luci.dart.try:flutter-analyze-try,analyzer-win-release-try,pkg-win-release-try Change-Id: I01beccac4969f832941c75eb6d7b49b54a0b085c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/377563 Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
These two are needed in the shared package so that plugins can run CorrectionProducers. They go in 'src' because plugin developers do not need to see this code. Work towards #53402 Cq-Include-Trybots: luci.dart.try:flutter-analyze-try,analyzer-win-release-try,pkg-win-release-try Change-Id: Iee5b5dd063d5b0a9b00c58b91a3e50328c8b5fc2 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/377860 Reviewed-by: Konstantin Shcheglov <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
Also move the top-level `computeFixes` convenience function. The `computeFixes` function is the main entrypoint that various code uses to, well, compute all fixes with a FixProcessor and a FixInFileProcessor. In order for plugin code to call `computeFixes`, we need that function and FixInFileProcessor in the analysis_server_plugin package. Work towards #53402 Cq-Include-Trybots: luci.dart.try:flutter-analyze-try,analyzer-win-release-try,pkg-win-release-try Change-Id: Id5f02864762eeefb92055a3534e91b60b76dfeb2 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/378220 Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
This adds and tests support for adding file overlays, changing them, and removing them. Work towards #53402 Change-Id: I7939f8199639f7e336cd97515c2f7d48b1d777a0 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/384305 Reviewed-by: Brian Wilkerson <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
This generator will be used create shared plugin package entrypoints from plugin configurations. Work towards #53402 Change-Id: If5c6aeb7bc7845311975928fba1cb9c8d273423c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/384681 Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]>
👋 This seems to be actively worked on ; which is cool! But it also makes me re-evaluate how much effort I should spend working on custom_lint. |
Yes, actively working on it, and hope to send out something in the next month or so. Definitely < 6 months 😁 stay tuned. |
This change is the final piece in enabling plugins in the new style to be launched from a specification in analysis options. Work towards #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 <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
I missed the fact that assists were part of the "In the future". Assists are key IMO. I'd say they are critical. |
I'm very open to bumping the pirority on Assists. They "should" be easy to add, as we already have quick fixes. I'll sync with the team on this for MVP. |
This change adds a build target (see utils/analysis_server/BUILD.gn) called 'analysis_server_aot'. This new target is _not_ included in the Dart SDK (the create_sdk target). It's "opt-in" "for development." The name of the new output file matches that of other snapshots (see the dartdevc snapshots). Then we do special work in the plugin manager if "we are AOT." An analysis server running as AOT cannot spawn Isolates from a dart source files; we must first compile a dart source file to AOT as well, then we can spawn an Isolate to that AOT file. _Then_ when we run pub, we can no longer rely on using `Platform.executable`. `dartaotruntime pub get` is not a thing. We must instead find the `dart` tool on disk. To do that, we copy some complex discovery code from dartdev. Work towards #53402 Work towards #53576 Work towards #50498 Manually tested: * [+] analysis_server JIT snapshot works in IDE. * [+] analysis_server JIT snapshot works in IDE, with a legacy plugin (custom_lint). * [+] analysis_server JIT snapshot works at commandline. * [+] analysis_server AOT snapshot works in IDE. * [x] analysis_server AOT snapshot works in IDE, with a legacy plugin (custom_lint) - BROKEN. Need similar work that is done for new plugins. * [x] analysis_server AOT snapshot works at commandline - BROKEN. I think a fair bit of refactoring is required in dartdev lib/src/analysis_server.dart to use `VmInteropHandler.run` or similar. Change-Id: I53173c716fa2a763331ef524a96304f62165810e Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/417942 Commit-Queue: Samuel Rawlins <[email protected]> Reviewed-by: Siva Annamalai <[email protected]>
Tasks needed to deliver a new analyzer plugin system:
Tech preview
dart analyze
and affect the RC #59646General availability
Product excellence
dart fix
.The text was updated successfully, but these errors were encountered: