Check for repeated values in case conditionals

[agrimm]

Repeated values should be checked for in case conditionals, because they're code that shouldn't have any effect (if it does, then that's a bad sign!).

Expected behavior

With the following code, RuboCop warns about "bar" appearing in the second when conditional

case foo
when "bar"
  do_x
when "bar"
  do_y
end

Actual behavior

No offense is raised.

Steps to reproduce the problem

Feature request, not bug.

RuboCop version

Include the output of rubocop -V:

$ rubocop -V
0.42.0 (using Parser 2.3.1.2, running on ruby 2.1.10 x86_64-darwin14.0)

[swcraig]

Hi there,

I am working on this.
I have created a working, spec'd out cop for redundant conditional values as mentioned above.

Should I be dealing with multi-value cases (such as those that I mention below)? Or just create a PR for the single-value case expressions as noted by @agrimm ?

I am running into a bit of trouble dealing multi-value cases such as

case x
when 'a', 'b'
  first_method
when 'b', 'a'
  second_method
end

and,

result =
  case x
  when 'a', 'b' then 'some_string'
  when 'b', 'a', 'c' then 'another_string
  end

Specifically I am not sure how to "extract" the when conditions so that I can perform some validation logic only on them, and not the result statements.
For example, for the second case, the node I receive from the on_case() method gives me something such as

(when
  (str 'a')
  (str 'b')
  (str 'some_string'))
 # etc...

How am I supposed to differentiate between the when conditions and the result? Does anyone have any tips?

I am new to contributing to rubocop but would very much like to get started, any help is appreciated.

[Drenmi]

@swcraig: The when node body is always a single node (and the last one.) You can use this fact to extract the expression nodes. 😀

Example:

def when_expressions(when_node)
  when_node.to_a[0...-1]
end

I prototyped this cop last night, but I'd much rather see a new contributor take this up. Feel free to ping me if you have any more questions. 😀

[swcraig]

Awesome @Drenmi , thank you!

The node structures makes more sense now, and I have multi-value cases working regardless of ordering.

I feel that the following should maybe be a new issue (it is checking for redundant operational logic, rather than just checking for repeated values in case conditionals); I have resolved the case presented in the original comment including multi-value.

I can create a PR right now that fixes this issue unless the following should also be addressed:

I am now considering expressions with logical operators and all of the different cases that these would bring...
Is there a simple built-in way to check for equality between logical expressions?
For example:

case x
when a && b
  first
when b && (a || true)
  second
end

This is obviously a very simple, contrived case, but could become very complex using multiple operators and different ordering of variables.

I would need to be checking whether the when expressions (or "chunks" of them) are logically equivalent and thus repeated throughout the case expression.

Is this out of scope and should be a new ticket?

[swcraig]

Disregard the previous commit.
Clearly I am trying to figure out this git workflow and the above is not related at all.

[swcraig]

Does this PR look correct?

[Drenmi]

Is there a simple built-in way to check for equality between logical expressions?

I think this is a bit out of scope for this cop. It is neat that we get the root node of the expressions, and can compare them directly. It makes the cop easy to understand, and I would personally be happy keeping it to that right now. 😀

(I vaguely recall there being a suggestion for a cop checking for ambiguous or redundant predicates.)

[swcraig]

@Drenmi Thank you again for all your help on this!