Skip to content

Commit

Permalink
[Fix rubocop#5380] Fix false negative in Layout/IndentationWidth cop (
Browse files Browse the repository at this point in the history
rubocop#5962)

Fix `Layout/IndentationWidth` not detecting invalid indentation of methods
in an access modifier section.
  • Loading branch information
tatsuyafw authored and bbatsov committed Jun 6, 2018
1 parent a1f335c commit 8fb4166
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 8 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Bug fixes

* [#5917](https://github.com/bbatsov/rubocop/issues/5917): Fix erroneous warning for `inherit_mode` directive. ([@jonas054][])
* [#5380](https://github.com/rubocop-hq/rubocop/issues/5380): Fix false negative in `Layout/IndentationWidth` when an access modifier section has an invalid indentation body. ([@tatsuyafw][])
* [#5909](https://github.com/rubocop-hq/rubocop/pull/5909): Even when a module has no public methods, `Layout/IndentationConsistency` should still register an offense for private methods. ([@jaredbeck][])
* [#5958](https://github.com/rubocop-hq/rubocop/issues/5958): Handle empty method body in `Rails/BulkChangeTable`. ([@wata727][])
* [#5954](https://github.com/bbatsov/rubocop/issues/5954): Make `Style/UnneededCondition` cop accepts a case of condition and `if_branch` are same when using `elsif` branch. ([@koic][])
Expand Down Expand Up @@ -3409,3 +3410,4 @@
[@EiNSTeiN-]: https://github.com/EiNSTeiN-
[@nroman-stripe]: https://github.com/nroman-stripe
[@sunny]: https://github.com/sunny
[@tatsuyafw]: https://github.com/tatsuyafw
14 changes: 10 additions & 4 deletions lib/rubocop/cop/layout/indentation_width.rb
Original file line number Diff line number Diff line change
Expand Up @@ -152,11 +152,17 @@ def check_members(base, members)
check_indentation(base, members.first)

return unless members.any? && members.first.begin_type?
return unless indentation_consistency_style == 'rails'

each_member(members) do |member, previous_modifier|
check_indentation(previous_modifier, member,
indentation_consistency_style)
if indentation_consistency_style == 'rails'
each_member(members) do |member, previous_modifier|
check_indentation(previous_modifier, member,
indentation_consistency_style)
end
else
members.first.children.each do |member|
next if member.send_type? && member.access_modifier?
check_indentation(base, member)
end
end
end

Expand Down
30 changes: 26 additions & 4 deletions spec/fixtures/html_formatter/expected.html
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ <h1 class="title">RuboCop Inspection Report</h1>
<div class="infobox">
<div class="total">
3 files inspected,
19 offenses detected:
21 offenses detected:
</div>
<ul class="offenses-list">

Expand All @@ -381,7 +381,7 @@ <h1 class="title">RuboCop Inspection Report</h1>

<li>
<a href="#offense_app/controllers/books_controller.rb">
app/controllers/books_controller.rb - 11 offenses
app/controllers/books_controller.rb - 13 offenses
</a>
</li>

Expand Down Expand Up @@ -432,7 +432,7 @@ <h1 class="title">RuboCop Inspection Report</h1>

<div class="offense-box" id="offense_app/controllers/books_controller.rb">
<div class="box-title-placeholder"><h3>&nbsp;</h3></div>
<div class="box-title"><h3>app/controllers/books_controller.rb - 11 offenses</h3></div>
<div class="box-title"><h3>app/controllers/books_controller.rb - 13 offenses</h3></div>
<div class="offense-reports">

<div class="report">
Expand Down Expand Up @@ -523,6 +523,17 @@ <h1 class="title">RuboCop Inspection Report</h1>

</div>

<div class="report">
<div class="meta">
<span class="location">Line #66</span>
<span class="severity convention">convention:</span>
<span class="message">Layout/IndentationWidth: Use 2 (not 4) spaces for indentation.</span>
</div>

<pre><code><span class="highlight convention"> </span>def set_book</code></pre>

</div>

<div class="report">
<div class="meta">
<span class="location">Line #66</span>
Expand All @@ -545,6 +556,17 @@ <h1 class="title">RuboCop Inspection Report</h1>

</div>

<div class="report">
<div class="meta">
<span class="location">Line #71</span>
<span class="severity convention">convention:</span>
<span class="message">Layout/IndentationWidth: Use 2 (not 4) spaces for indentation.</span>
</div>

<pre><code><span class="highlight convention"> </span>def book_params</code></pre>

</div>

<div class="report">
<div class="meta">
<span class="location">Line #71</span>
Expand Down Expand Up @@ -639,7 +661,7 @@ <h1 class="title">RuboCop Inspection Report</h1>
</div>
<footer>
Generated by <a href="https://github.com/rubocop-hq/rubocop">RuboCop</a>
<span class="version">0.51.0</span>
<span class="version">0.57.0</span>
</footer>
</body>
</html>
39 changes: 39 additions & 0 deletions spec/rubocop/cop/layout/indentation_width_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -960,6 +960,23 @@ class Test
RUBY
end

context 'with access modifier' do
it 'registers an offense for bad indentation of sections' do
expect_offense(<<-RUBY.strip_indent)
class Test
public
def e
^^^^ Use 2 (not 4) spaces for indentation.
end
def f
^^^^ Use 2 (not 4) spaces for indentation.
end
end
RUBY
end
end

context 'when consistency style is normal' do
it 'accepts indented public, protected, and private' do
expect_no_offenses(<<-RUBY.strip_indent)
Expand All @@ -981,6 +998,28 @@ def g
end
RUBY
end

it 'registers offenses for rails indentation' do
inspect_source(<<-RUBY.strip_indent)
class Test
def e
end
protected
def f
end
private
def g
end
end
RUBY
expect(cop.messages)
.to eq(['Use 2 (not 4) spaces for indentation.'] * 2)
expect(cop.offenses.map(&:line)).to eq([7, 12])
end
end

context 'when consistency style is rails' do
Expand Down

0 comments on commit 8fb4166

Please sign in to comment.