dart-lang/sdk

Quick-fixes are not provided over LSP for non-Dart files (pubspec.yaml, analysis_options.yaml and plugin-contributed)

zmtzawqlp opened this issue · 17 comments

This tracker is for issues related to:

  • Analyzer

i make a plugin use analyzer_plugin, and i want to add a fix for a yaml error, but it seems not work at my side. full example is attached, please check it . thanks.

  • Dart SDK Version (dart --version)
    Dart SDK version: 2.17.6 (stable) (Tue Jul 12 12:54:37 2022 +0200) on "macos_x64"

example.zip

I downloaded the project, but couldn't figure out the error. There are 73 errors, which one?

Please include the details of what is wrong, and how would you expect it to be fixed.

I assume it is related to depending on external packages, which is reported in #42159

If it is related to custom_lint, then its code is not provided.

i make a plugin use analyzer_plugin, and i want to add a fix for a yaml error, but it seems not work at my side.

That isn't surprising. If I remember the timing correctly, the analyzer_plugin experiment happened before we had support in the analysis server for generating fixes for issues in YAML files.

Sorry for the inconvenience, but because the plugin support is experimental, we have no plans at this point to add support for YAML files to it. If in the future we decide to support plugins, in one form or another, then we would definitely consider the possibility of allowing equal support for YAML files.

i am follow the doc https://github.com/dart-lang/sdk/blob/master/pkg/analyzer_plugin/doc/tutorial/tutorial.md to build a plugin. the code are in plugin.dart

├─ example
│  ├─ custom_lint
│  │  └─ tools
│  │     └─ analyzer_plugin
│  │        ├─ bin
│  │        │  └─ plugin.dart

i make a plugin use analyzer_plugin, and i want to add a fix for a yaml error, but it seems not work at my side.

That isn't surprising. If I remember the timing correctly, the analyzer_plugin experiment happened before we had support in the analysis server for generating fixes for issues in YAML files.

Sorry for the inconvenience, but because the plugin support is experimental, we have no plans at this point to add support for YAML files to it. If in the future we decide to support plugins, in one form or another, then we would definitely consider the possibility of allowing equal support for YAML files.

thanks for your quick reply, I did it because i found that in dart sdk, try to fix MISSING_NAME error for pubspec.yaml

but it seems not working, so i new this issue to confirm whether it's my fault at my side.
截屏2022-10-27 09 25 09

if we don't support fix files except dart file for now, maybe we should add a doc for this, thanks.

I downloaded the project, but couldn't figure out the error. There are 73 errors, which one?

Please include the details of what is wrong, and how would you expect it to be fixed.

I assume it is related to depending on external packages, which is reported in #42159

If it is related to custom_lint, then its code is not provided.

demo is run on Dart SDK version: 2.17.6 (stable) (Tue Jul 12 12:54:37 2022 +0200) on "macos_x64", i can see a custom lint at my side, but no fix.

截屏2022-10-27 09 29 48

... try to fix MISSING_NAME error for pubspec.yaml ...

Have you considered contributing to the analysis_server package directly? If you have reasons not to do so, that's fine, but if you just hadn't considered the possibility I thought I should at least mention it.

Have you considered contributing to the analysis_server package directly? If you have reasons not to do so, that's fine, but if you just hadn't considered the possibility I thought I should at least mention it.

ok, i just want to confirm whether i wrote wrong code. i will make a pr later for https://github.com/dart-lang/sdk/blob/master/pkg/analyzer_plugin/doc/tutorial/fixes.md

and i have another question, if we define a custom plugin, and add a fix as doc.

class MyPlugin extends ServerPlugin with FixesMixin, DartFixesMixin {
  // ...

  @override
  List<FixContributor> getFixContributors(String path) {
    return <FixContributor>[MyFixContributor()];
  }
}

but in fact, custom errors are not in ResolvedUnitResult.errors , so do i miss any other important info about it?

return result.errors.where((AnalysisError error) {

but in fact, custom errors are not in ResolvedUnitResult.errors, so do i miss any other important info about it?

The diagnostics reported to the client is the only piece of information in ResolvedUnitResult that plugins can contribute to. But to be clear, the ResolvedUnitResult is the result of performing all non-plugin related analyses.

@bwilkerson

https://github.com/dart-lang/sdk/blob/master/pkg/analyzer_plugin/doc/tutorial/fixes.md

so whether the document is not right? The user builds a plugin with FixesMixin, DartFixesMixin, but the custom errors are not coming from here.

The documentation might be considered to be incomplete in that it doesn't mention the fact that the analysis server doesn't send any diagnostics created by one of the plugins to any of the plugins, including the plugin that produced the diagnostic.

It also doesn't discuss the basic architecture of the code base, which is what determines the behavior above. The analyzer package performs basic, built-in, analysis of Dart code. This includes the creation of diagnostics that are required by the specification, additional enabled-by-default warnings, and any lint rules that the client of the analyzer package might have registered.

The analysis_server package is where the majority of the IDE support resides and where the knowledge of plugins lives. The analyzer package has no knowledge of plugins, so it can't return any diagnostics produced by those plugins from its APIs, and ResolvedUnitResult is part of the analyzer package's API.

One implication is that plugins need to remember the diagnostics they produce if they're going to provide fixes for those diagnostics. Another implication is that one plugin can't provide fixes for diagnostics produced by a different plugin. Those implications are also not included in the documentation.

But do remember that the documentation is preliminary, and documents a mechanism that we don't support and actively encourage the community to not use. The current plugin support is merely a proof-of-concept demonstrating that we could build some kind of plugability into our tool chain. If we were going to actually support such a feature, I would want it to be much easier to use than the current prototype.

ok, i got it, thank you for clarification.

Without additional information we're not able to resolve this issue, so it will be closed at this time. You're still free to add more info and respond to any questions above, though. We'll re-open the case if you do. Thanks for your contribution!

(Note: we recently added a 'no response' bot to our repo; it may have closed existing issues with the needs-info label incorrectly. If you think your issue was effected, please feel free to reopen the issue)

it seems that it support in android studio but fail in vscode, does vscode have limitation ?

pq commented

It looks like non-Dart fixes were never implemented in the LSP server. I'll look at fixing this.

I've a fix for this in progress. It adds the servers existing pubspec/analysis_options fixes to LSP, and as a result of no longer skipping fixes for non-Dart files, I expected fixes for non-Dart files from plugins will also show up (although I haven't tested this end-to-end and the caveats about plugin support above still apply).

(I've tweaked the title of this issue to be just about these fixes so when it's closed by the bot it doesn't look like it's intended that custom errors will show up in ResolvedUnitResult, something that is working as intended as described above).