Skip to content

Commit 0e1687c

Browse files
committed
Implement RSpec/DescribedClassModuleWrapping cop
Here's the situation that this can help discourage within Rails code bases: I can tell a small story as to why this should be a cop to consider. Specifically, re-opening modules in tests can confuse the [Rails autoloading mechanism](https://guides.rubyonrails.org/autoloading_and_reloading_constants.html). (Note: This is all pre-zeitwork on Rails 5.2 and earlier. I'm not sure if Rails 6+ with zeitwerk will change this.) If you write: ```ruby module Foo module Bar RSpec.describe Baz do # ... end end end ``` And then you have code that refers to a constant defined on `Foo::Bar`, e.g. `Foo::Bar::WIDGETS`, this test code will fail if eagerloading is disabled. Why? Rails autoloading thinks it's already loaded the Foo::Bar module because it's defined. Even though `app/models/foo/bar.rb` might define the `WIDGETS` constant, our test will not load the file because our test file has already defined the module. To confuse the matter more, turning on eager loading will cause the test to pass, because all code in `app/` will be loaded before the test executes. Not reopening the module, e.g. ```ruby RSpec.describe Foo::Bar::Baz do # ... end ``` will cause the test to pass in both an eagerloaded and not eagerloaded configuration.
1 parent 6049c9a commit 0e1687c

File tree

7 files changed

+122
-0
lines changed

7 files changed

+122
-0
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
## Master (Unreleased)
44

5+
* Implement `RSpec/DescribedClassModuleWrapping` to disallow RSpec statements within a module. ([@kellysutton][])
56
* Fix documentation rake task to support Rubocop 0.75. ([@nickcampbell18][])
67
* Fix `RSpec/SubjectStub` to detect implicit subjects stubbed. ([@QQism][])
78

@@ -457,3 +458,4 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features.
457458
[@onumis]: https://github.com/onumis
458459
[@nickcampbell18]: https://github.com/nickcampbell18
459460
[@QQism]: https://github.com/QQism
461+
[@kellysutton]: https://github.com/kellysutton

config/default.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,11 @@ RSpec/DescribedClass:
8888
- explicit
8989
StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/DescribedClass
9090

91+
RSpec/DescribedClassModuleWrapping:
92+
Description: Avoid opening modules and defining specs within them.
93+
Enabled: false
94+
StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/DescribedClassModuleWrapping
95+
9196
RSpec/Dialect:
9297
Description: This cop enforces custom RSpec dialects.
9398
Enabled: false
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
# frozen_string_literal: true
2+
3+
module RuboCop
4+
module Cop
5+
module RSpec
6+
# Avoid opening modules and defining specs within them.
7+
#
8+
# @example
9+
# # bad
10+
# module MyModule
11+
# RSpec.describe MyClass do
12+
# # ...
13+
# end
14+
# end
15+
#
16+
# # good
17+
# RSpec.describe MyModule::MyClass do
18+
# # ...
19+
# end
20+
#
21+
# @see https://github.com/rubocop-hq/rubocop-rspec/issues/735
22+
class DescribedClassModuleWrapping < Cop
23+
MSG = 'Avoid opening modules and defining specs within them.'
24+
25+
def_node_search :find_rspec_blocks,
26+
ExampleGroups::ALL.block_pattern
27+
28+
def on_module(node)
29+
find_rspec_blocks(node) do
30+
add_offense(node)
31+
end
32+
end
33+
end
34+
end
35+
end
36+
end

lib/rubocop/cop/rspec_cops.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
require_relative 'rspec/describe_method'
2626
require_relative 'rspec/describe_symbol'
2727
require_relative 'rspec/described_class'
28+
require_relative 'rspec/described_class_module_wrapping'
2829
require_relative 'rspec/dialect'
2930
require_relative 'rspec/empty_example_group'
3031
require_relative 'rspec/empty_line_after_example'

manual/cops.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
* [RSpec/DescribeMethod](cops_rspec.md#rspecdescribemethod)
2525
* [RSpec/DescribeSymbol](cops_rspec.md#rspecdescribesymbol)
2626
* [RSpec/DescribedClass](cops_rspec.md#rspecdescribedclass)
27+
* [RSpec/DescribedClassModuleWrapping](cops_rspec.md#rspecdescribedclassmodulewrapping)
2728
* [RSpec/Dialect](cops_rspec.md#rspecdialect)
2829
* [RSpec/EmptyExampleGroup](cops_rspec.md#rspecemptyexamplegroup)
2930
* [RSpec/EmptyLineAfterExample](cops_rspec.md#rspecemptylineafterexample)

manual/cops_rspec.md

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,34 @@ EnforcedStyle | `described_class` | `described_class`, `explicit`
469469

470470
* [https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/DescribedClass](https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/DescribedClass)
471471

472+
## RSpec/DescribedClassModuleWrapping
473+
474+
Enabled by default | Supports autocorrection
475+
--- | ---
476+
Disabled | No
477+
478+
Avoid opening modules and defining specs within them.
479+
480+
### Examples
481+
482+
```ruby
483+
# bad
484+
module MyModule
485+
RSpec.describe MyClass do
486+
# ...
487+
end
488+
end
489+
490+
# good
491+
RSpec.describe MyModule::MyClass do
492+
# ...
493+
end
494+
```
495+
496+
### References
497+
498+
* [https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/DescribedClassModuleWrapping](https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/DescribedClassModuleWrapping)
499+
472500
## RSpec/Dialect
473501

474502
Enabled by default | Supports autocorrection
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
# frozen_string_literal: true
2+
3+
RSpec.describe RuboCop::Cop::RSpec::DescribedClassModuleWrapping do
4+
subject(:cop) { described_class.new }
5+
6+
it 'allows a describe block in the outermost scope' do
7+
expect_no_offenses(<<-RUBY)
8+
RSpec.describe MyClass do
9+
subject { "MyClass" }
10+
end
11+
RUBY
12+
end
13+
14+
it 'registers an offense when RSpec.describe is nested within a module' do
15+
expect_offense(<<-RUBY)
16+
module MyModule
17+
^^^^^^^^^^^^^^^ Avoid opening modules and defining specs within them.
18+
RSpec.describe MyClass do
19+
20+
subject { "MyClass" }
21+
end
22+
end
23+
RUBY
24+
end
25+
26+
it 'registers an offense when RSpec.describe is nested within two modules' do
27+
expect_offense(<<-RUBY)
28+
module MyFirstModule
29+
^^^^^^^^^^^^^^^^^^^^ Avoid opening modules and defining specs within them.
30+
module MySecondModule
31+
^^^^^^^^^^^^^^^^^^^^^ Avoid opening modules and defining specs within them.
32+
RSpec.describe MyClass do
33+
34+
subject { "MyClass" }
35+
end
36+
end
37+
end
38+
RUBY
39+
end
40+
41+
it 'allows a module that does not contain RSpec.describe' do
42+
expect_no_offenses(<<-RUBY)
43+
module MyModule
44+
def some_method
45+
end
46+
end
47+
RUBY
48+
end
49+
end

0 commit comments

Comments
 (0)