diff --git a/changelog/fix_fix_layoutdotposition_false_positives.md b/changelog/fix_fix_layoutdotposition_false_positives.md new file mode 100644 index 00000000000..5722cde04c6 --- /dev/null +++ b/changelog/fix_fix_layoutdotposition_false_positives.md @@ -0,0 +1 @@ +* [#10165](https://github.com/rubocop/rubocop/issues/10165): Fix `Layout/DotPosition` false positives when the selector and receiver are on the same line. ([@dvandersluis][]) diff --git a/lib/rubocop/cop/layout/dot_position.rb b/lib/rubocop/cop/layout/dot_position.rb index b4e2554607e..98da99adc1e 100644 --- a/lib/rubocop/cop/layout/dot_position.rb +++ b/lib/rubocop/cop/layout/dot_position.rb @@ -68,18 +68,14 @@ def message(dot) end def proper_dot_position?(node) - selector_line = selector_range(node).line + selector_range = selector_range(node) - # If the receiver is a HEREDOC and the selector is on the same line - # then there is nothing to do - return true if heredoc?(node.receiver) && node.receiver.loc.first_line == selector_line + return true if same_line?(selector_range, selector_range(node.receiver)) + selector_line = selector_range.line receiver_line = receiver_end_line(node.receiver) dot_line = node.loc.dot.line - # receiver and selector are on the same line - return true if selector_line == receiver_line - # don't register an offense if there is a line comment between the # dot and the selector otherwise, we might break the code while # "correcting" it (even if there is just an extra blank line, treat @@ -124,6 +120,8 @@ def heredoc?(node) end def selector_range(node) + return node unless node.call_type? + # l.(1) has no selector, so we use the opening parenthesis instead node.loc.selector || node.loc.begin end diff --git a/lib/rubocop/cop/layout/space_inside_parens.rb b/lib/rubocop/cop/layout/space_inside_parens.rb index bcde717e469..4f1416e4f04 100644 --- a/lib/rubocop/cop/layout/space_inside_parens.rb +++ b/lib/rubocop/cop/layout/space_inside_parens.rb @@ -146,10 +146,6 @@ def correct_missing_space(token1, token2) end end - def same_line?(token1, token2) - token1.line == token2.line - end - def parens?(token1, token2) token1.left_parens? || token2.right_parens? end diff --git a/lib/rubocop/cop/style/commented_keyword.rb b/lib/rubocop/cop/style/commented_keyword.rb index db06cec1178..f2fea5076e9 100644 --- a/lib/rubocop/cop/style/commented_keyword.rb +++ b/lib/rubocop/cop/style/commented_keyword.rb @@ -52,9 +52,11 @@ class CommentedKeyword < Base ALLOWED_COMMENTS = %w[:nodoc: :yields: rubocop:disable rubocop:todo].freeze ALLOWED_COMMENT_REGEXES = ALLOWED_COMMENTS.map { |c| /#\s*#{c}/ }.freeze + REGEXP = /(?\S+).*#/.freeze + def on_new_investigation processed_source.comments.each do |comment| - next unless offensive?(comment) && (match = line(comment).match(/(?\S+).*#/)) + next unless offensive?(comment) && (match = source_line(comment).match(REGEXP)) register_offense(comment, match[:keyword]) end @@ -76,12 +78,12 @@ def register_offense(comment, matched_keyword) end def offensive?(comment) - line = line(comment) + line = source_line(comment) KEYWORD_REGEXES.any? { |r| r.match?(line) } && ALLOWED_COMMENT_REGEXES.none? { |r| r.match?(line) } end - def line(comment) + def source_line(comment) comment.location.expression.source_line end end diff --git a/lib/rubocop/cop/util.rb b/lib/rubocop/cop/util.rb index 50bb2c256c7..3971c1e150a 100644 --- a/lib/rubocop/cop/util.rb +++ b/lib/rubocop/cop/util.rb @@ -126,10 +126,18 @@ def interpret_string_escapes(string) StringInterpreter.interpret(string) end + def line(node_or_range) + if node_or_range.respond_to?(:line) + node_or_range.line + elsif node_or_range.respond_to?(:loc) + node_or_range.loc.line + end + end + def same_line?(node1, node2) - # rubocop:disable InternalAffairs/LocationLineEqualityComparison - node1.respond_to?(:loc) && node2.respond_to?(:loc) && node1.loc.line == node2.loc.line - # rubocop:enable InternalAffairs/LocationLineEqualityComparison + line1 = line(node1) + line2 = line(node2) + line1 && line2 && line1 == line2 end def indent(node, offset: 0) diff --git a/spec/rubocop/cop/layout/dot_position_spec.rb b/spec/rubocop/cop/layout/dot_position_spec.rb index c13a188e35f..257c6375364 100644 --- a/spec/rubocop/cop/layout/dot_position_spec.rb +++ b/spec/rubocop/cop/layout/dot_position_spec.rb @@ -257,6 +257,16 @@ RUBY end end + + context 'with another method on the same line' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + foo(<<~HEREDOC).squish + something + HEREDOC + RUBY + end + end end context 'when the receiver is a heredoc' do @@ -445,6 +455,16 @@ RUBY end end + + context 'with another method on the same line' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + foo(<<~HEREDOC).squish + something + HEREDOC + RUBY + end + end end context 'when the receiver is a heredoc' do @@ -467,7 +487,7 @@ end context 'when there is a heredoc with a following method' do - it 'does not register an offense' do + it 'does not register an offense for a heredoc' do expect_no_offenses(<<~RUBY) <<~HEREDOC.squish something diff --git a/spec/rubocop/cop/util_spec.rb b/spec/rubocop/cop/util_spec.rb index 6cafec12199..90ef77d0cef 100644 --- a/spec/rubocop/cop/util_spec.rb +++ b/spec/rubocop/cop/util_spec.rb @@ -43,4 +43,37 @@ def some_method it { is_expected.to eq('SupportedStylesInsidePipes') } end end + + describe '#same_line?' do + let(:source) do + <<-RUBY + @foo + @bar + @baz + RUBY + end + + let(:processed_source) { parse_source(source) } + let(:ast) { processed_source.ast } + let(:nodes) { ast.each_descendant(:ivar).to_a } + let(:node1) { nodes[0] } + let(:node2) { nodes[1] } + let(:node3) { nodes[2] } + + it 'returns true when two nodes are on the same line' do + expect(described_class.same_line?(node1, node2)).to eq(true) + end + + it 'returns false when two nodes are not on the same line' do + expect(described_class.same_line?(node1, node3)).to be_falsey + end + + it 'can use ranges' do + expect(described_class.same_line?(node1.loc.expression, node2)).to eq(true) + end + + it 'returns false if an argument is not a node or range' do + expect(described_class.same_line?(node1, 5)).to be_falsey + expect(described_class.same_line?(5, node2)).to be_falsey + end + end end