sdk icon indicating copy to clipboard operation
sdk copied to clipboard

[analyzer] consider a more gradual reanalysis after `analysis_options` file is changed

Open incendial opened this issue 2 years ago • 2 comments

Saving analysis_options restarts the analysis server completely making it hard to lint analysis options in general (shows stale issues on save), but also consumes resources.

You can repro this when renaming a valid rule to an invalid one in the linter rules section in any analysis_options file. Even when you change the name back.

Background

In our discussion about the analyzer_plugin changes @srawlins wrote:

That's a really good question. I think it is possible to support this, orthogonal to plugin support. Things like, if analysis_options changes due to:

  • a change in enabled experiments, then maybe have to throw everything away and re-analyze from zero
  • a lint rule is removed, drop all reported diagnostics of that rule,
  • a lint rule is added, just run linter code over sources, reporting new diagnostics
  • etc.

and we agreed to move the discussion to the SDK repo.

incendial avatar May 17 '23 15:05 incendial

CC @scheglov @bwilkerson I like this, but I think we've discussed it could be a fair bit of work. Amongst edits to analysis_options.yaml, I imagine:

  • < 1% are to change the enabled_experiments section, so we can reduce carving out new analysis contexts.
  • < 5% are changes in the language section, so we can reduce both restarts.
  • > 90% are to change the linter section, and we can be a little bit smart about "re-linting," since lint rules are executed in their own pass, separate from resolution and compile-time error reporting.
    • I don't know if it would be worth it to try to re-lint on the individual level. For example, if we detect that the change to the file was just to add lint_rule_one, then we could just run the registered visitors for lint_rule_one. That might be a lot of refactoring.
    • Either way, even re-linting all lint rules would require handling removed lint rules. We'd have to take the existing reported diagnostics and remove all lint diagnostics before re-running all lint rules.

srawlins avatar May 17 '23 16:05 srawlins

This works as intended.

  1. Users change analysis_options.yaml files rarely, so it is not worth investing our time into optimizing it.
  2. Identifying fine grained changes in options is hard and error prone. We could do this, but not in a "cavalry attack" way. We would need to rewrite carefully how we parse options, and how we compute diagnostics for them. Then we could do some diffing on these parsed structures, and more specific reactions on changes.

scheglov avatar May 17 '23 19:05 scheglov