FactoryBot/AttributeDefinedStatically doesn't work well

[chubchenko]

When processing the following code:

# spec/lib/application/configuration_spec.rb

describe '.ignore' do
  context 'when the block is given' do
    it 'changes the count of the registered regex' do
      expect do
        described_class.ignore do |ignore|
          ignore << /(a|b)/
          ignore.push(/.*/)
        end
      end.to change(described_class.ignore, :count).by(2)
    end
  end
end

I have the next recommendation:

spec/lib/application/configuration_spec.rb:33:13: C: FactoryBot/AttributeDefinedStatically: Use a block to declare attribute values.
            ignore.push(/.*/)
            ^^^^^^^^^^^^

Also, I have the same recommendation in another file:

# spec/dummy/config/initializers/seo.rb

config.ignore do |ignore|
  ignore.push(%r{\A/admin.*}.freeze)
  ignore.push(%r{\A/\z}.freeze)
end

spec/dummy/config/initializers/seo.rb:28:5: C: FactoryBot/AttributeDefinedStatically: Use a block to declare attribute values.
    ignore.push(%r{\A/admin.*}.freeze)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^
spec/dummy/config/initializers/seo.rb:29:5: C: FactoryBot/AttributeDefinedStatically: Use a block to declare attribute values.
    ignore.push(%r{\A/\z}.freeze)
    ^^^^^^^^^^^^^^^^^^^^^^

rubocop -V
0.93.1 (using Parser 2.7.2.0, rubocop-ast 0.8.0, running on ruby 2.6.5 x86_64-darwin20)

bundle info rubocop-rspec

* rubocop-rspec (1.43.2)
	Summary: Code style checking for RSpec files
	Homepage: https://github.com/rubocop-hq/rubocop-rspec
	Path: /Users/chubchenko/.rvm/gems/ruby-2.6.5/gems/rubocop-rspec-1.43.2

[pirj]

Wondering why this https://github.com/rubocop-hq/rubocop-rspec/blob/e11f6e3c4a539203185e0e11089c8fe2e8f05393/config/default.yml#L8 doesn't work for you.
FactoryBot cops should only scan their directories.
Would you like to debug this a little further, say, by running rubocop with --debug flag, or setting a breakpoint somewhere here https://github.com/rubocop-hq/rubocop-rspec/blob/0d5e03d686d569f9e6f2c00ddce0b04d6463cb3c/lib/rubocop/cop/rspec/base.rb#L39 ?

[chubchenko]

Regarding the

AllCops:
  RSpec/FactoryBot:
    Patterns:
    - spec/factories/**/*.rb

this was the first thing I tried, but my attempt failed - the same behavior as before 😞 .

Here is an example of output with the --debug option:

bundle exec --gemfile gemfiles/rubocop.gemfile rubocop spec/dummy/config/initializers/seo.rb --debug

For /Users/chubchenko/applications/seo: configuration from /Users/chubchenko/applications/seo/.rubocop.yml
configuration from /Users/chubchenko/.rvm/gems/ruby-2.6.5/gems/rubocop-performance-1.8.1/config/default.yml
configuration from /Users/chubchenko/.rvm/gems/ruby-2.6.5/gems/rubocop-performance-1.8.1/config/default.yml
Default configuration from /Users/chubchenko/.rvm/gems/ruby-2.6.5/gems/rubocop-0.93.1/config/default.yml
configuration from /Users/chubchenko/.rvm/gems/ruby-2.6.5/gems/rubocop-rails-2.8.1/config/default.yml
configuration from /Users/chubchenko/.rvm/gems/ruby-2.6.5/gems/rubocop-rails-2.8.1/config/default.yml
configuration from /Users/chubchenko/.rvm/gems/ruby-2.6.5/gems/rubocop-rspec-1.43.2/config/default.yml
configuration from /Users/chubchenko/.rvm/gems/ruby-2.6.5/gems/rubocop-rspec-1.43.2/config/default.yml
Inspecting 1 file
Scanning /Users/chubchenko/applications/cms-seo/spec/dummy/config/initializers/seo.rb
Loading cache from /Users/chubchenko/.cache/rubocop_cache/6655e55d54761e1e00b5da5b9a104451067030e3/6d7a3b621ca1730e04accd938619e4bdab66cfb1/48dc38c32c72316fef6fe5bddb16f10a6b0fd8cc
C

