From 44442c1f22bb1cc4d83844bea2bd345b63ef209f Mon Sep 17 00:00:00 2001 From: Tyler Porter Date: Sat, 21 Mar 2020 20:03:17 -0400 Subject: [PATCH] [Fix #7492] Add Style/TrailingCommaInBlockArgs --- CHANGELOG.md | 1 + config/default.yml | 6 + lib/rubocop.rb | 1 + .../cop/style/trailing_comma_in_block_args.rb | 85 +++++++++++ manual/cops.md | 1 + manual/cops_style.md | 48 ++++++ .../trailing_comma_in_block_args_spec.rb | 142 ++++++++++++++++++ 7 files changed, 284 insertions(+) create mode 100644 lib/rubocop/cop/style/trailing_comma_in_block_args.rb create mode 100644 spec/rubocop/cop/style/trailing_comma_in_block_args_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index c1d4b19e00e..4ba1f05602a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ * [#7784](https://github.com/rubocop-hq/rubocop/pull/7784): Support Ruby 2.7's numbered parameter for `Lint/SafeNavigationChain`. ([@koic][]) * [#7331](https://github.com/rubocop-hq/rubocop/issues/7331): Add `forbidden` option to `Style/ModuleFunction` cop. ([@weh][]) * [#7699](https://github.com/rubocop-hq/rubocop/pull/7699): Add new `Lint/StructNewOverride` cop. ([@ybiquitous][]) +* [#7637](https://github.com/rubocop-hq/rubocop/pull/7637): Add new `Style/TrailingCommaInBlockArgs` cop. ([@pawptart][]) ### Bug fixes diff --git a/config/default.yml b/config/default.yml index 79a77e574fa..4c1935d70ba 100644 --- a/config/default.yml +++ b/config/default.yml @@ -3851,6 +3851,12 @@ Style/TrailingCommaInArrayLiteral: - consistent_comma - no_comma +Style/TrailingCommaInBlockArgs: + Description: 'Checks for useless trailing commas in block arguments.' + Enabled: false + Safe: false + VersionAdded: '0.81' + Style/TrailingCommaInHashLiteral: Description: 'Checks for trailing comma in hash literals.' Enabled: true diff --git a/lib/rubocop.rb b/lib/rubocop.rb index fb6a3887742..5cd3f0e0f9e 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -558,6 +558,7 @@ require_relative 'rubocop/cop/style/trailing_body_on_module' require_relative 'rubocop/cop/style/trailing_comma_in_arguments' require_relative 'rubocop/cop/style/trailing_comma_in_array_literal' +require_relative 'rubocop/cop/style/trailing_comma_in_block_args' require_relative 'rubocop/cop/style/trailing_comma_in_hash_literal' require_relative 'rubocop/cop/style/trailing_method_end_statement' require_relative 'rubocop/cop/style/trailing_underscore_variable' diff --git a/lib/rubocop/cop/style/trailing_comma_in_block_args.rb b/lib/rubocop/cop/style/trailing_comma_in_block_args.rb new file mode 100644 index 00000000000..ea0ed46e006 --- /dev/null +++ b/lib/rubocop/cop/style/trailing_comma_in_block_args.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Style + # This cop checks whether trailing commas in block arguments are + # required. Blocks with only one argument and a trailing comma require + # that comma to be present. Blocks with more than one argument never + # require a trailing comma. + # + # @example + # # bad + # add { |foo, bar,| foo + bar } + # + # # good + # add { |foo, bar| foo + bar } + # + # # good + # add { |foo,| foo } + # + # # good + # add { foo } + # + # # bad + # add do |foo, bar,| + # foo + bar + # end + # + # # good + # add do |foo, bar| + # foo + bar + # end + # + # # good + # add do |foo,| + # foo + # end + # + # # good + # add do + # foo + bar + # end + class TrailingCommaInBlockArgs < Cop + MSG = 'Useless trailing comma present in block arguments.' + + def on_block(node) + return unless useless_trailing_comma?(node) + + add_offense(node, location: last_comma(node).pos) + end + + def autocorrect(node) + ->(corrector) { corrector.replace(last_comma(node).pos, '') } + end + + private + + def useless_trailing_comma?(node) + arg_count(node) > 1 && trailing_comma?(node) + end + + def arg_count(node) + node.arguments.each_descendant(:arg, :optarg, :kwoptarg).to_a.size + end + + def trailing_comma?(node) + argument_tokens(node).last.comma? + end + + def last_comma(node) + argument_tokens(node).last + end + + def argument_tokens(node) + pipes = tokens(node).select { |token| token.type == :tPIPE } + begin_pos, end_pos = pipes.map do |pipe| + tokens(node).index(pipe) + end + + tokens(node)[begin_pos + 1..end_pos - 1] + end + end + end + end +end diff --git a/manual/cops.md b/manual/cops.md index 3469d012440..d233f13ffce 100644 --- a/manual/cops.md +++ b/manual/cops.md @@ -465,6 +465,7 @@ In the following section you find all available cops: * [Style/TrailingBodyOnModule](cops_style.md#styletrailingbodyonmodule) * [Style/TrailingCommaInArguments](cops_style.md#styletrailingcommainarguments) * [Style/TrailingCommaInArrayLiteral](cops_style.md#styletrailingcommainarrayliteral) +* [Style/TrailingCommaInBlockArgs](cops_style.md#styletrailingcommainblockargs) * [Style/TrailingCommaInHashLiteral](cops_style.md#styletrailingcommainhashliteral) * [Style/TrailingMethodEndStatement](cops_style.md#styletrailingmethodendstatement) * [Style/TrailingUnderscoreVariable](cops_style.md#styletrailingunderscorevariable) diff --git a/manual/cops_style.md b/manual/cops_style.md index cba3722fc7d..7734460bb94 100644 --- a/manual/cops_style.md +++ b/manual/cops_style.md @@ -7223,6 +7223,54 @@ EnforcedStyleForMultiline | `no_comma` | `comma`, `consistent_comma`, `no_comma` * [https://rubystyle.guide#no-trailing-array-commas](https://rubystyle.guide#no-trailing-array-commas) +## Style/TrailingCommaInBlockArgs + +Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged +--- | --- | --- | --- | --- +Disabled | No | Yes (Unsafe) | 0.81 | - + +This cop checks whether trailing commas in block arguments are +required. Blocks with only one argument and a trailing comma require +that comma to be present. Blocks with more than one argument never +require a trailing comma. + + add do |foo, bar,| + foo + bar + end + + # good + add do |foo, bar| + foo + bar + end + + # good + add do |foo,| + foo + end + + # good + add do + foo + bar + end + +### Examples + +```ruby +# bad +add { |foo, bar,| foo + bar } + + # good +add { |foo, bar| foo + bar } + +# good +add { |foo,| foo } + +# good +add { foo } + +# bad +``` + ## Style/TrailingCommaInHashLiteral Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged diff --git a/spec/rubocop/cop/style/trailing_comma_in_block_args_spec.rb b/spec/rubocop/cop/style/trailing_comma_in_block_args_spec.rb new file mode 100644 index 00000000000..0d01b242b71 --- /dev/null +++ b/spec/rubocop/cop/style/trailing_comma_in_block_args_spec.rb @@ -0,0 +1,142 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Style::TrailingCommaInBlockArgs do + subject(:cop) { described_class.new(config) } + + let(:config) { RuboCop::Config.new } + + context 'curly brace block format' do + it 'registers an offense when a trailing comma is not needed' do + expect_offense(<<~RUBY) + test { |a, b,| a + b } + ^ Useless trailing comma present in block arguments. + RUBY + expect_correction(<<~RUBY) + test { |a, b| a + b } + RUBY + end + + it 'does not register an offense when a trailing comma is required' do + expect_no_offenses(<<~RUBY) + test { |a,| a } + RUBY + end + + it 'does not register an offense when no arguments are present' do + expect_no_offenses(<<~RUBY) + test { a } + RUBY + end + + it 'does not register an offense when more than one argument is ' \ + 'present with no trailing comma' do + expect_no_offenses(<<~RUBY) + test { |a, b| a + b } + RUBY + end + + it 'does not register an offense for default arguments' do + expect_no_offenses(<<~RUBY) + test { |a, b, c = nil| a + b + c } + RUBY + end + + it 'does not register an offense for keyword arguments' do + expect_no_offenses(<<~RUBY) + test { |a, b, c: 1| a + b + c } + RUBY + end + + it 'ignores commas in default argument strings' do + expect_no_offenses(<<~RUBY) + add { |foo, bar = ','| foo + bar } + RUBY + end + + it 'preserves semicolons in block/local variables' do + expect_no_offenses(<<~RUBY) + add { |foo, bar,; baz| foo + bar } + RUBY + end + end + + context 'do/end block format' do + it 'registers an offense when a trailing comma is not needed' do + expect_offense(<<~RUBY) + test do |a, b,| + ^ Useless trailing comma present in block arguments. + a + b + end + RUBY + expect_correction(<<~RUBY) + test do |a, b| + a + b + end + RUBY + end + + it 'does not register an offense when a trailing comma is required' do + expect_no_offenses(<<~RUBY) + test do |a,| + a + end + RUBY + end + + it 'does not register an offense when no arguments are present' do + expect_no_offenses(<<~RUBY) + test do + a + end + RUBY + end + + it 'does not register an offense for an empty block' do + expect_no_offenses(<<~RUBY) + test do || + end + RUBY + end + + it 'does not register an offense when more than one argument is ' \ + 'present with no trailing comma' do + expect_no_offenses(<<~RUBY) + test do |a, b| + a + b + end + RUBY + end + + it 'does not register an offense for default arguments' do + expect_no_offenses(<<~RUBY) + test do |a, b, c = nil| + a + b + c + end + RUBY + end + + it 'does not register an offense for keyword arguments' do + expect_no_offenses(<<~RUBY) + test do |a, b, c: 1| + a + b + c + end + RUBY + end + + it 'ignores commas in default argument strings' do + expect_no_offenses(<<~RUBY) + add do |foo, bar = ','| + foo + bar + end + RUBY + end + + it 'preserves semicolons in block/local variables' do + expect_no_offenses(<<~RUBY) + add do |foo, bar,; baz| + foo + bar + end + RUBY + end + end +end