Add cop flag explicit factory bot dsl method usage in association

[morissetcl]

Fixes #6

This cop will add an offense when a strategy is hardcoded to create an association in a factory.

  # bad - only works for one strategy
    factory :foo do
      profile { create(:profile) }
    end

  # good - implicit
    factory :foo do
      profile
    end
  
  # good - explicit
    factory :foo do
      association :profile
    end
    
  # good - inline
    factory :foo do
      profile { association :profile }
    end

---

Before submitting the PR make sure the following are checked:

• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Updated documentation.
• Added an entry to the CHANGELOG.md if the new code introduces user-observable changes.
• The build (bundle exec rake) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).

If you have created a new cop:

• Added the new cop to config/default.yml.
• The cop is configured as Enabled: pending in config/default.yml.
• The cop documents examples of good and bad code.
• The tests assert both that bad code is reported and that good code is not reported.
• Set VersionAdded: "<<next>>" in default/config.yml.

[pirj]

Fantastic!
The path to improve this hardcoded style is so clearly documented. 👏
Thanks for tackling this.
A few notes on how to (subjectively) simplify the code.

[morissetcl]

Thanks @pirj for the quick feedback.
I will address your comments in the next few days.

[morissetcl]

@pirj is there a more elegant way to do it?
This is the way I found to make this test pass.

[pirj]

It is as elegant as a node pattern can be 😅
Seriously, this construct that looks clumsy at first, replaces several lines of Ruby code, and the code that transpiles it into Ruby is optimised and known to have no side effects.

[morissetcl]

Haha sorry my comment was not clear at all 🙈

I was more talking about using two node matcher and used them in 3 nested blocks
https://github.com/rubocop/rubocop-factory_bot/pull/34/files#diff-10eb8e610da9d71b102d2bd7dad3e70e381264a4ff3fe0cb43b1c4085c2d8ae3R51-R59

[pirj]

Got it. One thing that seems like might be improved is the node.each_node call.
Seems like, but not necessarily have to be.
I see three ways to refactor.

1. A node matcher returns just one element. There's a def_node_search as well, but it searches with arbitrary depth which is sometimes incorrect:

factory :user do
  factory :admin do
    profile { build :profile }
  en
end

The build would be found twice - inside the factory :user block and inside the factory :admin block.
This also imposes a performance impact.

2. Node pattern that matches parent elements with ^. This is something outside my cognitive toolbox, I would have to read the doc to figure out how that node pattern works.

3. In the code trigger on on_send instead for HARDcODED sends, and check if the parent element is an association, and check if the parent's parent is a factory/trait.
   But here comes the trouble with a single-statement block and multi-statement:

$ ruby-parse -e 'factory { 1 }'
(block
  (send nil :factory)
  (args)
  (int 1))
$ ruby-parse -e 'factory { 1; 2 }'
warning: parser/current is loading parser/ruby30, which recognizes 3.0.6-compliant syntax, but you are running 3.0.3.
(block
  (send nil :factory)
  (args)
  (begin
    (int 1)
    (int 2)))

Notice the additional begin intermediate node for multi-statement. We'd have to account for that. In both parent checks.
And for the method 2 above, I can't tell off the top of my head how ^ works in such cases.

4. A compound node pattern, a search type, that would describe the structure. combine those existing two and add an additional < inner_pattern ... >. It's a monster.

I'd say your code is good as is, unless other guys come up with a novel idea.

[pirj]

Nitpicks only. Looks great, thanks a lot!

Good to merge on green ci.

With a tweak (added rubocop-factory_bot to .rubocop.yml) checked the cop against real-world-rspec. No errors, and 18 (I expected much more!) offences detected.
I've seen this in all projects I worked on. It's reassuring that it's not widely abused on open source projects.

/Users/pirj/source/real-world-rspec/gitlabhq/spec/factories/packages/packages.rb:246:27: C: FactoryBot/FactoryAssociationWithStrategy: Use an implicit, explicit or inline definition instead of hard coding a strategy for setting association within factory.
        conan_metadatum { build(:conan_metadatum, package: nil) } # rubocop:disable FactoryBot/InlineAssociation
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/

This alerted me. Does GitLab have a similar cop?
https://github.com/gitlabhq/gitlabhq/blob/master/rubocop/cop/rspec/factory_bot/inline_association.rb

For a follow-up, I suggest borrowing ideas from their spec to see if we cover what they have. I have never seen add_attribute in practice.

[pirj]

I've changed the description to "fixes #6". Do you think there's anything left there to address? Like auto-correction?

[morissetcl]

I've changed the description to "fixes #6". Do you think there's anything left there to address? Like auto-correction?

I think it's good.
I don't know what the autocorrect will be. There are several ways to solve this offense, so I'll let the developers choose what works best for them.

[ydah]

Thank you great works! ♥