From 6fd1bfe1ba4d71a80c29fba71866ceda0e701885 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Mon, 28 Sep 2020 22:09:21 +0900 Subject: [PATCH] Fix a false positive for `Style/RedundantCondition` This PR fixes the following incorrect autocorrect for `Style/RedundantCondition` when using assignment by hash key access. ### Before The result of auto-correction is syntax error. ```console % cat example.rb if @columns_cache[table_name] @columns_cache[table_name] else @columns_cache[table_name] = super(table_name) end % bundle exec rubocop -a --only Style/RedundantCondition (snip) Inspecting 1 file C Offenses: example.rb:1:1: C: [Corrected] Style/RedundantCondition: Use double pipes || instead. if @columns_cache[table_name] ... ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 1 file inspected, 1 offense detected, 1 offense corrected % cat example.rb @columns_cache[table_name] || []=(table_name, super(table_name)) % ruby -c example.rb example.rb:1: syntax error, unexpected '=', expecting end-of-input ...olumns_cache[table_name] || []=(table_name, super(table_name... ``` ## After The cop accepts the example case. --- CHANGELOG.md | 1 + lib/rubocop/cop/style/redundant_condition.rb | 6 +++++- spec/rubocop/cop/style/redundant_condition_spec.rb | 10 ++++++++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74a7c5edeb8..460a457cf2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ * [#8825](https://github.com/rubocop-hq/rubocop/issues/8825): Fix crash in `Style/ExplicitBlockArgument` when code is called outside of a method. ([@ghiculescu][]) * [#8354](https://github.com/rubocop-hq/rubocop/issues/8354): Detect regexp named captures in `Style/CaseLikeIf` cop. ([@dsavochkin][]) * [#8830](https://github.com/rubocop-hq/rubocop/issues/8830): Fix bad autocorrect of `Style/StringConcatenation` when string includes double quotes. ([@tleish][]) +* [#8807](https://github.com/rubocop-hq/rubocop/pull/8807): Fix a false positive for `Style/RedundantCondition` when using assignment by hash key access. ([@koic][]) ### Changes diff --git a/lib/rubocop/cop/style/redundant_condition.rb b/lib/rubocop/cop/style/redundant_condition.rb index d6e5284ed83..d048a1fe611 100644 --- a/lib/rubocop/cop/style/redundant_condition.rb +++ b/lib/rubocop/cop/style/redundant_condition.rb @@ -75,7 +75,7 @@ def range_of_offense(node) def offense?(node) condition, if_branch, else_branch = *node - return false if use_if_branch?(else_branch) + return false if use_if_branch?(else_branch) || use_hash_key_assignment?(else_branch) condition == if_branch && !node.elsif? && ( node.ternary? || @@ -88,6 +88,10 @@ def use_if_branch?(else_branch) else_branch&.if_type? end + def use_hash_key_assignment?(else_branch) + else_branch&.send_type? && else_branch&.method?(:[]=) + end + def else_source(else_branch) if require_parentheses?(else_branch) "(#{else_branch.source})" diff --git a/spec/rubocop/cop/style/redundant_condition_spec.rb b/spec/rubocop/cop/style/redundant_condition_spec.rb index 989e4eb8333..3e2ad9db831 100644 --- a/spec/rubocop/cop/style/redundant_condition_spec.rb +++ b/spec/rubocop/cop/style/redundant_condition_spec.rb @@ -42,6 +42,16 @@ RUBY end + it 'registers an offense and corrects when using assignment by hash key access' do + expect_no_offenses(<<~RUBY) + if @cache[key] + @cache[key] + else + @cache[key] = heavy_load[key] + end + RUBY + end + it 'registers an offense and corrects when `raise` without argument parentheses in `else`' do expect_offense(<<~RUBY) if b