Skip to content

Commit

Permalink
[Fix rubocop#8871] Fix a false positive for Style/RedundantBegin
Browse files Browse the repository at this point in the history
Fixes rubocop#8871.

This PR fixes a false positive for `Style/RedundantBegin`
when using `begin` for method argument or part of conditions.

`begin` keyword may be redundant when using only one expression
in `begin` of each issue case.
However, since it is the unintended case for rubocop#8822, I think it
can be implemented as an enhancement different from this bug fix.
  • Loading branch information
koic committed Oct 9, 2020
1 parent f488821 commit 817e470
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -9,6 +9,7 @@
* [#8869](https://github.com/rubocop-hq/rubocop/issues/8869): Fix a false positive for `Style/RedundantBegin` when using `begin` for or assignment and method call. ([@koic][])
* [#8862](https://github.com/rubocop-hq/rubocop/issues/8862): Fix an error for `Lint/AmbiguousRegexpLiteral` when using regexp without method calls in nested structure. ([@koic][])
* [#8872](https://github.com/rubocop-hq/rubocop/issues/8872): Fix an error for `Metrics/ClassLength` when multiple assignments to constants. ([@koic][])
* [#8871](https://github.com/rubocop-hq/rubocop/issues/8871): Fix a false positive for `Style/RedundantBegin` when using `begin` for method argument or part of conditions. ([@koic][])

## 0.93.0 (2020-10-08)

Expand Down
18 changes: 14 additions & 4 deletions lib/rubocop/cop/style/redundant_begin.rb
Expand Up @@ -85,10 +85,7 @@ def on_block(node)
end

def on_kwbegin(node)
return if node.each_ancestor.any?(&:assignment?) || node.parent&.post_condition_loop?

first_child = node.children.first
return if first_child.rescue_type? || first_child.ensure_type?
return if contain_rescue_or_ensure?(node) || valid_context_using_only_begin?(node)

register_offense(node)
end
Expand All @@ -101,6 +98,19 @@ def register_offense(node)
corrector.remove(node.loc.end)
end
end

def contain_rescue_or_ensure?(node)
first_child = node.children.first

first_child.rescue_type? || first_child.ensure_type?
end

def valid_context_using_only_begin?(node)
parent = node.parent

node.each_ancestor.any?(&:assignment?) || parent&.post_condition_loop? ||
parent&.send_type? || parent&.operator_keyword?
end
end
end
end
Expand Down
27 changes: 27 additions & 0 deletions spec/rubocop/cop/style/redundant_begin_spec.rb
Expand Up @@ -233,6 +233,33 @@ def method
RUBY
end

it 'does not register an offense when using `begin` for method argument' do
expect_no_offenses(<<~RUBY)
do_something begin
foo
bar
end
RUBY
end

it 'does not register an offense when using `begin` for logical operator conditions' do
expect_no_offenses(<<~RUBY)
condition && begin
foo
bar
end
RUBY
end

it 'does not register an offense when using `begin` for semantic operator conditions' do
expect_no_offenses(<<~RUBY)
condition and begin
foo
bar
end
RUBY
end

context '< Ruby 2.5', :ruby24 do
it 'accepts a do-end block with a begin-end' do
expect_no_offenses(<<~RUBY)
Expand Down

0 comments on commit 817e470

Please sign in to comment.