Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
ydakuka opened this issue May 16, 2023 · 11 comments
Open

Comments

@ydakuka
Copy link

ydakuka commented May 16, 2023

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 ydakuka changed the title Bug Report: --only option ignore FactoryBot/AttributeDefinedStatically Bug Report: --only option ignores FactoryBot/AttributeDefinedStatically May 16, 2023
@ydah
Copy link
Member

ydah commented May 17, 2023

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

@ydakuka
Copy link
Author

ydakuka commented May 17, 2023

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
Copy link
Author

ydakuka commented May 17, 2023

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
Copy link
Member

pirj commented May 17, 2023

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
Copy link
Member

pirj commented May 17, 2023

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
Copy link
Member

ydah commented May 18, 2023

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
Copy link
Member

ydah commented May 18, 2023

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

true is returned here:

def proc?(attribute)
value_matcher(attribute).to_a.all?(&:block_pass_type?)
end

Therefore, it is not an offense:

next if proc?(attribute) || association?(attribute.first_argument)

@ydah
Copy link
Member

ydah commented May 18, 2023

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
Copy link
Member

pirj commented May 18, 2023

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
Copy link
Member

pirj commented May 18, 2023

Can you please create a reproduction repo, @ydah ?

@ydah
Copy link
Member

ydah commented May 20, 2023

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants