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

Guard clauses on multi-line statements #3570

Closed
booch opened this Issue Oct 4, 2016 · 5 comments

Comments

Projects
None yet
5 participants
@booch

booch commented Oct 4, 2016

In doing a code review, I came across something like the following:

      foo[:bar] = {
        # Several more lines
      } unless baz.nil?

I was rather surprised that RuboCop didn't flag that. Guard clauses are great, but if the guard clause is so far away from the beginning of the statement, it gets too confusing. This seems like a cop worth adding. (And I suppose there should be a corresponding change to the Ruby style guide.)

@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Oct 4, 2016

Collaborator

Agreed.

Collaborator

bbatsov commented Oct 4, 2016

Agreed.

@palkan

This comment has been minimized.

Show comment
Hide comment
@palkan

palkan Oct 12, 2016

Contributor

if the guard clause is so far away from the beginning of the statement

How many lines are "so far away"? I think, >1. Or should it be configurable?

Contributor

palkan commented Oct 12, 2016

if the guard clause is so far away from the beginning of the statement

How many lines are "so far away"? I think, >1. Or should it be configurable?

@Drenmi

This comment has been minimized.

Show comment
Hide comment
@Drenmi

Drenmi Oct 12, 2016

Collaborator

How many lines are "so far away"? I think, >1. Or should it be configurable?

+1 for "more than one." I think multiline is the most unambiguous and least arbitrary condition.

Collaborator

Drenmi commented Oct 12, 2016

How many lines are "so far away"? I think, >1. Or should it be configurable?

+1 for "more than one." I think multiline is the most unambiguous and least arbitrary condition.

@tessi

This comment has been minimized.

Show comment
Hide comment
@tessi

tessi Oct 15, 2016

Contributor

I'm 👍 for that change and startet with proposing that advise to the styleguide. As I'm new to rubocop, I am not sure yet how to implement this best in rubocop. We'll see if I can manage to follow up with a PR.

Contributor

tessi commented Oct 15, 2016

I'm 👍 for that change and startet with proposing that advise to the styleguide. As I'm new to rubocop, I am not sure yet how to implement this best in rubocop. We'll see if I can manage to follow up with a PR.

tessi added a commit to tessi/rubocop that referenced this issue Oct 17, 2016

tessi added a commit to tessi/rubocop that referenced this issue Oct 17, 2016

tessi added a commit to tessi/rubocop that referenced this issue Oct 17, 2016

@tessi

This comment has been minimized.

Show comment
Hide comment
@tessi

tessi Oct 17, 2016

Contributor

There is a PR now (#3636). I'd be happy for some reviews :)

Contributor

tessi commented Oct 17, 2016

There is a PR now (#3636). I'd be happy for some reviews :)

tessi added a commit to tessi/rubocop that referenced this issue Oct 17, 2016

tessi added a commit to tessi/rubocop that referenced this issue Oct 18, 2016

tessi added a commit to tessi/rubocop that referenced this issue Oct 18, 2016

@bbatsov bbatsov closed this in 5b64541 Oct 24, 2016

bbatsov added a commit that referenced this issue Oct 24, 2016

Merge pull request #3636 from tessi/feature/3570-guard-clauses-on-mul…
…ti-line-statements

[Fix #3570] avoid if-modifiers on multi-line statements

b-t-g pushed a commit to b-t-g/rubocop that referenced this issue Oct 29, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment