rubocop-factory_bot icon indicating copy to clipboard operation
rubocop-factory_bot copied to clipboard

Bug Report: --only option ignores FactoryBot/AttributeDefinedStatically

Open ydakuka opened this issue 2 years ago • 11 comments

Actual behavior

ydakuka@yauhenid:~/Work/project$ bin/rails_docker rubocop spec/factories/masks.rb
Inspecting 1 file
C

Offenses:

spec/factories/masks.rb:6:5: C: [Correctable] FactoryBot/AssociationStyle: Use implicit style to define associations.
    association :blocklist, strategy: :build
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
spec/factories/masks.rb:6:5: C: [Correctable] FactoryBot/AttributeDefinedStatically: Use a block to declare attribute values.
    association :blocklist, strategy: :build
    ^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 2 offenses detected, 2 offenses autocorrectable
ydakuka@yauhenid:~/Work/project$ bin/rails_docker rubocop spec/factories/masks.rb --only FactoryBot/AssociationStyle
Inspecting 1 file
C

Offenses:

spec/factories/masks.rb:6:5: C: [Correctable] FactoryBot/AssociationStyle: Use implicit style to define associations.
    association :blocklist, strategy: :build
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected, 1 offense autocorrectable
ydakuka@yauhenid:~/Work/project$ bin/rails_docker rubocop spec/factories/masks.rb --only FactoryBot/AttributeDefinedStatically
Inspecting 1 file
.

1 file inspected, no offenses detected

RuboCop version

ydakuka@yauhenid:~/Work/project$ bin/rails_docker rubocop -V
1.50.2 (using Parser 3.2.2.1, rubocop-ast 1.28.1, running on ruby 2.7.8) [x86_64-linux]
  - rubocop-capybara 2.18.0
  - rubocop-performance 1.17.1
  - rubocop-rails 2.19.1
  - rubocop-rake 0.6.0
  - rubocop-rspec 2.22.0

FactoryBot version

factory_bot (6.2.1)
  activesupport (>= 5.0.0)
factory_bot_rails (6.2.0)
  factory_bot (~> 6.2.0)
  railties (>= 5.0.0)

ydakuka avatar May 16 '23 11:05 ydakuka

Thank you for report. Could you describe the code to reproduce?

ydah avatar May 17 '23 15:05 ydah

First case.

I have the factory:

# frozen_string_literal: true

FactoryBot.define do
  factory :mask do
    association :blocklist, strategy: :build
  end
end

I run rubocop:

ydakuka@yauhenid:~/Work/project$ bin/rails_docker rubocop spec/factories/masks.rb
Inspecting 1 file
C

Offenses:

spec/factories/masks.rb:5:5: C: [Correctable] FactoryBot/AssociationStyle: Use implicit style to define associations.
    association :blocklist, strategy: :build
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
spec/factories/masks.rb:5:5: C: [Correctable] FactoryBot/AttributeDefinedStatically: Use a block to declare attribute values.
    association :blocklist, strategy: :build
    ^^^^^^^^^^^^^^^^^^^^^^^^^^

I run rubocop with --only FactoryBot/AssociationStyle:

ydakuka@yauhenid:~/Work/project$ bin/rails_docker rubocop spec/factories/masks.rb --only FactoryBot/AssociationStyle
Inspecting 1 file
C

Offenses:

spec/factories/masks.rb:5:5: C: [Correctable] FactoryBot/AssociationStyle: Use implicit style to define associations.
    association :blocklist, strategy: :build
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected, 1 offense autocorrectable

I run rubocop with --only FactoryBot/AttributeDefinedStatically:

ydakuka@yauhenid:~/Work/project$ bin/rails_docker rubocop spec/factories/masks.rb --only FactoryBot/AttributeDefinedStatically
Inspecting 1 file
.

1 file inspected, no offenses detected

ydakuka avatar May 17 '23 16:05 ydakuka

Second case.

I have the factory:

FactoryBot.define do
  factory :mask do
    association :blocklist, strategy: :build
  end
end

I run rubocop:

ydakuka@yauhenid:~/Work/project$ bin/rails_docker rubocop spec/factories/masks.rb
Inspecting 1 file
C

Offenses:

spec/factories/masks.rb:1:1: C: [Correctable] Style/FrozenStringLiteralComment: Missing frozen string literal comment.
FactoryBot.define do
^
spec/factories/masks.rb:3:5: C: [Correctable] FactoryBot/AssociationStyle: Use implicit style to define associations.
    association :blocklist, strategy: :build
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 2 offenses detected, 2 offenses autocorrectable

It doesn't show me the FactoryBot/AttributeDefinedStatically cop.

ydakuka avatar May 17 '23 16:05 ydakuka

This is quite interesting. @ydah are you able to reproduce?

