Cop(s) Idea: remove redundant booleans from control flow

[jayqui]

Is your feature request related to a problem? Please describe.

Basic problem & use case

It is possible to run across cases where there are booleans in the antecedents of conditionals (true ? foo : bar). This kind of case can be simplified to return the logical branch that the statement will always results in (foo). A cop that could take care of this might be called Lint/BooleanCondition.

Perhaps sometimes a well-intentioned developer could straightforwardly make this kind of mistake. Anyway, another use case in production code is with what my team calls "fully-enabled feature flags": a bit of conditional code that will forever more evaluate to true, which we can reasonably gsub to just true. If we had Lint/BooleanCondition cop, then we could automate the feature flags away:

# original
feature_flags(:foo_flag_enabled, user) ? render(:new_page) : render(:legacy_page)

# substitute `true` for fully enabled flag
true ? render(:new_page) : render(:legacy_page)

# apply `Lint/BooleanCondition`
render(:new_page)

Similar cases

antecendents that always evaluate to true

Sometimes a conditional can have an antecedent that will always evaluate to true, especially when it contains no variables, e.g. 2 > 1 ? foo : bar, which will always evaluate to foo. Perhaps that kind of case could also be taken care of by a Lint/BooleanCondition cop.

conjunctions and disjunctions

true && foo will always evaluate to foo. true || foo will always evaluate to true. Perhaps such cases should also fit into the scope of Lint/BooleanCondition. If not, then those cases could have their own cops, Lint/BooleanInConjunction and Lint/BooleanInDisjucntion.

unless, case, etc.

Perhaps it goes without saying, but we would want Lint/BooleanCondition to handle the various kinds of conditionals Ruby allows for, including if, the ternary operator, unless, case, and possibly and and or.

Describe the solution you'd like

A solution that could auto-correct away such cases as I will post in the first comment.

[jayqui]

1. Boolean as antecedent of a conditional (Lint/BooleanCondition)

1.1 if/else with true

# Bad
true ? foo : bar

# Bad
if true
  foo
elsif anything
  bar
else
  "never going to get here"
end

# Good 
foo

1.2 if/else with false

# Bad
false ? foo : bar

# Bad
if false
  foo
else
  bar
end

# Good
bar

1.3 if/elsif/else with false as first antecedent

# Bad
if false
  "a" # never going to get here
elsif bar
  "b"
else
  "c"
end

# Good
if bar
  "b"
else
  "c"
end

1.4 if/elsif/else with false as an elsif antecedent

# Bad
if foo
  "a"
elsif false
  "b" # never going to get here
else
  "c"
end

# Good
if foo
  "a"
else
  "c"
end

1.5 if/elsif/else with true as an elsif antecedent

# Bad
if foo
  "a"
elsif true
  "b" 
else
  "c" # never going to get here
end

# Bad
case
when foo then "a"
when true then "b"
else "c" # never going to get here
end

# Good
if foo
  "a"
else
  "b"
end

# Good
case
when foo then "a"
else "b"
end

2. Boolean within conjunction (Lint/BooleanInConjunction)

2.1 true as leading conjunct

# Bad
true && "anything"

# Good
true

2.2 true as following conjunct

# Bad
"anything" && true

# Good
!!"anything"

2.3 false as leading conjunct

# Bad
false && "anything"

# Good
false

2.4 false as following conjunct

# Bad
"anything" && false

# Good
false

2.5 true as middle conjunct

# Bad
foo && true && bar

# Good
foo && bar

2.6 false as middle conjunct

# Bad
foo && false && bar

# Good
false

3. Boolean within disjucntion (Lint/BooleanInDisjucntion)

3.1 true as leading disjunct

# Bad
true || "anything"

# Good
true

3.2 true as following disjunct

# Bad
"anything" || true

# Good
true

3.3 false as leading disjunct

# Bad
false || "anything"

# Good
"anything"

3.4 false as following disjunct

# Bad
"anything" || false

# Good
"anything"

3.5 true as middle disjunct

# Bad
foo || true || bar

# Good
true

3.6 false as middle disjunct

# Bad
foo || false || bar

# Good
foo || bar

[dvandersluis]

Note two of your examples are not strictly equivalent (assuming you don't literally mean a string "anything", but rather allow for a method anything):

# Bad
anything || true

# Good
true

In the first case anything will be called and could have side effects. In the second case, it would not be called.

# Bad
foo || true || bar

# Good
true

In the first case foo will be called (bar will not be so is redundant due to short-circuiting). In the second case, neither would be.

[dvandersluis]

2 > 1 ? foo : bar

Although this is easy to see that it is always true, it is not easy to detect via static analysis as the clause (or any other code) is not evaluated and there are many forms it could take.

[vlad-pisanov]

Sounds like an extension of Lint/LiteralAsCondition which already covers many of these cases:

false ? foo : bar
# W: Lint/LiteralAsCondition: Literal false appeared as a condition.

if false
  "a"
elsif bar
  "b"
else
  "c"
end
# W: Lint/LiteralAsCondition: Literal false appeared as a condition.