Offenses:

spec/dummy/config/initializers/seo.rb:28:5: C: FactoryBot/AttributeDefinedStatically: Use a block to declare attribute values.
    ignore.push(%r{\A/admin.*}.freeze)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
spec/dummy/config/initializers/seo.rb:29:5: C: FactoryBot/AttributeDefinedStatically: Use a block to declare attribute values.
    ignore.push(%r{\A/\z}.freeze)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 2 offenses detected, 2 offenses auto-correctable
Finished in 0.7074670000001788 seconds

[pirj]

Do you mind sharing the /Users/chubchenko/applications/seo/.rubocop.yml config?

[pirj]

@bquorning Do you think we need to make some changes to the relevant_rubocop_rspec_file? in order to support RSpec/FactoryBot/Patterns setting? It seems that the Patterns setting is one of our own, not supported by the RuboCop config itself. And our logic on top of it it only respects RSpec/Patterns, not RSpec/FactoryBot/Patterns.

[chubchenko]

Yes, no problem. Here is the configuration file:

require:
  - "rubocop-performance"
  - "rubocop-rails"
  - "rubocop-rspec"

AllCops:
  Exclude:
    - "bin/*"
    - "spec/dummy/bin/*"
    - "spec/dummy/db/**/*"
  TargetRubyVersion: 2.5
  NewCops: enable

############### Layout ###############

Layout/LineLength:
  Max: 120

############### Naming ###############

Naming/PredicateName:
  AllowedMethods:
    - "is_a?"
    - "have_redirect"

############### Style ###############

Style/Documentation:
  Enabled: false

############### Metrics ###############

Metrics/AbcSize:
  IgnoredMethods:
    - "change"
    - "redirectable"

Metrics/BlockLength:
  ExcludedMethods:
    - "register"
    - "describe"
    - "shared_examples"

Metrics/MethodLength:
  ExcludedMethods:
    - "change"

############### Rails ###############

Rails/SkipsModelValidations:
  Exclude:
    - "app/models/seo_module/redirect.rb"
    - "app/services/seo_module/redirect/kill_cycling.rb"
    - "lib/seo_module/admin/redirect.rb"

Rails/DynamicFindBy:
  Whitelist:
    - "find_by_page_type_and_slug"

############### RSpec ###############

RSpec/DescribeClass:
  Exclude:
    - "spec/system/**/*"

RSpec/ExampleLength:
  Exclude:
    - "spec/system/**/*"
  Max: 10

RSpec/MultipleExpectations:
  Exclude:
    - "spec/system/**/*"
  Max: 2

[bquorning]

Do you think we need to make some changes to the relevant_rubocop_rspec_file? in order to support RSpec/FactoryBot/Patterns setting?

Actually, I started thinking maybe it’s time to move the FactoryBot code into a separate gem (e.g. RuboCop-FactoryBot). Do the three FactoryBot cops rely on any RSpec cop specific code (the Language module, etc.) at all?

[pirj]

I support the idea. Reasoning:

• no common code (that I could find with RSpec cops)
• do not overlap on scanned files (FactoryClassName, AttributeDefinedStatically)
• it's probably the best moment to extract at about the moment when RuboCop hits 1.0
• it makes it possible to find misuses when using FactoryBot with Minitest as well
• ... I have a number of FactoryBot-related cop ideas based on my recent projects code assession

Cons:

• CreateList really should watch both spec files and factories, since both may call create in a loop, or create_list. We'll probably have to keep it here

Side note: we will still have to use über-departments since the whole problem was due to Rails/HttpStatus and RSpec/FilePath. But we can opt-out of this in rspec-factory_bot.

Concerning file locations, as per FactoryBot doc, we should be looking in:

test/factories.rb
spec/factories.rb
test/factories/*.rb
spec/factories/*.rb

Another wild idea would be to submit those cops directly to FactoryBot itself, but according to my recent experience, their maintainers are not too responsive.

[pirj]

This should be fixed in #1063 that is released in 2.0.

Please feel free to reopen if it still doesn't work properly.