Merge pull request #6702 from abrom/fix-trailing-comma-in-heredoc

[Fix #6701] Trailing comma detection regression with HEREDOC

CHANGELOG.md

@@ -17,6 +17,7 @@
 * [#6382](https://github.com/rubocop-hq/rubocop/issues/6382): Fix `Layout/IndentationWidth` with `Layout/EndAlignment` set to start_of_line. ([@dischorde][], [@siegfault][], [@mhelmetag][])
 * [#6710](https://github.com/rubocop-hq/rubocop/issues/6710): Fix `Naming/MemoizedInstanceVariableName` on method starts with underscore. ([@pocke][])
 * [#6722](https://github.com/rubocop-hq/rubocop/issues/6722): Fix an error for `Style/OneLineConditional` when `then` branch has no body. ([@koic][])
+* [#6702](https://github.com/rubocop-hq/rubocop/pull/6702): Fix `TrailingComma` regression where heredoc with commas caused false positives. ([@abrom][])
 
 ### Changes
 

lib/rubocop/cop/mixin/trailing_comma.rb

@@ -17,11 +17,13 @@ def style_parameter_name
       end
 
       def check(node, items, kind, begin_pos, end_pos)
-        return if heredoc?(items.last)
-
         after_last_item = range_between(begin_pos, end_pos)
 
-        comma_offset = after_last_item.source =~ /,/
+        # If there is any heredoc in items, then match the comma succeeding
+        # any whitespace (except newlines), otherwise allow for newlines
+        comma_regex = any_heredoc?(items) ? /\A[^\S\n]*,/ : /\A\s*,/
+        comma_offset = after_last_item.source =~ comma_regex &&
+                       after_last_item.source.index(',')
 
         if comma_offset && !inside_comment?(after_last_item, comma_offset)
           check_comma(node, kind, after_last_item.begin_pos + comma_offset)
@@ -167,6 +169,10 @@ def avoid_autocorrect?(_nodes)
         false
       end
 
+      def any_heredoc?(items)
+        items.any? { |item| heredoc?(item) }
+      end
+
       def heredoc?(node)
         return false unless node.is_a?(RuboCop::AST::Node)
         return true if node.loc.respond_to?(:heredoc_body)

spec/rubocop/cop/style/trailing_comma_in_arguments_spec.rb

@@ -11,6 +11,14 @@
       RUBY
     end
 
+    it 'registers an offense for trailing comma preceded by whitespace' \
+       ' in a method call' do
+      expect_offense(<<-RUBY.strip_indent)
+        some_method(a, b, c , )
+                            ^ Avoid comma after the last parameter of a method call#{extra_info}.
+      RUBY
+    end
+
     it 'registers an offense for trailing comma in a method call with hash' \
        ' parameters at the end' do
       expect_offense(<<-RUBY.strip_indent)
@@ -57,6 +65,14 @@
       new_source = autocorrect_source('some_method(a, b, c: 0, d: 1, )')
       expect(new_source).to eq('some_method(a, b, c: 0, d: 1 )')
     end
+
+    it 'accepts heredoc without trailing comma' do
+      expect_no_offenses(<<-RUBY.strip_indent)
+        route(1, <<-HELP.chomp)
+        ...
+        HELP
+      RUBY
+    end
   end
 
   context 'with single line list of values' do
@@ -167,6 +183,33 @@
         RUBY
       end
 
+      it 'accepts comma inside a modified heredoc parameter' do
+        expect_no_offenses(<<-RUBY.strip_indent)
+          some_method(
+            <<-LOREM.delete("\\n")
+              Something with a , in it
+            LOREM
+          )
+        RUBY
+      end
+
+      it 'auto-corrects unwanted comma after modified heredoc parameter' do
+        new_source = autocorrect_source(<<-RUBY.strip_indent)
+          some_method(
+            <<-LOREM.delete("\\n"),
+              Something with a , in it
+            LOREM
+          )
+        RUBY
+        expect(new_source).to eq(<<-RUBY.strip_indent)
+          some_method(
+            <<-LOREM.delete("\\n")
+              Something with a , in it
+            LOREM
+          )
+        RUBY
+      end
+
       context 'when there is string interpolation inside heredoc parameter' do
         it 'accepts comma inside a heredoc parameter' do
           expect_no_offenses(<<-RUBY.strip_indent)
@@ -280,7 +323,7 @@
       it 'accepts missing comma after heredoc with comments' do
         expect_no_offenses(<<-RUBY.strip_indent)
           route(
-            <<-HELP.chomp
+            a, <<-HELP.chomp
             ,
             # some comment
             HELP
@@ -437,15 +480,19 @@
         RUBY
       end
 
-      it 'accepts missing comma after a heredoc' do
-        # A heredoc that's the last item in a literal or parameter list can not
-        # have a trailing comma. It's a syntax error.
-        expect_no_offenses(<<-RUBY.strip_indent)
+      it 'auto-corrects missing comma after a heredoc' do
+        new_source = autocorrect_source(<<-RUBY.strip_indent)
           route(1, <<-HELP.chomp
           ...
           HELP
           )
         RUBY
+        expect(new_source).to eq(<<-RUBY.strip_indent)
+          route(1, <<-HELP.chomp,
+          ...
+          HELP
+          )
+        RUBY
       end
 
       it 'auto-corrects missing comma in a method call with hash parameters' \

spec/rubocop/cop/style/trailing_comma_in_array_literal_spec.rb

@@ -100,6 +100,33 @@
                    ]
         RUBY
       end
+
+      it 'accepts HEREDOC with commas' do
+        expect_no_offenses(<<-RUBY.strip_indent)
+          [
+            <<-TEXT, 123
+              Something with a , in it
+            TEXT
+          ]
+        RUBY
+      end
+
+      it 'auto-corrects unwanted comma where HEREDOC has commas' do
+        new_source = autocorrect_source(<<-RUBY.strip_indent)
+          [
+            <<-TEXT, 123,
+              Something with a , in it
+            TEXT
+          ]
+        RUBY
+        expect(new_source).to eq(<<-RUBY.strip_indent)
+          [
+            <<-TEXT, 123
+              Something with a , in it
+            TEXT
+          ]
+        RUBY
+      end
     end
 
     context 'when EnforcedStyleForMultiline is comma' do

spec/rubocop/cop/style/trailing_comma_in_hash_literal_spec.rb

@@ -154,12 +154,10 @@
         RUBY
       end
 
-      it 'accepts missing comma after a heredoc' do
-        # A heredoc that's the last item in a literal or parameter list can not
-        # have a trailing comma. It's a syntax error.
+      it 'accepts trailing comma after a heredoc' do
         expect_no_offenses(<<-RUBY.strip_indent)
           route(help: {
-            'auth' => <<-HELP.chomp
+            'auth' => <<-HELP.chomp,
           ...
           HELP
           })
@@ -238,12 +236,10 @@
         RUBY
       end
 
-      it 'accepts missing comma after a heredoc' do
-        # A heredoc that's the last item in a literal or parameter list can not
-        # have a trailing comma. It's a syntax error.
+      it 'accepts trailing comma after a heredoc' do
         expect_no_offenses(<<-RUBY.strip_indent)
           route(help: {
-            'auth' => <<-HELP.chomp
+            'auth' => <<-HELP.chomp,
           ...
           HELP
           })