Skip to content

Commit

Permalink
Add new InternalAffairs/RedundantDescribedClassAsSubject cop
Browse files Browse the repository at this point in the history
This PR adds new `InternalAffairs/RedundantDescribedClassAsSubject` cop.
It checks for redundant `subject(:cop) { described_class.new }`.

```ruby
# bad
RSpec.describe RuboCop::Cop::Department::Foo do
  subject(:cop) { described_class.new(config) }
end

# good
RSpec.describe RuboCop::Cop::Department::Foo, :config do
end
```

No changelog entry has been added to the changelog due to this cop is
for use inside development.

And redundant `let(:config) { RuboCop::Config.new }` can also be removed.
I will prepare the implementation as a different cop.
  • Loading branch information
koic authored and bbatsov committed Jan 27, 2021
1 parent 6d6708d commit e3ca0e3
Show file tree
Hide file tree
Showing 270 changed files with 377 additions and 794 deletions.
1 change: 1 addition & 0 deletions lib/rubocop/cop/internal_affairs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
require_relative 'internal_affairs/node_destructuring'
require_relative 'internal_affairs/node_type_predicate'
require_relative 'internal_affairs/offense_location_keyword'
require_relative 'internal_affairs/redundant_described_class_as_subject'
require_relative 'internal_affairs/redundant_message_argument'
require_relative 'internal_affairs/redundant_location_argument'
require_relative 'internal_affairs/style_detected_api_use'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# frozen_string_literal: true

module RuboCop
module Cop
module InternalAffairs
# This cop checks for redundant `subject(:cop) { described_class.new }`.
#
# @example
# # bad
# RSpec.describe RuboCop::Cop::Department::Foo do
# subject(:cop) { described_class.new(config) }
# end
#
# # good
# RSpec.describe RuboCop::Cop::Department::Foo, :config do
# end
#
class RedundantDescribedClassAsSubject < Base
include RangeHelp
extend AutoCorrector

MSG = 'Remove the redundant `subject`%<additional_message>s.'

def_node_matcher :described_class_subject?, <<~PATTERN
(block
(send nil? :subject
(sym :cop))
(args)
(send
(send nil? :described_class) :new
$...))
PATTERN

def on_block(node)
return unless (described_class_arguments = described_class_subject?(node))
return if described_class_arguments.count >= 2

describe = find_describe_method_node(node)

unless (exist_config = describe.last_argument.source == ':config')
additional_message = ' and specify `:config` in `describe`'
end

message = format(MSG, additional_message: additional_message)

add_offense(node, message: message) do |corrector|
corrector.remove(range_by_whole_lines(node.source_range, include_final_newline: true))

corrector.insert_after(describe.last_argument, ', :config') unless exist_config
end
end

private

def find_describe_method_node(block_node)
block_node.ancestors.find { |node| node.block_type? && node.method?(:describe) }.send_node
end
end
end
end
end
4 changes: 1 addition & 3 deletions spec/rubocop/cop/internal_affairs/method_name_equal_spec.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::InternalAffairs::MethodNameEqual do
subject(:cop) { described_class.new(config) }

RSpec.describe RuboCop::Cop::InternalAffairs::MethodNameEqual, :config do
let(:config) { RuboCop::Config.new }

it 'registers an offense when using `#method == :do_something`' do
Expand Down
4 changes: 1 addition & 3 deletions spec/rubocop/cop/internal_affairs/node_destructuring_spec.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::InternalAffairs::NodeDestructuring do
subject(:cop) { described_class.new }

RSpec.describe RuboCop::Cop::InternalAffairs::NodeDestructuring, :config do
context 'when destructuring using `node.children`' do
it 'registers an offense when receiver is named `node`' do
expect_offense(<<~RUBY, 'example_cop.rb')
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::InternalAffairs::NodeTypePredicate do
subject(:cop) { described_class.new }

RSpec.describe RuboCop::Cop::InternalAffairs::NodeTypePredicate, :config do
context 'comparison node type check' do
it 'registers an offense and auto-corrects' do
expect_offense(<<~RUBY)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::InternalAffairs::OffenseLocationKeyword do
subject(:cop) { described_class.new }

RSpec.describe RuboCop::Cop::InternalAffairs::OffenseLocationKeyword, :config do
context 'when `node.loc.selector` is passed' do
it 'registers an offense' do
expect_offense(<<~RUBY)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::InternalAffairs::RedundantDescribedClassAsSubject, :config do
it 'registers an offense when using `subject(:cop)` and `:config` is not specified in `describe`' do
expect_offense(<<~RUBY)
RSpec.describe RuboCop::Cop::Lint::RegexpAsCondition do
subject(:cop) { described_class.new(config) }
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Remove the redundant `subject` and specify `:config` in `describe`.
let(:config) { RuboCop::Config.new }
end
RUBY

