Skip to content

Commit

Permalink
[Fix #7506] Move LineLength code to mixin and use in IfUnlessModifier
Browse files Browse the repository at this point in the history 
Extract some code that can detect lines that are too long with all the special
cases that are controlled through configuration parameters of the
Metrics/LineLength cop.

The code that is moved to the LineLengthHelp mixin can then be used in
IfUnlessModifier so that it only reports lines on modifier form that are too
long when the LineLength cop would report them.
  • Loading branch information
@jonas054
jonas054 committed Nov 27, 2019
1 parent bcd21f9 commit bfdda17
Show file tree
Hide file tree
Showing 7 changed files with 227 additions and 84 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Expand Up @@ -12,6 +12,7 @@
* [#7515](https://github.com/rubocop-hq/rubocop/issues/7515): Fix a false negative for `Style/RedundantParentheses` when calling a method with safe navigation operator. ([@koic][])
* [#7477](https://github.com/rubocop-hq/rubocop/issues/7477): Fix line length autocorrect for semicolons in string literals. ([@maxh][])
* [#7522](https://github.com/rubocop-hq/rubocop/pull/7522): Fix a false-positive edge case (`n % 2 == 2`) for `Style/EvenOdd`. ([@buehmann][])
* [#7506](https://github.com/rubocop-hq/rubocop/issues/7506): Make `Style/IfUnlessModifier` respect all settings in `Metrics/LineLength`. ([@jonas054][])

### Changes

Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
View file
Expand Up @@ -123,6 +123,7 @@
require_relative 'rubocop/cop/mixin/ignored_methods'
require_relative 'rubocop/cop/mixin/integer_node'
require_relative 'rubocop/cop/mixin/interpolation'
require_relative 'rubocop/cop/mixin/line_length_help'
require_relative 'rubocop/cop/mixin/match_range'
require_relative 'rubocop/cop/mixin/method_complexity'
require_relative 'rubocop/cop/mixin/method_preference'
Expand Down
81 changes: 4 additions & 77 deletions lib/rubocop/cop/metrics/line_length.rb
View file
Expand Up @@ -2,7 +2,6 @@

require 'uri'

# rubocop:disable Metrics/ClassLength
module RuboCop
module Cop
module Metrics
Expand Down Expand Up @@ -58,6 +57,7 @@ class LineLength < Cop
include ConfigurableMax
include IgnoredPattern
include RangeHelp
include LineLengthHelp

MSG = 'Line is too long. [%<length>d/%<max>d]'

Expand Down Expand Up @@ -130,20 +130,6 @@ def heredocs
@heredocs ||= extract_heredocs(processed_source.ast)
end

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

def indentation_difference(line)
return 0 unless tab_indentation_width

line.match(/^\t*/)[0].size * (tab_indentation_width - 1)
end

def line_length(line)
line.length + indentation_difference(line)
end

def highlight_start(line)
max - indentation_difference(line)
end
Expand Down Expand Up @@ -225,51 +211,10 @@ def line_in_permitted_heredoc?(line_number)
end
end

def allow_uri?
cop_config['AllowURI']
end

def ignore_cop_directives?
cop_config['IgnoreCopDirectives']
end

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

def find_excessive_uri_range(line)
last_uri_match = match_uris(line).last
return nil unless last_uri_match

begin_position, end_position =
last_uri_match.offset(0).map do |pos|
pos + indentation_difference(line)
end
return nil if begin_position < max && end_position < max

begin_position...end_position
end

def match_uris(string)
matches = []
string.scan(uri_regexp) do
matches << $LAST_MATCH_INFO if valid_uri?($LAST_MATCH_INFO[0])
def line_in_heredoc?(line_number)
heredocs.any? do |range, _delimiter|
range.cover?(line_number)
end
matches
end

def valid_uri?(uri_ish_string)
URI.parse(uri_ish_string)
true
rescue URI::InvalidURIError, NoMethodError
false
end

def uri_regexp
@uri_regexp ||=
URI::DEFAULT_PARSER.make_regexp(cop_config['URISchemes'])
end

def check_directive_line(line, line_index)
Expand All @@ -287,23 +232,6 @@ def check_directive_line(line, line_index)
)
end

def directive_on_source_line?(line_index)
source_line_number = line_index + processed_source.buffer.first_line
comment =
processed_source
.comments
.detect { |e| e.location.line == source_line_number }

return false unless comment

comment.text.match(CommentConfig::COMMENT_DIRECTIVE_REGEXP)
end

def line_length_without_directive(line)
before_comment, = line.split(CommentConfig::COMMENT_DIRECTIVE_REGEXP)
before_comment.rstrip.length
end

def check_uri_line(line, line_index)
uri_range = find_excessive_uri_range(line)
return if uri_range && allowed_uri_position?(line, uri_range)
Expand All @@ -318,4 +246,3 @@ def check_uri_line(line, line_index)
end
end
end
# rubocop:enable Metrics/ClassLength
88 changes: 88 additions & 0 deletions lib/rubocop/cop/mixin/line_length_help.rb
View file
@@ -0,0 +1,88 @@
# frozen_string_literal: true

module RuboCop
module Cop
# Help methods for determining if a line is too long.
module LineLengthHelp
private

def ignore_cop_directives?
config.for_cop('Metrics/LineLength')['IgnoreCopDirectives']
end

def directive_on_source_line?(line_index)
source_line_number = line_index + processed_source.buffer.first_line
comment =
processed_source.comments
.detect { |e| e.location.line == source_line_number }

return false unless comment

comment.text.match(CommentConfig::COMMENT_DIRECTIVE_REGEXP)
end

def allow_uri?
config.for_cop('Metrics/LineLength')['AllowURI']
end

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

def line_length(line)
line.length + indentation_difference(line)
end

def find_excessive_uri_range(line)
last_uri_match = match_uris(line).last
return nil unless last_uri_match

begin_position, end_position = last_uri_match.offset(0).map do |pos|
pos + indentation_difference(line)
end
return nil if begin_position < max_line_length &&
end_position < max_line_length

begin_position...end_position
end

def match_uris(string)
matches = []
string.scan(uri_regexp) do
matches << $LAST_MATCH_INFO if valid_uri?($LAST_MATCH_INFO[0])
end
matches
end

def indentation_difference(line)
return 0 unless tab_indentation_width

line.match(/^\t*/)[0].size * (tab_indentation_width - 1)
end

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

def uri_regexp
@uri_regexp ||=
URI::DEFAULT_PARSER
.make_regexp(config.for_cop('Metrics/LineLength')['URISchemes'])
end

def valid_uri?(uri_ish_string)
URI.parse(uri_ish_string)
true
rescue URI::InvalidURIError, NoMethodError
false
end

def line_length_without_directive(line)
before_comment, = line.split(CommentConfig::COMMENT_DIRECTIVE_REGEXP)
before_comment.rstrip.length
end
end
end
end
37 changes: 36 additions & 1 deletion lib/rubocop/cop/style/if_unless_modifier.rb
View file
Expand Up @@ -32,6 +32,8 @@ module Style
# end
class IfUnlessModifier < Cop
include StatementModifier
include LineLengthHelp
include IgnoredPattern

MSG_USE_MODIFIER = 'Favor modifier `%<keyword>s` usage when having a ' \
'single-line body. Another good alternative is ' \
Expand Down Expand Up @@ -66,14 +68,47 @@ def autocorrect(node)

private

def ignored_patterns
config.for_cop('Metrics/LineLength')['IgnoredPatterns'] || []
end

def too_long_single_line?(node)
return false unless max_line_length

range = node.source_range
return false unless range.first_line == range.last_line
return false unless line_length_enabled_at_line?(range.first_line)

range.last_column > max_line_length
line = range.source_line
return false if line_length(line) <= max_line_length

too_long_line_based_on_config?(range, line)
end

def too_long_line_based_on_config?(range, line)
return false if matches_ignored_pattern?(line)

too_long = too_long_line_based_on_ignore_cop_directives?(range, line)
return too_long unless too_long == :undetermined

too_long_line_based_on_allow_uri?(line)
end

def too_long_line_based_on_ignore_cop_directives?(range, line)
if ignore_cop_directives? && directive_on_source_line?(range.line - 1)
return line_length_without_directive(line) > max_line_length
end

:undetermined
end

def too_long_line_based_on_allow_uri?(line)
if allow_uri?
uri_range = find_excessive_uri_range(line)
return false if uri_range && allowed_uri_position?(line, uri_range)
end

true
end

def line_length_enabled_at_line?(line)
Expand Down
18 changes: 14 additions & 4 deletions spec/rubocop/cop/metrics/line_length_spec.rb
View file
Expand Up @@ -39,7 +39,17 @@
context 'when AllowURI option is enabled' do
let(:cop_config) { { 'Max' => 80, 'AllowURI' => true } }

context 'and all the excessive characters are part of an URL' do
context 'and the URL fits within the max allowed characters' do
it 'registers an offense for the line' do
expect_offense(<<-RUBY)
# Some documentation comment...
# See: https://github.com/rubocop-hq/rubocop and then words that are not part of a URL
^^^^^^^^^^^^^^^^ Line is too long. [96/80]
RUBY
end
end

context 'and all the excessive characters are part of a URL' do
it 'accepts the line' do
expect_no_offenses(<<-RUBY)
# Some documentation comment...
Expand Down Expand Up @@ -73,7 +83,7 @@
end
end

context 'and the excessive characters include part of an URL ' \
context 'and the excessive characters include part of a URL ' \
'and another word' do
it 'registers an offense for the line' do
expect_offense(<<-RUBY)
Expand All @@ -85,7 +95,7 @@
end

context 'and an error other than URI::InvalidURIError is raised ' \
'while validating an URI-ish string' do
'while validating a URI-ish string' do
let(:cop_config) do
{ 'Max' => 80, 'AllowURI' => true, 'URISchemes' => %w[LDAP] }
end
Expand Down Expand Up @@ -204,7 +214,7 @@ def test_some_other_long_test_description_which_exceeds_length
context 'when AllowURI option is disabled' do
let(:cop_config) { { 'Max' => 80, 'AllowURI' => false } }

context 'and all the excessive characters are part of an URL' do
context 'and all the excessive characters are part of a URL' do
it 'registers an offense for the line' do
expect_offense(<<-RUBY)
# Lorem ipsum dolar sit amet.
Expand Down

0 comments on commit bfdda17

Please sign in to comment.