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.

Sign up for GitHub

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

Jump to bottom

Add new Style/EmptyLinesAroundExceptionHandlingKeywords cop #3995

Merged
merged 1 commit into from
Feb 5, 2017
Merged

Add new Style/EmptyLinesAroundExceptionHandlingKeywords cop #3995

merged 1 commit into from
Feb 5, 2017

Conversation

pocke
Copy link
Collaborator

@pocke pocke commented Jan 31, 2017 

 # bad
begin
  foo

rescue

  bar
end

 # good
begin
  foo
rescue
  bar
end

See also #3984

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).

@pocke pocke force-pushed the Style/EmptyLinesAroundExceptionHandlingKeywords branch from c9164de to e4c3cd9 Compare January 31, 2017 14:38
bbatsov
bbatsov reviewed Feb 1, 2017
View reviewed changes
lib/rubocop/cop/style/empty_lines_around_exception_handling_keywords.rb Outdated
module RuboCop
module Cop
module Style
# This cops checks if empty lines exist around the bodies of begin
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add backquotes around keyword names.

bbatsov
bbatsov reviewed Feb 1, 2017
View reviewed changes
lib/rubocop/cop/style/empty_lines_around_exception_handling_keywords.rb Outdated
module Cop
module Style
# This cops checks if empty lines exist around the bodies of begin
# sections. This cop doesn't check empty lines at begin body starting
Copy link
Collaborator

Choose a reason for hiding this comment

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

at begin body beginning/end.

bbatsov
bbatsov reviewed Feb 1, 2017
View reviewed changes
lib/rubocop/cop/style/empty_lines_around_exception_handling_keywords.rb Outdated
end

def keyword_locations_in_rescue(node)
_, *resbodies, _ = *node
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd use _something names here. They add more clarity IMO.

bbatsov
bbatsov reviewed Feb 1, 2017
View reviewed changes
spec/rubocop/cop/style/empty_lines_around_exception_handling_keywords_spec.rb Outdated
include_examples :offense,
'rescue section ending for method definition',
<<-CODE, <<-CORRECTION
def foo
Copy link
Collaborator

Choose a reason for hiding this comment

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

I really hate outdented hereblocks. Use the strip_margin method from powerpack to pretty those up a bit.

bbatsov
bbatsov reviewed Feb 1, 2017
View reviewed changes
lib/rubocop/cop/style/empty_lines_around_exception_handling_keywords.rb Outdated
end
end

def style
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this a placeholder for making the cop configurable?

bbatsov
bbatsov reviewed Feb 1, 2017
View reviewed changes
lib/rubocop/cop/style/empty_lines_around_exception_handling_keywords.rb Outdated
line = loc.line
keyword = loc.source
# on the keyword
check_line(style, line, format(MSG, 'on', keyword), &:empty?)
Copy link
Collaborator

Choose a reason for hiding this comment

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

on -> before

bbatsov
bbatsov reviewed Feb 1, 2017
View reviewed changes
lib/rubocop/cop/style/empty_lines_around_exception_handling_keywords.rb Outdated
# on the keyword
check_line(style, line, format(MSG, 'on', keyword), &:empty?)
# under the keyword
check_line(style, line - 2, format(MSG, 'under', keyword), &:empty?)
Copy link
Collaborator

Choose a reason for hiding this comment

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

under -> after

bbatsov
bbatsov reviewed Feb 1, 2017
View reviewed changes
lib/rubocop/cop/style/empty_lines_around_exception_handling_keywords.rb Outdated
include EmptyLinesAroundBody
include OnMethodDef

MSG = 'Extra empty line detected %s the %s.'.freeze
Copy link
Collaborator

Choose a reason for hiding this comment

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

the %s -> %s (in backquotes)

@pocke pocke force-pushed the Style/EmptyLinesAroundExceptionHandlingKeywords branch from e4c3cd9 to 163ce88 Compare February 4, 2017 05:53
@pocke
Copy link
Collaborator Author

pocke commented Feb 4, 2017

I really hate outdented hereblocks. Use the strip_margin method from powerpack to pretty those up a bit.

Thanks for the nice suggestion 👍
strip_indent method is used in the repository instead of strip_margin.
So, I use the method instead. https://github.com/bbatsov/rubocop/search?utf8=%E2%9C%93&q=strip_indent

Is this a placeholder for making the cop configurable?

No, I imitated here. https://github.com/bbatsov/rubocop/blob/ec3123fc3454b080e1100e35480c6466d1240fff/lib/rubocop/cop/style/empty_lines_around_method_body.rb#L35-L37
However, I think we can use the method for making configurable. I'll implement the configuration feature if there is a demand.

I've updated the other suggestions.

@bbatsov
Copy link
Collaborator

bbatsov commented Feb 4, 2017

Looks good but has to be rebased.

@pocke
Add new Style/EmptyLinesAroundExceptionHandlingKeywords cop
6f02896 
```ruby
 # bad
begin
  foo

rescue

  bar
end

 # good
begin
  foo
rescue
  bar
end
```

See also rubocop#3984

Fix
@pocke pocke force-pushed the Style/EmptyLinesAroundExceptionHandlingKeywords branch from 163ce88 to 6f02896 Compare February 5, 2017 05:17
@pocke
Copy link
Collaborator Author

pocke commented Feb 5, 2017

I've rebased. Thanks.

@bbatsov bbatsov merged commit c820b4f into rubocop:master Feb 5, 2017
@pocke pocke deleted the Style/EmptyLinesAroundExceptionHandlingKeywords branch February 5, 2017 12:43
@dependabot dependabot bot mentioned this pull request Mar 12, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bbatsov bbatsov bbatsov left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@pocke @bbatsov