spec/factories/masks.rb:5:5: C: [Correctable] FactoryBot/AttributeDefinedStatically: Use a block to declare attribute values.
    association :blocklist, strategy: :build
    ^^^^^^^^^^^^^^^^^^^^^^^^^^

This shouldn't happen, as association is a reserved word, and the cop is aware of that.

pirj avatar May 17 '23 20:05 pirj

I can't reproduce locally.

@ydakuka can you please set a breakpoint in on_block of lib/rubocop/cop/factory_bot/attribute_defined_statically.rb here and see what RuboCop::FactoryBot.reserved_methods and node are?

pirj avatar May 17 '23 20:05 pirj

I reproduced it:

❯ bundle exec rubocop spec/factories.rb --only FactoryBot/AttributeDefinedStatically    
Inspecting 1 file
.

1 file inspected, no offenses detected
❯ bundle exec rubocop spec/factories.rb                                                 
Inspecting 1 file
C

Offenses:

spec/factories.rb:5:5: C: [Correctable] FactoryBot/AssociationStyle: Use implicit style to define associations.
    association :blocklist, strategy: :build
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
spec/factories.rb:5:5: C: [Correctable] FactoryBot/AttributeDefinedStatically: Use a block to declare attribute values.
    association :blocklist, strategy: :build
    ^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 2 offenses detected, 2 offenses autocorrectable

ydah avatar May 18 '23 02:05 ydah

It certainly does not seem to offense the current implementation of FactoryBot/AttributeDefinedStatically when --only is used.

true is returned here: https://github.com/rubocop/rubocop-factory_bot/blob/81d95db1836be1f4a6726100e131cd53fe2f9008/lib/rubocop/cop/factory_bot/attribute_defined_statically.rb#L80-L82

Therefore, it is not an offense: https://github.com/rubocop/rubocop-factory_bot/blob/81d95db1836be1f4a6726100e131cd53fe2f9008/lib/rubocop/cop/factory_bot/attribute_defined_statically.rb#L48

ydah avatar May 18 '23 03:05 ydah

If we run RuboCop without adding the --only option, does that mean that the cop that can be autocorrected will be autocorrected internally and then checked for further offense in the autocorrected code? Somehow this case seems to be working that way.

Original code:

# frozen_string_literal: true

FactoryBot.define do
  factory :mask do
    association :blocklist, strategy: :build
  end
end

Run bundle exec rubocop -A spec/factories.rb --only FactoryBot/AssociationStyle This will cause an offense of FactoryBot/AttributeDefinedStatically.

# frozen_string_literal: true

FactoryBot.define do
  factory :mask do
    blocklist strategy: :build
  end
end

Run bundle exec rubocop -A spec/factories.rb --only FactoryBot/AttributeDefinedStatically

# frozen_string_literal: true

FactoryBot.define do
  factory :mask do
    blocklist { { strategy: :build } }
  end
end

ydah avatar May 18 '23 03:05 ydah

Good question! @rubocop/rubocop-core To me, it doesn’t make much sense, and I would expect all cops to inspect the original code.

pirj avatar May 18 '23 06:05 pirj

Can you please create a reproduction repo, @ydah ?

pirj avatar May 18 '23 06:05 pirj

A repository has been created to reproduce: https://github.com/ydah/reproduction-44-rubocop-factory_bot

But I noticed an interesting behavior.

First:

❯ bundle exec rubocop spec/factories.rb                                             
Inspecting 1 file
C

Offenses:

spec/factories.rb:5:5: C: [Correctable] FactoryBot/AssociationStyle: Use implicit style to define associations.
    association :blocklist, strategy: :build
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected, 1 offense autocorrectable

Running with automatic correction:

❯ bundle exec rubocop spec/factories.rb -A                                          
Inspecting 1 file
C

Offenses:

spec/factories.rb:5:5: C: [Corrected] FactoryBot/AssociationStyle: Use implicit style to define associations.
    association :blocklist, strategy: :build
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
spec/factories.rb:5:5: C: [Corrected] FactoryBot/AttributeDefinedStatically: Use a block to declare attribute values.
    blocklist strategy: :build
    ^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 2 offenses detected, 2 offenses corrected

Run again with revert auto correct:

❯ bundle exec rubocop spec/factories.rb   
Inspecting 1 file
C

Offenses:

spec/factories.rb:5:5: C: [Correctable] FactoryBot/AssociationStyle: Use implicit style to define associations.
    association :blocklist, strategy: :build
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
spec/factories.rb:5:5: C: [Correctable] FactoryBot/AttributeDefinedStatically: Use a block to declare attribute values.
    association :blocklist, strategy: :build
    ^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 2 offenses detected, 2 offenses autocorrectable

Perhaps this behavior is referring to the cache?

ydah avatar May 20 '23 09:05 ydah