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

[Fix #10136] Update Lint/AssignmentInCondition to not consider assignments within blocks in conditions #10137

Merged
merged 1 commit into from Sep 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/fix_update_lintassignmentincondition_to_not.md
@@ -0,0 +1 @@
* [#10136](https://github.com/rubocop/rubocop/issues/10136): Update `Lint/AssignmentInCondition` to not consider assignments within blocks in conditions. ([@dvandersluis][])
12 changes: 7 additions & 5 deletions lib/rubocop/cop/lint/assignment_in_condition.rb
Expand Up @@ -49,7 +49,7 @@ class AssignmentInCondition < Base
def on_if(node)
return if node.condition.block_type?

traverse_node(node.condition, ASGN_TYPES) do |asgn_node|
traverse_node(node.condition) do |asgn_node|
next :skip_children if skip_children?(asgn_node)
next if allowed_construct?(asgn_node)

Expand Down Expand Up @@ -83,13 +83,15 @@ def skip_children?(asgn_node)
(safe_assignment_allowed? && safe_assignment?(asgn_node))
end

# each_node/visit_descendants_with_types with :skip_children
def traverse_node(node, types, &block)
result = yield node if types.include?(node.type)
def traverse_node(node, &block)
# if the node is a block, any assignments are irrelevant
return if node.block_type?

result = yield node if ASGN_TYPES.include?(node.type)

return if result == :skip_children

node.each_child_node { |child| traverse_node(child, types, &block) }
node.each_child_node { |child| traverse_node(child, &block) }
end
end
end
Expand Down
15 changes: 15 additions & 0 deletions spec/rubocop/cop/lint/assignment_in_condition_spec.rb
Expand Up @@ -90,6 +90,21 @@
expect_no_offenses('return 1 if any_errors? { o = file }.present?')
end

it 'accepts assignment in a block after ||' do
expect_no_offenses(<<~RUBY)
if x?(bar) || y? { z = baz }
foo
end
RUBY
end

it 'registers an offense for = in condition inside a block' do
expect_offense(<<~RUBY)
foo { x if y = z }
^ Use `==` if you meant to do a comparison or wrap the expression in parentheses to indicate you meant to assign in a condition.
RUBY
end

it 'accepts ||= in condition' do
expect_no_offenses('raise StandardError unless foo ||= bar')
end
Expand Down