Add support for "fix all" in the analysis server

[DanTup]

Related to discussion in Dart-Code/Dart-Code#1393 (and a little #37227). Having a server action (similar to "sort members" of format) that can apply all minor fixes in a file (in a set of edits from a single call) would allow people to set it up to run on-save (eg. via VS Code's support).

In many cases these might by the same as the preferred fixes in #37227, though I'm not sure if always. It'd certainly be a nice way to fix up a lot of lints (for ex. if you enable always_specify_types and this just fixed all the places they were missing).

[bwilkerson]

We have a command-line tool called dartfix that does something similar. It currently uses an experimental server protocol, so this is potentially doable. The dartfix tool only applies fixes (it doesn't apply formatting or sorting changes) which makes it much easier to merge the results of multiple edits, but even then there are some interesting challenges around things like not adding multiple imports for the same library.

One interesting design question that we still haven't answered for dartfix is which fix to apply in the cases where there are multiple supported fixes. For something like always_specify_types there's really only one good way to fix it, but for many diagnostics there are multiple valid fixes available and deciding which to apply is a harder question.

Another interesting design question is how to control which problems to fix. Do all users want members sorted? Which lints should be fixed? For dartfix we're allowing users to specify on the command line the set of fixes to apply. We have a default set that we use if none are specified, but I'm not sure how well the default set matches what our users want. For something like this, would we decide for users what to fix, or would we have some way for users to configure it (maybe in the analysis options file)?

[DanTup]

I looked at dartfix when I opened this, but the description says:

dartfix is a command-line tool for migrating your Dart code to use newer syntax styles.

so I wasn't sure if this were related (though maybe migrating to new syntax is just a set of particular fixes).

One interesting design question that we still haven't answered for dartfix is which fix to apply in the cases where there are multiple supported fixes.

I think this relates a little to #37227. I think if there isn't an clear best fix, we shouldn't apply any (at least in the case for things like VS Code's "fix all" that can be run on-save.. it might be different in an interactive setting where the user could express a preference).

Do all users want members sorted?

If they're enabled a lint that raises warnings when things are not sorted, I would say sorting them seems like the obvious fix the user would do, so we should do it. That said, sort members is more disruptive than most, and I think in VS Code they can already set that to run on-save, so we might want to leave it out (which might mean allowing the IDE to provide exclusions to the server or something).

Which lints should be fixed?

I don't know these will enough to be confident in this answer, but I'd generally say if a user has enabled a lint that's what they want. Not wanting it is probably the exception (for me, I tend to enable all lints, then when I see a few warnings on code I think is fine, I'll just turn a lint off).

If we got lots of reports from users that said they want to use auto-fix-on-save but there are some lints that they want enabled, but not auto-fixed (which doesn't sound like it should be that common to me) we could always extend analysis options with a list of "lints to exclude from auto fix" (this seems less maintenance than duping the whole list of lints for which ones should auto-fix)?

[bwilkerson]

dartfix is primarily focused on migration, but so far most of that work has been done by applying fixes / assists to compute the edits. My comment wasn't intended to suggest that we don't need this functionality because we have dartfix, it was intended to point to prior art as a proof that the work is at least potentially feasible. Sorry if that wasn't clear.

We might even be able to share a single server protocol between the two uses if we're careful with the design.

[DanTup]

I didn't take it to mean that, I think pointing to it makes sense (I think when I read the description I dismissed it too quickly). There does seem like a lot of overlap here.., for ex:

• new feature: don't need new/const
• lint: don't use new/const where not needed
• auto-fix: remove new/const where not needed

Whether all syntax migrations would work as a lint to enforce using the new one (and a fix), I'm not sure - but it does seem like there's a lot that could be shared here :-)

[DanTup]

For VS Code when using LSP, there's now a Fix All command that fixes anything "dart fix" would for the current file (it can also be set to run on-save):

85d455f

That takes care of my original request here, though I'm not sure if this should remain open to add something similar for IntelliJ/AS?

[bwilkerson]

I believe that it's working in IntelliJ as well. I just tried it using the following test case:

void f(int a, int b) {
  (a + b);
  (b + a);
}

and I got the expected "fix in file" item in the menu.

As a result, I'm going to close the issue.