rubocop / rubocop-factory_bot

Code style checking for factory_bot files.
https://docs.rubocop.org/rubocop-factory_bot
MIT License
44 stars 13 forks source link

Bug Report: --only option ignores FactoryBot/AttributeDefinedStatically #44

Open ydakuka opened 1 year ago

ydakuka commented 1 year ago

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)
ydah commented 1 year ago

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

ydakuka commented 1 year ago

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 commented 1 year ago

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.

pirj commented 1 year ago

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 commented 1 year ago

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?

ydah commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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
pirj commented 1 year ago

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 commented 1 year ago

Can you please create a reproduction repo, @ydah ?

ydah commented 1 year ago

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?