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

Add ExplicitOnly configuration option to RSpec/FactoryBot/ConsistentParenthesesStyle, RSpec/FactoryBot/CreateList and RSpec/FactoryBot/FactoryNameStyle #1487

Closed

Conversation

ydah
Copy link
Member

@ydah ydah commented Nov 17, 2022

Follow up: #1482 (comment)
Resolve: rubocop/rubocop-factory_bot#5


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 modified an existing cop's configuration options:

  • Set VersionChanged in config/default.yml to the next major version.

@@ -929,7 +929,9 @@ RSpec/FactoryBot/ConsistentParenthesesStyle:
SupportedStyles:
- require_parentheses
- omit_parentheses
ExplicitOnly: false
Copy link
Member

Choose a reason for hiding this comment

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

I was thinking whether this could be applied to RSpec/FactoryBot (the whole department) and affect the matcher on the receiver (whether to include nil receiver or not). @pirj WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed it is, looks good. So we cut out ExplicitOnly into a common class to make it easier to handle, and added the configuration values to the other cop. WDYT?

@ydah ydah force-pushed the add_option_for_consistent_parentheses_style branch from 530d4b7 to 4db3f31 Compare November 18, 2022 14:51
@pirj
Copy link
Member

pirj commented Nov 18, 2022

We have FactoryBot/SyntaxMethods already that enforces FactoryBot.create vs just create, right?
Is there a situation where non-explicit and explicit would be used sparingly?
I'm hinting that those two cops together can enforce any given style combination, and this makes the option redundant.
Can you please help me understand the need for this option?

@Darhazer
Copy link
Member

Suppose you want to use FactoryBot only with explicit receiver - as you are using methods like create either from another gem or defined yourself. So you disable the SyntaxMethods cop. The idea is to prevent the other FactoryBot cops of raising false positives.

@ydah
Copy link
Member Author

ydah commented Nov 18, 2022

My understanding is that FactoryBot/SyntaxMethods can explicitly force FactoryBot to be a receiver or not.
Therefore, methods such as create that do not explicitly have a receiver with FactoryBot can be considered as not belonging to FactoryBot, so setting this option to true will help exclude methods with the same name other than FactoryBot, which would be a false positive. We believe that this option will help to exclude methods with the same name other than FactoryBot that are false positive.

ydah added a commit to ydah/rubocop-rspec that referenced this pull request Nov 24, 2022
ydah added a commit to ydah/rubocop-rspec that referenced this pull request Nov 24, 2022
@ydah ydah force-pushed the add_option_for_consistent_parentheses_style branch from 4db3f31 to f1caccb Compare November 24, 2022 02:58
@ydah ydah changed the title Add ExplicitOnly configuration option to RSpec/FactoryBot/ConsistentParenthesesStyle Add ExplicitOnly configuration option to RSpec/FactoryBot/ConsistentParenthesesStyle, RSpec/FactoryBot/CreateList and RSpec/FactoryBot/FactoryNameStyle. Nov 24, 2022
@ydah ydah changed the title Add ExplicitOnly configuration option to RSpec/FactoryBot/ConsistentParenthesesStyle, RSpec/FactoryBot/CreateList and RSpec/FactoryBot/FactoryNameStyle. Add ExplicitOnly configuration option to RSpec/FactoryBot/ConsistentParenthesesStyle, RSpec/FactoryBot/CreateList and RSpec/FactoryBot/FactoryNameStyle Nov 24, 2022
ydah added a commit to ydah/rubocop-rspec that referenced this pull request Nov 24, 2022
ydah added a commit to ydah/rubocop-rspec that referenced this pull request Nov 24, 2022
@ydah ydah force-pushed the add_option_for_consistent_parentheses_style branch from f1caccb to c10b14b Compare November 24, 2022 03:01
ydah added a commit to ydah/rubocop-rspec that referenced this pull request Nov 24, 2022
ydah added a commit to ydah/rubocop-rspec that referenced this pull request Nov 24, 2022
@ydah ydah force-pushed the add_option_for_consistent_parentheses_style branch from c10b14b to eea2e53 Compare November 24, 2022 03:35
@ydah ydah requested review from pirj and Darhazer and removed request for pirj November 24, 2022 03:36
ydah added a commit to ydah/rubocop-rspec that referenced this pull request Dec 4, 2022
ydah added a commit to ydah/rubocop-rspec that referenced this pull request Dec 4, 2022
@ydah ydah force-pushed the add_option_for_consistent_parentheses_style branch from eea2e53 to 19c2b54 Compare December 4, 2022 07:16
ydah added a commit to ydah/rubocop-rspec that referenced this pull request Dec 12, 2022
ydah added a commit to ydah/rubocop-rspec that referenced this pull request Dec 12, 2022
@ydah ydah force-pushed the add_option_for_consistent_parentheses_style branch from 19c2b54 to 6563c10 Compare December 12, 2022 05:27
@ydah
Copy link
Member Author

