Analyzer plugins are not supported in _inner_ analysis options.

[srawlins]

Right now, with the new analyzer system, the analysis server only searches an analysis options file at the context root for plugins. I don't think we have a published document about what an analysis context / context root is. But I think it is essentially the root of a Pub workspace (either the workspace created by a single package, or a workspace created by a parent pubspec files that specifies workspace:).

So you can write a plugins section in an inner analysis options file, and we just don't do anything with it. That's not great. We have some options:

1. Statically report this with a diagnostic. This is easy, and a great solution to offer with the current behavior.
2. Change the behavior! Start up a separate plugin isolate each time we find plugins enabled in any analysis options. This shouldn't be too hard, but the current architecture might make this tricky. Should be doable though. There is a question of whether it is worth it, given it really may accidentally increase folks' DAS memory usage. We specifically went out of our way to ban more than one legacy plugin, because of the accidental memory issues and the perception of memory usage in DAS. The current behavior aligns closer with that "one-legacy-plugin" policy. But maybe we've done enough with the memory issues with the new system that we can get back to the wild west, and allow plugins anywhere.

(There are bigger design possibilities where we look at the whole "context collection" space for all plugin sets, and try to find maximal sets of those sets, and start up a minimum number of isolates to serve all of the contexts. While very cool, this sounds hard, complex, error-prone, and also time-expensive for the user while we seek out all analysis options files and query pub for version constraint compatibilities.)

I think for the MVP release, we should just do 1.

CC @bkonyi @jensjoha @bwilkerson

[dcharkes]

Also, a toplevel analysis_options.yaml applies to all subdirectories. Not only to subdirectories that are part of a pub workspace.

[srawlins]

Also, a toplevel analysis_options.yaml applies to all subdirectories. Not only to subdirectories that are part of a pub workspace.

Ooh. That would be a bigger bug. But I believe you 😅 . I will look into this as well...

[jensjoha]

How about a half-bread where it collects them all and joins them (I'm assuming they're compatible here --- but I guess they should be if it's in a workspace), but only runs the ones the individual analysis_options asked for?

Say we have this directory structure:

.
├── analysis_options.yaml
├── pkg1
│   └── analysis_options.yaml
├── pkg2
│   └── analysis_options.yaml
└── pkg3
    └── analysis_options.yaml

where the root one is the workspace root or whatnot.
Say then also that pkg1 has plugin1, pkg2 has none and pkg3 has plugin3.

• Scenario 1: We then open the entire root dir in our IDE. It should find plugin1 and plugin2 and launch a single isolate. It should only run and report plugin1 stuff for pkg1 stuff and plugin3 stuff for pkg3 stuff.
• Scenario 2: We open pkg1 and pkg2 in our IDE. It should find plugin1 and launch a single isolate. It should only run and report plugin1 stuff for pkg1.
• Scenario 3: We open only pkg2 in our IDE. It should find no plugins and launch no isolates.

Can that be done?

[srawlins]

I think this can be done as well.

(I'm assuming they're compatible here --- but I guess they should be if it's in a workspace)

This would be a good restriction, I think. But hard to enforce. If "compatible" means that we have to create a synthetic package with the plugin1 and plugin3 dependencies, and ask pub if they're compatible, then... that might just be too expensive. If they're not compatible, do we just not start any plugins, reporting an error? We definitely don't want a non-deterministic situation, where two developers who work on the same workspace get different analysis results, because one only opens pkg1 and the other opens the whole workspace. We don't want different results just because you open different roots. If one developer saw a plugin error and the other did not, that would be bad. If one developer saw more plugin lints being reported, and the other did not, that would be bad.

[bwilkerson]

I think for the MVP release, we should just do 1.

I agree. Anything else would likely push out the release until the next stable release.

I don't think we have a published document about what an analysis context / context root is.

See analyzer/doc/tutorial/analysis.md:

An _analysis context_ is a representation of a set of code that should be analyzed using the same sources of metadata (same `package_config.json` file).

If they're not compatible, do we just not start any plugins, reporting an error?

If that isn't the case then I think it will be very hard for users to understand what's going on.

We definitely don't want a non-deterministic situation, where two developers who work on the same workspace get different analysis results ...

+1

We don't want different results just because you open different roots.

+1

The complexity of the design is part of why I don't think we could ship on schedule if we don't do (1) for the MVP.