Skip to content

Commit

Permalink
[Fix #9601] Make Style/RedundantBegin aware of begin blocks aroun…
Browse files Browse the repository at this point in the history
…d memoization

Fixes #9601.

This PR makes `Style/RedundantBegin` aware of redundant `begin`/`end` blocks
around memoization.
  • Loading branch information
koic authored and bbatsov committed Mar 16, 2021
1 parent f18b7b6 commit 1d4959d
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 16 deletions.
@@ -0,0 +1 @@
* [#9601](https://github.com/rubocop/rubocop/issues/9601): Make `Style/RedundantBegin` aware of redundant `begin`/`end` blocks around memoization. ([@koic][])
10 changes: 4 additions & 6 deletions lib/rubocop/cop/mixin/uncommunicative_name.rb
Expand Up @@ -53,12 +53,10 @@ def uppercase?(name)
end

def name_type(node)
@name_type ||= begin
case node.type
when :block then 'block parameter'
when :def, :defs then 'method parameter'
end
end
@name_type ||= case node.type
when :block then 'block parameter'
when :def, :defs then 'method parameter'
end
end

def num_offense(node, range)
Expand Down
29 changes: 26 additions & 3 deletions lib/rubocop/cop/style/redundant_begin.rb
Expand Up @@ -63,6 +63,7 @@ module Style
# end
# end
class RedundantBegin < Base
include RangeHelp
extend AutoCorrector

MSG = 'Redundant `begin` block detected.'
Expand Down Expand Up @@ -95,12 +96,26 @@ def on_kwbegin(node)
private

def register_offense(node)
add_offense(node.loc.begin) do |corrector|
corrector.remove(node.loc.begin)
offense_range = node.loc.begin

add_offense(offense_range) do |corrector|
if any_ancestor_assignment_node?(node)
replace_begin_with_statement(corrector, offense_range, node)
else
corrector.remove(offense_range)
end

corrector.remove(node.loc.end)
end
end

def replace_begin_with_statement(corrector, offense_range, node)
first_child = node.children.first

corrector.replace(offense_range, first_child.source)
corrector.remove(range_between(offense_range.end_pos, first_child.source_range.end_pos))
end

def empty_begin?(node)
node.children.empty?
end
Expand All @@ -114,9 +129,17 @@ def contain_rescue_or_ensure?(node)
def valid_context_using_only_begin?(node)
parent = node.parent

node.each_ancestor.any?(&:assignment?) || parent&.post_condition_loop? ||
valid_begin_assignment?(node) || parent&.post_condition_loop? ||
parent&.send_type? || parent&.operator_keyword?
end

def valid_begin_assignment?(node)
any_ancestor_assignment_node?(node) && !node.children.one?
end

def any_ancestor_assignment_node?(node)
node.each_ancestor.any?(&:assignment?)
end
end
end
end
Expand Down
9 changes: 3 additions & 6 deletions lib/rubocop/ext/regexp_parser.rb
Expand Up @@ -39,9 +39,8 @@ def start_index

# Shortcut to `loc.expression`
def expression
@expression ||= begin
origin.adjust(begin_pos: start_index, end_pos: start_index + full_length)
end
end_pos = start_index + full_length
@expression ||= origin.adjust(begin_pos: start_index, end_pos: end_pos)
end
end

Expand All @@ -60,9 +59,7 @@ def expression
#
# Please open issue if you need other locations
def loc
@loc ||= begin
Map.new(expression, **build_location)
end
@loc ||= Map.new(expression, **build_location)
end

private
Expand Down
41 changes: 40 additions & 1 deletion spec/rubocop/cop/style/redundant_begin_spec.rb
Expand Up @@ -195,7 +195,39 @@ def method
RUBY
end

it 'does not register an offense when using `begin` for or assignment' do
it 'registers and corrects an offense when using `begin` with single statement for or assignment' do
expect_offense(<<~RUBY)
var ||= begin
^^^^^ Redundant `begin` block detected.
foo
end
RUBY

expect_correction(<<~RUBY)
var ||= foo
RUBY
end

it 'registers and corrects an offense when using `begin` with single statement that called a block for or assignment' do
expect_offense(<<~RUBY)
var ||= begin
^^^^^ Redundant `begin` block detected.
foo do |arg|
bar
end
end
RUBY

expect_correction(<<~RUBY)
var ||= foo do |arg|
bar
end
RUBY
end

it 'does not register an offense when using `begin` with multiple statement for or assignment' do
expect_no_offenses(<<~RUBY)
var ||= begin
foo
Expand All @@ -204,6 +236,13 @@ def method
RUBY
end

it 'does not register an offense when using `begin` with no statements for or assignment' do
expect_no_offenses(<<~RUBY)
var ||= begin
end
RUBY
end

it 'does not register an offense when using `begin` with `while`' do
expect_no_offenses(<<~RUBY)
begin
Expand Down

0 comments on commit 1d4959d

Please sign in to comment.