expect_correction(<<~RUBY)
RSpec.describe RuboCop::Cop::Lint::RegexpAsCondition, :config do
let(:config) { RuboCop::Config.new }
end
RUBY
end

it 'registers an offense when using `subject(:cop)` and `:config` is already specified in `describe`' do
expect_offense(<<~RUBY)
RSpec.describe RuboCop::Cop::Lint::RegexpAsCondition, :config do
subject(:cop) { described_class.new(config) }
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Remove the redundant `subject`.
let(:config) { RuboCop::Config.new }
end
RUBY

expect_correction(<<~RUBY)
RSpec.describe RuboCop::Cop::Lint::RegexpAsCondition, :config do
let(:config) { RuboCop::Config.new }
end
RUBY
end

it 'registers an offense when using `subject(:cop)` with no argument `described_class.new` and `:config` is specified' do
expect_offense(<<~RUBY)
RSpec.describe RuboCop::Cop::Lint::RegexpAsCondition, :config do
subject(:cop) { described_class.new }
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Remove the redundant `subject`.
let(:config) { RuboCop::Config.new }
end
RUBY

expect_correction(<<~RUBY)
RSpec.describe RuboCop::Cop::Lint::RegexpAsCondition, :config do
let(:config) { RuboCop::Config.new }
end
RUBY
end

it 'does not register an offense when using `subject(:cop)` with multiple arguments to `described_class.new`' do
expect_no_offenses(<<~RUBY)
RSpec.describe RuboCop::Cop::Lint::RegexpAsCondition do
subject(:cop) { described_class.new(config, options) }
end
RUBY
end
end
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::InternalAffairs::RedundantLocationArgument do
subject(:cop) { described_class.new }

RSpec.describe RuboCop::Cop::InternalAffairs::RedundantLocationArgument, :config do
context 'when location argument is passed' do
context 'when location argument is :expression' do
it 'registers an offense' do
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::InternalAffairs::RedundantMessageArgument do
subject(:cop) { described_class.new }

RSpec.describe RuboCop::Cop::InternalAffairs::RedundantMessageArgument, :config do
context 'when `MSG` is passed' do
it 'registers an offense' do
expect_offense(<<~RUBY, 'example_cop.rb')
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::InternalAffairs::StyleDetectedApiUse do
subject(:cop) { described_class.new(config) }

RSpec.describe RuboCop::Cop::InternalAffairs::StyleDetectedApiUse, :config do
let(:config) { RuboCop::Config.new }

it 'registers an offense when correct_style_detected is used without a negative *_style_detected follow up' do
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::InternalAffairs::UselessMessageAssertion do
subject(:cop) { described_class.new }

RSpec.describe RuboCop::Cop::InternalAffairs::UselessMessageAssertion, :config do
it 'registers an offense for specs that assert using the MSG' do
expect_offense(<<~RUBY, 'example_spec.rb')
it 'uses described_class::MSG to specify the expected message' do
Expand Down
4 changes: 1 addition & 3 deletions spec/rubocop/cop/layout/access_modifier_indentation_spec.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Layout::AccessModifierIndentation do
subject(:cop) { described_class.new(config) }

RSpec.describe RuboCop::Cop::Layout::AccessModifierIndentation, :config do
let(:config) do
c = cop_config.merge('SupportedStyles' => %w[indent outdent])
RuboCop::Config
Expand Down
4 changes: 1 addition & 3 deletions spec/rubocop/cop/layout/argument_alignment_spec.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Layout::ArgumentAlignment do
subject(:cop) { described_class.new(config) }

