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

FactoryBot/CreateList: Prefer 1.times.map conflicts with Lint/UselessTimes #78

Open
taryneast opened this issue Sep 27, 2023 · 1 comment

Comments

@taryneast
Copy link

taryneast commented Sep 27, 2023

The first line of this spec-code :

         [ create(:event_delivery, invoice: invoice, flat_rate: false, carer: carer) ] +
            create_list(:event_delivery, 2, invoice: invoice, flat_rate: true, carer: carer)

Triggered an offense of the form: FactoryBot/CreateList: Prefer 1.times.map.
and was listed as correctable.

However when correcting it, it triggered a different rubocop as follows:

Inspecting 1 file
W

Offenses:

<filename here>:270:11: C: [Corrected] FactoryBot/CreateList: Prefer 1.times.map.
          [create(:event_delivery, invoice: invoice, flat_rate: false, carer: carer)] +
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
<filename here>:270:11: W: Lint/UselessTimes: Useless call to 1.times detected.
          1.times.map { create(:event_delivery, invoice: invoice, flat_rate: false, carer: carer) } +
          ^^^^^^^

1 file inspected, 2 offenses detected, 1 offense corrected

It seems that 1.times.map is not the right solution to this, but instead it should be:

         create_list(:event_delivery, 1, invoice: invoice, flat_rate: false, carer: carer) +
            create_list(:event_delivery, 2, invoice: invoice, flat_rate: true, carer: carer)

Which seems to make both cops happy.
Any chance we could update the cop to do this instead?

@pirj
Copy link
Member

pirj commented Sep 28, 2023

Thanks for reporting.

We could allow for length 1 even if we detect that arguments include method calls here

return if arguments_include_method_call?(node.body)
and line 125, too.

I don't think our specs describe how we deal with most cases for a single-entry list. Would you like to discuss that for two combinations of styles and the presence of method calls in arguments?

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

2 participants