Skip to content

Commit

Permalink
[Fix #6566] Fix a false positive for `Layout/EmptyLinesAroundAccessMo…
Browse files Browse the repository at this point in the history
…difier`

Fixes #6566.

This PR fixes a false positive for `Layout/EmptyLinesAroundAccessModifier`
when at the end of specifying a superclass is missing blank line.

The following is originally no offense code.
(It is probably a regression by #6307)

```console
% cat example.rb
class SomeController < SomeOtherController
  def index; end

  private
end

% rubocop example.rb --only Layout/EmptyLinesAroundAccessModifier -a
Inspecting 1 file
C

Offenses:

example.rb:4:3: C: [Corrected] Layout/EmptyLinesAroundAccessModifier:
Keep a blank line before and after private.
  private
  ^^^^^^^

1 file inspected, 1 offense detected, 1 offense corrected
```

Auto-correct adds a blank line after `private`. This is due to false positives.

```diff
 % cat example.rb
 class SomeController < SomeOtherController
   def index; end

   private
+
 end
```

This caused the infinite loop in `Layout/EmptyLinesAroundAccessModifier` and
`Layout/EmptyLinesAroundClassBody`.

```console
% rubocop example.rb --only Layout/EmptyLinesAroundClassBody -a
Inspecting 1 file
C

Offenses:

example.rb:5:1: C: [Corrected] Layout/EmptyLinesAroundClassBody: Extra
empty line detected at class body end.

1 file inspected, 1 offense detected, 1 offense corrected
```

Auto-correct removes the blank line from after of `private`.

```diff
 % cat example.rb
 class SomeController < SomeOtherController
   def index; end

   private
-
 end
```

It loops to the first code. This PR fixes the infinite loop.
  • Loading branch information
koic authored and bbatsov committed Dec 14, 2018
1 parent eb1bd63 commit ab49526
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* [#6554](https://github.com/rubocop-hq/rubocop/issues/6554): Prevent Layout/RescueEnsureAlignment cop from breaking on block assignment when assignment is on a separate line. ([@timmcanty][])
* [#6343](https://github.com/rubocop-hq/rubocop/pull/6343): Optimise `--auto-gen-config` when `Metrics/LineLength` cop is disabled. ([@tom-lord][])
* [#6389](https://github.com/rubocop-hq/rubocop/pull/6389): Fix false negative for `Style/TrailingCommaInHashLitera`/`Style/TrailingCommaInArrayLiteral` when there is a comment in the last line. ([@bayandin][])
* [#6566](https://github.com/rubocop-hq/rubocop/issues/6566): Fix false positive for `Layout/EmptyLinesAroundAccessModifier` when at the end of specifying a superclass is missing blank line. ([@koic][])

## 0.61.1 (2018-12-06)

Expand Down
21 changes: 14 additions & 7 deletions lib/rubocop/cop/layout/empty_lines_around_access_modifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,24 @@ def initialize(config = nil, options = nil)
def on_class(node)
_name, superclass, _body = *node

@class_or_module_def = superclass || node.source_range
@class_or_module_def_first_line = if superclass
superclass.first_line
else
node.source_range.first_line
end
@class_or_module_def_last_line = node.source_range.last_line
end

def on_module(node)
@class_or_module_def = node.source_range
@class_or_module_def_first_line = node.source_range.first_line
@class_or_module_def_last_line = node.source_range.last_line
end

def on_sclass(node)
self_node, _body = *node

@class_or_module_def = self_node.source_range
@class_or_module_def_first_line = self_node.source_range.first_line
@class_or_module_def_last_line = self_node.source_range.last_line
end

def on_block(node)
Expand Down Expand Up @@ -106,9 +113,9 @@ def empty_lines_around?(node)
end

def class_def?(line)
return false unless @class_or_module_def
return false unless @class_or_module_def_first_line

line == @class_or_module_def.first_line + 1
line == @class_or_module_def_first_line + 1
end

def block_start?(line)
Expand All @@ -118,9 +125,9 @@ def block_start?(line)
end

def body_end?(line)
return false unless @class_or_module_def
return false unless @class_or_module_def_last_line

line == @class_or_module_def.last_line - 1
line == @class_or_module_def_last_line - 1
end

def message(node)
Expand Down
11 changes: 11 additions & 0 deletions spec/rubocop/cop/layout/empty_lines_around_access_modifier_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,17 @@ def test; end
RUBY
end

it 'accepts missing blank line when at the end of specifying ' \
'a superclass' do
expect_no_offenses(<<-RUBY.strip_indent)
class Test < Base
def test; end
#{access_modifier}
end
RUBY
end

it 'requires blank line when next line started with end' do
inspect_source(<<-RUBY.strip_indent)
class Test
Expand Down

0 comments on commit ab49526

Please sign in to comment.