jfmengels / elm-review

Analyzes Elm projects, to help find mistakes before your users find them.
https://package.elm-lang.org/packages/jfmengels/elm-review/latest/
Other
252 stars 13 forks source link

Add a way to specify a limit to how many fixes are batched together when running --fix-all #50

Closed jfmengels closed 3 years ago

jfmengels commented 3 years ago

A growing frustration I have with automatic fixes is that --fix is kind of too slow/tedious to apply, while --fix-all can take too much time when there are a lot of issues to fix.

In my current case, I'm writing a new rule and testing the automatic fixes on a codebase with over 2000 errors to make sure that the fix won't introduce compiler errors. --fix-all takes a long long time to run and would show me too many instances where the fix introduced errors. --fix works well but when errors become more rare takes me quite a while to go to the next error.

I propose a way to set a batch size for when running --fix-all, which I imagine to look like elm-review --fix-all=20 and elm-review --fix-all 20. I would prefer only the former to work, especially with regards to backwards compatibility (it could swallow arguments that would have been accepted before) but I am not sure whether the arguments parser we use allows for making a distinction between the two. If we can't figure out a way to do that, then we might need to add this only in the next major release or by using a different flag. What do you think?

When run with such an argument, elm-review would apply N fixes (20 in the previous example), give the same report as what you'd get with --fix-all at the moment, then when accepted, start over and apply fixes up to the same limit.

I am not entirely sure what we would need to do if the user refuses the applied fix. We can't put these fixes in a list of refused fixes like we do with --fix because only the first fix is sure to have the same position in the source code as before (the first fix might change the position of the errors reported after it). My current thinking is that refusing the suggested fix will cause elm-review to continue as if it didn't run in watch mode (until a change is being made if --watch is used), just like the current behavior of --fix-all.

I think this would help with introducing a new rule to a codebase too. When there are too many errors, running --fix-all takes a long time and produces a big diff. Limiting the size of the batch makes for a more reviewable diff, which you can look at, accept, commit before continuing in the same manner. You can also stop whenever you want, so you could potentially do this over multiple days.