Skip to content

Commit

Permalink
Merge pull request #1825 from rubocop/only-static-constants
Browse files Browse the repository at this point in the history
Add configuration option `OnlyStaticConstants` to `RSpec/DescribedClass`
  • Loading branch information
bquorning committed Mar 3, 2024
2 parents dd6ae03 + 350b50f commit f2593cd
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 39 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -3,6 +3,7 @@
## Master (Unreleased)

- Fix a false positive for `RSpec/RepeatedSubjectCall` when `subject.method_call`. ([@ydah])
- Add configuration option `OnlyStaticConstants` to `RSpec/DescribedClass`. ([@ydah])

## 2.27.0 (2024-03-01)

Expand Down
3 changes: 2 additions & 1 deletion config/default.yml
Expand Up @@ -283,9 +283,10 @@ RSpec/DescribedClass:
SupportedStyles:
- described_class
- explicit
OnlyStaticConstants: true
SafeAutoCorrect: false
VersionAdded: '1.0'
VersionChanged: '1.11'
VersionChanged: "<<next>>"
Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/DescribedClass

RSpec/DescribedClassModuleWrapping:
Expand Down
32 changes: 29 additions & 3 deletions docs/modules/ROOT/pages/cops_rspec.adoc
Expand Up @@ -887,16 +887,18 @@ end
| Yes
| Always (Unsafe)
| 1.0
| 1.11
| <<next>>
|===
Checks that tests use `described_class`.
If the first argument of describe is a class, the class is exposed to
each example via described_class.
This cop can be configured using the `EnforcedStyle` and `SkipBlocks`
options.
This cop can be configured using the `EnforcedStyle`, `SkipBlocks`
and `OnlyStaticConstants` options.
`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
Expand Down Expand Up @@ -924,6 +926,26 @@ describe MyClass do
end
----
==== `OnlyStaticConstants: true` (default)
[source,ruby]
----
# good
describe MyClass do
subject { MyClass::CONSTANT }
end
----
==== `OnlyStaticConstants: false`
[source,ruby]
----
# bad
describe MyClass do
subject { MyClass::CONSTANT }
end
----
==== `EnforcedStyle: explicit`
[source,ruby]
Expand Down Expand Up @@ -967,6 +989,10 @@ end
| EnforcedStyle
| `described_class`
| `described_class`, `explicit`
| OnlyStaticConstants
| `true`
| Boolean
|===
=== References
Expand Down
31 changes: 26 additions & 5 deletions lib/rubocop/cop/rspec/described_class.rb
Expand Up @@ -8,8 +8,10 @@ module RSpec
# If the first argument of describe is a class, the class is exposed to
# each example via described_class.
#
# This cop can be configured using the `EnforcedStyle` and `SkipBlocks`
# options.
# This cop can be configured using the `EnforcedStyle`, `SkipBlocks`
# and `OnlyStaticConstants` options.
# `OnlyStaticConstants` is only relevant when `EnforcedStyle` is
# `described_class`.
#
# @example `EnforcedStyle: described_class` (default)
# # bad
Expand All @@ -22,6 +24,18 @@ module RSpec
# subject { described_class.do_something }
# end
#
# @example `OnlyStaticConstants: true` (default)
# # good
# describe MyClass do
# subject { MyClass::CONSTANT }
# end
#
# @example `OnlyStaticConstants: false`
# # bad
# describe MyClass do
# subject { MyClass::CONSTANT }
# end
#
# @example `EnforcedStyle: explicit`
# # bad
# describe MyClass do
Expand Down Expand Up @@ -54,7 +68,7 @@ module RSpec
# end
# end
#
class DescribedClass < Base
class DescribedClass < Base # rubocop:disable Metrics/ClassLength
extend AutoCorrector
include ConfigurableEnforcedStyle
include Namespace
Expand Down Expand Up @@ -112,14 +126,17 @@ def autocorrect(corrector, match)

def find_usage(node, &block)
yield(node) if offensive?(node)

return if scope_change?(node)
return if scope_change?(node) || allowed?(node)

node.each_child_node do |child|
find_usage(child, &block)
end
end

def allowed?(node)
node.const_type? && only_static_constants?
end

def message(offense)
if style == :described_class
format(MSG, replacement: DESCRIBED_CLASS, src: offense)
Expand All @@ -139,6 +156,10 @@ def skippable_block?(node)
node.block_type? && !rspec_block?(node) && cop_config['SkipBlocks']
end

def only_static_constants?
cop_config.fetch('OnlyStaticConstants', true)
end

def offensive?(node)
if style == :described_class
offensive_described_class?(node)
Expand Down
88 changes: 58 additions & 30 deletions spec/rubocop/cop/rspec/described_class_spec.rb
Expand Up @@ -148,36 +148,6 @@ module MyModule
RUBY
end

it 'flags class with constant' do
expect_offense(<<~RUBY)
describe MyClass do
subject { MyClass::FOO }
^^^^^^^ Use `described_class` instead of `MyClass`.
end
RUBY

expect_correction(<<~RUBY)
describe MyClass do
subject { described_class::FOO }
end
RUBY
end

it 'flags class with subclasses' do
expect_offense(<<~RUBY)
describe MyClass do
subject { MyClass::Subclass }
^^^^^^^ Use `described_class` instead of `MyClass`.
end
RUBY

expect_correction(<<~RUBY)
describe MyClass do
subject { described_class::Subclass }
end
RUBY
end

it 'ignores non-matching namespace defined on `describe` level' do
expect_no_offenses(<<~RUBY)
describe MyNamespace::MyClass do
Expand Down Expand Up @@ -310,6 +280,64 @@ module SomeGem
end
RUBY
end

context 'when OnlyStaticConstants is `true`' do
let(:cop_config) do
{ 'EnforcedStyle' => :described_class, 'OnlyStaticConstants' => true }
end

it 'ignores class with constant' do
expect_no_offenses(<<~RUBY)
describe MyClass do
subject { MyClass::FOO }
end
RUBY
end

it 'ignores class with subclasses' do
expect_no_offenses(<<~RUBY)
describe MyClass do
subject { MyClass::Subclass }
end
RUBY
end
end

context 'when OnlyStaticConstants is `false`' do
let(:cop_config) do
{ 'EnforcedStyle' => :described_class, 'OnlyStaticConstants' => false }
end

it 'flags class with constant' do
expect_offense(<<~RUBY)
describe MyClass do
subject { MyClass::FOO }
^^^^^^^ Use `described_class` instead of `MyClass`.
end
RUBY

expect_correction(<<~RUBY)
describe MyClass do
subject { described_class::FOO }
end
RUBY
end

it 'flags class with subclasses' do
expect_offense(<<~RUBY)
describe MyClass do
subject { MyClass::Subclass }
^^^^^^^ Use `described_class` instead of `MyClass`.
end
RUBY

expect_correction(<<~RUBY)
describe MyClass do
subject { described_class::Subclass }
end
RUBY
end
end
end

context 'when EnforcedStyle is :explicit' do
Expand Down

0 comments on commit f2593cd

Please sign in to comment.