diff --git a/CHANGELOG.md b/CHANGELOG.md index bf5e082ab..f65e661fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ ## Master (Unreleased) +## 2.4.0 (2021-06-09) + +* Update `RSpec/FilePath` to check suffix when given a non-constant top-level node (e.g. features). ([@topalovic][]) +* Add missing documentation for `single_statement_only` style of `RSpec/ImplicitSubject` cop. ([@tejasbubane][]) +* Fix an exception in `DescribedClass` when accessing a constant on a variable in a spec that is nested in a namespace. ([@rrosenblum][]) +* Add new `RSpec/IdenticalEqualityAssertion` cop. ([@tejasbubane][]) +* Add `RSpec/Rails/AvoidSetupHook` cop. ([@paydaylight][]) +* Fix false negative in `RSpec/ExpectChange` cop with block style and chained method call. ([@tejasbubane][]) + ## 2.3.0 (2021-04-28) * Allow `RSpec/ContextWording` to accept multi-word prefixes. ([@hosamaly][]) @@ -613,3 +622,6 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features. [@hosamaly]: https://github.com/hosamaly [@stephannv]: https://github.com/stephannv [@Tietew]: https://github.com/Tietew +[@rrosenblum]: https://github.com/rrosenblum +[@paydaylight]: https://github.com/paydaylight +[@topalovic]: https://github.com/topalovic diff --git a/config/default.yml b/config/default.yml index 1b3b7d3f4..fe526a173 100644 --- a/config/default.yml +++ b/config/default.yml @@ -351,6 +351,12 @@ RSpec/HooksBeforeExamples: VersionAdded: '1.29' StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/HooksBeforeExamples +RSpec/IdenticalEqualityAssertion: + Description: Checks for equality assertions with identical expressions on both sides. + Enabled: pending + VersionAdded: '2.4' + StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/IdenticalEqualityAssertion + RSpec/ImplicitBlockExpectation: Description: Check that implicit block expectation syntax is not used. Enabled: true @@ -753,6 +759,12 @@ RSpec/FactoryBot/FactoryClassName: VersionChanged: '2.0' StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/FactoryBot/FactoryClassName +RSpec/Rails/AvoidSetupHook: + Description: Checks that tests use RSpec `before` hook over Rails `setup` method. + Enabled: pending + VersionAdded: '2.4' + StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Rails/AvoidSetupHook + RSpec/Rails/HttpStatus: Description: Enforces use of symbolic or numeric value to describe HTTP status. Enabled: true diff --git a/docs/antora.yml b/docs/antora.yml index c1b2ea6aa..84ca297f8 100644 --- a/docs/antora.yml +++ b/docs/antora.yml @@ -1,5 +1,5 @@ name: rubocop-rspec title: RuboCop RSpec -version: '2.3' +version: '2.4' nav: - modules/ROOT/nav.adoc diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index 4330d141f..3a28ab91f 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -35,6 +35,7 @@ * xref:cops_rspec.adoc#rspecfocus[RSpec/Focus] * xref:cops_rspec.adoc#rspechookargument[RSpec/HookArgument] * xref:cops_rspec.adoc#rspechooksbeforeexamples[RSpec/HooksBeforeExamples] +* xref:cops_rspec.adoc#rspecidenticalequalityassertion[RSpec/IdenticalEqualityAssertion] * xref:cops_rspec.adoc#rspecimplicitblockexpectation[RSpec/ImplicitBlockExpectation] * xref:cops_rspec.adoc#rspecimplicitexpect[RSpec/ImplicitExpect] * xref:cops_rspec.adoc#rspecimplicitsubject[RSpec/ImplicitSubject] @@ -96,6 +97,7 @@ === Department xref:cops_rspec/rails.adoc[RSpec/Rails] +* xref:cops_rspec/rails.adoc#rspecrails/avoidsetuphook[RSpec/Rails/AvoidSetupHook] * xref:cops_rspec/rails.adoc#rspecrails/httpstatus[RSpec/Rails/HttpStatus] // END_COP_LIST diff --git a/docs/modules/ROOT/pages/cops_rspec.adoc b/docs/modules/ROOT/pages/cops_rspec.adoc index 6bd0f07ed..9cd6d51fa 100644 --- a/docs/modules/ROOT/pages/cops_rspec.adoc +++ b/docs/modules/ROOT/pages/cops_rspec.adoc @@ -1720,6 +1720,37 @@ end * https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/HooksBeforeExamples +== RSpec/IdenticalEqualityAssertion + +|=== +| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged + +| Pending +| Yes +| No +| 2.4 +| - +|=== + +Checks for equality assertions with identical expressions on both sides. + +=== Examples + +[source,ruby] +---- +# bad +expect(foo.bar).to eq(foo.bar) +expect(foo.bar).to eql(foo.bar) + +# good +expect(foo.bar).to eq(2) +expect(foo.bar).to eql(2) +---- + +=== References + +* https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/IdenticalEqualityAssertion + == RSpec/ImplicitBlockExpectation |=== @@ -1827,7 +1858,7 @@ This cop can be configured using the `EnforcedStyle` option === Examples -==== `EnforcedStyle: single_line_only` +==== `EnforcedStyle: single_line_only` (default) [source,ruby] ---- @@ -1843,6 +1874,26 @@ it do end ---- +==== `EnforcedStyle: single_statement_only` + +[source,ruby] +---- +# bad +it do + foo = 1 + is_expected.to be_truthy +end + +# good +it do + foo = 1 + expect(subject).to be_truthy +end +it do + is_expected.to be_truthy +end +---- + ==== `EnforcedStyle: disallow` [source,ruby] diff --git a/docs/modules/ROOT/pages/cops_rspec/rails.adoc b/docs/modules/ROOT/pages/cops_rspec/rails.adoc index 9ef62fe06..b782e4ca7 100644 --- a/docs/modules/ROOT/pages/cops_rspec/rails.adoc +++ b/docs/modules/ROOT/pages/cops_rspec/rails.adoc @@ -1,5 +1,38 @@ = RSpec/Rails +== RSpec/Rails/AvoidSetupHook + +|=== +| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged + +| Pending +| Yes +| Yes +| 2.4 +| - +|=== + +Checks that tests use RSpec `before` hook over Rails `setup` method. + +=== Examples + +[source,ruby] +---- +# bad +setup do + allow(foo).to receive(:bar) +end + +# good +before do + allow(foo).to receive(:bar) +end +---- + +=== References + +* https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Rails/AvoidSetupHook + == RSpec/Rails/HttpStatus |=== diff --git a/lib/rubocop/cop/rspec/described_class.rb b/lib/rubocop/cop/rspec/described_class.rb index aee738e01..05b646ae6 100644 --- a/lib/rubocop/cop/rspec/described_class.rb +++ b/lib/rubocop/cop/rspec/described_class.rb @@ -203,7 +203,7 @@ def const_name(node) [name] elsif namespace.const_type? [*const_name(namespace), name] - elsif namespace.lvar_type? || namespace.cbase_type? + elsif %i[lvar cbase send].include?(namespace.type) [nil, name] end end diff --git a/lib/rubocop/cop/rspec/expect_change.rb b/lib/rubocop/cop/rspec/expect_change.rb index 7c95b772b..7f24332b9 100644 --- a/lib/rubocop/cop/rspec/expect_change.rb +++ b/lib/rubocop/cop/rspec/expect_change.rb @@ -39,7 +39,7 @@ class ExpectChange < Base # @!method expect_change_with_arguments(node) def_node_matcher :expect_change_with_arguments, <<-PATTERN - (send nil? :change ({const send} nil? $_) (sym $_)) + (send nil? :change $_ (sym $_)) PATTERN # @!method expect_change_with_block(node) @@ -55,9 +55,9 @@ def on_send(node) return unless style == :block expect_change_with_arguments(node) do |receiver, message| - msg = format(MSG_CALL, obj: receiver, attr: message) + msg = format(MSG_CALL, obj: receiver.source, attr: message) add_offense(node, message: msg) do |corrector| - replacement = "change { #{receiver}.#{message} }" + replacement = "change { #{receiver.source}.#{message} }" corrector.replace(node, replacement) end end diff --git a/lib/rubocop/cop/rspec/file_path.rb b/lib/rubocop/cop/rspec/file_path.rb index 4bf70ae89..c2ab8f584 100644 --- a/lib/rubocop/cop/rspec/file_path.rb +++ b/lib/rubocop/cop/rspec/file_path.rb @@ -61,10 +61,10 @@ class FilePath < Base MSG = 'Spec path should end with `%s`.' - # @!method const_described(node) - def_node_matcher :const_described, <<~PATTERN + # @!method example_group(node) + def_node_matcher :example_group, <<~PATTERN (block - $(send #rspec? _example_group $(const ...) $...) ... + $(send #rspec? _example_group $_ $...) ... ) PATTERN @@ -74,17 +74,17 @@ class FilePath < Base def on_top_level_example_group(node) return unless top_level_groups.one? - const_described(node) do |send_node, described_class, arguments| + example_group(node) do |send_node, example_group, arguments| next if routing_spec?(arguments) - ensure_correct_file_path(send_node, described_class, arguments) + ensure_correct_file_path(send_node, example_group, arguments) end end private - def ensure_correct_file_path(send_node, described_class, arguments) - pattern = pattern_for(described_class, arguments.first) + def ensure_correct_file_path(send_node, example_group, arguments) + pattern = pattern_for(example_group, arguments.first) return if filename_ends_with?(pattern) # For the suffix shown in the offense message, modify the regular @@ -99,11 +99,13 @@ def routing_spec?(args) args.any?(&method(:routing_metadata?)) end - def pattern_for(described_class, method_name) - return pattern_for_spec_suffix_only? if spec_suffix_only? + def pattern_for(example_group, method_name) + if spec_suffix_only? || !example_group.const_type? + return pattern_for_spec_suffix_only? + end [ - expected_path(described_class), + expected_path(example_group), name_pattern(method_name), '[^/]*_spec\.rb' ].join diff --git a/lib/rubocop/cop/rspec/identical_equality_assertion.rb b/lib/rubocop/cop/rspec/identical_equality_assertion.rb new file mode 100644 index 000000000..89f911b99 --- /dev/null +++ b/lib/rubocop/cop/rspec/identical_equality_assertion.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module RSpec + # Checks for equality assertions with identical expressions on both sides. + # + # @example + # + # # bad + # expect(foo.bar).to eq(foo.bar) + # expect(foo.bar).to eql(foo.bar) + # + # # good + # expect(foo.bar).to eq(2) + # expect(foo.bar).to eql(2) + # + class IdenticalEqualityAssertion < Base + MSG = 'Identical expressions on both sides of the equality ' \ + 'may indicate a flawed test.' + RESTRICT_ON_SEND = %i[to].freeze + + # @!method equality_check?(node) + def_node_matcher :equality_check?, <<~PATTERN + (send (send nil? :expect $_) :to + {(send nil? {:eql :eq :be} $_) + (send (send nil? :be) :== $_)}) + PATTERN + + def on_send(node) + equality_check?(node) do |left, right| + add_offense(node) if left == right + end + end + end + end + end +end diff --git a/lib/rubocop/cop/rspec/implicit_subject.rb b/lib/rubocop/cop/rspec/implicit_subject.rb index 21380de71..dafb02299 100644 --- a/lib/rubocop/cop/rspec/implicit_subject.rb +++ b/lib/rubocop/cop/rspec/implicit_subject.rb @@ -7,7 +7,7 @@ module RSpec # # This cop can be configured using the `EnforcedStyle` option # - # @example `EnforcedStyle: single_line_only` + # @example `EnforcedStyle: single_line_only` (default) # # bad # it do # is_expected.to be_truthy @@ -19,6 +19,22 @@ module RSpec # expect(subject).to be_truthy # end # + # @example `EnforcedStyle: single_statement_only` + # # bad + # it do + # foo = 1 + # is_expected.to be_truthy + # end + # + # # good + # it do + # foo = 1 + # expect(subject).to be_truthy + # end + # it do + # is_expected.to be_truthy + # end + # # @example `EnforcedStyle: disallow` # # bad # it { is_expected.to be_truthy } diff --git a/lib/rubocop/cop/rspec/rails/avoid_setup_hook.rb b/lib/rubocop/cop/rspec/rails/avoid_setup_hook.rb new file mode 100644 index 000000000..00c1abe88 --- /dev/null +++ b/lib/rubocop/cop/rspec/rails/avoid_setup_hook.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module RSpec + module Rails + # Checks that tests use RSpec `before` hook over Rails `setup` method. + # + # @example + # + # # bad + # setup do + # allow(foo).to receive(:bar) + # end + # + # # good + # before do + # allow(foo).to receive(:bar) + # end + # + class AvoidSetupHook < Base + extend AutoCorrector + + MSG = 'Use `before` instead of `setup`.' + + # @!method setup_call(node) + def_node_matcher :setup_call, <<-PATTERN + (block + $(send nil? :setup) + (args) _) + PATTERN + + def on_block(node) + setup_call(node) do |setup| + add_offense(node) do |corrector| + corrector.replace setup, 'before' + end + end + end + end + end + end + end +end diff --git a/lib/rubocop/cop/rspec_cops.rb b/lib/rubocop/cop/rspec_cops.rb index 1de7e025d..2263e0b9a 100644 --- a/lib/rubocop/cop/rspec_cops.rb +++ b/lib/rubocop/cop/rspec_cops.rb @@ -8,6 +8,7 @@ require_relative 'rspec/factory_bot/create_list' require_relative 'rspec/factory_bot/factory_class_name' +require_relative 'rspec/rails/avoid_setup_hook' begin require_relative 'rspec/rails/http_status' rescue LoadError @@ -47,6 +48,7 @@ require_relative 'rspec/focus' require_relative 'rspec/hook_argument' require_relative 'rspec/hooks_before_examples' +require_relative 'rspec/identical_equality_assertion' require_relative 'rspec/implicit_block_expectation' require_relative 'rspec/implicit_expect' require_relative 'rspec/implicit_subject' diff --git a/lib/rubocop/rspec/version.rb b/lib/rubocop/rspec/version.rb index 8e9024ea0..97e0d7ecd 100644 --- a/lib/rubocop/rspec/version.rb +++ b/lib/rubocop/rspec/version.rb @@ -4,7 +4,7 @@ module RuboCop module RSpec # Version information for the RSpec RuboCop plugin. module Version - STRING = '2.3.0' + STRING = '2.4.0' end end end diff --git a/spec/rubocop/cop/rspec/align_left_let_brace_spec.rb b/spec/rubocop/cop/rspec/align_left_let_brace_spec.rb index fea688d9f..22cc7878b 100644 --- a/spec/rubocop/cop/rspec/align_left_let_brace_spec.rb +++ b/spec/rubocop/cop/rspec/align_left_let_brace_spec.rb @@ -43,13 +43,13 @@ end # rubocop:enable RSpec/ExampleLength - it 'does not register offense for let with proc argument' do + it 'does not register an offense for let with proc argument' do expect_no_offenses(<<-RUBY) let(:user, &args[:build_user]) RUBY end - it 'works with empty file' do + it 'does not register an offense with empty file' do expect_no_offenses('') end end diff --git a/spec/rubocop/cop/rspec/align_right_let_brace_spec.rb b/spec/rubocop/cop/rspec/align_right_let_brace_spec.rb index 6538b4a50..dc2010334 100644 --- a/spec/rubocop/cop/rspec/align_right_let_brace_spec.rb +++ b/spec/rubocop/cop/rspec/align_right_let_brace_spec.rb @@ -43,13 +43,13 @@ end # rubocop:enable RSpec/ExampleLength - it 'does not register offense for let with proc argument' do + it 'does not register an offense for let with proc argument' do expect_no_offenses(<<-RUBY) let(:user, &args[:build_user]) RUBY end - it 'works with empty file' do + it 'does not register an offense with empty file' do expect_no_offenses('') end end diff --git a/spec/rubocop/cop/rspec/described_class_spec.rb b/spec/rubocop/cop/rspec/described_class_spec.rb index 17c949936..9da01955b 100644 --- a/spec/rubocop/cop/rspec/described_class_spec.rb +++ b/spec/rubocop/cop/rspec/described_class_spec.rb @@ -77,6 +77,17 @@ RUBY end + it 'allows accessing constants from variables when in a nested namespace' do + expect_no_offenses(<<~RUBY) + module Foo + describe MyClass do + let(:foo) { SomeClass } + let(:baz) { foo::CONST } + end + end + RUBY + end + it 'flags with metadata' do expect_offense(<<-RUBY) describe MyClass, some: :metadata do diff --git a/spec/rubocop/cop/rspec/empty_line_after_final_let_spec.rb b/spec/rubocop/cop/rspec/empty_line_after_final_let_spec.rb index 2e7d3231d..afab09e45 100644 --- a/spec/rubocop/cop/rspec/empty_line_after_final_let_spec.rb +++ b/spec/rubocop/cop/rspec/empty_line_after_final_let_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true RSpec.describe RuboCop::Cop::RSpec::EmptyLineAfterFinalLet do - it 'checks for empty line after last let' do + it 'registers an offense for empty line after last let' do expect_offense(<<-RUBY) RSpec.describe User do let(:a) { a } @@ -21,7 +21,7 @@ RUBY end - it 'check for empty line after the last `let!`' do + it 'registers an offense for empty line after the last `let!`' do expect_offense(<<-RUBY) RSpec.describe User do let(:a) { a } @@ -45,7 +45,7 @@ RUBY end - it 'checks for empty line after let with proc argument' do + it 'registers an offense for empty line after let with proc argument' do expect_offense(<<-RUBY) RSpec.describe User do let(:a) { a } @@ -65,30 +65,32 @@ RUBY end - it 'approves empty line after let' do + it 'does not register an offense for empty line after let' do expect_no_offenses(<<-RUBY) - RSpec.describe User do - let(:a) { a } - let(:b) { b } + RSpec.describe User do + let(:a) { a } + let(:b) { b } - it { expect(a).to eq(b) } - end + it { expect(a).to eq(b) } + end RUBY end - it 'allows comment followed by an empty line after let' do + it 'does not register an offense for comment '\ + 'followed by an empty line after let' do expect_no_offenses(<<-RUBY) - RSpec.describe User do - let(:a) { a } - let(:b) { b } - # end of setup + RSpec.describe User do + let(:a) { a } + let(:b) { b } + # end of setup - it { expect(a).to eq(b) } - end + it { expect(a).to eq(b) } + end RUBY end - it 'flags missing empty line after the comment that comes after last let' do + it 'registers an offense for missing empty line after the comment '\ + 'that comes after last let' do expect_offense(<<-RUBY) RSpec.describe User do let(:a) { a } @@ -110,7 +112,8 @@ RUBY end - it 'flags missing empty line after a multiline comment after last let' do + it 'registers an offense for missing empty line after a multiline comment '\ + 'after last let' do expect_offense(<<-RUBY) RSpec.describe User do let(:a) { a } @@ -134,20 +137,8 @@ RUBY end - it 'ignores empty lines between the lets' do - expect_offense(<<-RUBY) - RSpec.describe User do - let(:a) { a } - - subject { described_class } - - let!(:b) { b } - ^^^^^^^^^^^^^^ Add an empty line after the last `let!`. - it { expect(a).to eq(b) } - end - RUBY - - expect_correction(<<-RUBY) + it 'does not register an offense for empty lines between the lets' do + expect_no_offenses(<<-RUBY) RSpec.describe User do let(:a) { a } @@ -160,7 +151,7 @@ RUBY end - it 'handles let in tests' do + it 'does not register an offense for let in tests' do expect_no_offenses(<<-RUBY) RSpec.describe User do # This shouldn't really ever happen in a sane codebase but I still @@ -173,7 +164,7 @@ RUBY end - it 'handles multiline let block' do + it 'does not register an offense for multiline let block' do expect_no_offenses(<<-RUBY) RSpec.describe User do let(:a) { a } @@ -186,7 +177,7 @@ RUBY end - it 'handles let being the latest node' do + it 'does not register an offense for let being the latest node' do expect_no_offenses(<<-RUBY) RSpec.describe User do let(:a) { a } @@ -195,7 +186,7 @@ RUBY end - it 'handles HEREDOC for let' do + it 'does not register an offense for HEREDOC for let' do expect_no_offenses(<<-RUBY) RSpec.describe User do let(:foo) do @@ -212,7 +203,7 @@ RUBY end - it 'handles silly HEREDOC syntax for let' do + it 'does not register an offense for silly HEREDOC syntax for let' do expect_no_offenses(<<-RUBY) RSpec.describe 'silly heredoc syntax' do let(:foo) { <<-BAR } @@ -227,7 +218,7 @@ RUBY end - it 'handles silly HEREDOC offense' do + it 'registers an offense for silly HEREDOC offense' do expect_offense(<<-RUBY) RSpec.describe 'silly heredoc syntax' do let(:foo) { <<-BAR } diff --git a/spec/rubocop/cop/rspec/empty_line_after_hook_spec.rb b/spec/rubocop/cop/rspec/empty_line_after_hook_spec.rb index 3e98ec5c7..51b162729 100644 --- a/spec/rubocop/cop/rspec/empty_line_after_hook_spec.rb +++ b/spec/rubocop/cop/rspec/empty_line_after_hook_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true RSpec.describe RuboCop::Cop::RSpec::EmptyLineAfterHook do - it 'checks for empty line after `before` hook' do + it 'registers an offense for empty line after `before` hook' do expect_offense(<<-RUBY) RSpec.describe User do before { do_something } @@ -19,7 +19,7 @@ RUBY end - it 'checks for empty line after `after` hook' do + it 'registers an offense for empty line after `after` hook' do expect_offense(<<-RUBY) RSpec.describe User do after { do_something } @@ -37,7 +37,7 @@ RUBY end - it 'checks for empty line after `around` hook' do + it 'registers an offense for empty line after `around` hook' do expect_offense(<<-RUBY) RSpec.describe User do around { |test| test.run } @@ -55,7 +55,7 @@ RUBY end - it 'approves empty line after `before` hook' do + it 'does not register an offense for empty line after `before` hook' do expect_no_offenses(<<-RUBY) RSpec.describe User do before { do_something } @@ -65,7 +65,7 @@ RUBY end - it 'approves empty line after `after` hook' do + it 'does not register an offense for empty line after `after` hook' do expect_no_offenses(<<-RUBY) RSpec.describe User do after { do_something } @@ -75,7 +75,7 @@ RUBY end - it 'approves empty line after `around` hook' do + it 'does not register an offense for empty line after `around` hook' do expect_no_offenses(<<-RUBY) RSpec.describe User do around { |test| test.run } @@ -85,7 +85,7 @@ RUBY end - it 'handles multiline `before` block' do + it 'does not register an offense for multiline `before` block' do expect_no_offenses(<<-RUBY) RSpec.describe User do before do @@ -97,7 +97,7 @@ RUBY end - it 'handles multiline `after` block' do + it 'does not register an offense for multiline `after` block' do expect_no_offenses(<<-RUBY) RSpec.describe User do after do @@ -109,7 +109,7 @@ RUBY end - it 'handles multiline `around` block' do + it 'does not register an offense for multiline `around` block' do expect_no_offenses(<<-RUBY) RSpec.describe User do around do |test| @@ -121,7 +121,7 @@ RUBY end - it 'handles `before` being the latest node' do + it 'does not register an offense for `before` being the latest node' do expect_no_offenses(<<-RUBY) RSpec.describe User do before { do_something } diff --git a/spec/rubocop/cop/rspec/empty_line_after_subject_spec.rb b/spec/rubocop/cop/rspec/empty_line_after_subject_spec.rb index c4cdc0507..a9a3f4e60 100644 --- a/spec/rubocop/cop/rspec/empty_line_after_subject_spec.rb +++ b/spec/rubocop/cop/rspec/empty_line_after_subject_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true RSpec.describe RuboCop::Cop::RSpec::EmptyLineAfterSubject do - it 'checks for empty line after subject' do + it 'registers an offense for empty line after subject' do expect_offense(<<-RUBY) RSpec.describe User do subject { described_class.new } @@ -19,7 +19,7 @@ RUBY end - it 'checks for empty line after subject!' do + it 'registers an offense for empty line after subject!' do expect_offense(<<-RUBY) RSpec.describe User do subject! { described_class.new } @@ -37,7 +37,7 @@ RUBY end - it 'approves empty line after subject' do + it 'does not register an offense for empty line after subject' do expect_no_offenses(<<-RUBY) RSpec.describe User do subject { described_class.new } @@ -47,7 +47,7 @@ RUBY end - it 'approves empty line after subject!' do + it 'does not register an offense for empty line after subject!' do expect_no_offenses(<<-RUBY) RSpec.describe User do subject! { described_class.new } @@ -57,7 +57,7 @@ RUBY end - it 'handles subjects in tests' do + it 'does not register an offense for subjects in tests' do expect_no_offenses(<<-RUBY) RSpec.describe User do # This shouldn't really ever happen in a sane codebase but I still @@ -70,7 +70,7 @@ RUBY end - it 'handles multiline subject block' do + it 'does not register an offense for multiline subject block' do expect_no_offenses(<<-RUBY) RSpec.describe User do subject do @@ -82,7 +82,7 @@ RUBY end - it 'handles subject being the latest node' do + it 'does not register an offense for subject being the latest node' do expect_no_offenses(<<-RUBY) RSpec.describe User do subject { described_user } diff --git a/spec/rubocop/cop/rspec/expect_change_spec.rb b/spec/rubocop/cop/rspec/expect_change_spec.rb index 82476b151..95b7a5477 100644 --- a/spec/rubocop/cop/rspec/expect_change_spec.rb +++ b/spec/rubocop/cop/rspec/expect_change_spec.rb @@ -67,7 +67,7 @@ RUBY end - it 'flags change matcher when receiver is a variable' do + it 'flags change matcher when receiver is a constant' do expect_offense(<<-RUBY) it do expect { run }.to change(User, :count) @@ -82,6 +82,81 @@ RUBY end + it 'flags change matcher when receiver is a top-level constant' do + expect_offense(<<-RUBY) + it do + expect { run }.to change(::User, :count) + ^^^^^^^^^^^^^^^^^^^^^^ Prefer `change { ::User.count }`. + end + RUBY + + expect_correction(<<-RUBY) + it do + expect { run }.to change { ::User.count } + end + RUBY + end + + it 'flags change matcher when receiver is a variable' do + expect_offense(<<-RUBY) + it do + expect { run }.to change(user, :status) + ^^^^^^^^^^^^^^^^^^^^^ Prefer `change { user.status }`. + end + RUBY + + expect_correction(<<-RUBY) + it do + expect { run }.to change { user.status } + end + RUBY + end + + it 'registers an offense for change matcher with chained method call' do + expect_offense(<<-RUBY) + it do + expect { paint_users! }.to change(users.green, :count).by(1) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `change { users.green.count }`. + end + RUBY + + expect_correction(<<-RUBY) + it do + expect { paint_users! }.to change { users.green.count }.by(1) + end + RUBY + end + + it 'registers an offense for change matcher with an instance variable' do + expect_offense(<<-RUBY) + it do + expect { paint_users! }.to change(@food, :taste).to(:sour) + ^^^^^^^^^^^^^^^^^^^^^ Prefer `change { @food.taste }`. + end + RUBY + + expect_correction(<<-RUBY) + it do + expect { paint_users! }.to change { @food.taste }.to(:sour) + end + RUBY + end + + it 'registers an offense for change matcher with a global variable' do + expect_offense(<<-RUBY) + it do + expect { paint_users! }.to change($token, :value).to(nil) + ^^^^^^^^^^^^^^^^^^^^^^ Prefer `change { $token.value }`. + end + RUBY + + expect_correction(<<-RUBY) + it do + expect { paint_users! }.to change { $token.value }.to(nil) + end + RUBY + end + it 'ignores methods called change' do expect_no_offenses(<<-RUBY) it do diff --git a/spec/rubocop/cop/rspec/file_path_spec.rb b/spec/rubocop/cop/rspec/file_path_spec.rb index 87b80be1f..68dfeb2e4 100644 --- a/spec/rubocop/cop/rspec/file_path_spec.rb +++ b/spec/rubocop/cop/rspec/file_path_spec.rb @@ -64,6 +64,13 @@ RUBY end + it 'registers an offense for a feature file missing _spec' do + expect_offense(<<-RUBY, 'spec/features/my_feature.rb') + feature "my feature" do; end + ^^^^^^^^^^^^^^^^^^^^ Spec path should end with `*_spec.rb`. + RUBY + end + it 'registers an offense for a file without the .rb extension' do expect_offense(<<-RUBY, 'spec/models/user_specxrb') describe User do; end @@ -71,119 +78,121 @@ RUBY end - it 'ignores shared examples' do + it 'does not register an offense for shared examples' do expect_no_offenses(<<-RUBY, 'spec/models/user.rb') shared_examples_for 'foo' do; end RUBY end - it 'skips specs that do not describe a class / method' do + it 'does not register an offence for example groups '\ + 'do not describe a class / method' do expect_no_offenses(<<-RUBY, 'some/class/spec.rb') describe 'Test something' do; end RUBY end - it 'skips specs that do have multiple top level describes' do + it 'does not register an offense for multiple top level describes' do expect_no_offenses(<<-RUBY, 'some/class/spec.rb') describe MyClass, 'do_this' do; end describe MyClass, 'do_that' do; end RUBY end - it 'checks class specs' do + it 'does not register an offense for class specs' do expect_no_offenses(<<-RUBY, 'some/class_spec.rb') describe Some::Class do; end RUBY end - it 'allows different parent directories' do + it 'does not register an offense for different parent directories' do expect_no_offenses(<<-RUBY, 'parent_dir/some/class_spec.rb') describe Some::Class do; end RUBY end - it 'handles CamelCaps class names' do + it 'does not register an offense for CamelCaps class names' do expect_no_offenses(<<-RUBY, 'my_class_spec.rb') describe MyClass do; end RUBY end - it 'handles ACRONYMClassNames' do + it 'does not register an offense for ACRONYMClassNames' do expect_no_offenses(<<-RUBY, 'abc_one/two_spec.rb') describe ABCOne::Two do; end RUBY end - it 'handles ALLCAPS class names' do + it 'does not register an offense for ALLCAPS class names' do expect_no_offenses(<<-RUBY, 'allcaps_spec.rb') describe ALLCAPS do; end RUBY end - it 'handles alphanumeric class names' do + it 'does not register an offense for alphanumeric class names' do expect_no_offenses(<<-RUBY, 'ipv4_and_ipv6_spec.rb') describe IPV4AndIPV6 do; end RUBY end - it 'checks instance methods' do + it 'does not register an offense for instance methods' do expect_no_offenses(<<-RUBY, 'some/class/inst_spec.rb') describe Some::Class, '#inst' do; end RUBY end - it 'checks class methods' do + it 'does not register an offense for class methods' do expect_no_offenses(<<-RUBY, 'some/class/inst_spec.rb') describe Some::Class, '.inst' do; end RUBY end - it 'allows flat hierarchies for instance methods' do + it 'does not register an offense for flat hierarchies for instance methods' do expect_no_offenses(<<-RUBY, 'some/class_inst_spec.rb') describe Some::Class, '#inst' do; end RUBY end - it 'allows flat hierarchies for class methods' do + it 'does not register an offense for flat hierarchies for class methods' do expect_no_offenses(<<-RUBY, 'some/class_inst_spec.rb') describe Some::Class, '.inst' do; end RUBY end - it 'allows subdirs for instance methods' do + it 'does not register an offense for subdirs for instance methods' do filename = 'some/class/instance_methods/inst_spec.rb' expect_no_offenses(<<-RUBY, filename) describe Some::Class, '#inst' do; end RUBY end - it 'allows subdirs for class methods' do + it 'does not register an offense for subdirs for class methods' do filename = 'some/class/class_methods/inst_spec.rb' expect_no_offenses(<<-RUBY, filename) describe Some::Class, '.inst' do; end RUBY end - it 'ignores non-alphanumeric characters' do + it 'does not register an offense for non-alphanumeric characters' do expect_no_offenses(<<-RUBY, 'some/class/pred_spec.rb') describe Some::Class, '#pred?' do; end RUBY end - it 'allows bang method' do + it 'does not register an offense for bang method' do expect_no_offenses(<<-RUBY, 'some/class/bang_spec.rb') describe Some::Class, '#bang!' do; end RUBY end - it 'allows flexibility with predicates' do + it 'does not register an offence for an arbitrary spec suffix' do filename = 'some/class/thing_predicate_spec.rb' expect_no_offenses(<<-RUBY, filename) describe Some::Class, '#thing?' do; end RUBY end - it 'allows flexibility with operators' do + it 'does not register an offence for an arbitrary spec name '\ + 'for an operator method' do filename = 'my_little_class/spaceship_operator_spec.rb' expect_no_offenses(<<-RUBY, filename) describe MyLittleClass, '#<=>' do; end @@ -197,14 +206,14 @@ RUBY end - it 'detects the path based on a class name with long module' do + it 'registers an offense for path based on a class name with long module' do expect_offense(<<-RUBY, '/home/foo/spec/very/my_class_spec.rb') describe Very::Long::Namespace::MyClass do; end ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Spec path should end with `very/long/namespace/my_class*_spec.rb`. RUBY end - it 'detects the path based the absolute path of the file' do + it 'does not register offense for absolute file path' do allow(File).to receive(:expand_path).with('my_class_spec.rb').and_return( '/home/foo/spec/very/long/namespace/my_class_spec.rb' ) @@ -214,7 +223,7 @@ end # RSpec/FilePath runs on all files - not only **/*_spec.rb - it 'works on files defining an empty class' do + it 'does not register an offense for files defining an empty class' do expect_no_offenses(<<-RUBY) class Foo end @@ -224,13 +233,13 @@ class Foo context 'when configured with CustomTransform' do let(:cop_config) { { 'CustomTransform' => { 'FooFoo' => 'foofoo' } } } - it 'respects custom module name transformation' do + it 'does not register an offense for custom module name transformation' do expect_no_offenses(<<-RUBY, 'foofoo/some/class/bar_spec.rb') describe FooFoo::Some::Class, '#bar' do; end RUBY end - it 'ignores routing specs' do + it 'does not register an offense for routing specs' do expect_no_offenses(<<-RUBY, 'foofoo/some/class/bar_spec.rb') describe MyController, "#foo", type: :routing do; end RUBY @@ -240,7 +249,7 @@ class Foo context 'when configured with IgnoreMethods' do let(:cop_config) { { 'IgnoreMethods' => true } } - it 'does not care about the described method' do + it 'does not register an offense for the described method' do expect_no_offenses(<<-RUBY, 'my_class_spec.rb') describe MyClass, '#look_here_a_method' do; end RUBY @@ -250,7 +259,7 @@ class Foo context 'when configured with SpecSuffixOnly' do let(:cop_config) { { 'SpecSuffixOnly' => true } } - it 'does not care about the described class' do + it 'does not register an offense for the described class' do expect_no_offenses(<<-RUBY, 'whatever_spec.rb') describe MyClass do; end RUBY @@ -263,6 +272,13 @@ class Foo RUBY end + it 'registers an offense when a feature file is missing _spec.rb suffix' do + expect_offense(<<-RUBY, 'spec/my_feature.rb') + feature "my feature" do; end + ^^^^^^^^^^^^^^^^^^^^ Spec path should end with `*_spec.rb`. + RUBY + end + it 'registers an offense when the file extension is not .rb' do expect_offense(<<-RUBY, 'whatever_specxrb') describe MyClass do; end diff --git a/spec/rubocop/cop/rspec/identical_equality_assertion_spec.rb b/spec/rubocop/cop/rspec/identical_equality_assertion_spec.rb new file mode 100644 index 000000000..0524f4d20 --- /dev/null +++ b/spec/rubocop/cop/rspec/identical_equality_assertion_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::RSpec::IdenticalEqualityAssertion do + it 'registers an offense when using identical expressions with `eq`' do + expect_offense(<<~RUBY) + expect(foo.bar).to eq(foo.bar) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Identical expressions on both sides of the equality may indicate a flawed test. + RUBY + end + + it 'registers an offense when using identical expressions with `eql`' do + expect_offense(<<~RUBY) + expect(foo.bar.baz).to eql(foo.bar.baz) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Identical expressions on both sides of the equality may indicate a flawed test. + RUBY + end + + it 'registers an offense for trivial constants' do + expect_offense(<<~RUBY) + expect(42).to eq(42) + ^^^^^^^^^^^^^^^^^^^^ Identical expressions on both sides of the equality may indicate a flawed test. + RUBY + end + + it 'registers an offense for complex constants' do + expect_offense(<<~RUBY) + expect({a: 1, b: :b}).to eql({a: 1, b: :b}) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Identical expressions on both sides of the equality may indicate a flawed test. + RUBY + end + + it 'registers an offense for identical expression with be' do + expect_offense(<<~RUBY) + expect(foo.bar).to be(foo.bar) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Identical expressions on both sides of the equality may indicate a flawed test. + RUBY + end + + it 'registers an offense for identical expression with be ==' do + expect_offense(<<~RUBY) + expect(foo.bar).to be == foo.bar + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Identical expressions on both sides of the equality may indicate a flawed test. + RUBY + end + + it 'does not register offense for different expressions' do + expect_no_offenses(<<~RUBY) + expect(foo.bar).to eq(bar.foo) + RUBY + end + + it 'checks for whole expression' do + expect_no_offenses(<<~RUBY) + expect(Foo.new(1).foo).to eql(Foo.new(2).bar) + RUBY + end +end diff --git a/spec/rubocop/cop/rspec/leading_subject_spec.rb b/spec/rubocop/cop/rspec/leading_subject_spec.rb index 994595d22..847e343f3 100644 --- a/spec/rubocop/cop/rspec/leading_subject_spec.rb +++ b/spec/rubocop/cop/rspec/leading_subject_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true RSpec.describe RuboCop::Cop::RSpec::LeadingSubject do - it 'checks subject below let' do + it 'registers an offense for subject below let' do expect_offense(<<-RUBY) RSpec.describe User do let(:params) { foo } @@ -20,7 +20,7 @@ RUBY end - it 'checks subject below let!' do + it 'registers an offense for subject below let!' do expect_offense(<<-RUBY) RSpec.describe User do let!(:params) { foo } @@ -39,7 +39,7 @@ RUBY end - it 'checks subject below let with proc argument' do + it 'registers an offense for subject below let with proc argument' do expect_offense(<<-RUBY) RSpec.describe User do let(:user, &args[:build_user]) @@ -58,7 +58,7 @@ RUBY end - it 'approves of subject above let' do + it 'does not register an offense for subject above let' do expect_no_offenses(<<-RUBY) RSpec.describe User do subject { described_class.new } @@ -71,7 +71,7 @@ RUBY end - it 'handles subjects in contexts' do + it 'does not register an offense for subjects in contexts' do expect_no_offenses(<<-RUBY) RSpec.describe User do let(:params) { foo } @@ -83,7 +83,7 @@ RUBY end - it 'handles subjects in tests' do + it 'does not register an offense for subjects in tests' do expect_no_offenses(<<-RUBY) RSpec.describe User do # This shouldn't really ever happen in a sane codebase but I still @@ -96,7 +96,7 @@ RUBY end - it 'checks subject below hook' do + it 'registers an offense for subject below hook' do expect_offense(<<-RUBY) RSpec.describe User do before { allow(Foo).to receive(:bar) } @@ -115,7 +115,7 @@ RUBY end - it 'checks subject below example' do + it 'registers an offense for subject below example' do expect_offense(<<-RUBY) RSpec.describe User do it { is_expected.to be_present } @@ -134,7 +134,7 @@ RUBY end - it 'checks also when subject is below a non-offending node' do + it 'registers an offense when subject is below a non-offending node' do expect_offense(<<~RUBY) RSpec.describe do def helper_method @@ -159,7 +159,7 @@ def helper_method RUBY end - it 'flags subject below example group' do + it 'registers an offense for subject below example group' do expect_offense(<<-RUBY) RSpec.describe User do describe do @@ -182,7 +182,7 @@ def helper_method RUBY end - it 'flags subject below shared example group' do + it 'registers an offense for subject below shared example group' do expect_offense(<<-RUBY) RSpec.describe User do shared_examples_for 'used later' do @@ -205,7 +205,7 @@ def helper_method RUBY end - it 'flags subject below include' do + it 'registers an offense for subject below include' do expect_offense(<<-RUBY) RSpec.describe User do it_behaves_like 'a good citizen' @@ -224,7 +224,7 @@ def helper_method RUBY end - it 'flags subject below include with a block' do + it 'registers an offense for subject below include with a block' do expect_offense(<<-RUBY) RSpec.describe User do it_behaves_like 'a good citizen' do @@ -247,7 +247,7 @@ def helper_method RUBY end - it 'flags subject below include with a blockpass' do + it 'registers an offense for subject below include with a blockpass' do expect_offense(<<-RUBY) RSpec.describe User do block = ->{ } @@ -268,7 +268,7 @@ def helper_method RUBY end - it 'ignores subject nested inside a block' do + it 'does not register an offense for subject nested inside a block' do expect_no_offenses(<<-RUBY) RSpec.describe User do let(:foo) { 'bar' } diff --git a/spec/rubocop/cop/rspec/overwriting_setup_spec.rb b/spec/rubocop/cop/rspec/overwriting_setup_spec.rb index 7ae84b38a..7a56e9c2a 100644 --- a/spec/rubocop/cop/rspec/overwriting_setup_spec.rb +++ b/spec/rubocop/cop/rspec/overwriting_setup_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true RSpec.describe RuboCop::Cop::RSpec::OverwritingSetup do - it 'finds overwriten `let`' do + it 'registers an offense for overwriten `let`' do expect_offense(<<-RUBY) RSpec.describe User do let(:a) { a } @@ -11,7 +11,7 @@ RUBY end - it 'finds overwriten `subject`' do + it 'registers an offense for overwriten `subject`' do expect_offense(<<-RUBY) RSpec.describe User do subject(:a) { a } @@ -22,7 +22,7 @@ RUBY end - it 'works with `subject!` and `let!`' do + it 'registers an offense for `subject!` and `let!`' do expect_offense(<<-RUBY) RSpec.describe User do subject!(:a) { a } @@ -33,7 +33,7 @@ RUBY end - it 'finds `let!` overwriting `let`' do + it 'registers an offense for `let!` overwriting `let`' do expect_offense(<<-RUBY) RSpec.describe User do let(:a) { b } @@ -43,7 +43,7 @@ RUBY end - it 'ignores overwriting in different context' do + it 'does not register an offense for overwriting in different context' do expect_no_offenses(<<-RUBY) RSpec.describe User do let(:a) { a } @@ -55,7 +55,7 @@ RUBY end - it 'handles unnamed subjects' do + it 'registers an offense for overriding an unnamed subject' do expect_offense(<<-RUBY) RSpec.describe User do subject { a } @@ -66,7 +66,7 @@ RUBY end - it 'handles dynamic names for `let`' do + it 'does not register an offense for dynamic names for `let`' do expect_no_offenses(<<-RUBY) RSpec.describe User do subject(:name) { a } @@ -76,7 +76,7 @@ RUBY end - it 'handles string arguments' do + it 'registers an offense for string arguments' do expect_offense(<<-RUBY) RSpec.describe User do subject(:name) { a } diff --git a/spec/rubocop/cop/rspec/rails/avoid_setup_hook_spec.rb b/spec/rubocop/cop/rspec/rails/avoid_setup_hook_spec.rb new file mode 100644 index 000000000..01c952752 --- /dev/null +++ b/spec/rubocop/cop/rspec/rails/avoid_setup_hook_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::RSpec::Rails::AvoidSetupHook do + it 'registers an offense for `setup`' do + expect_offense(<<~RUBY) + setup do + ^^^^^^^^ Use `before` instead of `setup`. + allow(foo).to receive(:bar) + end + RUBY + + expect_correction(<<~RUBY) + before do + allow(foo).to receive(:bar) + end + RUBY + end + + it 'does not register an offense for `before`' do + expect_no_offenses(<<~RUBY) + before do + allow(foo).to receive(:bar) + end + RUBY + end + + it 'does not register an offense for an unrelated `setup` call' do + expect_no_offenses(<<~RUBY) + navigation.setup do + direction 'to infinity!' + end + RUBY + end +end diff --git a/spec/rubocop/cop/rspec/return_from_stub_spec.rb b/spec/rubocop/cop/rspec/return_from_stub_spec.rb index 390c83cec..5af3ca1bd 100644 --- a/spec/rubocop/cop/rspec/return_from_stub_spec.rb +++ b/spec/rubocop/cop/rspec/return_from_stub_spec.rb @@ -157,7 +157,7 @@ RUBY end - it 'finds concatenated strings with no variables' do + it 'registers an offense for concatenated strings with no variables' do expect_offense(<<-RUBY) it do allow(Foo).to receive(:bar) do @@ -176,7 +176,7 @@ RUBY end - it 'ignores stubs without return value' do + it 'does not register an offense for a stub without return value' do expect_no_offenses(<<-RUBY) it do allow(Foo).to receive(:bar) @@ -184,7 +184,7 @@ RUBY end - it 'handles stubs in a method' do + it 'does not register an offense for stub in a method' do expect_no_offenses(<<-RUBY) def stub_foo allow(Foo).to receive(:bar) @@ -196,7 +196,7 @@ def stub_foo context 'with EnforcedStyle `block`' do let(:enforced_style) { 'block' } - it 'finds static values returned from method' do + it 'registers an offense for static values returned from method' do expect_offense(<<-RUBY) it do allow(Foo).to receive(:bar).and_return(42) @@ -205,7 +205,7 @@ def stub_foo RUBY end - it 'finds static values returned from chained method' do + it 'registers an offense for static values returned from chained method' do expect_offense(<<-RUBY) it do allow(Foo).to receive(:bar).with(1).and_return(42) @@ -214,7 +214,7 @@ def stub_foo RUBY end - it 'ignores dynamic values returned from method' do + it 'does not register an offense for dynamic values returned from method' do expect_no_offenses(<<-RUBY) it do allow(Foo).to receive(:bar).and_return(baz)