ydah commented Dec 12, 2022

It was conflicted, so I rebased it to the latest master branch.

@pirj pirj requested a review from bquorning December 12, 2022 11:24
ydah added a commit to ydah/rubocop-rspec that referenced this pull request Dec 13, 2022
ydah added a commit to ydah/rubocop-rspec that referenced this pull request Dec 13, 2022
@ydah ydah requested a review from pirj January 12, 2023 04:32
ydah added a commit to ydah/rubocop-rspec that referenced this pull request Jan 13, 2023
ydah added a commit to ydah/rubocop-rspec that referenced this pull request Jan 13, 2023
@ydah ydah force-pushed the add_option_for_consistent_parentheses_style branch from 2517dc5 to c103f00 Compare January 13, 2023 13:15
ydah added a commit to ydah/rubocop-rspec that referenced this pull request Jan 13, 2023
ydah added a commit to ydah/rubocop-rspec that referenced this pull request Jan 13, 2023
@ydah ydah force-pushed the add_option_for_consistent_parentheses_style branch from c103f00 to 82d5cc0 Compare January 13, 2023 13:34
ydah added a commit to ydah/rubocop-rspec that referenced this pull request Jan 13, 2023
@ydah ydah force-pushed the add_option_for_consistent_parentheses_style branch from 82d5cc0 to 2e52c83 Compare January 13, 2023 13:34
ydah added a commit to ydah/rubocop-rspec that referenced this pull request Jan 17, 2023
ydah added a commit to ydah/rubocop-rspec that referenced this pull request Jan 17, 2023
@ydah ydah force-pushed the add_option_for_consistent_parentheses_style branch from 2e52c83 to e25faa0 Compare January 17, 2023 11:17
ydah added a commit to ydah/rubocop-rspec that referenced this pull request Jan 19, 2023
ydah added a commit to ydah/rubocop-rspec that referenced this pull request Jan 19, 2023
@ydah ydah force-pushed the add_option_for_consistent_parentheses_style branch from e25faa0 to 8c4e675 Compare January 19, 2023 10:59
@ydah
Copy link
Member Author

ydah commented Jan 19, 2023

I rebased and resolved the conflict.

@ydah ydah force-pushed the add_option_for_consistent_parentheses_style branch from 8c4e675 to 4676c23 Compare January 20, 2023 06:21
@pirj
Copy link
Member

pirj commented Jan 21, 2023

I'd hold this off until rubocop-factroy_bot extraction.

ydah added a commit to rubocop/rubocop-factory_bot that referenced this pull request May 6, 2023
ydah added a commit to rubocop/rubocop-factory_bot that referenced this pull request May 6, 2023
@ydah
Copy link
Member Author

ydah commented May 6, 2023

@ydah ydah closed this May 6, 2023
ydah added a commit to rubocop/rubocop-factory_bot that referenced this pull request May 7, 2023
ydah added a commit to rubocop/rubocop-factory_bot that referenced this pull request May 7, 2023
pirj pushed a commit to rubocop/rubocop-factory_bot that referenced this pull request May 14, 2023
pirj pushed a commit to rubocop/rubocop-factory_bot that referenced this pull request May 14, 2023
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

Successfully merging this pull request may close these issues.

FactoryBot/FactoryNameStyle false positive on rails generators
3 participants