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.57k forks source link

Support running the analyzer in the context of a specific configuration #49009

Open jacob314 opened 2 years ago

jacob314 commented 2 years ago

The analyzer supports config specific imports but the analysis server does not provide a way to change which config to use so IDE users always analyze with the default config which is not as useful as the config(s) from the platform(s) a user will run their app on.

Possible solutions for this could be:

  1. setting the config to analyze either as part of the analysis_options.yaml file
  2. providing a way to change the config dynamically by calling an analysis server api.
  3. Infeasible to implement: generate analysis errors for all configurations.

It is ok that changing the config to use requires reanalysis of the entire project. Only users actively editing config specific import code should need to change the config and normally a default to one of the real platforms should be acceptable. We'll need a good blog post explaining how to use this advanced functionality. The workflow would probably be that users have a way to switch the analyzer to use the config specific import that matches the config specified by a dart or Flutter run configuration. IDEs could also show a toast offering to suggest changing the config to analyze so that the current config specific import file being edited is analyzed.

For context see: https://github.com/dart-lang/sdk/issues/46264#issuecomment-1124501196 This is currently a case where compilers show more actionable warnings and errors than the analyzer.

bwilkerson commented 2 years ago
  1. setting the config to analyze either as part of the analysis_options.yaml file

Do we think that teams will want to share this configuration information, or will this more likely be a per-user setting? I'd guess the latter, but am interested in other opinions.

If it's a per-user setting, then we might want to put this information in a different file that isn't committed.

  1. providing a way to change the config dynamically by calling an analysis server api.

That's a possibility, but we'd need to

The workflow would probably be that users have a way to switch the analyzer to use the config specific import that matches the config specified by a dart or Flutter run configuration.

That sounds like it would work in an IDE, but I don't think they'd work well from the command-line (unless by "run configuration" you mean something different that what IDEs typically support).

pattobrien commented 2 years ago

Do we think that teams will want to share this configuration information, or will this more likely be a per-user setting? I'd guess the latter, but am interested in other opinions.

Another use case that I find lacking an intuitive solution is wanting different lints based on development phase (e.g. release branches vs feature branches). Take for example prefer_const_* and unnecessary_const lints, which are recommended for Flutter for performance optimization. During initial Widget prototyping, it becomes rather annoying and tedious to deal with so many performance-based optimizations one after another after another. And when it comes to (for example) Columns/Rows/ListViews at the center of many Screens, making a list of children widgets a constant requires you to then remove individually declared constants from any children. Errors/quick fixes could be utilized dozens of times per screen during this initial scaffolding stage.

It would be much more intuitive to integrate different analysis options based on something like development branches or even based on dev machine vs CI/CD machine. That way, I could still enforce the importance of these lint rules with my entire team at the times that make the most sense.

That said, workarounds do exist, they're just not elegant and sort of make certain rules feel more "forced" at certain times of the dev process more than others.

jacob314 commented 2 years ago

For prefer_const and unnecessary_const, we could offer a workflow where on file save, quick fixes for those lints are automatically applied for the best of both worlds. Are there other lints you would want off for your feature branches?

pattobrien commented 2 years ago

Off the top of my head, I can't think of anything else in the sdks that would be better served as an alternating configuration or alternate workflow. I'll sleep on this though.

Regarding analysis_plugin, I'm working on a proof of concept that would allow specialized lints/fixes to be packaged individually and bootstrapped to the analyzer dynamically (using analyzer plugin, similar to other recent custom lint packages). My intention is to see if there's greater interest in more package- or project-specific lints/fixes, no matter how opinionated, strict, or one-off. (discussion)

As food for thought, these are 2 of my proof of concept use cases I see a lot of potential for:

So +1 that having this API accessible via the analyzer_plugin would be appreciated.