Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add configuration option OnlyStaticConstants to RSpec/DescribedClass #1825

Merged
merged 1 commit into from Mar 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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