dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.23k stars 1.58k forks source link

[analyzer_plugin] Running build_runner does not invoke `analysis.updateContent` with the newly generated files in VScode #54113

Open rrousselGit opened 11 months ago

rrousselGit commented 11 months ago

Hello!

It appears that when generated files are generated using build_runner, analyzer_plugins do not receive a analysis.updateContent matching the generation.

Steps to reproduce:

This means that subsequent getFixes/getsAssists behave as if code-generation did not happen.

Here's a video showcasing the problem.

https://github.com/dart-lang/sdk/assets/20165741/3f9aeaf2-1b0d-4554-a424-c864409dc5e1

rrousselGit commented 11 months ago

It appears that instead of deleting the .g.dart in the IDE, we call mv dependencies.g.dart backup.dart, then the {type: remove} event isn't sent either.

It may be that for all file changes triggered by the terminal, analyzer_plugin isn't notified.

bwilkerson commented 11 months ago

Definitely something we will need to get right in the new plugin implementation.

@srawlins

rrousselGit commented 11 months ago

Definitely something we will need to get right in the new plugin implementation.

Is there a new implementation in progress?

bwilkerson commented 11 months ago

We're in the design phase, and once we have a design we can make a decision about whether we will implement it.

rrousselGit commented 11 months ago

Are you aware of a possible workaround to this issue? It's quite inconvenient for my users, as it causes confusing false positives/negatives in the IDE. I don't mind having to fix it on my end, but I'm just not sure how to get started with that.

bwilkerson commented 11 months ago

Assuming that the server is already watching the files in the directory that build_runner writes to, the fix would be to make sure that those changes are passed along to the plugins.

As for a workaround, that would probably be for each plugin to create their own watcher on that directory and tell the AnalysisContext when the files have been changed. It isn't clear to me that the workaround would be any less work than the fix, though.

rrousselGit commented 11 months ago

I'd love to fix it in the SDK directly. But I'm not sure where to even look at. Do you maybe have any tips on how to get started here?

bwilkerson commented 11 months ago

The analysis.handleWatchEvents request is sent to the plugins from PluginManager.broadcastWatchEvent

I don't know how much that helps, though.

Note that our use of the watcher package is wrapped so that we can use it from the ResourceProvider. That allows us to write in-memory tests that simulate the behavior of the watcher package without having to actually modify the disk. It might make it easier to debug the issue.

rrousselGit commented 7 months ago

About this, is there a document detailing how to use the local clone of analyzer_server in my IDE?

I have tests running and was looking into forking test_concurrentContextRebuilds to debug this. But so far I'm struggling to write a test for this, and would likely need a debugger first.