Rule for no "NoOp" expressions

[MartinSStewart]

What the rule should do: Remove code that is effectively a noop.

Example of things the rule would report:

x == True
x /= False
(\x -> x) -- So long as removing this doesn't cause there to be a syntax error.
1 * x
0 + x
"" ++ x

Example of things the rule would not report:

(x) -- elm-format already does a good job of removing unneeded parenthesis and sometimes you want to keep them for grouping stuff

I am looking for:
Ideally someone who will do the work for me :)
But otherwise this is just so I don't forget about this so I can get around to doing it sometime.

I've also posted the issue here jfmengels/elm-review-simplify#1 as I think this would be a good place for this rule to exist.

[lydell]

It could also make sense to reduce the scope. My guess would be that only looking for == True would catch 80% or more of all cases.

Maybe it would make sense to have rule that only deals with True/False together with ==//=? It could have a focus on being really friendly and teaching to beginners – both Elm beginners, and programming beginners. It’s common to think that you have to have a ==, /=, <, >= etc in your if condition as a beginner, until you learn that the value there just has to be True to move on to the then branch.

• x == True, True == x, x /= False, False /= x – recommend x.
• x == False, False == x, x /= True, True /= x – recommend not (x). Even better: if inside an if condition, recommend flipping the then and else: if x == False then a else b -> if x then b else a. Might be tricky in long else if chains, though.

[MartinSStewart]

True, the == True stuff could be its own rule.

I'm not certain about recommending not (x) though. I think there might be cases where x == False reads better than not (x). Something like

if someFunction "a" 5 15 == False then 

vs

if not (someFunction "a" 5 15) then

or

if someFunction "a" 5 15 |> not  then

Good point about swapping the order of the sub-expressions in the if then else though. I think it's achievable if the rule isn't applied to any chained if expressions.

[sparksp]

Could you recommend that (\x -> x) be replaced by identity? If they're using a function then they probably want a function, it might highlight that there's some pointless work being done around it, but it could also come up in parsers when you want to succeed identity.

Other useless maths that might also fit... 0 * x, x // 1, x / 1

Here's a bunch of unpublished rules that would cover cons and concat of lists, the later two have some pretty good fixes baked in too!

• NoListLiteralsConcat
• NoRedundantConcat
• NoRedundantCons

With regards to the not discussion, you could have a separate rule that reports == False et al, then it can be enabled if someone wants that rule or ignored otherwise. I personally prefer not to compare something with True or False.

[lydell]

For more simplification ideas, check out the fsharplint hints: https://github.com/fsprojects/FSharpLint/blob/0007ec1e83a1043d74bcd5a0067c6483657faa26/src/FSharpLint.Core/DefaultConfiguration.json#L271-L397

[sparksp]

I agree that you could highlight if expressions that could be flipped. Would you still want to report a case with else if?

Failure

if something == False then -- or: if not something then
    bish
else if somethingElse then
    bash
else
    bosh

Success

if something then
    if somethingElse then
        bash
    else
        bosh
else
    bish

[sparksp]

This leads to the possibility of simplifying more complicated boolean expressions, but perhaps that's a discussion for another issue? E.g., (A + ^AB) => (A + B)

[lydell]

I think flipping if only makes sense for “standalone” ifs, that aren’t the else part of another if or haven’t got an if as their else.