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

Fix a false positive for FactoryBot/AssociationStyle when EnforcedStyle: Explicit and using trait within trait #59

Merged
merged 1 commit into from
Sep 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
- Fix `FactoryBot/AssociationStyle` cop to ignore explicit associations with `strategy: :build`. ([@pirj])
- Change `FactoryBot/CreateList` so that it is not an offense if not repeated multiple times. ([@ydah])
- Fix a false positive for `FactoryBot/AssociationStyle` when `association` is called in trait block and column name is keyword. ([@ydah])
- Fix a false positive for `FactoryBot/AssociationStyle` when `EnforcedStyle: Explicit` and using trait within trait. ([@ydah])

## 2.23.1 (2023-05-15)

Expand Down
16 changes: 15 additions & 1 deletion lib/rubocop/cop/factory_bot/association_style.rb
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,11 @@ def on_send(node)
(send nil? :association $...)
PATTERN

# @!method trait_name(node)
def_node_search :trait_name, <<~PATTERN
(send nil? :trait (sym $_) )
PATTERN

def autocorrect(corrector, node)
if style == :explicit
autocorrect_to_explicit_style(corrector, node)
Expand Down Expand Up @@ -168,7 +173,8 @@ def autocorrect_to_implicit_style(corrector, node)

def bad?(node)
if style == :explicit
implicit_association?(node)
implicit_association?(node) &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cosmetic. Here it feels that ‘implicit_association?’ can return true when it only looks like an implicit association, not necessarily is.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should rename the matcher? I can't think of a good name now 🤔

!trait_within_trait?(node)
else
explicit_association?(node) &&
!with_strategy_build_option?(node) &&
Expand Down Expand Up @@ -239,6 +245,14 @@ def options_for_autocorrect_to_implicit_style(node)
end
options
end

def trait_within_trait?(node)
factory_node = node.ancestors.reverse.find do |ancestor|
ancestor.send_node.method?(:factory) if ancestor.block_type?
end

trait_name(factory_node).include?(node.method_name)
end
end
end
end
Expand Down
71 changes: 71 additions & 0 deletions spec/rubocop/cop/factory_bot/association_style_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -334,5 +334,76 @@ def inspected_source_filename
RUBY
end
end

context 'when using trait within trait' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
factory :order do
trait :completed do
completed_at { 3.days.ago }
end

trait :refunded do
completed
refunded_at { 1.day.ago }
end
end
RUBY
end
end

context 'when factory inside a factory' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
factory :order do
trait :completed do
completed_at { 3.days.ago }
end

factory :order_with_refund do
trait :refunded do
completed
refunded_at { 1.day.ago }
end
end
end
RUBY
end
end

context 'when use an association with the same name as trait' do
it 'registers and corrects an offense' do
expect_offense(<<~RUBY)
factory :order do
trait :completed do
completed_at { 3.days.ago }
end
end

factory :order_with_refund do
trait :refunded do
completed
^^^^^^^^^ Use explicit style to define associations.
refunded_at { 1.day.ago }
end
end
RUBY

expect_correction(<<~RUBY)
factory :order do
trait :completed do
completed_at { 3.days.ago }
end
end

factory :order_with_refund do
trait :refunded do
association :completed
refunded_at { 1.day.ago }
end
end
RUBY
end
end
end
end