Skip to content

Commit da35c8d

Browse files
committed
Introduce FactoryGirl/FactoryClassName cop
The cop ensures that when creating factories, we do not allow passing the class itself. Instead we require users to pass the class name. Although `FactoryBot` allows passing the class, we find that by doing so, application models are loaded during application initialization and many files are required without them having to. This affects the startup time as well as libraries like simplecov that fail to capture those files. Example: bad ``` factory :foo, class: Foo do end ``` good ``` factory :foo, class: 'Foo' do end ```
1 parent 46a37de commit da35c8d

File tree

7 files changed

+144
-0
lines changed

7 files changed

+144
-0
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
* Fix `RSpec/SubjectStub` to detect implicit subjects stubbed. ([@QQism][])
88
* Fix `RSpec/Pending` not flagging `skip` with string values. ([@pirj][])
99
* Add `AllowedExplicitMatchers` config option for `RSpec/PredicateMatcher`. ([@mkrawc][])
10+
* Add `FactoryBot/FactoryClassName` cop. ([@jfragoulis][])
1011

1112
## 1.36.0 (2019-09-27)
1213

@@ -462,3 +463,4 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features.
462463
[@QQism]: https://github.com/QQism
463464
[@kellysutton]: https://github.com/kellysutton
464465
[@mkrawc]: https://github.com/mkrawc
466+
[@jfragoulis]: https://github.com/jfragoulis

config/default.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,11 @@ FactoryBot/CreateList:
480480
- n_times
481481
StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/FactoryBot/CreateList
482482

483+
FactoryBot/FactoryClassName:
484+
Description: Use string value when setting the class attribute explicitly.
485+
Enabled: true
486+
StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/FactoryBot/FactoryClassName
487+
483488
Rails/HttpStatus:
484489
Description: Enforces use of symbolic or numeric value to describe HTTP status.
485490
Enabled: true
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
# frozen_string_literal: true
2+
3+
module RuboCop
4+
module Cop
5+
module RSpec
6+
module FactoryBot
7+
# Use string value when setting the class attribute explicitly.
8+
#
9+
# @example
10+
# # bad
11+
# factory :foo, class: Foo do
12+
# end
13+
#
14+
# # good
15+
# factory :foo, class: 'Foo' do
16+
# end
17+
class FactoryClassName < Cop
18+
MSG = "Pass '%<class_name>s' instead of %<class_name>s."
19+
20+
def_node_matcher :class_name, <<~PATTERN
21+
(send _ :factory _ (hash <(pair (sym :class) $(const ...)) ...>))
22+
PATTERN
23+
24+
def on_send(node)
25+
class_name(node) do |cn|
26+
add_offense(cn, message: format(MSG, class_name: cn.const_name))
27+
end
28+
end
29+
30+
def autocorrect(node)
31+
lambda do |corrector|
32+
corrector.replace(node.loc.expression, "'#{node.source}'")
33+
end
34+
end
35+
end
36+
end
37+
end
38+
end
39+
end

lib/rubocop/cop/rspec_cops.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
require_relative 'rspec/factory_bot/attribute_defined_statically'
77
require_relative 'rspec/factory_bot/create_list'
8+
require_relative 'rspec/factory_bot/factory_class_name'
89

910
begin
1011
require_relative 'rspec/rails/http_status'

manual/cops.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
* [FactoryBot/AttributeDefinedStatically](cops_factorybot.md#factorybotattributedefinedstatically)
1010
* [FactoryBot/CreateList](cops_factorybot.md#factorybotcreatelist)
11+
* [FactoryBot/FactoryClassName](cops_factorybot.md#factorybotfactoryclassname)
1112

1213
#### Department [RSpec](cops_rspec.md)
1314

manual/cops_factorybot.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,27 @@ EnforcedStyle | `create_list` | `create_list`, `n_times`
7777
### References
7878

7979
* [https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/FactoryBot/CreateList](https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/FactoryBot/CreateList)
80+
81+
## FactoryBot/FactoryClassName
82+
83+
Enabled by default | Supports autocorrection
84+
--- | ---
85+
Enabled | Yes
86+
87+
Use string value when setting the class attribute explicitly.
88+
89+
### Examples
90+
91+
```ruby
92+
# bad
93+
factory :foo, class: Foo do
94+
end
95+
96+
# good
97+
factory :foo, class: 'Foo' do
98+
end
99+
```
100+
101+
### References
102+
103+
* [https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/FactoryBot/FactoryClassName](https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/FactoryBot/FactoryClassName)
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
# frozen_string_literal: true
2+
3+
RSpec.describe RuboCop::Cop::RSpec::FactoryBot::FactoryClassName do
4+
subject(:cop) { described_class.new }
5+
6+
context 'when passing block' do
7+
it 'flags passing a class' do
8+
expect_offense(<<~RUBY)
9+
factory :foo, class: Foo do
10+
^^^ Pass 'Foo' instead of Foo.
11+
end
12+
RUBY
13+
14+
expect_correction(<<~RUBY)
15+
factory :foo, class: 'Foo' do
16+
end
17+
RUBY
18+
end
19+
20+
it 'flags passing a class from global namespace' do
21+
expect_offense(<<~RUBY)
22+
factory :foo, class: ::Foo do
23+
^^^^^ Pass 'Foo' instead of Foo.
24+
end
25+
RUBY
26+
27+
expect_correction(<<~RUBY)
28+
factory :foo, class: '::Foo' do
29+
end
30+
RUBY
31+
end
32+
33+
it 'flags passing a subclass' do
34+
expect_offense(<<~RUBY)
35+
factory :foo, class: Foo::Bar do
36+
^^^^^^^^ Pass 'Foo::Bar' instead of Foo::Bar.
37+
end
38+
RUBY
39+
40+
expect_correction(<<~RUBY)
41+
factory :foo, class: 'Foo::Bar' do
42+
end
43+
RUBY
44+
end
45+
46+
it 'ignores passing class name' do
47+
expect_no_offenses(<<~RUBY)
48+
factory :foo, class: 'Foo' do
49+
end
50+
RUBY
51+
end
52+
end
53+
54+
context 'when not passing block' do
55+
it 'flags passing a class' do
56+
expect_offense(<<~RUBY)
57+
factory :foo, class: Foo
58+
^^^ Pass 'Foo' instead of Foo.
59+
RUBY
60+
61+
expect_correction(<<~RUBY)
62+
factory :foo, class: 'Foo'
63+
RUBY
64+
end
65+
66+
it 'ignores passing class name' do
67+
expect_no_offenses(<<~RUBY)
68+
factory :foo, class: 'Foo'
69+
RUBY
70+
end
71+
end
72+
end

0 commit comments

Comments
 (0)