RSpec.describe RuboCop::Cop::Layout::ArgumentAlignment, :config do
let(:config) do
RuboCop::Config.new('Layout/ArgumentAlignment' => cop_config,
'Layout/IndentationWidth' => {
Expand Down
4 changes: 1 addition & 3 deletions spec/rubocop/cop/layout/array_alignment_spec.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Layout::ArrayAlignment do
subject(:cop) { described_class.new(config) }

RSpec.describe RuboCop::Cop::Layout::ArrayAlignment, :config do
let(:config) do
RuboCop::Config.new('Layout/ArrayAlignment' => cop_config,
'Layout/IndentationWidth' => {
Expand Down
4 changes: 1 addition & 3 deletions spec/rubocop/cop/layout/block_end_newline_spec.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Layout::BlockEndNewline do
subject(:cop) { described_class.new }

RSpec.describe RuboCop::Cop::Layout::BlockEndNewline, :config do
it 'accepts a one-liner' do
expect_no_offenses('test do foo end')
end
Expand Down
4 changes: 1 addition & 3 deletions spec/rubocop/cop/layout/case_indentation_spec.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Layout::CaseIndentation do
subject(:cop) { described_class.new(config) }

RSpec.describe RuboCop::Cop::Layout::CaseIndentation, :config do
let(:config) do
merged = RuboCop::ConfigLoader
.default_configuration['Layout/CaseIndentation'].merge(cop_config)
Expand Down
4 changes: 1 addition & 3 deletions spec/rubocop/cop/layout/closing_heredoc_indentation_spec.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Layout::ClosingHeredocIndentation do
subject(:cop) { described_class.new(config) }

RSpec.describe RuboCop::Cop::Layout::ClosingHeredocIndentation, :config do
let(:config) do
RuboCop::Config.new('Layout/ClosingHeredocIndentation' => cop_config)
end
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Layout::ClosingParenthesisIndentation do
subject(:cop) { described_class.new }

RSpec.describe RuboCop::Cop::Layout::ClosingParenthesisIndentation, :config do
context 'for method calls' do
context 'with line break before 1st parameter' do
it 'registers an offense for misaligned )' do
Expand Down
4 changes: 1 addition & 3 deletions spec/rubocop/cop/layout/comment_indentation_spec.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Layout::CommentIndentation do
subject(:cop) { described_class.new(config) }

RSpec.describe RuboCop::Cop::Layout::CommentIndentation, :config do
let(:config) do
RuboCop::Config
.new('Layout/IndentationWidth' => { 'Width' => indentation_width })
Expand Down
4 changes: 1 addition & 3 deletions spec/rubocop/cop/layout/condition_position_spec.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Layout::ConditionPosition do
subject(:cop) { described_class.new }

RSpec.describe RuboCop::Cop::Layout::ConditionPosition, :config do
%w[if unless while until].each do |keyword|
it 'registers an offense and corrects for condition on the next line' do
expect_offense(<<~RUBY)
Expand Down
4 changes: 1 addition & 3 deletions spec/rubocop/cop/layout/else_alignment_spec.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Layout::ElseAlignment do
subject(:cop) { described_class.new(config) }

RSpec.describe RuboCop::Cop::Layout::ElseAlignment, :config do
let(:config) do
RuboCop::Config.new('Layout/EndAlignment' => end_alignment_config)
end
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Layout::EmptyLineAfterGuardClause do
subject(:cop) { described_class.new }

RSpec.describe RuboCop::Cop::Layout::EmptyLineAfterGuardClause, :config do
it 'registers an offense and corrects a guard clause ' \
'not followed by empty line' do
expect_offense(<<~RUBY)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Layout::EmptyLineAfterMagicComment do
subject(:cop) { described_class.new(config) }

RSpec.describe RuboCop::Cop::Layout::EmptyLineAfterMagicComment, :config do
let(:config) { RuboCop::Config.new }

it 'registers an offense for code that immediately follows comment' do
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Layout::EmptyLineAfterMultilineCondition do
subject(:cop) { described_class.new }

RSpec.describe RuboCop::Cop::Layout::EmptyLineAfterMultilineCondition, :config do
it 'registers an offense when no new line after `if` with multiline condition' do
expect_offense(<<~RUBY)
if multiline &&
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Layout::EmptyLinesAroundAttributeAccessor, :config do
subject(:cop) { described_class.new(config) }

it 'registers an offense and corrects for code ' \
'that immediately follows accessor' do
expect_offense(<<~RUBY)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Layout::EmptyLinesAroundBeginBody do
subject(:cop) { described_class.new(config) }

RSpec.describe RuboCop::Cop::Layout::EmptyLinesAroundBeginBody, :config do
let(:config) { RuboCop::Config.new }

shared_examples 'accepts' do |name, code|
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Layout::EmptyLinesAroundExceptionHandlingKeywords do
subject(:cop) { described_class.new(config) }

RSpec.describe RuboCop::Cop::Layout::EmptyLinesAroundExceptionHandlingKeywords, :config do
let(:config) { RuboCop::Config.new }
let(:message) { '^{} Extra empty line detected' }

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Layout::EmptyLinesAroundMethodBody do
subject(:cop) { described_class.new }

RSpec.describe RuboCop::Cop::Layout::EmptyLinesAroundMethodBody, :config do
let(:beginning_offense_annotation) { '^{} Extra empty line detected at method body beginning.' }
let(:end_offense_annotation) { '^{} Extra empty line detected at method body end.' }

Expand Down
4 changes: 1 addition & 3 deletions spec/rubocop/cop/layout/empty_lines_spec.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Layout::EmptyLines do
subject(:cop) { described_class.new }

RSpec.describe RuboCop::Cop::Layout::EmptyLines, :config do
it 'registers an offense for consecutive empty lines' do
expect_offense(<<~RUBY)
test = 5
Expand Down
Loading

0 comments on commit e3ca0e3

Please sign in to comment.