jfmengels / elm-review-unused

Provides elm-review rules to detect unused elements in your Elm project
https://package.elm-lang.org/packages/jfmengels/elm-review-unused/latest/
BSD 3-Clause "New" or "Revised" License
23 stars 12 forks source link

Make autofix for NoUnused.Patterns simplify the patterns more in one go. #24

Open jfmengels opened 3 years ago

jfmengels commented 3 years ago

Currently, NoUnused.Patterns simplifies the patterns in the autofix. Meaning that you go from

( { a, b }, _ )

to (assuming neither a nor b is used)

( _, _ )        -- step 1
-- then
_               -- step 2

I think it would be nice if the autofix went straight to step 2 in this case. In doing so, we would waste less of the user's time by having him wait for the second prompt when running with --fix, and make --fix-all faster by skipping one (or more) fix cycles.

I think that each fix should only fix a single report at a time. So if you have ( a, b ) and both are unused, then if should probably be fixed to ( _, b ) then to _. But since we already report and remove both a and b when they are matched with { a, b }, I'm open to someone changing my mind.

For NoUnused.Variables reporting imports, I have so rarely encountered a case where I did not want to remove all of the imports at once. So for that one, I am enclined to have each report autofix the import to be as minimal as possible in one go, instead of letting each cycle do its job.

I would love for someone to work on what I proposed for NoUnused.Patterns, and I would also want to know what you think on whether to batch the fix for multiple reports in one go for NoUnused.Patterns and NoUnused.Variables.