Skip to content

Commit

Permalink
Fix false positives for Lint/LiteralAssignmentInCondition
Browse files Browse the repository at this point in the history
This PR fixes the following false positive for `Lint/LiteralAssignmentInCondition`
when a collection lireal contains non-literal elements:

```console
$ echo 'x = 1; if test = [42, x]; end' | be rubocop --stdin test.rb --only Lint/LiteralAssignmentInCondition
Inspecting 1 file
W

Offenses:

test.rb:1:16: W: Lint/LiteralAssignmentInCondition: Don't use literal assignment = [42, x] in conditional,
should be == or non-literal operand.
x = 1; if test = [42, x]; end
               ^^^^^^^^^

1 file inspected, 1 offense detected
```

In cases where non-literal elements are included in the collection, no warning will be displayed.

```console
$ ruby -we 'x = 1; if test = [42, x]; end'
```
  • Loading branch information
koic committed Dec 14, 2023
1 parent 7c80c79 commit 84b0d4d
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#12539](https://github.com/rubocop/rubocop/pull/12539): Fix false positives for `Lint/LiteralAssignmentInCondition` when a collection literal contains non-literal elements. ([@koic][])
17 changes: 12 additions & 5 deletions lib/rubocop/cop/lint/literal_assignment_in_condition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def on_if(node)
next unless asgn_node.loc.operator

rhs = asgn_node.to_a.last
next if !forbidden_literal?(rhs) || parallel_assignment_with_splat_operator?(rhs)
next if !all_literals?(rhs) || parallel_assignment_with_splat_operator?(rhs)

range = offense_range(asgn_node, rhs)

Expand All @@ -59,10 +59,17 @@ def traverse_node(node, &block)
node.each_child_node { |child| traverse_node(child, &block) }
end

def forbidden_literal?(node)
return false if node.dstr_type? || node.xstr_type?

node.respond_to?(:literal?) && node.literal?
def all_literals?(node)
case node.type
when :dstr, :xstr
false
when :array
node.values.all? { |value| all_literals?(value) }
when :hash
(node.values + node.keys).all? { |item| all_literals?(item) }
else
node.respond_to?(:literal?) && node.literal?
end
end

def parallel_assignment_with_splat_operator?(node)
Expand Down
47 changes: 46 additions & 1 deletion spec/rubocop/cop/lint/literal_assignment_in_condition_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,59 @@
RUBY
end

it 'registers an offense when assigning array literal to local variable in `if` condition' do
it 'registers an offense when assigning empty array literal to local variable in `if` condition' do
expect_offense(<<~RUBY)
if test = []
^^^^ Don't use literal assignment `= []` in conditional, should be `==` or non-literal operand.
end
RUBY
end

it 'registers an offense when assigning array literal with only literal elements to local variable in `if` condition' do
expect_offense(<<~RUBY)
if test = [1, 2, 3]
^^^^^^^^^^^ Don't use literal assignment `= [1, 2, 3]` in conditional, should be `==` or non-literal operand.
end
RUBY
end

it 'does not register an offense when assigning array literal with non-literal elements to local variable in `if` condition' do
expect_no_offenses(<<~RUBY)
if test = [42, x, y]
end
RUBY
end

it 'registers an offense when assigning empty hash literal to local variable in `if` condition' do
expect_offense(<<~RUBY)
if test = {}
^^^^ Don't use literal assignment `= {}` in conditional, should be `==` or non-literal operand.
end
RUBY
end

it 'registers an offense when assigning hash literal with only literal elements to local variable in `if` condition' do
expect_offense(<<~RUBY)
if test = {x: :y}
^^^^^^^^^ Don't use literal assignment `= {x: :y}` in conditional, should be `==` or non-literal operand.
end
RUBY
end

it 'does not register an offense when assigning hash literal with non-literal key to local variable in `if` condition' do
expect_no_offenses(<<~RUBY)
if test = {x => :y}
end
RUBY
end

it 'does not register an offense when assigning hash literal with non-literal value to local variable in `if` condition' do
expect_no_offenses(<<~RUBY)
if test = {x: y}
end
RUBY
end

it 'does not register an offense when assigning non-literal to local variable in `if` condition' do
expect_no_offenses(<<~RUBY)
if test = do_something
Expand Down

0 comments on commit 84b0d4d

Please sign in to comment.