diff --git a/CHANGELOG.md b/CHANGELOG.md index 7264bd8f7a8..c0082f2a5af 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/lib/rubocop/cop/mixin/trailing_comma.rb b/lib/rubocop/cop/mixin/trailing_comma.rb index 956d2bd9bb1..4f8819dc983 100644 --- a/lib/rubocop/cop/mixin/trailing_comma.rb +++ b/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) diff --git a/spec/rubocop/cop/style/trailing_comma_in_arguments_spec.rb b/spec/rubocop/cop/style/trailing_comma_in_arguments_spec.rb index c6019f05a40..4c1050cf0a0 100644 --- a/spec/rubocop/cop/style/trailing_comma_in_arguments_spec.rb +++ b/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' \ diff --git a/spec/rubocop/cop/style/trailing_comma_in_array_literal_spec.rb b/spec/rubocop/cop/style/trailing_comma_in_array_literal_spec.rb index 143fac27dde..5ea7908048b 100644 --- a/spec/rubocop/cop/style/trailing_comma_in_array_literal_spec.rb +++ b/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 diff --git a/spec/rubocop/cop/style/trailing_comma_in_hash_literal_spec.rb b/spec/rubocop/cop/style/trailing_comma_in_hash_literal_spec.rb index f250f0979a5..13d6c6f544c 100644 --- a/spec/rubocop/cop/style/trailing_comma_in_hash_literal_spec.rb +++ b/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 })