false positive on factory

[luizkowalski]

I have the following factory:

FactoryBot.define do
  factory :feature_tier, class: 'Services::FeatureTier' do
    feature { create(:feature) }
    tier { create(:tier) }
  end

and I'm getting a false positive (I think) when linting

spec/factories/services_features_tiers.rb:3:5: C: RSpec/EmptyLineAfterExampleGroup: Add an empty line after feature.
    feature { create(:feature) }
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I'm running Rubocop 0.56.0 with rubocop-rspec 1.27.0

[bquorning]

Thank you for the bug report @luizkowalski.

If you use factories to generate an object with an attribute named identically to certains RSpec DSL methods (describe, context, feature, example_group, etc.) you will get these false positives. Testing your example, I am seeing false positives for RSpec/EmptyExampleGroup, RSpec/EmptyLineAfterExampleGroup and RSpec/MissingExampleGroupArgument.

The real problem is probably that we even run the non-FactoryBot cops against the spec/factories.rb etc. files. I think removing "(?:^|/)spec/" from the AllCops::RSpec::Patterns configuration might be the solution.

[bquorning]

@backus Do you recall why "(?:^|/)spec/" was added as a file pattern in 65c594088597279a674a3e35d1c5cb18b297f181?

[backus]

Maybe to catch things like spec/spec_helper.rb and spec/support/ which maybe has example groups in it?

[januszm]

This still causes problems, see:

# migration for the Mymodel

class CreateMymodelFeatures < ActiveRecord::Migration[6.0]
--
def change
  create_table :mymodel_features do |t|
    t.string :foo
    t.string :feature, null: false
# ...

# factory
spec/factories/mymodel_features.rb:6:7: C: RSpec/MissingExampleGroupArgument:
  The first argument to feature should not be empty.
      feature { 'blablabla' }
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

if you call a db column feature and use FactoryBot, it'll cause false positives.

Looks like all factories (as well as app subdirectories) should be excluded from this check.

[pirj]

@januszm Pull request is welcome! cc @itay-grudev

I imagine you'd have to tweak this logic https://github.com/rubocop-hq/rubocop-rspec/blob/master/lib/rubocop/cop/rspec/cop.rb#L43 or maybe take a look at the other RuboCop extensions how they manage to do this. Last time I've checked, https://github.com/rubocop-hq/rubocop-minitest didn't have anything related to path checks.

Another option as previously mentioned would be to tweak the pattern here https://github.com/rubocop-hq/rubocop-rspec/blob/master/config/default.yml#L6

[boardfish]

I'm also experiencing this issue when setting an attribute called context:

spec/factories/tests.rb:6:5: C: RSpec/EmptyExampleGroup: Empty example group detected.
    context { 'MyString' }
    ^^^^^^^
spec/factories/tests.rb:6:5: C: [Corrected] RSpec/EmptyLineAfterExampleGroup: Add an empty line after context.
    context { 'MyString' }
    ^^^^^^^^^^^^^^^^^^^^^^
spec/factories/tests.rb:6:5: C: RSpec/MissingExampleGroupArgument: The first argument to context should not be empty.
    context { 'MyString' }
    ^^^^^^^^^^^^^^^^^^^^^^

Adding the following to .rubocop.yml fixed this:

RSpec/EmptyExampleGroup:
  Exclude:
    - 'spec/factories/**/*'

RSpec/EmptyLineAfterExampleGroup:
  Exclude:
    - 'spec/factories/**/*'

RSpec/MissingExampleGroupArgument:
  Exclude:
    - 'spec/factories/**/*'

But I imagine this should probably be integrated into the gem itself.

[sl4vr]

Common FactoryBot paths can be added to default exclude paths:

Exclude:
  - spec/factories.rb
  - spec/factories/**/*.rb
  - features/support/factories/**/*.rb

However it may break if user config has Exclude section as well since override policy applied AFAIK. But it's done same way for the Include, just mentioned in docs to keep in mind to list default paths, so probably that's an option here as well.

Another way might be to create some sort of mixin for non-top level cops to check if the current block/send is even inside any of example group or shared example/context group. I believe most of the cops shouldn't check for RSpec syntax outside of RSpec blocks. Though it may hit the performance, so maybe trade off solution could be to check that just any of top_level_groups is example group or shared example/context group and just stop investigation if not. It doesn't guarantee absence of false positives in weird RSpec syntax use cases, but it should cover common RSpec usage.