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

Error rework #138

Closed lue-bird closed 12 months ago

lue-bird commented 1 year ago

Refactoring simplify is a bottomless pit. In other languages, I think I wouldn't even bother starting but in elm it's fun :white_flower:

changes

cool stuff this PR enables

Adding generic checks for map, andThen etc (mostly thanks to the new WrapperProperties) makes it very easy to apply these checks for Random.andThen, Fuzz.andThen etc.

I've intentionally not added specific cases to existing function checks in this PR* to keep it focused, e.g not

Adding those low-hanging fruits could be done in a future PR.

* I did add 2 things: 1, List.member (List.singleton b) b --> b == b with expectNaN + edited test. Looking at the previous implementation, this seems to have been a bug. I also tested that member and == give the same result for NaN. 2, List.concatMap List.singleton x --> x + tests. Excluding it would have been quite a bit more work.

discussion

git

The technique you showed with stash push and pop worked wonderfully!

funny errors that were fixed

There were some really cursed errors in there, so far my favorite is

{ message = "The result of String.replace will be the original string"
, details = [ "The replacement doesn't haven't any noticeable impact. You can remove the call to String.replace." ]
}
jfmengels commented 1 year ago

All looks great so far :blush:

lue-bird commented 1 year ago

Oki, 444 commits, ready to review!