diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index d998a5008..4fe16c6dc 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -7,6 +7,7 @@ jobs: publish: name: Publish to RubyGems runs-on: ubuntu-latest + if: github.repository_owner == 'rubocop' permissions: actions: write contents: write diff --git a/.rubocop.yml b/.rubocop.yml index 3e2119c92..e8625243b 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -289,3 +289,7 @@ Performance/StringIdentifierArgument: {Enabled: true} Performance/StringInclude: {Enabled: true} Performance/Sum: {Enabled: true} Performance/ZipWithoutBlock: {Enabled: true} + +# Enable our own pending cops. + +RSpec/IncludeExamples: {Enabled: true} diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index f65177dd0..fb7116182 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -6,27 +6,6 @@ # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# This cop supports safe autocorrection (--autocorrect). -InternalAffairs/NodePatternGroups: - Exclude: - - 'lib/rubocop/cop/rspec/described_class.rb' - - 'lib/rubocop/cop/rspec/described_class_module_wrapping.rb' - - 'lib/rubocop/cop/rspec/hook_argument.rb' - - 'lib/rubocop/cop/rspec/hooks_before_examples.rb' - - 'lib/rubocop/cop/rspec/no_expectation_example.rb' - - 'lib/rubocop/cop/rspec/pending_without_reason.rb' - - 'lib/rubocop/cop/rspec/redundant_around.rb' - - 'lib/rubocop/rspec/language.rb' - -# This cop supports safe autocorrection (--autocorrect). -InternalAffairs/NodeTypeMultiplePredicates: - Exclude: - - 'lib/rubocop/cop/rspec/empty_example_group.rb' - - 'lib/rubocop/cop/rspec/predicate_matcher.rb' - - 'lib/rubocop/cop/rspec/receive_messages.rb' - - 'lib/rubocop/cop/rspec/variable_definition.rb' - - 'lib/rubocop/cop/rspec/variable_name.rb' - Rake/MethodDefinitionInTask: Exclude: - 'tasks/cut_release.rake' diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a430230a..33071e72e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,22 @@ ## Master (Unreleased) +- Mark `RSpec/IncludeExamples` as `SafeAutoCorrect: false`. ([@yujideveloper]) +- Fix a false positive for `RSpec/LeakyConstantDeclaration` when defining constants in explicit namespaces. ([@naveg]) +- Add support for error matchers (`raise_exception` and `raise_error`) to `RSpec/Dialect`. ([@lovro-bikic]) +- Don't register offenses for `RSpec/DescribedClass` within `Data.define` blocks. ([@lovro-bikic]) +- Add autocorrection support for `RSpec/IteratedExpectation` for single expectations. ([@lovro-bikic]) +- Exclude all cops from inspecting factorybot files, except if explicitly included. ([@Mth0158]) + +## 3.6.0 (2025-04-18) + +- Fix false positive in `RSpec/Pending`, where it would mark the default block `it` as an offense. ([@bquorning]) +- Fix issue when `Style/ContextWording` is configured with a Prefix being interpreted as a boolean, like `on`. ([@sakuro]) +- Add new `RSpec/IncludeExamples` cop to enforce using `it_behaves_like` over `include_examples`. ([@dvandersluis]) +- Change `RSpec/ScatteredSetup` to allow `around` hooks to be scattered. ([@ydah]) +- Fix an error `RSpec/ChangeByZero` cop when without expect block. ([@lee266]) +- Fix a false positive for `RSpec/DescribedClass` when `SkipBlocks` is true and numblocks are used. ([@earlopain]) + ## 3.5.0 (2025-02-16) - Don't let `RSpec/PredicateMatcher` replace `respond_to?` with two arguments with the RSpec `respond_to` matcher. ([@bquorning]) @@ -998,6 +1014,7 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features. [@krororo]: https://github.com/krororo [@kuahyeow]: https://github.com/kuahyeow [@lazycoder9]: https://github.com/lazycoder9 +[@lee266]: https://github.com/lee266 [@leoarnold]: https://github.com/leoarnold [@liberatys]: https://github.com/Liberatys [@lokhi]: https://github.com/lokhi @@ -1011,6 +1028,7 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features. [@mlarraz]: https://github.com/mlarraz [@mockdeep]: https://github.com/mockdeep [@mothonmars]: https://github.com/MothOnMars +[@mth0158]: https://github.com/Mth0158 [@mvz]: https://github.com/mvz [@naveg]: https://github.com/naveg [@nc-holodakg]: https://github.com/nc-holodakg @@ -1038,6 +1056,7 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features. [@rrosenblum]: https://github.com/rrosenblum [@rspeicher]: https://github.com/rspeicher [@rst-j]: https://github.com/RST-J +[@sakuro]: https://github.com/sakuro [@samrjenkins]: https://github.com/samrjenkins [@schmijos]: https://github.com/schmijos [@seanpdoyle]: https://github.com/seanpdoyle @@ -1062,5 +1081,6 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features. [@ydah]: https://github.com/ydah [@yevhene]: https://github.com/yevhene [@ypresto]: https://github.com/ypresto +[@yujideveloper]: https://github.com/yujideveloper [@zdennis]: https://github.com/zdennis [@zverok]: https://github.com/zverok diff --git a/config/default.yml b/config/default.yml index d52125d61..dd6206314 100644 --- a/config/default.yml +++ b/config/default.yml @@ -6,6 +6,10 @@ RSpec: Include: - "**/*_spec.rb" - "**/spec/**/*" + Exclude: + - "**/spec/factories.rb" + - "**/spec/factories/**/*.rb" + - "**/features/support/factories/**/*.rb" Language: inherit_mode: merge: @@ -79,6 +83,9 @@ RSpec: - prepend_after - after - append_after + ErrorMatchers: + - raise_error + - raise_exception Includes: inherit_mode: merge: @@ -532,6 +539,14 @@ RSpec/ImplicitSubject: VersionChanged: '2.13' Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ImplicitSubject +RSpec/IncludeExamples: + Description: Checks for usage of `include_examples`. + Enabled: pending + SafeAutoCorrect: false + VersionAdded: '3.6' + VersionChanged: "<>" + Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/IncludeExamples + RSpec/IndexedLet: Description: Do not set up test data using indexes (e.g., `item_1`, `item_2`). Enabled: true @@ -776,7 +791,7 @@ RSpec/ReceiveCounts: Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ReceiveCounts RSpec/ReceiveMessages: - Description: Checks for multiple messages stubbed on the same object. + Description: Prefer `receive_messages` over multiple `receive`s on the same object. Enabled: true SafeAutoCorrect: false VersionAdded: '2.23' diff --git a/docs/antora.yml b/docs/antora.yml index fe1caf800..51a533a15 100644 --- a/docs/antora.yml +++ b/docs/antora.yml @@ -1,5 +1,5 @@ name: rubocop-rspec title: RuboCop RSpec -version: '3.5' +version: ~ nav: - modules/ROOT/nav.adoc diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index 16ddf0fd6..08329092b 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -50,6 +50,7 @@ * xref:cops_rspec.adoc#rspecimplicitblockexpectation[RSpec/ImplicitBlockExpectation] * xref:cops_rspec.adoc#rspecimplicitexpect[RSpec/ImplicitExpect] * xref:cops_rspec.adoc#rspecimplicitsubject[RSpec/ImplicitSubject] +* xref:cops_rspec.adoc#rspecincludeexamples[RSpec/IncludeExamples] * xref:cops_rspec.adoc#rspecindexedlet[RSpec/IndexedLet] * xref:cops_rspec.adoc#rspecinstancespy[RSpec/InstanceSpy] * xref:cops_rspec.adoc#rspecinstancevariable[RSpec/InstanceVariable] diff --git a/docs/modules/ROOT/pages/cops_rspec.adoc b/docs/modules/ROOT/pages/cops_rspec.adoc index 319462940..2c0a7eba9 100644 --- a/docs/modules/ROOT/pages/cops_rspec.adoc +++ b/docs/modules/ROOT/pages/cops_rspec.adoc @@ -985,6 +985,13 @@ To narrow down this setting to only a specific directory, it is possible to use an overriding configuration file local to that directory. +[#safety-rspecdescribedclass] +=== Safety + +Autocorrection is unsafe when `SkipBlocks: false` because +`described_class` might not be available within the block (for +example, in rspec-rails's `controller` helper). + [#examples-rspecdescribedclass] === Examples @@ -1153,6 +1160,7 @@ A dialect can be based on the following RSpec methods: - let, let! - subject, subject! - expect, is_expected, expect_any_instance_of +- raise_error, raise_exception By default all of the RSpec methods and aliases are allowed. By setting a config like: @@ -2125,11 +2133,32 @@ expect(false).to eq(true) Checks for consistent style of change matcher. -Enforces either passing object and attribute as arguments to the matcher -or passing a block that reads the attribute value. +Enforces either passing a receiver and message as method arguments, +or a block. This cop can be configured using the `EnforcedStyle` option. +[#safety-rspecexpectchange] +=== Safety + +Autocorrection is unsafe because `method_call` style calls the +receiver *once* and sends the message to it before and after +calling the `expect` block, whereas `block` style calls the +expression *twice*, including the receiver. + +If your receiver is dynamic (e.g., the result of a method call) and +you expect it to be called before and after the `expect` block, +changing from `block` to `method_call` style may break your test. + +[source,ruby] +---- +expect { run }.to change { my_method.message } +# `my_method` is called before and after `run` + +expect { run }.to change(my_method, :message) +# `my_method` is called once, but `message` is called on it twice +---- + [#examples-rspecexpectchange] === Examples @@ -2743,6 +2772,99 @@ it { expect(named_subject).to be_truthy } * https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ImplicitSubject +[#rspecincludeexamples] +== RSpec/IncludeExamples + +|=== +| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed + +| Pending +| Yes +| Always (Unsafe) +| 3.6 +| <> +|=== + +Checks for usage of `include_examples`. + +`include_examples`, unlike `it_behaves_like`, does not create its +own context. As such, using `subject`, `let`, `before`/`after`, etc. +within shared examples included with `include_examples` can have +unexpected behavior and side effects. + +Prefer using `it_behaves_like` instead. + +---- + +[#safety-rspecincludeexamples] +=== Safety + +`include_examples` and `it_behaves_like` have different scoping +behaviors. +Changing `include_examples` to `it_behaves_like` creates a new +context, altering setup dependencies, which can lead to unexpected +test failures. +Specifically, the scope of hooks (`before`, `after`, `around`) +changes, which may prevent expected setup from being inherited +correctly. + +Additionally, `let` and `subject` are affected by scoping rules. +When `include_examples` is used, `let` and `subject` defined within +`shared_examples` are evaluated in the caller's context, allowing +access to their values. +In contrast, `it_behaves_like` creates a new context, preventing +access to `let` or `subject` values from the caller's context. + +[source,ruby] +---- +shared_examples "mock behavior" do + before do + allow(service).to receive(:call).and_return("mocked response") + end + + it "returns mocked response" do + expect(service.call).to eq "mocked response" + end +end + +context "working example with include_examples" do + let(:service) { double(:service) } + + include_examples "mock behavior" + + it "uses the mocked service" do + expect(service.call).to eq "mocked response" # Passes + end +end + +context "broken example with it_behaves_like" do + let(:service) { double(:service) } + + it_behaves_like "mock behavior" + + it "unexpectedly does not use the mocked service" do + # Fails because `it_behaves_like` does not apply the mock setup + expect(service.call).to eq "mocked response" + end +end + +[#examples-rspecincludeexamples] +=== Examples + +[source,ruby] +---- +# bad +include_examples 'examples' + +# good +it_behaves_like 'examples' +---- + +[#references-rspecincludeexamples] +=== References + +* https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/IncludeExamples + [#rspecindexedlet] == RSpec/IndexedLet @@ -3069,7 +3191,7 @@ it_should_behave_like 'a foo' | Enabled | Yes -| No +| Always | 1.14 | - |=== @@ -4827,7 +4949,7 @@ expect(foo).to receive(:bar).at_most(:twice).times | - |=== -Checks for multiple messages stubbed on the same object. +Prefer `receive_messages` over multiple `receive`s on the same object. [#safety-rspecreceivemessages] === Safety @@ -5464,7 +5586,9 @@ end Checks for setup scattered across multiple hooks in an example group. -Unify `before`, `after`, and `around` hooks when possible. +Unify `before` and `after` hooks when possible. +However, `around` hooks are allowed to be defined multiple times, +as unifying them would typically make the code harder to read. [#examples-rspecscatteredsetup] === Examples @@ -5484,6 +5608,12 @@ describe Foo do setup2 end end + +# good +describe Foo do + around { |example| before1; example.call; after1 } + around { |example| before2; example.call; after2 } +end ---- [#references-rspecscatteredsetup] diff --git a/lib/rubocop/cop/rspec/change_by_zero.rb b/lib/rubocop/cop/rspec/change_by_zero.rb index 71407f74b..64c5df4b2 100644 --- a/lib/rubocop/cop/rspec/change_by_zero.rb +++ b/lib/rubocop/cop/rspec/change_by_zero.rb @@ -102,6 +102,8 @@ def on_send(node) private def register_offense(node, change_node) + return unless node.parent.send_type? + if compound_expectations?(node) add_offense(node, message: message_compound(change_node)) do |corrector| @@ -116,8 +118,7 @@ def register_offense(node, change_node) end def compound_expectations?(node) - node.parent.send_type? && - %i[and or & |].include?(node.parent.method_name) + %i[and or & |].include?(node.parent.method_name) end def message(change_node) diff --git a/lib/rubocop/cop/rspec/context_wording.rb b/lib/rubocop/cop/rspec/context_wording.rb index 3fdba92f6..96c40a76e 100644 --- a/lib/rubocop/cop/rspec/context_wording.rb +++ b/lib/rubocop/cop/rspec/context_wording.rb @@ -115,7 +115,12 @@ def expect_patterns end def prefixes - Array(cop_config.fetch('Prefixes', [])) + Array(cop_config.fetch('Prefixes', [])).tap do |prefixes| + non_strings = prefixes.reject { |pre| pre.is_a?(String) } + unless non_strings.empty? + raise "Non-string prefixes #{non_strings.inspect} detected." + end + end end end end diff --git a/lib/rubocop/cop/rspec/described_class.rb b/lib/rubocop/cop/rspec/described_class.rb index 389e675c5..6558f64d5 100644 --- a/lib/rubocop/cop/rspec/described_class.rb +++ b/lib/rubocop/cop/rspec/described_class.rb @@ -13,6 +13,20 @@ module RSpec # `OnlyStaticConstants` is only relevant when `EnforcedStyle` is # `described_class`. # + # There's a known caveat with rspec-rails's `controller` helper that + # runs its block in a different context, and `described_class` is not + # available to it. `SkipBlocks` option excludes detection in all + # non-RSpec related blocks. + # + # To narrow down this setting to only a specific directory, it is + # possible to use an overriding configuration file local to that + # directory. + # + # @safety + # Autocorrection is unsafe when `SkipBlocks: false` because + # `described_class` might not be available within the block (for + # example, in rspec-rails's `controller` helper). + # # @example `EnforcedStyle: described_class` (default) # # bad # describe MyClass do @@ -47,15 +61,6 @@ module RSpec # subject { MyClass.do_something } # end # - # There's a known caveat with rspec-rails's `controller` helper that - # runs its block in a different context, and `described_class` is not - # available to it. `SkipBlocks` option excludes detection in all - # non-RSpec related blocks. - # - # To narrow down this setting to only a specific directory, it is - # possible to use an overriding configuration file local to that - # directory. - # # @example `SkipBlocks: true` # # spec/controllers/.rubocop.yml # # RSpec/DescribedClass: @@ -78,12 +83,18 @@ class DescribedClass < Base # rubocop:disable Metrics/ClassLength # @!method common_instance_exec_closure?(node) def_node_matcher :common_instance_exec_closure?, <<~PATTERN - (block (send (const nil? {:Class :Module :Struct}) :new ...) ...) + (block + { + (send (const nil? {:Class :Module :Struct}) :new ...) + (send (const nil? :Data) :define ...) + } + ... + ) PATTERN # @!method rspec_block?(node) def_node_matcher :rspec_block?, - '({block numblock} (send #rspec? #ALL.all ...) ...)' + '(any_block (send #rspec? #ALL.all ...) ...)' # @!method scope_changing_syntax?(node) def_node_matcher :scope_changing_syntax?, '{def class module}' @@ -153,7 +164,9 @@ def scope_change?(node) end def skippable_block?(node) - node.block_type? && !rspec_block?(node) && cop_config['SkipBlocks'] + return false unless cop_config['SkipBlocks'] + + node.any_block_type? && !rspec_block?(node) end def only_static_constants? diff --git a/lib/rubocop/cop/rspec/described_class_module_wrapping.rb b/lib/rubocop/cop/rspec/described_class_module_wrapping.rb index 263bec673..9ea0a786d 100644 --- a/lib/rubocop/cop/rspec/described_class_module_wrapping.rb +++ b/lib/rubocop/cop/rspec/described_class_module_wrapping.rb @@ -24,7 +24,7 @@ class DescribedClassModuleWrapping < Base # @!method include_rspec_blocks?(node) def_node_search :include_rspec_blocks?, <<~PATTERN - ({block numblock} (send #explicit_rspec? #ExampleGroups.all ...) ...) + (any_block (send #explicit_rspec? #ExampleGroups.all ...) ...) PATTERN def on_module(node) diff --git a/lib/rubocop/cop/rspec/dialect.rb b/lib/rubocop/cop/rspec/dialect.rb index 5bc91f0cc..f7541a7e9 100644 --- a/lib/rubocop/cop/rspec/dialect.rb +++ b/lib/rubocop/cop/rspec/dialect.rb @@ -21,6 +21,7 @@ module RSpec # - let, let! # - subject, subject! # - expect, is_expected, expect_any_instance_of + # - raise_error, raise_exception # # By default all of the RSpec methods and aliases are allowed. By setting # a config like: diff --git a/lib/rubocop/cop/rspec/empty_example_group.rb b/lib/rubocop/cop/rspec/empty_example_group.rb index b1e788b3c..3e0d7878e 100644 --- a/lib/rubocop/cop/rspec/empty_example_group.rb +++ b/lib/rubocop/cop/rspec/empty_example_group.rb @@ -137,7 +137,7 @@ class EmptyExampleGroup < Base PATTERN def on_block(node) # rubocop:disable InternalAffairs/NumblockHandler - return if node.each_ancestor(:def, :defs).any? + return if node.each_ancestor(:any_def).any? return if node.each_ancestor(:block).any? { |block| example?(block) } example_group_body(node) do |body| @@ -155,7 +155,7 @@ def offensive?(body) return true unless body return false if conditionals_with_examples?(body) - if body.if_type? || body.case_type? + if body.type?(:if, :case) !examples_in_branches?(body) else !examples?(body) @@ -163,7 +163,7 @@ def offensive?(body) end def conditionals_with_examples?(body) - return false unless body.begin_type? || body.case_type? + return false unless body.type?(:begin, :case) body.each_descendant(:if, :case).any? do |condition_node| examples_in_branches?(condition_node) @@ -172,7 +172,7 @@ def conditionals_with_examples?(body) def examples_in_branches?(condition_node) return false unless condition_node - return false if !condition_node.if_type? && !condition_node.case_type? + return false unless condition_node.type?(:if, :case) condition_node.branches.any? { |branch| examples?(branch) } end diff --git a/lib/rubocop/cop/rspec/expect_change.rb b/lib/rubocop/cop/rspec/expect_change.rb index 2b58e20af..12f339e2b 100644 --- a/lib/rubocop/cop/rspec/expect_change.rb +++ b/lib/rubocop/cop/rspec/expect_change.rb @@ -5,11 +5,30 @@ module Cop module RSpec # Checks for consistent style of change matcher. # - # Enforces either passing object and attribute as arguments to the matcher - # or passing a block that reads the attribute value. + # Enforces either passing a receiver and message as method arguments, + # or a block. # # This cop can be configured using the `EnforcedStyle` option. # + # @safety + # Autocorrection is unsafe because `method_call` style calls the + # receiver *once* and sends the message to it before and after + # calling the `expect` block, whereas `block` style calls the + # expression *twice*, including the receiver. + # + # If your receiver is dynamic (e.g., the result of a method call) and + # you expect it to be called before and after the `expect` block, + # changing from `block` to `method_call` style may break your test. + # + # [source,ruby] + # ---- + # expect { run }.to change { my_method.message } + # # `my_method` is called before and after `run` + # + # expect { run }.to change(my_method, :message) + # # `my_method` is called once, but `message` is called on it twice + # ---- + # # @example `EnforcedStyle: method_call` (default) # # bad # expect { run }.to change { Foo.bar } diff --git a/lib/rubocop/cop/rspec/focus.rb b/lib/rubocop/cop/rspec/focus.rb index 4be8a311e..5d247490b 100644 --- a/lib/rubocop/cop/rspec/focus.rb +++ b/lib/rubocop/cop/rspec/focus.rb @@ -73,7 +73,7 @@ class Focus < Base PATTERN def on_send(node) - return if node.chained? || node.each_ancestor(:def, :defs).any? + return if node.chained? || node.each_ancestor(:any_def).any? if focused_block?(node) on_focused_block(node) diff --git a/lib/rubocop/cop/rspec/hook_argument.rb b/lib/rubocop/cop/rspec/hook_argument.rb index 36b2823aa..654dd036f 100644 --- a/lib/rubocop/cop/rspec/hook_argument.rb +++ b/lib/rubocop/cop/rspec/hook_argument.rb @@ -67,12 +67,12 @@ class HookArgument < Base # @!method scoped_hook(node) def_node_matcher :scoped_hook, <<~PATTERN - ({block numblock} $(send _ #Hooks.all (sym ${:each :example})) ...) + (any_block $(send _ #Hooks.all (sym ${:each :example})) ...) PATTERN # @!method unscoped_hook(node) def_node_matcher :unscoped_hook, <<~PATTERN - ({block numblock} $(send _ #Hooks.all) ...) + (any_block $(send _ #Hooks.all) ...) PATTERN def on_block(node) diff --git a/lib/rubocop/cop/rspec/hooks_before_examples.rb b/lib/rubocop/cop/rspec/hooks_before_examples.rb index 93fb96fa9..e8aeeebb5 100644 --- a/lib/rubocop/cop/rspec/hooks_before_examples.rb +++ b/lib/rubocop/cop/rspec/hooks_before_examples.rb @@ -30,7 +30,7 @@ class HooksBeforeExamples < Base # @!method example_or_group?(node) def_node_matcher :example_or_group?, <<~PATTERN { - ({block numblock} { + (any_block { (send #rspec? #ExampleGroups.all ...) (send nil? #Examples.all ...) } ...) diff --git a/lib/rubocop/cop/rspec/include_examples.rb b/lib/rubocop/cop/rspec/include_examples.rb new file mode 100644 index 000000000..81b44ccf3 --- /dev/null +++ b/lib/rubocop/cop/rspec/include_examples.rb @@ -0,0 +1,90 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module RSpec + # Checks for usage of `include_examples`. + # + # `include_examples`, unlike `it_behaves_like`, does not create its + # own context. As such, using `subject`, `let`, `before`/`after`, etc. + # within shared examples included with `include_examples` can have + # unexpected behavior and side effects. + # + # Prefer using `it_behaves_like` instead. + # + # @safety + # `include_examples` and `it_behaves_like` have different scoping + # behaviors. + # Changing `include_examples` to `it_behaves_like` creates a new + # context, altering setup dependencies, which can lead to unexpected + # test failures. + # Specifically, the scope of hooks (`before`, `after`, `around`) + # changes, which may prevent expected setup from being inherited + # correctly. + # + # Additionally, `let` and `subject` are affected by scoping rules. + # When `include_examples` is used, `let` and `subject` defined within + # `shared_examples` are evaluated in the caller's context, allowing + # access to their values. + # In contrast, `it_behaves_like` creates a new context, preventing + # access to `let` or `subject` values from the caller's context. + # + # [source,ruby] + # ---- + # shared_examples "mock behavior" do + # before do + # allow(service).to receive(:call).and_return("mocked response") + # end + # + # it "returns mocked response" do + # expect(service.call).to eq "mocked response" + # end + # end + # + # context "working example with include_examples" do + # let(:service) { double(:service) } + # + # include_examples "mock behavior" + # + # it "uses the mocked service" do + # expect(service.call).to eq "mocked response" # Passes + # end + # end + # + # context "broken example with it_behaves_like" do + # let(:service) { double(:service) } + # + # it_behaves_like "mock behavior" + # + # it "unexpectedly does not use the mocked service" do + # # Fails because `it_behaves_like` does not apply the mock setup + # expect(service.call).to eq "mocked response" + # end + # end + # ---- + # + # @example + # # bad + # include_examples 'examples' + # + # # good + # it_behaves_like 'examples' + # + class IncludeExamples < Base + extend AutoCorrector + + MSG = 'Prefer `it_behaves_like` over `include_examples`.' + + RESTRICT_ON_SEND = %i[include_examples].freeze + + def on_send(node) + selector = node.loc.selector + + add_offense(selector) do |corrector| + corrector.replace(selector, 'it_behaves_like') + end + end + end + end + end +end diff --git a/lib/rubocop/cop/rspec/iterated_expectation.rb b/lib/rubocop/cop/rspec/iterated_expectation.rb index 2eb7ee6aa..473e7cc62 100644 --- a/lib/rubocop/cop/rspec/iterated_expectation.rb +++ b/lib/rubocop/cop/rspec/iterated_expectation.rb @@ -17,6 +17,8 @@ module RSpec # end # class IteratedExpectation < Base + extend AutoCorrector + MSG = 'Prefer using the `all` matcher instead ' \ 'of iterating over an array.' @@ -25,14 +27,14 @@ class IteratedExpectation < Base (block (send ... :each) (args (arg $_)) - $(...) + (...) ) PATTERN # @!method each_numblock?(node) def_node_matcher :each_numblock?, <<~PATTERN (numblock - (send ... :each) _ $(...) + (send ... :each) _ (...) ) PATTERN @@ -42,23 +44,43 @@ class IteratedExpectation < Base PATTERN def on_block(node) - each?(node) do |arg, body| - if single_expectation?(body, arg) || only_expectations?(body, arg) - add_offense(node.send_node) - end + each?(node) do |arg| + check_offense(node, arg) end end def on_numblock(node) - each_numblock?(node) do |body| - if single_expectation?(body, :_1) || only_expectations?(body, :_1) - add_offense(node.send_node) - end + each_numblock?(node) do + check_offense(node, :_1) end end private + def check_offense(node, argument) + if single_expectation?(node.body, argument) + add_offense(node.send_node) do |corrector| + next unless node.body.arguments.one? + next if uses_argument_in_matcher?(node, argument) + + corrector.replace(node, single_expectation_replacement(node)) + end + elsif only_expectations?(node.body, argument) + add_offense(node.send_node) + end + end + + def single_expectation_replacement(node) + collection = node.receiver.source + matcher = node.body.first_argument.source + + "expect(#{collection}).to all(#{matcher})" + end + + def uses_argument_in_matcher?(node, argument) + node.body.first_argument.each_descendant.any?(s(:lvar, argument)) + end + def single_expectation?(body, arg) expectation?(body, arg) end diff --git a/lib/rubocop/cop/rspec/leaky_constant_declaration.rb b/lib/rubocop/cop/rspec/leaky_constant_declaration.rb index 907b97dea..bb651d325 100644 --- a/lib/rubocop/cop/rspec/leaky_constant_declaration.rb +++ b/lib/rubocop/cop/rspec/leaky_constant_declaration.rb @@ -100,18 +100,21 @@ class LeakyConstantDeclaration < Base def on_casgn(node) return unless inside_describe_block?(node) + return if explicit_namespace?(node.namespace) add_offense(node, message: MSG_CONST) end def on_class(node) return unless inside_describe_block?(node) + return if explicit_namespace?(node.identifier.namespace) add_offense(node, message: MSG_CLASS) end def on_module(node) return unless inside_describe_block?(node) + return if explicit_namespace?(node.identifier.namespace) add_offense(node, message: MSG_MODULE) end @@ -121,6 +124,10 @@ def on_module(node) def inside_describe_block?(node) node.each_ancestor(:block).any? { |ancestor| spec_group?(ancestor) } end + + def explicit_namespace?(namespace) + !namespace.nil? + end end end end diff --git a/lib/rubocop/cop/rspec/nested_groups.rb b/lib/rubocop/cop/rspec/nested_groups.rb index 40e5aa2d8..f19412d0b 100644 --- a/lib/rubocop/cop/rspec/nested_groups.rb +++ b/lib/rubocop/cop/rspec/nested_groups.rb @@ -133,8 +133,8 @@ def find_nested_example_groups(node, nesting: 1, &block) def count_up_nesting?(node, example_group) example_group && - (node.block_type? && - !allowed_groups.include?(node.method_name.to_s)) + node.block_type? && + !allowed_groups.include?(node.method_name.to_s) end def message(nesting) diff --git a/lib/rubocop/cop/rspec/no_expectation_example.rb b/lib/rubocop/cop/rspec/no_expectation_example.rb index 5018a33cc..a800636df 100644 --- a/lib/rubocop/cop/rspec/no_expectation_example.rb +++ b/lib/rubocop/cop/rspec/no_expectation_example.rb @@ -65,7 +65,7 @@ class NoExpectationExample < Base # @param [RuboCop::AST::Node] node # @return [Boolean] def_node_matcher :regular_or_focused_example?, <<~PATTERN - ({block numblock} (send nil? {#Examples.regular #Examples.focused} ...) ...) + (any_block (send nil? {#Examples.regular #Examples.focused} ...) ...) PATTERN # @!method includes_expectation?(node) diff --git a/lib/rubocop/cop/rspec/pending.rb b/lib/rubocop/cop/rspec/pending.rb index af47a118d..13f71a76b 100644 --- a/lib/rubocop/cop/rspec/pending.rb +++ b/lib/rubocop/cop/rspec/pending.rb @@ -47,7 +47,7 @@ class Pending < Base # @!method skippable_example?(node) def_node_matcher :skippable_example?, <<~PATTERN - (send nil? #Examples.regular ...) + (send nil? #Examples.regular _ ...) PATTERN # @!method pending_block?(node) diff --git a/lib/rubocop/cop/rspec/pending_without_reason.rb b/lib/rubocop/cop/rspec/pending_without_reason.rb index 5f72bcd1f..eea8edaaa 100644 --- a/lib/rubocop/cop/rspec/pending_without_reason.rb +++ b/lib/rubocop/cop/rspec/pending_without_reason.rb @@ -63,8 +63,7 @@ class PendingWithoutReason < Base def_node_matcher :skipped_in_example?, <<~PATTERN { (send nil? ${#Examples.skipped #Examples.pending}) - (block (send nil? ${#Examples.skipped}) ...) - (numblock (send nil? ${#Examples.skipped}) ...) + (any_block (send nil? ${#Examples.skipped}) ...) } PATTERN @@ -75,7 +74,7 @@ class PendingWithoutReason < Base # @!method skipped_by_example_method_with_block?(node) def_node_matcher :skipped_by_example_method_with_block?, <<~PATTERN - ({block numblock} (send nil? ${#Examples.skipped #Examples.pending} ...) ...) + (any_block (send nil? ${#Examples.skipped #Examples.pending} ...) ...) PATTERN # @!method metadata_without_reason?(node) diff --git a/lib/rubocop/cop/rspec/predicate_matcher.rb b/lib/rubocop/cop/rspec/predicate_matcher.rb index a88516f25..fd7830ec4 100644 --- a/lib/rubocop/cop/rspec/predicate_matcher.rb +++ b/lib/rubocop/cop/rspec/predicate_matcher.rb @@ -182,7 +182,7 @@ def uncorrectable_matcher?(node, matcher) def heredoc_argument?(matcher) matcher.arguments.select do |arg| - arg.str_type? || arg.dstr_type? || arg.xstr_type? + arg.type?(:str, :dstr, :xstr) end.any?(&:heredoc?) end diff --git a/lib/rubocop/cop/rspec/receive_messages.rb b/lib/rubocop/cop/rspec/receive_messages.rb index d476604e5..4c1447ce8 100644 --- a/lib/rubocop/cop/rspec/receive_messages.rb +++ b/lib/rubocop/cop/rspec/receive_messages.rb @@ -3,7 +3,7 @@ module RuboCop module Cop module RSpec - # Checks for multiple messages stubbed on the same object. + # Prefer `receive_messages` over multiple `receive`s on the same object. # # @safety # The autocorrection is marked as unsafe, because it may change the @@ -148,8 +148,7 @@ def item_range_by_whole_lines(item) end def heredoc_or_splat?(node) - ((node.str_type? || node.dstr_type?) && node.heredoc?) || - node.splat_type? + (node.type?(:str, :dstr) && node.heredoc?) || node.splat_type? end def requires_quotes?(value) diff --git a/lib/rubocop/cop/rspec/redundant_around.rb b/lib/rubocop/cop/rspec/redundant_around.rb index f943d2b66..7522c9ce0 100644 --- a/lib/rubocop/cop/rspec/redundant_around.rb +++ b/lib/rubocop/cop/rspec/redundant_around.rb @@ -41,7 +41,7 @@ def on_send(node) # @!method match_redundant_around_hook_block?(node) def_node_matcher :match_redundant_around_hook_block?, <<~PATTERN - ({block numblock} (send _ :around ...) ... (send _ :run)) + (any_block (send _ :around ...) ... (send _ :run)) PATTERN # @!method match_redundant_around_hook_send?(node) diff --git a/lib/rubocop/cop/rspec/scattered_setup.rb b/lib/rubocop/cop/rspec/scattered_setup.rb index 56a0444ff..00f9f383e 100644 --- a/lib/rubocop/cop/rspec/scattered_setup.rb +++ b/lib/rubocop/cop/rspec/scattered_setup.rb @@ -5,7 +5,9 @@ module Cop module RSpec # Checks for setup scattered across multiple hooks in an example group. # - # Unify `before`, `after`, and `around` hooks when possible. + # Unify `before` and `after` hooks when possible. + # However, `around` hooks are allowed to be defined multiple times, + # as unifying them would typically make the code harder to read. # # @example # # bad @@ -22,6 +24,12 @@ module RSpec # end # end # + # # good + # describe Foo do + # around { |example| before1; example.call; after1 } + # around { |example| before2; example.call; after2 } + # end + # class ScatteredSetup < Base include FinalEndLocation include RangeHelp @@ -48,7 +56,7 @@ def on_block(node) # rubocop:disable InternalAffairs/NumblockHandler def repeated_hooks(node) hooks = RuboCop::RSpec::ExampleGroup.new(node) .hooks - .select(&:knowable_scope?) + .select { |hook| hook.knowable_scope? && hook.name != :around } .group_by { |hook| [hook.name, hook.scope, hook.metadata] } .values .reject(&:one?) diff --git a/lib/rubocop/cop/rspec/variable_definition.rb b/lib/rubocop/cop/rspec/variable_definition.rb index cb09b5ebd..b438e8609 100644 --- a/lib/rubocop/cop/rspec/variable_definition.rb +++ b/lib/rubocop/cop/rspec/variable_definition.rb @@ -69,7 +69,7 @@ def string?(node) end def symbol?(node) - node.sym_type? || node.dsym_type? + node.type?(:sym, :dsym) end end end diff --git a/lib/rubocop/cop/rspec/variable_name.rb b/lib/rubocop/cop/rspec/variable_name.rb index 957a95227..333400f12 100644 --- a/lib/rubocop/cop/rspec/variable_name.rb +++ b/lib/rubocop/cop/rspec/variable_name.rb @@ -50,7 +50,7 @@ def on_send(node) return unless inside_example_group?(node) variable_definition?(node) do |variable| - return if variable.dstr_type? || variable.dsym_type? + return if variable.type?(:dstr, :dsym) return if matches_allowed_pattern?(variable.value) check_name(node, variable.value, variable.source_range) diff --git a/lib/rubocop/cop/rspec_cops.rb b/lib/rubocop/cop/rspec_cops.rb index 5eed7e860..cf2879e11 100644 --- a/lib/rubocop/cop/rspec_cops.rb +++ b/lib/rubocop/cop/rspec_cops.rb @@ -48,6 +48,7 @@ require_relative 'rspec/implicit_block_expectation' require_relative 'rspec/implicit_expect' require_relative 'rspec/implicit_subject' +require_relative 'rspec/include_examples' require_relative 'rspec/indexed_let' require_relative 'rspec/instance_spy' require_relative 'rspec/instance_variable' diff --git a/lib/rubocop/rspec/hook.rb b/lib/rubocop/rspec/hook.rb index 90116a75e..4c2995aa7 100644 --- a/lib/rubocop/rspec/hook.rb +++ b/lib/rubocop/rspec/hook.rb @@ -64,7 +64,9 @@ def transform_metadata(meta) end def transform_true(node) - node.true_type? ? true : node + return true if node.true_type? + + node end def scope_name diff --git a/lib/rubocop/rspec/language.rb b/lib/rubocop/rspec/language.rb index dff05435c..284ed7211 100644 --- a/lib/rubocop/rspec/language.rb +++ b/lib/rubocop/rspec/language.rb @@ -26,7 +26,7 @@ class << self # @!method example_group?(node) def_node_matcher :example_group?, <<~PATTERN - ({block numblock} (send #rspec? #ExampleGroups.all ...) ...) + (any_block (send #rspec? #ExampleGroups.all ...) ...) PATTERN # @!method shared_group?(node) @@ -35,7 +35,7 @@ class << self # @!method spec_group?(node) def_node_matcher :spec_group?, <<~PATTERN - ({block numblock} (send #rspec? + (any_block (send #rspec? {#SharedGroups.all #ExampleGroups.all} ...) ...) PATTERN @@ -50,10 +50,7 @@ class << self # @!method hook?(node) def_node_matcher :hook?, <<~PATTERN - { - (numblock (send nil? #Hooks.all ...) ...) - (block (send nil? #Hooks.all ...) ...) - } + (any_block (send nil? #Hooks.all ...) ...) PATTERN # @!method let?(node) @@ -75,6 +72,12 @@ class << self # @!method subject?(node) def_node_matcher :subject?, '(block (send nil? #Subjects.all ...) ...)' + module ErrorMatchers # :nodoc: + def self.all(element) + Language.config['ErrorMatchers'].include?(element.to_s) + end + end + module ExampleGroups # :nodoc: class << self def all(element) @@ -203,14 +206,14 @@ def self.all(element) # This is used in Dialect and DescribeClass cops to detect RSpec blocks. module ALL # :nodoc: def self.all(element) - [ExampleGroups, Examples, Expectations, Helpers, Hooks, Includes, - Runners, SharedGroups, Subjects] + [ErrorMatchers, ExampleGroups, Examples, Expectations, Helpers, Hooks, + Includes, Runners, SharedGroups, Subjects] .find { |concept| concept.all(element) } end end - private_constant :ExampleGroups, :Examples, :Expectations, :Hooks, - :Includes, :Runners, :SharedGroups, :ALL + private_constant :ErrorMatchers, :ExampleGroups, :Examples, :Expectations, + :Hooks, :Includes, :Runners, :SharedGroups, :ALL end end end diff --git a/lib/rubocop/rspec/version.rb b/lib/rubocop/rspec/version.rb index 3389c5ce8..f8db04c2c 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 = '3.5.0' + STRING = '3.6.0' end end end diff --git a/spec/rubocop/cop/rspec/base_spec.rb b/spec/rubocop/cop/rspec/base_spec.rb index 2a6b85181..caba2662a 100644 --- a/spec/rubocop/cop/rspec/base_spec.rb +++ b/spec/rubocop/cop/rspec/base_spec.rb @@ -113,7 +113,7 @@ def on_block(node) # rubocop:disable InternalAffairs/NumblockHandler RUBY end - include_examples 'it detects `describe`' + it_behaves_like 'it detects `describe`' end context 'when `epic` is set as an alias to example group' do @@ -133,7 +133,7 @@ def on_block(node) # rubocop:disable InternalAffairs/NumblockHandler RUBY end - include_examples 'it detects `describe`' + it_behaves_like 'it detects `describe`' end end end diff --git a/spec/rubocop/cop/rspec/change_by_zero_spec.rb b/spec/rubocop/cop/rspec/change_by_zero_spec.rb index 4e9604768..675716fee 100644 --- a/spec/rubocop/cop/rspec/change_by_zero_spec.rb +++ b/spec/rubocop/cop/rspec/change_by_zero_spec.rb @@ -366,4 +366,12 @@ end RUBY end + + it 'does not register an offense when without expect block' do + expect_no_offenses(<<~RUBY) + it do + change(foo, :bar).by(0) + end + RUBY + end end diff --git a/spec/rubocop/cop/rspec/context_wording_spec.rb b/spec/rubocop/cop/rspec/context_wording_spec.rb index de3394b53..8820e1038 100644 --- a/spec/rubocop/cop/rspec/context_wording_spec.rb +++ b/spec/rubocop/cop/rspec/context_wording_spec.rb @@ -228,4 +228,38 @@ RUBY end end + + context 'when `Prefixes: [on]' do + let(:cop_config) do + YAML.safe_load(<<-CONFIG) + Prefixes: + - on # evaluates to true + CONFIG + end + + it 'fails' do + expect do + expect_no_offenses(<<~RUBY) + context 'on Linux' do + end + RUBY + end.to raise_error(/Non-string prefixes .+ detected/) + end + end + + context 'when `Prefixes: ["on"]' do + let(:cop_config) do + YAML.safe_load(<<-CONFIG) + Prefixes: + - "on" + CONFIG + end + + it 'does not fail' do + expect { expect_no_offenses(<<~RUBY) }.not_to raise_error + context 'on Linux' do + end + RUBY + end + end end diff --git a/spec/rubocop/cop/rspec/described_class_spec.rb b/spec/rubocop/cop/rspec/described_class_spec.rb index 758eebc0e..e20fb3bfb 100644 --- a/spec/rubocop/cop/rspec/described_class_spec.rb +++ b/spec/rubocop/cop/rspec/described_class_spec.rb @@ -10,7 +10,7 @@ expect_offense(<<~RUBY) describe MyClass do controller(ApplicationController) do - bar = MyClass + self.bar = MyClass end before do @@ -23,6 +23,43 @@ end end RUBY + + expect_correction(<<~RUBY) + describe MyClass do + controller(ApplicationController) do + self.bar = MyClass + end + + before do + described_class + + Foo.custom_block do + MyClass + end + end + end + RUBY + end + + it 'ignores offenses within non-rspec numblocks' do + expect_offense(<<~RUBY) + describe MyClass do + controller(ApplicationController) do + do_some_work(_1) + self.bar = MyClass + end + + before do + MyClass + ^^^^^^^ Use `described_class` instead of `MyClass`. + + Foo.custom_block do + do_some_work(_1) + MyClass + end + end + end + RUBY end end @@ -46,6 +83,22 @@ end end RUBY + + expect_correction(<<~RUBY) + describe MyClass do + controller(ApplicationController) do + bar = described_class + end + + before do + described_class + + Foo.custom_block do + described_class + end + end + end + RUBY end end @@ -95,6 +148,12 @@ module Foo ^^^^^^^ Use `described_class` instead of `MyClass`. end RUBY + + expect_correction(<<~RUBY) + describe MyClass, some: :metadata do + subject { described_class } + end + RUBY end it 'ignores described class as string' do @@ -119,6 +178,7 @@ module Foo Class.new { foo = MyClass } Module.new { bar = MyClass } Struct.new { lol = MyClass } + Data.define { dat = MyClass } def method include MyClass @@ -146,6 +206,16 @@ module MyModule end end RUBY + + expect_correction(<<~RUBY) + describe MyClass do + describe MyClass::Foo do + subject { described_class } + + let(:foo) { MyClass } + end + end + RUBY end it 'ignores non-matching namespace defined on `describe` level' do @@ -174,6 +244,12 @@ module MyNamespace ^^^^^^^^^^^^^^^^^^^^ Use `described_class` instead of `MyNamespace::MyClass`. end RUBY + + expect_correction(<<~RUBY) + describe MyNamespace::MyClass do + subject { described_class } + end + RUBY end it 'ignores non-matching namespace in usages' do @@ -248,6 +324,22 @@ module D end end RUBY + + expect_correction(<<~RUBY) + module A + class B::C + module D + describe E do + subject { described_class } + let(:one) { described_class } + let(:two) { described_class } + let(:six) { described_class } + let(:ten) { described_class } + end + end + end + end + RUBY end it 'accepts an empty block' do diff --git a/spec/rubocop/cop/rspec/dialect_spec.rb b/spec/rubocop/cop/rspec/dialect_spec.rb index 75ba063d0..7bcf707cd 100644 --- a/spec/rubocop/cop/rspec/dialect_spec.rb +++ b/spec/rubocop/cop/rspec/dialect_spec.rb @@ -73,4 +73,29 @@ RUBY end end + + context 'with error matchers config' do + let(:cop_config) do + { + 'PreferredMethods' => { + 'raise_exception' => 'raise_error' + } + } + end + + it 'registers an offense for `raise_exception`' do + expect_offense(<<~RUBY) + it 'raises an error' do + expect { subject }.to raise_exception(StandardError) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `raise_error` over `raise_exception`. + end + RUBY + + expect_correction(<<~RUBY) + it 'raises an error' do + expect { subject }.to raise_error(StandardError) + end + RUBY + end + end end diff --git a/spec/rubocop/cop/rspec/empty_example_group_spec.rb b/spec/rubocop/cop/rspec/empty_example_group_spec.rb index 8fd1134cb..74016c940 100644 --- a/spec/rubocop/cop/rspec/empty_example_group_spec.rb +++ b/spec/rubocop/cop/rspec/empty_example_group_spec.rb @@ -67,6 +67,8 @@ end end RUBY + + expect_correction("\n\n") end it 'ignores example group with examples defined in `if` branches' do @@ -123,6 +125,8 @@ end end RUBY + + expect_correction("\n\n") end it 'ignores example group with examples defined in `case` branches' do @@ -186,6 +190,8 @@ end end RUBY + + expect_correction("\n\n") end it 'ignores example group with examples defined in iterator' do @@ -222,6 +228,8 @@ more_surroundings end RUBY + + expect_correction('') end it 'ignores example group with examples defined in `if` branch ' \ @@ -407,6 +415,8 @@ end end RUBY + + expect_correction('') end context 'when a custom include method is specified' do diff --git a/spec/rubocop/cop/rspec/empty_line_after_example_spec.rb b/spec/rubocop/cop/rspec/empty_line_after_example_spec.rb index bbab767a9..ca9613cbc 100644 --- a/spec/rubocop/cop/rspec/empty_line_after_example_spec.rb +++ b/spec/rubocop/cop/rspec/empty_line_after_example_spec.rb @@ -99,6 +99,19 @@ it { } end RUBY + + expect_correction(<<~RUBY) + RSpec.context 'foo' do + it { } + it { } + + it 'does this' do + end + + it { } + it { } + end + RUBY end it 'does not register an offense for a comment followed by an empty line' do @@ -258,6 +271,14 @@ it { two } end RUBY + + expect_correction(<<~RUBY) + RSpec.describe Foo do + it { one } + + it { two } + end + RUBY end end end 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 e24ceae8a..f1ea5eadc 100644 --- a/spec/rubocop/cop/rspec/empty_line_after_hook_spec.rb +++ b/spec/rubocop/cop/rspec/empty_line_after_hook_spec.rb @@ -310,10 +310,10 @@ end context 'when AllowConsecutiveOneLiners option has default value `true`' do - include_examples 'always require empty line after hook groups' - include_examples 'never allows consecutive multiline blocks' + it_behaves_like 'always require empty line after hook groups' + it_behaves_like 'never allows consecutive multiline blocks' - it 'allows multiple one-liner blocks' do + it 'ignores multiple one-liner blocks' do expect_offense(<<~RUBY) RSpec.describe User do before { do_something } @@ -333,7 +333,7 @@ RUBY end - it 'allows multiple one-liner blocks with comments' do + it 'ignores multiple one-liner blocks with comments' do expect_offense(<<~RUBY) RSpec.describe User do before { do_something } @@ -381,8 +381,8 @@ context 'when AllowConsecutiveOneLiners option `false`' do let(:cop_config) { { 'AllowConsecutiveOneLiners' => false } } - include_examples 'always require empty line after hook groups' - include_examples 'never allows consecutive multiline blocks' + it_behaves_like 'always require empty line after hook groups' + it_behaves_like 'never allows consecutive multiline blocks' it 'registers an offense for multiple one-liner same hook blocks' do expect_offense(<<~RUBY) diff --git a/spec/rubocop/cop/rspec/example_wording_spec.rb b/spec/rubocop/cop/rspec/example_wording_spec.rb index 40645886b..5f93c79df 100644 --- a/spec/rubocop/cop/rspec/example_wording_spec.rb +++ b/spec/rubocop/cop/rspec/example_wording_spec.rb @@ -329,6 +329,8 @@ DESC end RUBY + + expect_no_corrections end it 'flags an unclear description' do @@ -337,6 +339,8 @@ ^^^^^ Your example description is insufficient. end RUBY + + expect_no_corrections end it 'flags an unclear description despite extra spaces' do @@ -345,6 +349,8 @@ ^^^^^^^^^^^ Your example description is insufficient. end RUBY + + expect_no_corrections end it 'flags an unclear description despite uppercase and lowercase strings' do @@ -353,6 +359,8 @@ ^^^^^^ Your example description is insufficient. end RUBY + + expect_no_corrections end context 'when `DisallowedExamples: Workz`' do @@ -373,6 +381,8 @@ " " do end RUBY + + expect_no_corrections end it 'flags an unclear description' do @@ -381,6 +391,8 @@ ^^^^^ Your example description is insufficient. end RUBY + + expect_no_corrections end it 'flags an unclear description despite uppercase and lowercase strings' do @@ -389,6 +401,8 @@ ^^^^^^ Your example description is insufficient. end RUBY + + expect_no_corrections end end end diff --git a/spec/rubocop/cop/rspec/expect_change_spec.rb b/spec/rubocop/cop/rspec/expect_change_spec.rb index 755d59ae9..e26b54234 100644 --- a/spec/rubocop/cop/rspec/expect_change_spec.rb +++ b/spec/rubocop/cop/rspec/expect_change_spec.rb @@ -46,6 +46,12 @@ ^^^^^^^^^^^^^^^^^^^^^ Prefer `change(User, :count)`. end RUBY + + expect_correction(<<~RUBY) + it do + expect(run).to change(User, :count).by(1) + end + RUBY end it 'ignores the usage that adheres to the enforced style' do @@ -256,6 +262,12 @@ ^^^^^^^^^^^^^^^^^^^^ Prefer `change { User.count }`. end RUBY + + expect_correction(<<~RUBY) + it do + expect(run).to change { User.count }.by(1) + end + RUBY end end end diff --git a/spec/rubocop/cop/rspec/hook_argument_spec.rb b/spec/rubocop/cop/rspec/hook_argument_spec.rb index 2223c0f52..6a7fbd6e8 100644 --- a/spec/rubocop/cop/rspec/hook_argument_spec.rb +++ b/spec/rubocop/cop/rspec/hook_argument_spec.rb @@ -29,10 +29,10 @@ end shared_examples 'an example hook' do - include_examples 'ignored hooks' - include_examples 'detects style', 'before(:each) { foo }', 'each' - include_examples 'detects style', 'before(:example) { foo }', 'example' - include_examples 'detects style', 'before { foo }', 'implicit' + it_behaves_like 'ignored hooks' + it_behaves_like 'detects style', 'before(:each) { foo }', 'each' + it_behaves_like 'detects style', 'before(:example) { foo }', 'example' + it_behaves_like 'detects style', 'before { foo }', 'implicit' end context 'when EnforcedStyle is :implicit' do @@ -87,7 +87,7 @@ RUBY end - include_examples 'an example hook' + it_behaves_like 'an example hook' context 'when Ruby 2.7', :ruby27 do it 'detects :each for hooks' do @@ -172,7 +172,7 @@ RUBY end - include_examples 'an example hook' + it_behaves_like 'an example hook' context 'when Ruby 2.7', :ruby27 do it 'does not flag :each for hooks' do @@ -257,7 +257,7 @@ RUBY end - include_examples 'an example hook' + it_behaves_like 'an example hook' context 'when Ruby 2.7', :ruby27 do it 'does not flag :example for hooks' do diff --git a/spec/rubocop/cop/rspec/implicit_expect_spec.rb b/spec/rubocop/cop/rspec/implicit_expect_spec.rb index d0e6ae909..143b37ac9 100644 --- a/spec/rubocop/cop/rspec/implicit_expect_spec.rb +++ b/spec/rubocop/cop/rspec/implicit_expect_spec.rb @@ -40,7 +40,7 @@ expect_no_offenses('it { is_expected.not_to be_truthy }') end - include_examples 'detects style', 'it { should be_truthy }', 'should' + it_behaves_like 'detects style', 'it { should be_truthy }', 'should' end context 'when EnforcedStyle is should' do @@ -89,12 +89,12 @@ expect_no_offenses('it { should_not be_truthy }') end - include_examples 'detects style', - 'it { is_expected.to be_truthy }', - 'is_expected' + it_behaves_like 'detects style', + 'it { is_expected.to be_truthy }', + 'is_expected' - include_examples 'detects style', - 'it { should be_truthy }', - 'should' + it_behaves_like 'detects style', + 'it { should be_truthy }', + 'should' end end diff --git a/spec/rubocop/cop/rspec/include_examples_spec.rb b/spec/rubocop/cop/rspec/include_examples_spec.rb new file mode 100644 index 000000000..f0ff6e77e --- /dev/null +++ b/spec/rubocop/cop/rspec/include_examples_spec.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::RSpec::IncludeExamples, :config do + it 'registers an offense and corrects for `include_examples`' do + expect_offense(<<~RUBY) + include_examples 'examples' + ^^^^^^^^^^^^^^^^ Prefer `it_behaves_like` over `include_examples`. + RUBY + + expect_correction(<<~RUBY) + it_behaves_like 'examples' + RUBY + end + + it 'does not register an offense for `it_behaves_like`' do + expect_no_offenses(<<~RUBY) + it_behaves_like 'examples' + RUBY + end + + it 'does not register an offense for `it_should_behave_like`' do + expect_no_offenses(<<~RUBY) + it_should_behave_like 'examples' + RUBY + end + + it 'does not register an offense for `include_context`' do + expect_no_offenses(<<~RUBY) + include_context 'context' + RUBY + end +end diff --git a/spec/rubocop/cop/rspec/iterated_expectation_spec.rb b/spec/rubocop/cop/rspec/iterated_expectation_spec.rb index 81b379071..219f0a458 100644 --- a/spec/rubocop/cop/rspec/iterated_expectation_spec.rb +++ b/spec/rubocop/cop/rspec/iterated_expectation_spec.rb @@ -8,6 +8,12 @@ ^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using the `all` matcher instead of iterating over an array. end RUBY + + expect_correction(<<~RUBY) + it 'validates users' do + expect([user1, user2, user3]).to all(be_valid) + end + RUBY end it 'flags `each` when expectation calls method with arguments' do @@ -17,6 +23,35 @@ ^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using the `all` matcher instead of iterating over an array. end RUBY + + expect_correction(<<~RUBY) + it 'validates users' do + expect([user1, user2, user3]).to all(be_a(User)) + end + RUBY + end + + it 'flags `each` when the expectation specifies an error message, but ' \ + 'does not correct' do + expect_offense(<<~RUBY) + it 'validates users' do + [user1, user2, user3].each { |user| expect(user).to be_a(User), "user is not a User" } + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using the `all` matcher instead of iterating over an array. + end + RUBY + + expect_no_corrections + end + + it 'flags `each` when matcher uses block argument, but does not correct' do + expect_offense(<<~RUBY) + it 'validates users' do + [user1, user2, user3].each { |user| expect(user).to receive(:flag).and_return(user.flag) } + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using the `all` matcher instead of iterating over an array. + end + RUBY + + expect_no_corrections end it 'ignores `each` without expectation' do @@ -45,6 +80,8 @@ end end RUBY + + expect_no_corrections end it 'ignore `each` when the body does not contain only expectations' do @@ -94,6 +131,12 @@ ^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using the `all` matcher instead of iterating over an array. end RUBY + + expect_correction(<<~RUBY) + it 'validates users' do + expect([user1, user2, user3]).to all(be_valid) + end + RUBY end it 'flags `each` when expectation calls method with arguments' do @@ -103,6 +146,12 @@ ^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using the `all` matcher instead of iterating over an array. end RUBY + + expect_correction(<<~RUBY) + it 'validates users' do + expect([user1, user2, user3]).to all(be_a(User)) + end + RUBY end it 'ignores `each` without expectation' do @@ -123,6 +172,8 @@ end end RUBY + + expect_no_corrections end it 'ignore `each` when the body does not contain only expectations' do diff --git a/spec/rubocop/cop/rspec/leaky_constant_declaration_spec.rb b/spec/rubocop/cop/rspec/leaky_constant_declaration_spec.rb index c3a6beb82..0a1164efb 100644 --- a/spec/rubocop/cop/rspec/leaky_constant_declaration_spec.rb +++ b/spec/rubocop/cop/rspec/leaky_constant_declaration_spec.rb @@ -31,6 +31,30 @@ RUBY end + it 'ignores constant defined on the example group' do + expect_no_offenses(<<~RUBY) + describe SomeClass do + self::CONSTANT = "Accessible as self.class::CONSTANT".freeze + end + RUBY + end + + it 'ignores constant defined in an explicit namespace' do + expect_no_offenses(<<~RUBY) + describe SomeClass do + Foo::CONSTANT = "Accessible as Foo::CONSTANT".freeze + end + RUBY + end + + it 'ignores classes defined explicitly in the global namespace' do + expect_no_offenses(<<~RUBY) + describe SomeClass do + ::CONSTANT = "Accessible as ::CONSTANT".freeze + end + RUBY + end + it 'ignores outside of example/shared group' do expect_no_offenses(<<~RUBY) factory :some_class do @@ -60,15 +84,32 @@ def method end end end - end + end RUBY end - it 'flags namespaced class' do - expect_offense(<<~RUBY) + it 'ignores classes defined on the example group' do + expect_no_offenses(<<~RUBY) + describe SomeClass do + class self::DummyClass + end + end + RUBY + end + + it 'ignores classes defined in an explicit namespace' do + expect_no_offenses(<<~RUBY) describe SomeClass do - class SomeModule::AnotherModule::DummyClass - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Stub class constant instead of declaring explicitly. + class Foo::DummyClass + end + end + RUBY + end + + it 'ignores classes defined explicitly in the global namespace' do + expect_no_offenses(<<~RUBY) + describe SomeClass do + class ::DummyClass end end RUBY @@ -93,6 +134,33 @@ module DummyModule RUBY end + it 'ignores modules defined on the example group' do + expect_no_offenses(<<~RUBY) + describe SomeClass do + module self::DummyModule + end + end + RUBY + end + + it 'ignores modules defined in an explicit namespace' do + expect_no_offenses(<<~RUBY) + describe SomeClass do + module Foo::DummyModule + end + end + RUBY + end + + it 'ignores modules defined explicitly in the global namespace' do + expect_no_offenses(<<~RUBY) + describe SomeClass do + module ::DummyModule + end + end + RUBY + end + it 'ignores outside of example/shared group' do expect_no_offenses(<<~RUBY) module Dummymodule diff --git a/spec/rubocop/cop/rspec/message_expectation_spec.rb b/spec/rubocop/cop/rspec/message_expectation_spec.rb index 1b1868cd9..3693244e8 100644 --- a/spec/rubocop/cop/rspec/message_expectation_spec.rb +++ b/spec/rubocop/cop/rspec/message_expectation_spec.rb @@ -17,8 +17,8 @@ expect_no_offenses('allow(foo).to receive(:bar)') end - include_examples 'detects style', 'allow(foo).to receive(:bar)', 'allow' - include_examples 'detects style', 'expect(foo).to receive(:bar)', 'expect' + it_behaves_like 'detects style', 'allow(foo).to receive(:bar)', 'allow' + it_behaves_like 'detects style', 'expect(foo).to receive(:bar)', 'expect' end context 'when EnforcedStyle is expect' do @@ -37,7 +37,7 @@ expect_no_offenses('expect(foo).to receive(:bar)') end - include_examples 'detects style', 'expect(foo).to receive(:bar)', 'expect' - include_examples 'detects style', 'allow(foo).to receive(:bar)', 'allow' + it_behaves_like 'detects style', 'expect(foo).to receive(:bar)', 'expect' + it_behaves_like 'detects style', 'allow(foo).to receive(:bar)', 'allow' end end diff --git a/spec/rubocop/cop/rspec/message_spies_spec.rb b/spec/rubocop/cop/rspec/message_spies_spec.rb index dcba75250..66baf7cd1 100644 --- a/spec/rubocop/cop/rspec/message_spies_spec.rb +++ b/spec/rubocop/cop/rspec/message_spies_spec.rb @@ -67,11 +67,11 @@ expect_no_offenses('expect(foo).to have_received(:bar)') end - include_examples 'detects style', 'expect(foo).to receive(:bar)', 'receive' + it_behaves_like 'detects style', 'expect(foo).to receive(:bar)', 'receive' - include_examples 'detects style', - 'expect(foo).to have_received(:bar)', - 'have_received' + it_behaves_like 'detects style', + 'expect(foo).to have_received(:bar)', + 'have_received' end context 'when EnforcedStyle is receive' do @@ -140,10 +140,10 @@ expect_no_offenses('expect(foo).to receive(:bar)') end - include_examples 'detects style', 'expect(foo).to receive(:bar)', 'receive' + it_behaves_like 'detects style', 'expect(foo).to receive(:bar)', 'receive' - include_examples 'detects style', - 'expect(foo).to have_received(:bar)', - 'have_received' + it_behaves_like 'detects style', + 'expect(foo).to have_received(:bar)', + 'have_received' end end diff --git a/spec/rubocop/cop/rspec/pending_spec.rb b/spec/rubocop/cop/rspec/pending_spec.rb index 4453dd82a..14b09f316 100644 --- a/spec/rubocop/cop/rspec/pending_spec.rb +++ b/spec/rubocop/cop/rspec/pending_spec.rb @@ -225,4 +225,12 @@ subject { Project.pending } RUBY end + + it 'ignores default block parameter' do + expect_no_offenses(<<~RUBY) + expect( + foo.map { it.reverse } + ).to include(:bar) + RUBY + end end diff --git a/spec/rubocop/cop/rspec/predicate_matcher_spec.rb b/spec/rubocop/cop/rspec/predicate_matcher_spec.rb index 6f2429b1a..56885be61 100644 --- a/spec/rubocop/cop/rspec/predicate_matcher_spec.rb +++ b/spec/rubocop/cop/rspec/predicate_matcher_spec.rb @@ -186,7 +186,7 @@ context 'when strict is true' do let(:strict) { true } - include_examples 'inflected common' + it_behaves_like 'inflected common' it 'accepts strict checking boolean matcher' do expect_no_offenses(<<~RUBY) @@ -203,7 +203,7 @@ context 'when strict is false' do let(:strict) { false } - include_examples 'inflected common' + it_behaves_like 'inflected common' it 'registers an offense for a predicate method in actual' do expect_offense(<<~RUBY) @@ -257,6 +257,13 @@ expect(foo).to have_something ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `has_something?` over `have_something` matcher. RUBY + + expect_correction(<<~RUBY) + expect(foo.empty?).to #{matcher_true} + expect(foo.empty?).to #{matcher_true}, 'fail' + expect(foo.empty?).to #{matcher_false} + expect(foo.has_something?).to #{matcher_true} + RUBY end it 'registers an offense for a predicate mather with argument' do @@ -266,6 +273,11 @@ expect(foo).to have_key(1) ^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `has_key?` over `have_key` matcher. RUBY + + expect_correction(<<~RUBY) + expect(foo.something?(1, 2)).to #{matcher_true} + expect(foo.has_key?(1)).to #{matcher_true} + RUBY end it 'registers an offense for a predicate matcher with a block' do @@ -279,6 +291,13 @@ expect(foo).to be_something(x) { |y| y.present? } ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `something?` over `be_something` matcher. RUBY + + expect_correction(<<~RUBY) + expect(foo.all?(&:present?)).to #{matcher_true} + expect(foo.all? { |x| x.present? }).to #{matcher_true} + expect(foo.all? { present }).to #{matcher_true} + expect(foo.something?(x) { |y| y.present? }).to #{matcher_true} + RUBY end it 'accepts built in matchers' do @@ -489,13 +508,13 @@ context 'when strict is true' do let(:strict) { true } - include_examples 'explicit', 'be(true)', 'be(false)' + it_behaves_like 'explicit', 'be(true)', 'be(false)' end context 'when strict is false' do let(:strict) { false } - include_examples 'explicit', 'be_truthy', 'be_falsey' + it_behaves_like 'explicit', 'be_truthy', 'be_falsey' end 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 2b5451ffe..4b101a305 100644 --- a/spec/rubocop/cop/rspec/return_from_stub_spec.rb +++ b/spec/rubocop/cop/rspec/return_from_stub_spec.rb @@ -218,6 +218,12 @@ def stub_foo ^^^^^^^^^^ Use block for static values. end RUBY + + expect_correction(<<~RUBY) + it do + allow(Foo).to receive(:bar) { 42 } + end + RUBY end it 'registers an offense for static values returned from chained method' do @@ -227,6 +233,12 @@ def stub_foo ^^^^^^^^^^ Use block for static values. end RUBY + + expect_correction(<<~RUBY) + it do + allow(Foo).to receive(:bar).with(1) { 42 } + end + RUBY end it 'registers, but does not try to autocorrect, heredocs' do diff --git a/spec/rubocop/cop/rspec/scattered_setup_spec.rb b/spec/rubocop/cop/rspec/scattered_setup_spec.rb index 7e9afea79..33957dc68 100644 --- a/spec/rubocop/cop/rspec/scattered_setup_spec.rb +++ b/spec/rubocop/cop/rspec/scattered_setup_spec.rb @@ -58,6 +58,15 @@ RUBY end + it 'ignores around hooks' do + expect_no_offenses(<<~RUBY) + describe Foo do + around { bar } + around { baz } + end + RUBY + end + it 'ignores different hooks' do expect_no_offenses(<<~RUBY) describe Foo do diff --git a/spec/rubocop/cop/rspec/sort_metadata_spec.rb b/spec/rubocop/cop/rspec/sort_metadata_spec.rb index d2f9e631f..9b8c3c66a 100644 --- a/spec/rubocop/cop/rspec/sort_metadata_spec.rb +++ b/spec/rubocop/cop/rspec/sort_metadata_spec.rb @@ -269,6 +269,16 @@ end end RUBY + + expect_correction(<<~RUBY) + RSpec.describan "Algo", :a, :b do + contexto_compartido 'una situación complicada', baz: true, foo: 'bar' do + end + + ejemplo "hablando español", baz: true, foo: 'bar' do + end + end + RUBY end end end diff --git a/spec/rubocop/cop/rspec/subject_declaration_spec.rb b/spec/rubocop/cop/rspec/subject_declaration_spec.rb index 894b304cd..e9292c5a9 100644 --- a/spec/rubocop/cop/rspec/subject_declaration_spec.rb +++ b/spec/rubocop/cop/rspec/subject_declaration_spec.rb @@ -51,10 +51,10 @@ end end - include_examples 'flag unclear subject declaration', :subject - include_examples 'flag unclear subject declaration', 'subject' - include_examples 'flag unclear subject declaration', :subject! - include_examples 'flag unclear subject declaration', 'subject!' + it_behaves_like 'flag unclear subject declaration', :subject + it_behaves_like 'flag unclear subject declaration', 'subject' + it_behaves_like 'flag unclear subject declaration', :subject! + it_behaves_like 'flag unclear subject declaration', 'subject!' context 'when subject helper is used directly' do it 'does not register an offense on named subject' do diff --git a/spec/rubocop/rspec/hook_spec.rb b/spec/rubocop/rspec/hook_spec.rb index 9cf2e59b4..d105a1c7b 100644 --- a/spec/rubocop/rspec/hook_spec.rb +++ b/spec/rubocop/rspec/hook_spec.rb @@ -64,14 +64,14 @@ def hook(source) end end - include_examples 'standardizes scope', 'before(:each) { }', :each - include_examples 'standardizes scope', 'around(:example) { }', :each - include_examples 'standardizes scope', 'after { }', :each + it_behaves_like 'standardizes scope', 'before(:each) { }', :each + it_behaves_like 'standardizes scope', 'around(:example) { }', :each + it_behaves_like 'standardizes scope', 'after { }', :each - include_examples 'standardizes scope', 'before(:all) { }', :context - include_examples 'standardizes scope', 'around(:context) { }', :context + it_behaves_like 'standardizes scope', 'before(:all) { }', :context + it_behaves_like 'standardizes scope', 'around(:context) { }', :context - include_examples 'standardizes scope', 'after(:suite) { }', :suite + it_behaves_like 'standardizes scope', 'after(:suite) { }', :suite end describe '#metadata' do diff --git a/tasks/cops_documentation.rake b/tasks/cops_documentation.rake index b257dea1d..691e6fb4f 100644 --- a/tasks/cops_documentation.rake +++ b/tasks/cops_documentation.rake @@ -12,10 +12,8 @@ end desc 'Generate docs of all cops departments' task generate_cops_documentation: :yard_for_generate_documentation do - RuboCop::ConfigLoader.inject_defaults!("#{__dir__}/../config/default.yml") - generator = CopsDocumentationGenerator.new( - departments: %w[RSpec] + departments: %w[RSpec], plugin_name: 'rubocop-rspec' ) generator.call end