Add new Style/EmptyLinesAroundBeginBody cop
#3984
Conversation
pocke
force-pushed
the
empty_lines_around_begin_block
branch
from
2b96b47
to
e341e6c
Compare
Example ------ ```ruby # good begin foo end # bad begin foo end ``` Note -------- This cop does not detect the following code. ```ruby begin foo # Empty line is here! rescue bar end begin foo rescue # Empty line is here! bar end begin foo # Empty line is here! ensure bar end ``` There are two reasons for this. First, the feature can't be implemented by `check` method that is defined by `EmptyLinesAroundBody` mixin. Second, I think the feature should be implemented as another cop(e.g. `Style/EmptyLinesAroundBeginSection`). I would like to check empty lines around begin-end block body always. However, I add an empty line around `ensure`, `rescue`, `else` keywords sometimes. ```ruby begin foo bar baz rescue something end ``` So, the features should be separateness to disable the one only. I'll implement the cop for empty lines around the keywords with another PR.
pocke
force-pushed
the
empty_lines_around_begin_block
branch
from
c8f19b2
to
a12eff9
Compare
| @@ -223,6 +223,10 @@ Style/EmptyLinesAroundAccessModifier: | |||
| Description: "Keep blank lines around access modifiers." | |||
| Enabled: true | |||
|
|
|||
| Style/EmptyLinesAroundBeginBody: | |||
There was a problem hiding this comment.
Don't we have a style guide rule for this?
There was a problem hiding this comment.
No, we don't have the style guide. And we also don't have style guides for other EmptyLines* cops.
I'll add the documentation.
| class EmptyLinesAroundBeginBody < Cop | ||
| include EmptyLinesAroundBody | ||
|
|
||
| KIND = 'begin'.freeze |
There was a problem hiding this comment.
This looks a bit confusing. Maybe we should turn it into a method instead?
There was a problem hiding this comment.
Why are you confusing? I think it is good to keep constant assignment.
Note: The begin keyword should be surrounded by backtick. :) I'll fix it.
| shared_examples :offense do |name, code, correction| | ||
| it "registers an offense for #{name} with a blank" do | ||
| inspect_source(cop, code) | ||
| expect(cop.offenses.size).to eq(1) |
There was a problem hiding this comment.
I don't see a check for the message generated by the cop.
There was a problem hiding this comment.
I've made to check it 👍
| end | ||
|
|
||
| include_examples :offense, 'begin body starting', <<-CODE, <<-CORRECTION | ||
| begin |
There was a problem hiding this comment.
Same remark as from your other PR - use strip_margin so we can pretty up the heredocs.
There was a problem hiding this comment.
We should probably add some cop for newer Rubies and a rule in the style guide to use the new heredocs syntax - it's so much nicer...
```ruby # bad begin foo rescue bar end # good begin foo rescue bar end ``` See also rubocop#3984 Fix
```ruby # bad begin foo rescue bar end # good begin foo rescue bar end ``` See also #3984 Fix
Example
Note
This cop does not detect the following code.
There are two reasons for this.
First, the feature can't be implemented by
checkmethod that is defined byEmptyLinesAroundBodymixin.Second, I think the feature should be implemented as another cop(e.g.
Style/EmptyLinesAroundBeginSection).I would like to check empty lines around begin-end block body always.
However, I add an empty line around
ensure,rescue,elsekeywords sometimes.So, the features should be separateness to disable the one only.
I'll implement the cop for empty lines around the keywords with another PR.
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 generate_cops_documentation(required only when you've added a new cop or changed the configuration/documentation of an existing cop).