Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Style/CombinableLoops incorrectly triggers on rails validates_each #8848

Closed
TikiTDO opened this issue Oct 3, 2020 · 2 comments · Fixed by #8851
Closed

Style/CombinableLoops incorrectly triggers on rails validates_each #8848

TikiTDO opened this issue Oct 3, 2020 · 2 comments · Fixed by #8851

Comments

@TikiTDO
Copy link
Contributor

TikiTDO commented Oct 3, 2020

The Style/CombinableLoops cop assumes that any method that ends with _each is an iterator, and then applies the cop logic to the method body. This rule triggers on the rails validates_each method, described here, which is not actually an iterator.

The below code will trigger the cop:

class Example < ActiveRecord::Base
  # Has two attributes :array_values, :other_array_values, both of which are arrays of strings

  validates_each :array_values, :other_array_values do |record, attribute, array_value|
    array_value.each do |array_value_item|
      record.errors.add(attribute, "invalid") unless is_valid_item?(array_value_item)
    end
  end

  def is_valid_item?(item)
    return item !== "invalid"
  end
end

Expected behavior

The Style/CombinableLoops should not necessarily trigger on methods with names that happen to end with _each, since that suffix doesn't necessarily imply that it's an iterator.

Actual behavior

The above code is not actually a combinable loop, because the validates_each is not really an iterator, but simply a method that can apply some validation logic to a value which may in turn be an enumerable. As such it should not trigger this cop.

Steps to reproduce the problem

See example above in the issue description above. There is no way to combine validates_each and the iteration over the array_value array.

RuboCop version

└> bundle exec rubocop -V
0.92.0 (using Parser 2.7.1.5, rubocop-ast 0.7.1, running on ruby 2.7.1 x86_64-linux)
@dvandersluis
Copy link
Member

Style/CombinableLoops was changed in #8731 which seems to have fixed this bug, I can't reproduce this error.

@dvandersluis
Copy link
Member

Actually I did find a case that this does register incorrectly, writing a PR now.

dvandersluis added a commit to dvandersluis/rubocop that referenced this issue Oct 4, 2020
…hen using the same method with different arguments.
@koic koic closed this as completed in #8851 Oct 4, 2020
koic added a commit that referenced this issue Oct 4, 2020
[Fix #8848] Fix a false positive for `Style/CombinableLoops` when using the same method with different arguments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants