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

[Fix #7795] Make Layout/EmptyLineAfterGuardClause aware of and return #7796

Conversation

koic
Copy link
Member

@koic koic commented Mar 12, 2020

Resolves #7795.

This PR makes Layout/EmptyLineAfterGuardClause aware of and return.

It was proposed based on the following code controlled by Rails controller.

render :foo and return if condition

do_something

I think that it can be generalized as a control flow.

foo(arg) and return if condition

do_something

So it will be newly detected by Layout/EmptyLineAfterGuardClause cop.


Before submitting the PR make sure the following are checked:

  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.

@koic koic force-pushed the make_empty_line_after_guard_clause_aware_of_and_return branch from 066c001 to 4007253 Compare March 12, 2020 15:44
Copy link
Collaborator

@Drenmi Drenmi left a comment

Choose a reason for hiding this comment

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

I think this is fair. It is a guard clause by our definition of the word.

lib/rubocop/cop/layout/empty_line_after_guard_clause.rb Outdated Show resolved Hide resolved
@koic koic force-pushed the make_empty_line_after_guard_clause_aware_of_and_return branch from 4007253 to e191613 Compare March 13, 2020 05:18
lib/rubocop/ast/node.rb Outdated Show resolved Hide resolved
@koic koic force-pushed the make_empty_line_after_guard_clause_aware_of_and_return branch from 26467b6 to c281156 Compare March 13, 2020 06:21
@koic
Copy link
Member Author

koic commented Mar 13, 2020

@Drenmi Thanks for your kindness review!

@bbatsov
Copy link
Collaborator

bbatsov commented Mar 18, 2020

Shouldn't we also cover usages of or as well?

@koic koic force-pushed the make_empty_line_after_guard_clause_aware_of_and_return branch from c281156 to 3e7cd7e Compare March 18, 2020 10:51
@koic
Copy link
Member Author

koic commented Mar 18, 2020

Sure! I updated this PR to support or.

@bbatsov
Copy link
Collaborator

bbatsov commented Mar 19, 2020

@koic Looks good! Feel free to merge it and the other PRs I just approved. I was a bit too lazy to resolve the merge conflicts in the changelog myself. :-)

@koic
Copy link
Member Author

koic commented Mar 19, 2020

Yeah, I will resolve conflicts and merge. Thank you for the review!

…and return`

Resolves rubocop#7795.

This PR makes `Layout/EmptyLineAfterGuardClause` aware of `and return`.

It was proposed based on the following code controlled by Rails
controller.

```ruby
render :foo and return if condition

do_something
```

I think that it can be generalized as a control flow.

```ruby
foo(arg) and return if condition

do_something
```

So it will be newly detected by `Layout/EmptyLineAfterGuardClause` cop.
@koic koic force-pushed the make_empty_line_after_guard_clause_aware_of_and_return branch from 3e7cd7e to 2e9f9e2 Compare March 19, 2020 10:52
@koic koic merged commit 69bc623 into rubocop:master Mar 19, 2020
@koic koic deleted the make_empty_line_after_guard_clause_aware_of_and_return branch March 19, 2020 11:04
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.

Enforce an empty line after render(:partail) and return
3 participants