Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix #9601] Make Style/RedundantBegin aware of begin blocks around memoization #9602

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -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