search

jfmengels / elm-review-simplify

Provides elm-review rules to simplify your Elm code
https://package.elm-lang.org/packages/jfmengels/elm-review-simplify/latest/
BSD 3-Clause "New" or "Revised" License
20 stars 9 forks source link

Add expectNaN configuration option #120

Closed jfmengels closed 1 year ago

jfmengels commented 1 year ago

New opt-in configuration option expectNaN which will disable some simplifications when the user indicates their project is likely to use NaN values. This disables the following simplifications:

When expectNaN is not used, n * 0 to 0 will be reported (as before) but will now have an autofix.

I may have missed other places where NaN could be a problem. This will enable working on https://github.com/jfmengels/elm-review-simplify/issues/41 safely.

PR can be reviewed commit by commit.

lue-bird commented 1 year ago

Very minor: For all the errors that don't provide fixes because of the possibility of NaN, it might make sense to add a reminder that the |> Simplify.expectNaN configuration is active and that a fix is only suggested when this setting isn't added.

jfmengels commented 1 year ago

For all the errors that don't provide fixes because of the possibility of NaN, it might make sense to add a reminder that the |> Simplify.expectNaN configuration is active and that a fix is only suggested when this setting isn't added.

If I'm not mistaken, for all the places that care about NaN the error is disabled, not just the fix. But I agree that we should probably mention the configuration if we only skip the fix.