Merge pull request #10168 from dvandersluis/issue/10165

[Fix #10165] Fix `Layout/DotPosition` false positives when the selector and receiver are on the same line

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][])

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

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

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 = /(?<keyword>\S+).*#/.freeze
+
         def on_new_investigation
           processed_source.comments.each do |comment|
-            next unless offensive?(comment) && (match = line(comment).match(/(?<keyword>\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

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)

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

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