From 0cc37f6a603b70f65f904766a21741c89f55289f Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sun, 16 May 2021 16:36:10 +0900 Subject: [PATCH] Add new `Lint/EmptyInPattern` cop This PR adds new `Lint/EmptyInPattern` cop for Ruby 2.7's pattern matching. It checks for the presence of `in` branches without a body. ```ruby # bad case condition in [a] do_something in [a, b] end # good case condition in [a] do_something in [a, b] nil end # good - AllowComments: true (default) case condition in [a] do_something in [a, b] # noop end # bad - AllowComments: false case condition in [a] do_something in [a, b] # noop end ``` This cop is similar to `Lint/EmptyWhen`, but with different supported syntax and Ruby version (requires 2.7 or higher). And this PR use https://github.com/rubocop/rubocop-ast/pull/183 feature, so it requires RuboCop AST 1.6.0 or higher. --- .../new_add_new_lint_empty_in_pattern_cop.md | 1 + config/default.yml | 5 + lib/rubocop.rb | 1 + lib/rubocop/cop/lint/empty_in_pattern.rb | 62 +++++++ rubocop.gemspec | 2 +- .../rubocop/cop/lint/empty_in_pattern_spec.rb | 165 ++++++++++++++++++ 6 files changed, 235 insertions(+), 1 deletion(-) create mode 100644 changelog/new_add_new_lint_empty_in_pattern_cop.md create mode 100644 lib/rubocop/cop/lint/empty_in_pattern.rb create mode 100644 spec/rubocop/cop/lint/empty_in_pattern_spec.rb diff --git a/changelog/new_add_new_lint_empty_in_pattern_cop.md b/changelog/new_add_new_lint_empty_in_pattern_cop.md new file mode 100644 index 00000000000..4d5657cc689 --- /dev/null +++ b/changelog/new_add_new_lint_empty_in_pattern_cop.md @@ -0,0 +1 @@ +* [#9825](https://github.com/rubocop/rubocop/pull/9825): Add new `Lint/EmptyInPattern` cop. ([@koic][]) diff --git a/config/default.yml b/config/default.yml index 8372c36860c..9b995fd4798 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1646,6 +1646,11 @@ Lint/EmptyFile: AllowComments: true VersionAdded: '0.90' +Lint/EmptyInPattern: + Description: 'Checks for the presence of `in` pattern branches without a body.' + Enabled: pending + VersionAdded: '<>' + Lint/EmptyInterpolation: Description: 'Checks for empty string interpolation.' Enabled: true diff --git a/lib/rubocop.rb b/lib/rubocop.rb index 057fdee0f9b..a25dbc1d354 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -288,6 +288,7 @@ require_relative 'rubocop/cop/lint/empty_ensure' require_relative 'rubocop/cop/lint/empty_expression' require_relative 'rubocop/cop/lint/empty_file' +require_relative 'rubocop/cop/lint/empty_in_pattern' require_relative 'rubocop/cop/lint/empty_interpolation' require_relative 'rubocop/cop/lint/empty_when' require_relative 'rubocop/cop/lint/ensure_return' diff --git a/lib/rubocop/cop/lint/empty_in_pattern.rb b/lib/rubocop/cop/lint/empty_in_pattern.rb new file mode 100644 index 00000000000..08ba49f8b38 --- /dev/null +++ b/lib/rubocop/cop/lint/empty_in_pattern.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Lint + # This cop checks for the presence of `in` pattern branches without a body. + # + # @example + # + # # bad + # case condition + # in [a] + # do_something + # in [a, b] + # end + # + # # good + # case condition + # in [a] + # do_something + # in [a, b] + # nil + # end + # + # @example AllowComments: true (default) + # + # # good + # case condition + # in [a] + # do_something + # in [a, b] + # # noop + # end + # + # @example AllowComments: false + # + # # bad + # case condition + # in [a] + # do_something + # in [a, b] + # # noop + # end + # + class EmptyInPattern < Base + extend TargetRubyVersion + + MSG = 'Avoid `in` branches without a body.' + + minimum_target_ruby_version 2.7 + + def on_case_match(node) + node.in_pattern_branches.each do |branch| + next if branch.body || cop_config['AllowComments'] && comment_lines?(node) + + add_offense(branch) + end + end + end + end + end +end diff --git a/rubocop.gemspec b/rubocop.gemspec index d1cac1db8bc..49530d0bc48 100644 --- a/rubocop.gemspec +++ b/rubocop.gemspec @@ -35,7 +35,7 @@ Gem::Specification.new do |s| s.add_runtime_dependency('rainbow', '>= 2.2.2', '< 4.0') s.add_runtime_dependency('regexp_parser', '>= 1.8', '< 3.0') s.add_runtime_dependency('rexml') - s.add_runtime_dependency('rubocop-ast', '>= 1.5.0', '< 2.0') + s.add_runtime_dependency('rubocop-ast', '>= 1.6.0', '< 2.0') s.add_runtime_dependency('ruby-progressbar', '~> 1.7') s.add_runtime_dependency('unicode-display_width', '>= 1.4.0', '< 3.0') diff --git a/spec/rubocop/cop/lint/empty_in_pattern_spec.rb b/spec/rubocop/cop/lint/empty_in_pattern_spec.rb new file mode 100644 index 00000000000..8d5ea713b30 --- /dev/null +++ b/spec/rubocop/cop/lint/empty_in_pattern_spec.rb @@ -0,0 +1,165 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Lint::EmptyInPattern, :config do + let(:cop_config) { { 'AllowComments' => false } } + + context 'when a `in` body is missing', :ruby27 do + it 'registers an offense for a missing `in` body' do + expect_offense(<<~RUBY) + case foo + in [a] then 1 + in [a, b] # nothing + ^^^^^^^^^ Avoid `in` branches without a body. + end + RUBY + + expect_no_corrections + end + + it 'registers an offense for missing `in` body followed by `else`' do + expect_offense(<<~RUBY) + case foo + in [a] then 1 + in [a, b] # nothing + ^^^^^^^^^ Avoid `in` branches without a body. + else 3 + end + RUBY + + expect_no_corrections + end + + it 'registers an offense for missing `in` ... `then` body' do + expect_offense(<<~RUBY) + case foo + in [a] then 1 + in [a, b] then # nothing + ^^^^^^^^^ Avoid `in` branches without a body. + end + RUBY + + expect_no_corrections + end + + it 'registers an offense for missing `in` ... then `body` followed by `else`' do + expect_offense(<<~RUBY) + case foo + in [a] then 1 + in [a, b] then # nothing + ^^^^^^^^^ Avoid `in` branches without a body. + else 3 + end + RUBY + + expect_no_corrections + end + + it 'registers an offense for missing `in` body with a comment' do + expect_offense(<<~RUBY) + case foo + in [a] + 1 + in [a, b] + ^^^^^^^^^ Avoid `in` branches without a body. + # nothing + end + RUBY + + expect_no_corrections + end + + it 'registers an offense for missing `in` body with a comment followed by `else`' do + expect_offense(<<~RUBY) + case foo + in [a] + 1 + in [a, b] + ^^^^^^^^^ Avoid `in` branches without a body. + # nothing + else + 3 + end + RUBY + + expect_no_corrections + end + end + + context 'when a `in` body is present', :ruby27 do + it 'accepts `case` with `in` ... `then` statements' do + expect_no_offenses(<<~RUBY) + case foo + in [a] then 1 + in [a, b] then 2 + end + RUBY + end + + it 'accepts `case` with `in` ... `then` statements and else clause' do + expect_no_offenses(<<~RUBY) + case foo + in [a] then 1 + in [a, b] then 2 + else 3 + end + RUBY + end + + it 'accepts `case` with `in` bodies' do + expect_no_offenses(<<~RUBY) + case foo + in [a] + 1 + in [a, b] + 2 + end + RUBY + end + + it 'accepts `case` with `in` bodies and `else` clause' do + expect_no_offenses(<<~RUBY) + case foo + in [a] + 1 + in [a, b] + 2 + else + 3 + end + RUBY + end + end + + context 'when `AllowComments: true`', :ruby27 do + let(:cop_config) { { 'AllowComments' => true } } + + it 'accepts an empty `in` body with a comment' do + expect_no_offenses(<<~RUBY) + case condition + in [a] + do_something + in [a, b] + # do nothing + end + RUBY + end + end + + context 'when `AllowComments: false`', :ruby27 do + let(:cop_config) { { 'AllowComments' => false } } + + it 'registers an offense for empty `in` body with a comment' do + expect_offense(<<~RUBY) + case condition + in [a] + do_something + in [a, b] + ^^^^^^^^^ Avoid `in` branches without a body. + # do nothing + end + RUBY + + expect_no_corrections + end + end +end