Fix false positive for Style/EmptyLineAfterGuardClause
#5700
Conversation
Related rubocop#5679. This PR fixes a false positive for Style/EmptyLineAfterGuardClause when guard clause is before `else`. ## Reproduction code ```ruby def foo if cond return another_object if something_different? else bar end end ``` ## Expected behavior No offenses. ## Actual behavior and Steps to reproduce the problem ```console % rubocop /tmp/example.rb --only Style/EmptyLineAfterGuardClause Inspecting 1 file C Offenses: /tmp/example.rb:3:5: C: Style/EmptyLineAfterGuardClause: Add empty line after guard clause. return another_object if something_different? ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 1 file inspected, 1 offense detected ``` ## RuboCop version ```console % rubocop -V 0.53.0 (using Parser 2.5.0.4, running on ruby 2.5.0 x86_64-darwin17) ``` This PR fixes the above false positive.
| @@ -73,6 +74,15 @@ def next_line_rescue_or_ensure?(node) | |||
| parent.nil? || parent.rescue_type? || parent.ensure_type? | |||
| end | |||
|
|
|||
| def next_sibling_parent_empty_or_else?(node) | |||
| next_sibling = node.parent.children[node.sibling_index + 1] | |||
| return true if next_sibling.nil? | |||
There was a problem hiding this comment.
Why is this cop Style/EmptyLineAfterGuardClause and not Style/EmptyLinesAroundGuardClause. 😕
There was a problem hiding this comment.
This cop is responsible for checking that there is a blank line after guard clause (not around), so I think that this name is good 😃
cc/ @unkmas
There was a problem hiding this comment.
I'm not saying the name is wrong, I'm saying that only checking after the guard clause seems like only "half a cop." 🙂
There was a problem hiding this comment.
@Drenmi I don't think, that before guard clause always should be a blank line.
def foo
result = SomeAction.peform
return 'WTF' if result.failure?
result.payload
endThis piece of code feels very natural for me.
There was a problem hiding this comment.
@unkmas: That's fine. It should be configurable to everyone's own preference. But by naming the cop like this, I can't add configuration options to it. 🙂 I would need to add another, Style/EmpyLineBeforeGuardClause that duplicates the implementation.
There was a problem hiding this comment.
I see. I understand about the question of Style/EmptyLinesAroundGuardClause.
However, I think that the style of putting a blank line before guard clause feel a bit iffy 💦
In the case of this cop, I think it is important to focus only on after of guard clause.
There was a problem hiding this comment.
Well. There are as many "feelings" about code as there are developers, which is why the vision of RuboCop has shifted towards being fully configurable. 🙂
This PR enables `Layout/EmptyLineAfterGuardClause` cop by default. This cop has been introduced by rubocop#5522, with setting that is disabled by default. After that, I improved this cop by rubocop#5679, rubocop#5700, rubocop#5720, and rubocop#5760. I think that this cop can be enabled by default as the background of the issue rubocop#5376. And this PR applies auto-correction using this cop.
This PR enables `Layout/EmptyLineAfterGuardClause` cop by default. This cop has been introduced by #5522, with setting that is disabled by default. After that, I improved this cop by #5679, #5700, #5720, and #5760. I think that this cop can be enabled by default as the background of the issue #5376. And this PR applies auto-correction using this cop.
Related #5679.
This PR fixes a false positive for Style/EmptyLineAfterGuardClause when guard clause is before
else.Reproduction code
Expected behavior
No offenses.
Actual behavior and Steps to reproduce the problem
RuboCop version
This PR fixes the above false positive.
Before submitting the PR make sure the following are checked:
[Fix #issue-number](if the related issue exists).master(if not - rebase it).and description in grammatically correct, complete sentences.
rake defaultorrake parallel. It executes all tests and RuboCop for itself, and generates the documentation.