flutter-intellij icon indicating copy to clipboard operation
flutter-intellij copied to clipboard

Allow apply multiple fixes from analyser window at once on Intellij Plugin

Open danielgomezrico opened this issue 3 years ago • 6 comments

Use case

Using Flutter Intellij plugin version 58.0.3 it shows the list of warnings and info results from the linter, and it lets you go to each file and gives you suggestions to fix it. The proposal is to allow multiple fixes at the same time from the list of results.

The list is this one: 2021-07-03 11 07 56

The specific issue comes from migrating from flutter 1 to 2 where we get a lot of new warnings and info results, and a lot of them are pretty simple, and without this I need to go to each file and fix it, it would be pretty fast if I could select multiple items at once and click fix, test if it worked and commit.

Proposal

I would love to have an option after I right-click a list of items (like on the gif) and get an option "Fix all of this automatically":

2021-07-03 11 07 56

danielgomezrico avatar Jul 03 '21 16:07 danielgomezrico

I'm removing [flutter_tools] from the title because the specific backend that would implement this functionality is probably the analyzer and not the flutter tool

jonahwilliams avatar Jul 03 '21 16:07 jonahwilliams

I think this issue belongs to https://github.com/flutter/flutter-intellij/issues so transferring it to appropriate repo.

darshankawar avatar Apr 19 '22 12:04 darshankawar

@bwilkerson This just came to my attention. What do you think? Is this an analyzer issue, or could it all be implemented by the Flutter plugin?

stevemessick avatar Apr 25 '22 22:04 stevemessick

Good question, but not necessarily a clear answer.

I don't know much about how that view is implemented, but I know that the fixes produced by the analysis server can be invoked from the context menu when there is a single diagnostic selected. I'm guessing that when a diagnostic is selected that server is asked for the fixes in the same way that it's asked for fixes when the cursor is on the line containing the highlight region for the diagnostic. If so, the view has most of the information needed to support the request.

I say "most" because there's a twist. For a given diagnostic there will sometimes be multiple possible kinds of fixes. In those cases we would probably not want to offer to apply a fix for it without requiring the user to specify which fix to apply. Even when there's only one kind of fix, there can be multiple "fixes" because we'll also offer to ignore the error, either in one place or for the whole file. And if the same kind of diagnostic is reported at multiple locations within the file, there will also be a fix-all-in-file option. Knowing which of multiple options to choose for the user might be error prone. (It might not be as bad as I expect, though.)

Even if we ignore the question of deciding which fix to apply, the view would need to apply the fix for one diagnostic, allow server to re-analyze the world, map the new set of diagnostics back to the old list to determine which diagnostics are still being reported, and then apply the fix for the next diagnostic. That would be required because the edits for two separate fixes are not guaranteed to be able to be applied together. Attempting to apply all the edits in a single pass could easily corrupt the user's code. (For example, if an undefined name is referenced in multiple locations, adding an import would fix it everywhere, but if you applied the fix multiple times you'd get multiple identical imports added.)

This is all kind of ugly because the plugin doesn't have the full context that the server has. Which suggests that, if we decide to support this request, we should consider adding a protocol to the server to compute a single change for an explicit list of diagnostics. That would take a bit of work, but probably not as much as trying to implement the same support in the plugin.

We do have a somewhat similar tool today: dart fix will fix all of the diagnostics we know how to fix across an entire directory. What's being requested in something half-way between the fix-a-single-diagnostic and the fix-everything-in-this-directory support that we already have.

bwilkerson avatar Apr 25 '22 22:04 bwilkerson

Thanks for the details @bwilkerson. Regarding the two complications you mentioned, I would think we should ignore any selected problems that require making a choice, but also choose to apply the fix rather than ignore it, since the user gave us that instruction, but I could see a counter-argument, too. I can't transfer issues to dart-lang/sdk anymore, so maybe I'll add an issue there. That would benefit VS Code, too, at least potentially.

@danielgomezrico Are you aware of the dart fix command that was mentioned above?

stevemessick avatar Apr 25 '22 23:04 stevemessick

Wow nice, thanks for that, I will try it but the issue was related to Intellij

danielgomezrico avatar Apr 26 '22 00:04 danielgomezrico