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

combine map, then map(N) #284

Open miniBill opened 8 months ago

miniBill commented 8 months ago

Example of things the rule would report:

List.map2 (\x y -> f x y) (List.map g xs) ys
--> List.map2 (\x y -> f (g x) y) xs ys

(and all the variations: mapN, or multiple maps in children)

I am looking for:

lue-bird commented 8 months ago

Sounds fun and also be quite hard to get right. Simple cases like

List.map g (List.map (\x -> f x) list)
--> List.map (\x -> g (f x))

seem alright while for example

List.map (\string -> g string) (List.map String.fromInt list)
--> List.map (\string -> g (String.fromInt string))

plain uses wrong variable names (your example has the same mistake btw, g needs to be a lambda so we have a name for its argument) so we can't reasonably fix it.

And many other more complex situations need to be avoided I'm guessing

List.map2 (\x y -> g x y) (List.map (\y -> f y) aList) bList
--> List.map (\y?? y -> g (f y) y)

List.map (\x -> g x x) (List.map (\y -> f y) aList) bList
--> List.map (\y -> let x = f y in g x x) -- should we do this fix?

List.map2 (\f -> f) aList (List.map g bList)
--> List.map2 (\f -> f << g) aList bList

for the same reason we don't currently simplify

List.map g (List.map f list)
--> List.map (g << f) list

etc for code style reasons

miniBill commented 8 months ago
List.map (\string -> g string) (List.map String.fromInt list)
--> List.map (\string -> g (String.fromInt string)) list

Wait, what is wrong with this transformation?


List.map2 (\x y -> g x y) (List.map (\y -> f y) aList) bList

should go to

List.map2 (\x y -> g x ((\y_ -> f y_) y)) aList bList

I guess? Either that, or not touch it.


List.map (\x -> g x x) (List.map (\y -> f y) aList)

This should not be touched


List.map2 (\f -> f) aList (List.map g bList)

Oh, this is probably also in the "don't touch" category

lue-bird commented 8 months ago
List.map (\string -> g string) (List.map String.fromInt list)
--> List.map (\string -> g (String.fromInt string)) list

Wait, what is wrong with this transformation?

Notice how the code says String.fromInt string. The string argument from the later map refers to the already transformed value.

2 of your suggestions fall into the same problem category

List.map2 (\x y -> f x y) (List.map g xs) ys
--> List.map2 (\x y -> f (g x) y) xs ys

List.map2 (\x y -> g x y) (List.map (\y -> f y) aList) bList
--> List.map2 (\x y -> g x ((\y_ -> f y_) y)) aList bList
miniBill commented 8 months ago

I see! Then maybe it makes sense to just not autofix it except for the simplest cases?