diff --git a/changelog/change_make_style_redundant_begin_aware_of_memoization.md b/changelog/change_make_style_redundant_begin_aware_of_memoization.md new file mode 100644 index 00000000000..b07bf396675 --- /dev/null +++ b/changelog/change_make_style_redundant_begin_aware_of_memoization.md @@ -0,0 +1 @@ +* [#9601](https://github.com/rubocop/rubocop/issues/9601): Make `Style/RedundantBegin` aware of redundant `begin`/`end` blocks around memoization. ([@koic][]) diff --git a/lib/rubocop/cop/mixin/uncommunicative_name.rb b/lib/rubocop/cop/mixin/uncommunicative_name.rb index 83ebca23278..80ea3b987f1 100644 --- a/lib/rubocop/cop/mixin/uncommunicative_name.rb +++ b/lib/rubocop/cop/mixin/uncommunicative_name.rb @@ -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) diff --git a/lib/rubocop/cop/style/redundant_begin.rb b/lib/rubocop/cop/style/redundant_begin.rb index 41e75034284..6ccf4b3dc09 100644 --- a/lib/rubocop/cop/style/redundant_begin.rb +++ b/lib/rubocop/cop/style/redundant_begin.rb @@ -63,6 +63,7 @@ module Style # end # end class RedundantBegin < Base + include RangeHelp extend AutoCorrector MSG = 'Redundant `begin` block detected.' @@ -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 @@ -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 diff --git a/lib/rubocop/ext/regexp_parser.rb b/lib/rubocop/ext/regexp_parser.rb index d61b1125774..ee8ec38635d 100644 --- a/lib/rubocop/ext/regexp_parser.rb +++ b/lib/rubocop/ext/regexp_parser.rb @@ -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 @@ -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 diff --git a/spec/rubocop/cop/style/redundant_begin_spec.rb b/spec/rubocop/cop/style/redundant_begin_spec.rb index cd5faba2dae..dde33561915 100644 --- a/spec/rubocop/cop/style/redundant_begin_spec.rb +++ b/spec/rubocop/cop/style/redundant_begin_spec.rb @@ -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 @@ -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