[Rails/Blank
& Rails/Present
cops] new cop configs allowing return unless
present?
|blank?
for multiple unless
guard clauses
#1327
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposition
TL;DR
Both
Rails/Blank
cop andRails/Present
cop are really useful, but theirUnlessPresent
andUnlessBlank
rules can be frustrating when dealing with multiplereturn unless
guard clauses, so we should have some cop configs allowing thosereturn unless foo.present?
/return unless foo.blank?
ONLY in this very specific case (i.e. in multipleunless
guard clauses).Context
Here the current targeted rules in a guard clause context:
(This is related to the
Rails/Blank
cop here, but that's the same inverted logic forRails/Present
.)Specific case this PR aims to address
Those rules are simple and legit in most cases 👌, but we should be able to disable them specifically on multiple
return unless
guard clauses like in this example:New cop configs
In order to address the case described just above ☝️, this PR propose those two new optional cop configs: (which of course should stay
false
by default to avoid any changed behaviors)Reasoning
Arguments in favor of these two new cop configs:
return unless a_condition
is a very common idiom in guard clauses, as it simply means thata_condition
is required in order to go further.return unless bar.present?
) triggers theUnlessPresent
rule, which suggests changing the code toreturn if bar.blank?
. But IMO:.present?
in some cases, which is not good for explicitness, and therefore a counterproductive incentive.Before submitting the PR make sure the following are checked:
Commit message starts with[Fix #issue-number]
(if the related issue exists).master
(if not - rebase it).bundle exec rake default
. It executes all tests and runs RuboCop on its own code.{change_type}_{change_description}.md
if the new code introduces user-observable changes. See changelog entry format for details.If this is a new cop, consider making a corresponding update to the Rails Style Guide.