diff --git a/CHANGELOG.md b/CHANGELOG.md index b919cc612a3e..cc4f80ae4447 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ * [#6311](https://github.com/rubocop-hq/rubocop/pull/6311): Prevent `Style/Semicolon` from breaking on single line if-then-else in assignment. ([@drenmi][]) * [#6315](https://github.com/rubocop-hq/rubocop/pull/6315): Fix an error for `Rails/HasManyOrHasOneDependent` when an Active Record model does not have any relations. ([@koic][]) * [#6316](https://github.com/rubocop-hq/rubocop/issues/6316): Fix an auto-correct error for `Style/For` when setting `EnforcedStyle: each` with range provided to the `for` loop without a `do` keyword or semicolon and without enclosing parenthesis. ([@lukasz-wojcik][]) +* [#6326](https://github.com/rubocop-hq/rubocop/issues/6326): Fix an alignment source for `Layout/RescueEnsureAlignment` when using inline access modifier. ([@andrew-aladev][]) ### Changes @@ -3625,5 +3626,6 @@ [@lukasz-wojcik]: https://github.com/lukasz-wojcik [@albaer]: https://github.com/albaer [@Kevinrob]: https://github.com/Kevinrob +[@andrew-aladev]: https://github.com/andrew-aladev [@y-yagi]: https://github.com/y-yagi -[@DiscoStarslayer]: https://github.com/DiscoStarslayer +[@DiscoStarslayer]: https://github.com/DiscoStarslayer \ No newline at end of file diff --git a/lib/rubocop/cop/layout/rescue_ensure_alignment.rb b/lib/rubocop/cop/layout/rescue_ensure_alignment.rb index 59a724433ce8..ef06d9d44549 100644 --- a/lib/rubocop/cop/layout/rescue_ensure_alignment.rb +++ b/lib/rubocop/cop/layout/rescue_ensure_alignment.rb @@ -29,6 +29,7 @@ class RescueEnsureAlignment < Cop '%d, %d.'.freeze ANCESTOR_TYPES = %i[kwbegin def defs class module].freeze RUBY_2_5_ANCESTOR_TYPES = (ANCESTOR_TYPES + [:block]).freeze + ANCESTOR_TYPES_WITH_ACCESS_MODIFIERS = %i[def defs].freeze def on_resbody(node) check(node) unless modifier?(node) @@ -38,6 +39,18 @@ def on_ensure(node) check(node) end + def autocorrect(node) + whitespace = whitespace_range(node) + # Some inline node is sitting before current node. + return nil unless whitespace.source.strip.empty? + + alignment_node = alignment_node(node) + return false if alignment_node.nil? + + new_column = alignment_node.loc.column + ->(corrector) { corrector.replace(whitespace, ' ' * new_column) } + end + def investigate(processed_source) @modifier_locations = processed_source.tokens.each_with_object([]) do |token, locations| @@ -47,50 +60,88 @@ def investigate(processed_source) end end - def autocorrect(node) - whitespace = whitespace_range(node) - return false unless whitespace.source.strip.empty? + private - new_column = ancestor_node(node).loc.end.column - ->(corrector) { corrector.replace(whitespace, ' ' * new_column) } + # Check alignment of node with rescue or ensure modifiers. + + def check(node) + alignment_node = alignment_node(node) + return if alignment_node.nil? + + alignment_loc = alignment_node.loc.expression + kw_loc = node.loc.keyword + + return if + alignment_loc.column == kw_loc.column || + alignment_loc.line == kw_loc.line + + add_offense( + node, + location: kw_loc, + message: format_message(alignment_node, alignment_loc, kw_loc) + ) end - private + def format_message(alignment_node, alignment_loc, kw_loc) + format( + MSG, + kw_loc: kw_loc.source, + kw_loc_line: kw_loc.line, + kw_loc_column: kw_loc.column, + beginning: alignment_source(alignment_node, alignment_loc), + begin_loc_line: alignment_loc.line, + begin_loc_column: alignment_loc.column + ) + end - def check(node) + def alignment_source(node, starting_loc) + ending_loc = + case node.type + when :block, :kwbegin + node.loc.begin + when :def, :defs, :class, :module + node.loc.name + else + # It is a wrapper with access modifier. + node.child_nodes.first.loc.name + end + + range_between(starting_loc.begin_pos, ending_loc.end_pos).source + end + + # We will use ancestor or wrapper with access modifier. + + def alignment_node(node) ancestor_node = ancestor_node(node) - alignment_loc = ancestor_node.loc.expression - kw_loc = node.loc.keyword + return nil if ancestor_node.nil? - return if alignment_loc.column == kw_loc.column - return if alignment_loc.line == kw_loc.line + access_modifier_node = access_modifier_node(ancestor_node) + return ancestor_node if access_modifier_node.nil? - add_offense(node, - location: kw_loc, - message: format_message(kw_loc, - alignment_loc, - ancestor_node)) + access_modifier_node end - def format_message(kw_loc, alignment_loc, ancestor_node) - format(MSG, - kw_loc: kw_loc.source, - kw_loc_line: kw_loc.line, - kw_loc_column: kw_loc.column, - beginning: alignment_source(ancestor_node), - begin_loc_line: alignment_loc.line, - begin_loc_column: alignment_loc.column) + def ancestor_node(node) + ancestor_types = + if target_ruby_version >= 2.5 + RUBY_2_5_ANCESTOR_TYPES + else + ANCESTOR_TYPES + end + + node.each_ancestor(*ancestor_types).first end - def alignment_source(node) - ending = case node.type - when :def, :defs, :class, :module - node.loc.name - when :block, :kwbegin - node.loc.begin - end + def access_modifier_node(node) + return nil unless + ANCESTOR_TYPES_WITH_ACCESS_MODIFIERS.include?(node.type) + + access_modifier_node = node.ancestors.first + return nil unless + access_modifier_node.respond_to?(:access_modifier?) && + access_modifier_node.access_modifier? - range_between(node.loc.expression.begin_pos, ending.end_pos).source + access_modifier_node end def modifier?(node) @@ -100,21 +151,11 @@ def modifier?(node) end def whitespace_range(node) - begin_pos = node.loc.keyword.begin_pos + begin_pos = node.loc.keyword.begin_pos current_column = node.loc.keyword.column range_between(begin_pos - current_column, begin_pos) end - - def ancestor_node(node) - types = if target_ruby_version >= 2.5 - RUBY_2_5_ANCESTOR_TYPES - else - ANCESTOR_TYPES - end - - node.each_ancestor(*types).first - end end end end diff --git a/spec/rubocop/cop/layout/rescue_ensure_alignment_spec.rb b/spec/rubocop/cop/layout/rescue_ensure_alignment_spec.rb index 3057c610b8a4..ef063d50fa21 100644 --- a/spec/rubocop/cop/layout/rescue_ensure_alignment_spec.rb +++ b/spec/rubocop/cop/layout/rescue_ensure_alignment_spec.rb @@ -434,4 +434,249 @@ def foo RUBY end end + + context 'allows inline access modifier' do + let(:cop_config) do + { + 'Style/AccessModifierDeclarations' => + { 'EnforcedStyle' => 'inline' } + } + end + + context 'rescue with def' do + it 'registers an offense' do + expect_offense(<<-RUBY.strip_indent) + private def test + 'foo' + rescue + ^^^^^^ `rescue` at 3, 2 is not aligned with `private def test` at 1, 0. + 'baz' + end + RUBY + end + + it 'correct alignment' do + expect_no_offenses(<<-RUBY.strip_indent) + private def test + 'foo' + rescue + 'baz' + end + RUBY + end + + it 'corrects the alignment' do + new_source = autocorrect_source(<<-RUBY.strip_indent) + private def test + 'foo' + rescue + 'baz' + end + RUBY + + expect(new_source).to eq(<<-RUBY.strip_indent) + private def test + 'foo' + rescue + 'baz' + end + RUBY + end + end + + context 'rescue with defs' do + it 'registers an offense' do + expect_offense(<<-RUBY.strip_indent) + private def Test.test + 'foo' + rescue + ^^^^^^ `rescue` at 3, 2 is not aligned with `private def Test.test` at 1, 0. + 'baz' + end + RUBY + end + + it 'correct alignment' do + expect_no_offenses(<<-RUBY.strip_indent) + private def Test.test + 'foo' + rescue + 'baz' + end + RUBY + end + + it 'corrects the alignment' do + new_source = autocorrect_source(<<-RUBY.strip_indent) + private def Test.test + 'foo' + rescue + 'baz' + end + RUBY + + expect(new_source).to eq(<<-RUBY.strip_indent) + private def Test.test + 'foo' + rescue + 'baz' + end + RUBY + end + end + + context 'ensure with def' do + it 'registers an offense' do + expect_offense(<<-RUBY.strip_indent) + private def test + 'foo' + ensure + ^^^^^^ `ensure` at 3, 2 is not aligned with `private def test` at 1, 0. + 'baz' + end + RUBY + end + + it 'correct alignment' do + expect_no_offenses(<<-RUBY.strip_indent) + private def test + 'foo' + ensure + 'baz' + end + RUBY + end + + it 'corrects the alignment' do + new_source = autocorrect_source(<<-RUBY.strip_indent) + private def test + 'foo' + ensure + 'baz' + end + RUBY + + expect(new_source).to eq(<<-RUBY.strip_indent) + private def test + 'foo' + ensure + 'baz' + end + RUBY + end + end + + context 'ensure with defs' do + it 'registers an offense' do + expect_offense(<<-RUBY.strip_indent) + private def Test.test + 'foo' + ensure + ^^^^^^ `ensure` at 3, 2 is not aligned with `private def Test.test` at 1, 0. + 'baz' + end + RUBY + end + + it 'correct alignment' do + expect_no_offenses(<<-RUBY.strip_indent) + private def Test.test + 'foo' + ensure + 'baz' + end + RUBY + end + + it 'corrects the alignment' do + new_source = autocorrect_source(<<-RUBY.strip_indent) + private def Test.test + 'foo' + ensure + 'baz' + end + RUBY + + expect(new_source).to eq(<<-RUBY.strip_indent) + private def Test.test + 'foo' + ensure + 'baz' + end + RUBY + end + end + end + + context 'allows inline expression before' do + context 'rescue' do + it 'registers an offense' do + expect_offense(<<-RUBY.strip_indent) + def test + 'foo'; rescue; 'baz' + ^^^^^^ `rescue` at 2, 9 is not aligned with `def test` at 1, 0. + end + + def test + begin + 'foo'; rescue; 'baz' + ^^^^^^ `rescue` at 7, 11 is not aligned with `begin` at 6, 2. + end + end + RUBY + end + + it 'does not correct alignment' do + source = <<-RUBY.strip_indent + def test + 'foo'; rescue; 'baz' + end + + def test + begin + 'foo'; rescue; 'baz' + end + end + RUBY + + new_source = autocorrect_source(source) + expect(new_source).to eq(source) + end + end + + context 'ensure' do + it 'registers an offense' do + expect_offense(<<-RUBY.strip_indent) + def test + 'foo'; ensure; 'baz' + ^^^^^^ `ensure` at 2, 9 is not aligned with `def test` at 1, 0. + end + + def test + begin + 'foo'; ensure; 'baz' + ^^^^^^ `ensure` at 7, 11 is not aligned with `begin` at 6, 2. + end + end + RUBY + end + + it 'does not correct alignment' do + source = <<-RUBY.strip_indent + def test + 'foo'; ensure; 'baz' + end + + def test + begin + 'foo'; ensure; 'baz' + end + end + RUBY + + new_source = autocorrect_source(source) + expect(new_source).to eq(source) + end + end + end end