Simplify code with elm-review

I released a new version of elm-review-unused, and also an entirely new elm-review package named elm-review-simplify. I hope you find them useful :blush:

15 Likes

I released v1.0.2, which will now do these additional kinds of simplifications:

### Maybe

    Maybe.map identity x
    --> x

    Maybe.map f Nothing
    --> Nothing

    Maybe.withDefault x Nothing
    --> x

    Maybe.withDefault x (Just y)
    --> y


### Lists

    List.partition fn []
    --> ( [], [] )

    List.partition (always True) list
    --> ( list, [] )

### Set

    Set.isEmpty Set.empty
    --> True

    Set.member x Set.empty
    --> False

    Set.fromList []
    --> Set.empty

    Set.toList Set.empty
    --> []

    Set.length Set.empty
    --> 0

    Set.intersect Set.empty set
    --> Set.empty

    Set.diff Set.empty set
    --> Set.empty

    Set.diff set Set.empty
    --> set

    Set.union set Set.empty
    --> set

    Set.insert x Set.empty
    --> Set.singleton x

    Set.partition fn Set.empty
    --> ( Set.empty, Set.empty )

    Set.partition (always True) set
    --> ( list, Set.empty )


### Dict

    Dict.isEmpty Dict.empty
    --> True

    Dict.fromList []
    --> Dict.empty

    Dict.toList Dict.empty
    --> []

    Dict.size Dict.empty
    --> 0

And a lot more will be coming in the future as this rule is still far from done.

I really like that it will now do this kind of change for Maybes:

Screenshot from 2021-04-28 09-47-10

(see here for a step-by-step breakdown of the change: https://twitter.com/jfmengels/status/1387161093676666884)

1 Like

I wonder if there is a possible mechanism that packages could contribute simplifications based on their own domain knowledge?

For instance in elm-visualization, the following simplification would work:

Axis.left [ Axis.ticks [ 3, 4, 5], Axis.tickCount 10 ] xScale

-->

Axis.left [ Axis.ticks [ 3, 4, 5] ] xScale

however, including that in the package wouldn’t make much sense as many projects don’t really need it.

That looks like a bad api. It should be impossible to pass those two values. Making impossible states impossible, yada yada.

This is just an example.

There is nothing impossible about this. It’s merely redundant. Just as calling List.map identity [1,2,3] is.

Yeah. I can think of are several ways:

  1. Create a new elm-review package with a simplification rule (elm-review-visualization, with a Visualization.Simplify rule or something like that). You could take a look at how Simplify does it.
  2. Simplify could take a list of simplification “plugins” as the argument, which could be provided by elm-review packages or through custom in your review/ folder`.

I was leaning towards 2 when I initially started, but there were some drawbacks. First is that it puts more burden on the user to configure elm-review, and second is that it requires nice abstractions so that writing a list of simplifications is easy.

At the moment, I have a few nice abstractions/setups with which it only takes me a few minutes to add a new simplification (documentation and tests included), that’s why there are already more than a hundred kinds of simplifications. But at the same time I know that some things I don’t handle at all or that require relatively custom implementations, so I’m not at a point where I can accept a plugin system. So maybe later, if somehow it makes sense.

So in short, for now go for solution 1.

1 Like

I’m reading docs for elm-visualization and I see:

Pass a list of ticks to be rendered explicitely.

and

How many tickmarks to approximately generate.

So if I pass 5 ticks and pass count as 10 I can’t get consistent result. And as you could see in the code of Axis.elm, ticks overrides value of result of applying tickCount to Scale.ticks.

This topic was automatically closed 10 days after the last reply. New replies are no longer allowed.