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.

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

[Fix #4882] Use IndentationWidth of Layout/Tab for other cops #5785

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
* [#5990](https://github.com/bbatsov/rubocop/pull/5990): Drop support for MRI 2.1. ([@drenmi][])
* [#3299](https://github.com/bbatsov/rubocop/issues/3299): `Lint/UselessAccessModifier` now warns when `private_class_method` is used without arguments. ([@Darhazer][])
* [#6026](https://github.com/rubocop-hq/rubocop/pull/6026): Exclude `refine` by default from `Metrics/BlockLength` cop. ([@kddeisz][])
* [#4882](https://github.com/bbatsov/rubocop/issues/4882): Use `IndentationWidth` of `Layout/Tab` for other cops. ([@AlexWayfer][])

## 0.57.2 (2018-06-12)

Expand Down
34 changes: 28 additions & 6 deletions lib/rubocop/cop/metrics/line_length.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ module Cop
module Metrics
# This cop checks the length of lines in the source code.
# The maximum length is configurable.
# The tab size is configured in the `IndentationWidth`
# of the `Layout/Tab` cop.
class LineLength < Cop
include ConfigurableMax
include IgnoredPattern
Expand All @@ -23,8 +25,24 @@ def investigate(processed_source)

private

def tab_indentation_width
config.for_cop('Layout/Tab')['IndentationWidth']
end

def line_length(line)
if tab_indentation_width
line = line.gsub("\t", ' ' * tab_indentation_width)
end
line.length
end

def highligh_start(line)
return max unless tab_indentation_width
max - (tab_indentation_width - 1) * line.count("\t")
end

def check_line(line, index, heredocs)
return if line.length <= max
return if line_length(line) <= max
return if ignored_line?(line, index, heredocs)

if ignore_cop_directives? && directive_on_source_line?(index)
Expand All @@ -33,7 +51,10 @@ def check_line(line, index, heredocs)
return check_uri_line(line, index) if allow_uri?

register_offense(
source_range(processed_source.buffer, index + 1, 0...line.length),
source_range(
processed_source.buffer, index,
highligh_start(line)...line_length(line)
),
line
)
end
Expand All @@ -44,10 +65,10 @@ def ignored_line?(line, index, heredocs)
end

def register_offense(loc, line)
message = format(MSG, length: line.length, max: max)
message = format(MSG, length: line_length(line), max: max)

add_offense(nil, location: loc, message: message) do
self.max = line.length
self.max = line_length(line)
end
end

Expand All @@ -59,7 +80,7 @@ def excess_range(uri_range, line, index)
end

source_range(processed_source.buffer, index + 1,
excessive_position...(line.length))
excessive_position...(line_length(line)))
end

def max
Expand Down Expand Up @@ -100,7 +121,8 @@ def ignore_cop_directives?

def allowed_uri_position?(line, uri_range)
uri_range.begin < max &&
(uri_range.end == line.length || uri_range.end == line.length - 1)
(uri_range.end == line_length(line) ||
uri_range.end == line_length(line) - 1)
end

def find_excessive_uri_range(line)
Expand Down
7 changes: 6 additions & 1 deletion lib/rubocop/cop/mixin/statement_modifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def modifier_fits_on_single_line?(node)
end

def length_in_modifier_form(node, cond, body_length)
indentation = node.loc.keyword.column
indentation = node.loc.keyword.column * indentation_multiplier
kw_length = node.loc.keyword.size
cond_length = cond.source_range.size
space = 1
Expand All @@ -49,6 +49,11 @@ def length_in_modifier_form(node, cond, body_length)
def max_line_length
config.for_cop('Metrics/LineLength')['Max']
end

def indentation_multiplier
return 1 if config.for_cop('Layout/Tab')['Enabled']
config.for_cop('Layout/Tab')['IndentationWidth']
end
end
end
end
3 changes: 2 additions & 1 deletion lib/rubocop/cop/style/if_unless_modifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ module Cop
module Style
# Checks for if and unless statements that would fit on one line
# if written as a modifier if/unless. The maximum line length is
# configured in the `Metrics/LineLength` cop.
# configured in the `Metrics/LineLength` cop. The tab size is configured
# in the `IndentationWidth` of the `Layout/Tab` cop.
#
# @example
# # bad
Expand Down
2 changes: 2 additions & 0 deletions manual/cops_metrics.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ Enabled | No

This cop checks the length of lines in the source code.
The maximum length is configurable.
The tab size is configured in the `IndentationWidth`
of the `Layout/Tab` cop.

### Configurable attributes

Expand Down
3 changes: 2 additions & 1 deletion manual/cops_style.md
Original file line number Diff line number Diff line change
Expand Up @@ -2376,7 +2376,8 @@ Enabled | Yes

Checks for if and unless statements that would fit on one line
if written as a modifier if/unless. The maximum line length is
configured in the `Metrics/LineLength` cop.
configured in the `Metrics/LineLength` cop. The tab size is configured
in the `IndentationWidth` of the `Layout/Tab` cop.

### Examples

Expand Down
43 changes: 43 additions & 0 deletions spec/rubocop/cop/metrics/line_length_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,13 @@ def test_some_other_long_test_description_which_exceeds_length
inspect_source(source)
expect(cop.offenses.size).to eq(1)
end

it 'highlights excessive characters' do
inspect_source(source)
expect(cop.highlights).to eq(
['061837309890f04ab08c']
)
end
end
end

Expand Down Expand Up @@ -334,4 +341,40 @@ def method_definition_that_is_just_under_the_line_length_limit(foo) # rubocop:di
end
end
end

context 'affecting by IndentationWidth from Layout\Tab' do
let(:config) do
RuboCop::Config.new(
'Layout/IndentationWidth' => {
'Width' => 1
},
'Layout/Tab' => {
'Enabled' => false,
'IndentationWidth' => 2
},
'Metrics/LineLength' => {
'Max' => 80
}
)
end

it "registers an offense for a line that's including 1 tab with size 2" \
' and 79 other characters' do
inspect_source("\t" + '#' * 79)
expect(cop.offenses.size).to eq(1)
expect(cop.offenses.first.message).to eq('Line is too long. [81/80]')
expect(cop.config_to_allow_offenses).to eq('Max' => 81)
end

it 'highlights excessive characters' do
inspect_source("\t" + '#' * 78 + 'abc')
expect(cop.highlights).to eq(['abc'])
end

it "accepts a line that's including 1 tab with size 2" \
' and 78 other characters' do
inspect_source("\t" + '#' * 78)
expect(cop.offenses.empty?).to be(true)
end
end
end
58 changes: 58 additions & 0 deletions spec/rubocop/cop/style/if_unless_modifier_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -348,4 +348,62 @@
RUBY
end
end

context 'with disabled Layout/Tab cop' do
let(:config) do
RuboCop::Config.new(
'Layout/IndentationWidth' => {
'Width' => 1
},
'Layout/Tab' => {
'Enabled' => false,
'IndentationWidth' => 2
},
'Metrics/LineLength' => { 'Max' => 80 }
)
end

context 'with tabs indentation' do
let(:source) do
# Empty lines should make no difference.
<<-RUBY
if #{condition}
#{body}
end
RUBY
end

let(:body) { 'b' * 38 }

context 'it fits on one line' do
let(:condition) { 'a' * 26 }

it 'registers an offense' do
# This if statement fits exactly on one line if written as a
# modifier.
expect("#{body} if #{condition}".length).to eq(68)

inspect_source(source)
expect(cop.messages).to eq(
['Favor modifier `if` usage when having a single-line' \
' body. Another good alternative is the usage of control flow' \
' `&&`/`||`.']
)
end
end

context "it doesn't fit on one line" do
let(:condition) { 'a' * 27 }

it "doesn't register an offense" do
# This if statement fits exactly on one line if written as a
# modifier.
expect("#{body} if #{condition}".length).to eq(69)

inspect_source(source)
expect(cop.offenses.empty?).to be(true)
end
end
end
end
end