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 new `Style/EmptyMethod` cop #3720

Merged
merged 1 commit into from Nov 14, 2016

Conversation

Projects
None yet
3 participants
@Drenmi
Collaborator

Drenmi commented Nov 14, 2016

This cop checks for the style of empty method definitions. By default it enforces a compact style, i.e.:

# EnforcedStyle: compact
def foo; end

but it can be configured to enforce a loose style as well, i.e.:

# EnforcedStyle: expanded
def foo
end

The cop comes with auto-correct.


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).
  • Used the same coding conventions as the rest of the project.
  • 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.
  • All tests are passing.
  • The new code doesn't generate RuboCop offenses.
  • The PR relates to only one subject with a clear title
    and description in grammatically correct, complete sentences.
  • Updated cop documentation with rake generate_cops_documentation (required only when you've added a new cop or changed the configuration/documentation of an existing cop).
Show outdated Hide outdated config/default.yml
EnforcedStyle: compact
SupportedStyles:
- compact
- loose

This comment has been minimized.

@bbatsov

bbatsov Nov 14, 2016

Collaborator

I think I prefer "expanded" or something like this.

@bbatsov

bbatsov Nov 14, 2016

Collaborator

I think I prefer "expanded" or something like this.

This comment has been minimized.

@Drenmi

Drenmi Nov 14, 2016

Collaborator

Fixed. 😀

@Drenmi

Drenmi Nov 14, 2016

Collaborator

Fixed. 😀

@@ -32,13 +32,11 @@ def autocorrect(node)
# Dummy implementation of method in ConfigurableEnforcedStyle that is
# called from StringHelp.
def opposite_style_detected
end
def opposite_style_detected; end

This comment has been minimized.

@bbatsov

bbatsov Nov 14, 2016

Collaborator

By the way ; is not really needed here. Maybe we should have two styles for enforcing its presence or absence.

@bbatsov

bbatsov Nov 14, 2016

Collaborator

By the way ; is not really needed here. Maybe we should have two styles for enforcing its presence or absence.

This comment has been minimized.

@Drenmi

Drenmi Nov 14, 2016

Collaborator

Hm. I think you might be confusing the first ; in the definition and the (optional) second one when there's a method body, i.e.:

def foo; bar end

Defining without the ; will raise:

SyntaxError: (irb):1: syntax error, unexpected keyword_end, expecting ';' or '\n'

😊

@Drenmi

Drenmi Nov 14, 2016

Collaborator

Hm. I think you might be confusing the first ; in the definition and the (optional) second one when there's a method body, i.e.:

def foo; bar end

Defining without the ; will raise:

SyntaxError: (irb):1: syntax error, unexpected keyword_end, expecting ';' or '\n'

😊

This comment has been minimized.

@bbatsov

bbatsov Nov 14, 2016

Collaborator

Guess I am. :-)

@bbatsov

bbatsov Nov 14, 2016

Collaborator

Guess I am. :-)

Add new `Style/EmptyMethod` cop
This cop checks for the style of empty method definitions. By default it
enforces a compact style, i.e.:

```
def foo; end
```

but it can be configured to enforce a expanded style as well, i.e.:

```
def foo
end
```

The cop comes with auto-correct.

@bbatsov bbatsov merged commit 26b5b9c into rubocop-hq:master Nov 14, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@lucascaton

This comment has been minimized.

Show comment
Hide comment
@lucascaton

lucascaton Dec 4, 2016

I'm pretty sure most developers will agree it should defaults to expanded.
Also: https://github.com/bbatsov/ruby-style-guide#no-single-line-methods.

Cheers.

I'm pretty sure most developers will agree it should defaults to expanded.
Also: https://github.com/bbatsov/ruby-style-guide#no-single-line-methods.

Cheers.

@Drenmi

This comment has been minimized.

Show comment
Hide comment
@Drenmi

Drenmi Dec 8, 2016

Collaborator

@lucascaton: The best way to test is to open up an issue or a PR in the style guide repository to see if people agree. 😀

For now, the section you linked (and that is also linked in the cop itself) states:

One exception to the rule are empty-body methods.

# good
def no_op; end
Collaborator

Drenmi commented Dec 8, 2016

@lucascaton: The best way to test is to open up an issue or a PR in the style guide repository to see if people agree. 😀

For now, the section you linked (and that is also linked in the cop itself) states:

One exception to the rule are empty-body methods.

# good
def no_op; end

@Drenmi Drenmi deleted the Drenmi:feature/empty-method-cop branch Dec 29, 2016

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