Skip to content

Commit

Permalink
Fix a false positive for Style/RedundantCondition
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
koic committed Oct 3, 2020
1 parent 5920918 commit 6fd1bfe
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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

Expand Down
6 changes: 5 additions & 1 deletion lib/rubocop/cop/style/redundant_condition.rb
Expand Up @@ -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? ||
Expand All @@ -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})"
Expand Down
10 changes: 10 additions & 0 deletions spec/rubocop/cop/style/redundant_condition_spec.rb
Expand Up @@ -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
Expand Down

0 comments on commit 6fd1bfe

Please sign in to comment.