From a8fdeb09679f5ee85f949ef19d1229d903e27956 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Thu, 14 Dec 2023 00:48:49 +0900 Subject: [PATCH] Fix false positives for `Lint/LiteralAssignmentInCondition` 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' ``` --- ...or_lint_literal_assignment_in_condition.md | 1 + .../lint/literal_assignment_in_condition.rb | 17 +++++-- .../literal_assignment_in_condition_spec.rb | 47 ++++++++++++++++++- 3 files changed, 59 insertions(+), 6 deletions(-) create mode 100644 changelog/fix_false_positives_for_lint_literal_assignment_in_condition.md diff --git a/changelog/fix_false_positives_for_lint_literal_assignment_in_condition.md b/changelog/fix_false_positives_for_lint_literal_assignment_in_condition.md new file mode 100644 index 00000000000..d9df43ef1cb --- /dev/null +++ b/changelog/fix_false_positives_for_lint_literal_assignment_in_condition.md @@ -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][]) diff --git a/lib/rubocop/cop/lint/literal_assignment_in_condition.rb b/lib/rubocop/cop/lint/literal_assignment_in_condition.rb index bc47630f316..a922ba916af 100644 --- a/lib/rubocop/cop/lint/literal_assignment_in_condition.rb +++ b/lib/rubocop/cop/lint/literal_assignment_in_condition.rb @@ -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) @@ -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) diff --git a/spec/rubocop/cop/lint/literal_assignment_in_condition_spec.rb b/spec/rubocop/cop/lint/literal_assignment_in_condition_spec.rb index 5701e5be0dc..c7cb812c607 100644 --- a/spec/rubocop/cop/lint/literal_assignment_in_condition_spec.rb +++ b/spec/rubocop/cop/lint/literal_assignment_in_condition_spec.rb @@ -31,7 +31,7 @@ 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. @@ -39,6 +39,51 @@ 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