[Fix #713] Add cop TrailingComma.

CHANGELOG.md

@@ -10,6 +10,7 @@
 * [#702](https://github.com/bbatsov/rubocop/issues/702): Improve `rubocop-todo.yml` with comments about offence count, configuration parameters, and auto-correction support. ([@jonas054][])
 * Add new command-line flag `-D/--display-cop-names` to trigger the display of cop names in offence messages. ([@bbatsov][])
 * [#733](https://github.com/bbatsov/rubocop/pull/733): `NumericLiterals` cop does auto-correction. ([@dblock][])
+* [#713](https://github.com/bbatsov/rubocop/issues/713): New cop `TrailingComma` checks for comma after the last item in a hash, array, or method call parameter list. ([@jonas054][])
 
 ### Changes
 

config/default.yml

@@ -249,6 +249,11 @@ SpaceInsideHashLiteralBraces:
     - space
     - no_space
 
+TrailingComma:
+  EnforcedStyleForMultiline: no_comma
+  SupportedStyles:
+    - comma
+    - no_comma
 
 # TrivialAccessors doesn't require exact name matches and doesn't allow
 # predicated methods by default.

config/enabled.yml

@@ -450,6 +450,10 @@ TrailingBlankLines:
   Description: 'Checks for superfluous trailing blank lines.'
   Enabled: true
 
+TrailingComma:
+  Description: 'Checks for trailing comma in parameter lists and literals.'
+  Enabled: true
+
 TrailingWhitespace:
   Description: 'Avoid trailing whitespace.'
   Enabled: true

lib/rubocop.rb

@@ -175,6 +175,7 @@
 require 'rubocop/cop/style/symbol_array'
 require 'rubocop/cop/style/tab'
 require 'rubocop/cop/style/trailing_blank_lines'
+require 'rubocop/cop/style/trailing_comma'
 require 'rubocop/cop/style/trailing_whitespace'
 require 'rubocop/cop/style/trivial_accessors'
 require 'rubocop/cop/style/unless_else'

lib/rubocop/cop/style/trailing_comma.rb

@@ -0,0 +1,94 @@
+# encoding: utf-8
+
+module Rubocop
+  module Cop
+    module Style
+      # This cop checks for trailing comma in parameter lists and literals.
+      class TrailingComma < Cop
+        include ConfigurableEnforcedStyle
+
+        MSG = '%s comma after the last %s.'
+
+        def on_array(node)
+          check_literal(node, 'item of %s array')
+        end
+
+        def on_hash(node)
+          check_literal(node, 'item of %s hash')
+        end
+
+        def on_send(node)
+          _receiver, _method_name, *args = *node
+          return if args.empty?
+          # It's impossible for a method call without parentheses to have
+          # a trailing comma.
+          return unless brackets?(node)
+
+          check(node, args, 'parameter of %s method call',
+                args.last.loc.expression.end_pos, node.loc.expression.end_pos)
+        end
+
+        private
+
+        def parameter_name
+          'EnforcedStyleForMultiline'
+        end
+
+        def check_literal(node, kind)
+          return if node.children.empty?
+          # A braceless hash is the last parameter of a method call and will be
+          # checked as such.
+          return unless brackets?(node)
+
+          check(node, node.children, kind,
+                node.children.last.loc.expression.end_pos,
+                node.loc.end.begin_pos)
+        end
+
+        def check(node, items, kind, begin_pos, end_pos)
+          sb = items.first.loc.expression.source_buffer
+          after_last_item = Parser::Source::Range.new(sb, begin_pos, end_pos)
+          comma_offset = after_last_item.source =~ /,/
+          should_have_comma = style == :comma && multiline?(node)
+          if comma_offset
+            unless should_have_comma
+              avoid_comma(items, kind,
+                          after_last_item.begin_pos + comma_offset, sb)
+            end
+          elsif should_have_comma
+            put_comma(items, kind, sb)
+          end
+        end
+
+        # Returns true if the node has round/square/curly brackets.
+        def brackets?(node)
+          !node.loc.end.nil?
+        end
+
+        # Returns true if the round/square/curly brackets of the given node are
+        # on different lines.
+        def multiline?(node)
+          [node.loc.begin, node.loc.end].map(&:line).uniq.size > 1
+        end
+
+        def avoid_comma(items, kind, comma_begin_pos, sb)
+          range = Parser::Source::Range.new(sb, comma_begin_pos,
+                                            comma_begin_pos + 1)
+          article = kind =~ /array/ ? 'an' : 'a'
+          add_offence(nil, range,
+                      sprintf(MSG, 'Avoid', sprintf(kind, article)))
+        end
+
+        def put_comma(items, kind, sb)
+          last_expr = items.last.loc.expression
+          ix = last_expr.source.rindex("\n") || 0
+          ix += last_expr.source[ix..-1] =~ /\S/
+          range = Parser::Source::Range.new(sb, last_expr.begin_pos + ix,
+                                            last_expr.end_pos)
+          add_offence(nil, range,
+                      sprintf(MSG, 'Put a', sprintf(kind, 'a multiline')))
+        end
+      end
+    end
+  end
+end

spec/rubocop/cop/style/trailing_comma_spec.rb

@@ -0,0 +1,200 @@
+# encoding: utf-8
+
+require 'spec_helper'
+
+describe Rubocop::Cop::Style::TrailingComma, :config do
+  subject(:cop) { described_class.new(config) }
+  let(:cop_config) { { 'EnforcedStyleForMultiline' => 'no_comma' } }
+
+  context 'with single line list of values' do
+    it 'registers an offence for trailing comma in an Array literal' do
+      inspect_source(cop, 'VALUES = [1001, 2020, 3333, ]')
+      expect(cop.messages)
+        .to eq(['Avoid comma after the last item of an array.'])
+      expect(cop.highlights).to eq([','])
+    end
+
+    it 'registers an offence for trailing comma in a Hash literal' do
+      inspect_source(cop, 'MAP = { a: 1001, b: 2020, c: 3333, }')
+      expect(cop.messages)
+        .to eq(['Avoid comma after the last item of a hash.'])
+      expect(cop.highlights).to eq([','])
+    end
+
+    it 'registers an offence for trailing comma in a method call' do
+      inspect_source(cop, 'some_method(a, b, c, )')
+      expect(cop.messages)
+        .to eq(['Avoid comma after the last parameter of a method call.'])
+      expect(cop.highlights).to eq([','])
+    end
+
+    it 'registers an offence for trailing comma in a method call with hash' +
+      ' parameters at the end' do
+      inspect_source(cop, 'some_method(a, b, c: 0, d: 1, )')
+      expect(cop.messages)
+        .to eq(['Avoid comma after the last parameter of a method call.'])
+      expect(cop.highlights).to eq([','])
+    end
+
+    it 'accepts Array literal without trailing comma' do
+      inspect_source(cop, 'VALUES = [1001, 2020, 3333]')
+      expect(cop.offences).to be_empty
+    end
+
+    it 'accepts empty Array literal' do
+      inspect_source(cop, 'VALUES = []')
+      expect(cop.offences).to be_empty
+    end
+
+    it 'accepts rescue clause' do
+      # The list of rescued classes is an array.
+      inspect_source(cop, ['begin',
+                           '  do_something',
+                           'rescue RuntimeError',
+                           'end'])
+      expect(cop.offences).to be_empty
+    end
+
+    it 'accepts Hash literal without trailing comma' do
+      inspect_source(cop, 'MAP = { a: 1001, b: 2020, c: 3333 }')
+      expect(cop.offences).to be_empty
+    end
+
+    it 'accepts empty Hash literal' do
+      inspect_source(cop, 'MAP = {}')
+      expect(cop.offences).to be_empty
+    end
+
+    it 'accepts method call without trailing comma' do
+      inspect_source(cop, 'some_method(a, b, c)')
+      expect(cop.offences).to be_empty
+    end
+
+    it 'accepts method call without parameters' do
+      inspect_source(cop, 'some_method')
+      expect(cop.offences).to be_empty
+    end
+  end
+
+  context 'with multi-line list of values' do
+    context 'when EnforcedStyleForMultiline is no_comma' do
+      it 'registers an offence for trailing comma in an Array literal' do
+        inspect_source(cop, ['VALUES = [',
+                             '           1001,',
+                             '           2020,',
+                             '           3333,',
+                             '         ]'])
+        expect(cop.highlights).to eq([','])
+      end
+
+      it 'registers an offence for trailing comma in a Hash literal' do
+        inspect_source(cop, ['MAP = { a: 1001,',
+                             '        b: 2020,',
+                             '        c: 3333,',
+                             '      }'])
+        expect(cop.highlights).to eq([','])
+      end
+
+      it 'registers an offence for trailing comma in a method call with ' +
+        'hash parameters at the end' do
+        inspect_source(cop, ['some_method(',
+                             '              a,',
+                             '              b,',
+                             '              c: 0,',
+                             '              d: 1,)'])
+        expect(cop.highlights).to eq([','])
+      end
+
+      it 'accepts an Array literal with no trailing comma' do
+        inspect_source(cop, ['VALUES = [ 1001,',
+                             '           2020,',
+                             '           3333 ]'])
+        expect(cop.offences).to be_empty
+      end
+
+      it 'accepts a Hash literal with no trailing comma' do
+        inspect_source(cop, ['MAP = {',
+                             '        a: 1001,',
+                             '        b: 2020,',
+                             '        c: 3333',
+                             '      }'])
+        expect(cop.offences).to be_empty
+      end
+
+      it 'accepts a method call with ' +
+        'hash parameters at the end and no trailing comma' do
+        inspect_source(cop, ['some_method(a,',
+                             '            b,',
+                             '            c: 0,',
+                             '            d: 1',
+                             '           )'])
+        expect(cop.offences).to be_empty
+      end
+    end
+
+    context 'when EnforcedStyleForMultiline is comma' do
+      let(:cop_config) { { 'EnforcedStyleForMultiline' => 'comma' } }
+
+      it 'registers an offence for no trailing comma in an Array literal' do
+        inspect_source(cop, ['VALUES = [',
+                             '           1001,',
+                             '           2020,',
+                             '           3333]'])
+        expect(cop.messages)
+          .to eq(['Put a comma after the last item of a multiline array.'])
+        expect(cop.highlights).to eq(['3333'])
+      end
+
+      it 'registers an offence for no trailing comma in a Hash literal' do
+        inspect_source(cop, ['MAP = { a: 1001,',
+                             '        b: 2020,',
+                             '        c: 3333 }'])
+        expect(cop.messages)
+          .to eq(['Put a comma after the last item of a multiline hash.'])
+        expect(cop.highlights).to eq(['c: 3333'])
+      end
+
+      it 'registers an offence for no trailing comma in a method call with' +
+        ' hash parameters at the end' do
+        inspect_source(cop, ['some_method(',
+                             '              a,',
+                             '              b,',
+                             '              c: 0,',
+                             '              d: 1',
+                             '           )'])
+        expect(cop.messages)
+          .to eq(['Put a comma after the last parameter of a multiline ' +
+                  'method call.'])
+        expect(cop.highlights).to eq(['d: 1'])
+      end
+
+      it 'accepts trailing comma in an Array literal' do
+        inspect_source(cop, ['VALUES = [1001,',
+                             '          2020,',
+                             '          3333,',
+                             '         ]'])
+        expect(cop.offences).to be_empty
+      end
+
+      it 'accepts trailing comma in a Hash literal' do
+        inspect_source(cop, ['MAP = {',
+                             '        a: 1001,',
+                             '        b: 2020,',
+                             '        c: 3333,',
+                             '      }'])
+        expect(cop.offences).to be_empty
+      end
+
+      it 'accepts trailing comma in a method call with hash' +
+        ' parameters at the end' do
+        inspect_source(cop, ['some_method(',
+                             '              a,',
+                             '              b,',
+                             '              c: 0,',
+                             '              d: 1,',
+                             '           )'])
+        expect(cop.offences).to be_empty
+      end
+    end
+  end
+end