diff --git a/CHANGELOG.md b/CHANGELOG.md index ad784be2c36..51d1fc33d5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ * [#7905](https://github.com/rubocop-hq/rubocop/pull/7905): Fix an error when running `rubocop --only` or `rubocop --except` options without cop name argument. ([@koic][]) * [#7903](https://github.com/rubocop-hq/rubocop/pull/7903): Fix an incorrect autocorrect for `Style/HashTransformKeys` and `Style/HashTransformValues` cops when line break before `to_h` method. ([@diogoosorio][], [@koic][]) * [#7899](https://github.com/rubocop-hq/rubocop/issues/7899): Fix an infinite loop error for `Layout/SpaceAroundOperators` with `Layout/ExtraSpacing` when using `ForceEqualSignAlignment: true`. ([@koic][]) +* [#7885](https://github.com/rubocop-hq/rubocop/issues/7885): Fix `Style/IfUnlessModifier` logic when tabs are used for indentation. ([@jonas054][]) ### Changes diff --git a/lib/rubocop/cop/layout/line_length.rb b/lib/rubocop/cop/layout/line_length.rb index 408acbbb726..1c655bdd7e5 100644 --- a/lib/rubocop/cop/layout/line_length.rb +++ b/lib/rubocop/cop/layout/line_length.rb @@ -160,7 +160,10 @@ def heredocs end def highlight_start(line) - max - indentation_difference(line) + # TODO: The max with 0 is a quick fix to avoid crashes when a line + # begins with many tabs, but getting a correct highlighting range + # when tabs are used for indentation doesn't work currently. + [max - indentation_difference(line), 0].max end def check_line(line, line_index) diff --git a/lib/rubocop/cop/mixin/line_length_help.rb b/lib/rubocop/cop/mixin/line_length_help.rb index 293ef38c27d..066f1f2d9d8 100644 --- a/lib/rubocop/cop/mixin/line_length_help.rb +++ b/lib/rubocop/cop/mixin/line_length_help.rb @@ -63,7 +63,8 @@ def indentation_difference(line) end def tab_indentation_width - config.for_cop('Layout/IndentationStyle')['IndentationWidth'] + config.for_cop('Layout/IndentationStyle')['IndentationWidth'] || + config.for_cop('Layout/IndentationWidth')['Width'] end def uri_regexp diff --git a/lib/rubocop/cop/mixin/statement_modifier.rb b/lib/rubocop/cop/mixin/statement_modifier.rb index dff5564c312..88ea52a0334 100644 --- a/lib/rubocop/cop/mixin/statement_modifier.rb +++ b/lib/rubocop/cop/mixin/statement_modifier.rb @@ -4,6 +4,8 @@ module RuboCop module Cop # Common functionality for modifier cops. module StatementModifier + include LineLengthHelp + private def single_line_as_modifier?(node) @@ -34,21 +36,14 @@ def non_eligible_condition?(condition) def modifier_fits_on_single_line?(node) return true unless max_line_length - modifier_length = length_in_modifier_form(node, node.condition, - node.body.source_length) - - modifier_length <= max_line_length + length_in_modifier_form(node, node.condition) <= max_line_length end - def length_in_modifier_form(node, cond, body_length) + def length_in_modifier_form(node, cond) keyword = node.loc.keyword - - indentation = keyword.column * indentation_multiplier - kw_length = keyword.size - cond_length = cond.source_range.size - space = 1 - - indentation + body_length + space + kw_length + space + cond_length + indentation = keyword.source_line[/^\s*/] + line_length("#{indentation}#{node.body.source} #{keyword.source} " \ + "#{cond.source}") end def max_line_length @@ -56,17 +51,6 @@ def max_line_length config.for_cop('Layout/LineLength')['Max'] end - - def indentation_multiplier - return 1 if config.for_cop('Layout/IndentationStyle')['Enabled'] - - default_configuration = RuboCop::ConfigLoader.default_configuration - config.for_cop('Layout/IndentationStyle')['IndentationWidth'] || - config.for_cop('Layout/IndentationWidth')['Width'] || - default_configuration - .for_cop('Layout/IndentationStyle')['IndentationWidth'] || - default_configuration.for_cop('Layout/IndentationWidth')['Width'] - end end end end diff --git a/spec/rubocop/cop/layout/line_length_spec.rb b/spec/rubocop/cop/layout/line_length_spec.rb index 99c1de8d5bd..b35e6cc25a8 100644 --- a/spec/rubocop/cop/layout/line_length_spec.rb +++ b/spec/rubocop/cop/layout/line_length_spec.rb @@ -5,6 +5,15 @@ let(:cop_config) { { 'Max' => 80, 'IgnoredPatterns' => nil } } + let(:config) do + RuboCop::Config.new( + 'Layout/LineLength' => { + 'URISchemes' => %w[http https] + }.merge(cop_config), + 'Layout/IndentationStyle' => { 'IndentationWidth' => 2 } + ) + end + it "registers an offense for a line that's 81 characters wide" do inspect_source('#' * 81) expect(cop.offenses.size).to eq(1) @@ -36,6 +45,21 @@ expect(cop.messages).to eq(['Line is too long. [150/80]']) end + context 'when line is indented with tabs' do + let(:cop_config) { { 'Max' => 10, 'IgnoredPatterns' => nil } } + + it 'accepts a short line' do + expect_no_offenses("\t\t\t123") + end + + it 'registers an offense for a long line' do + expect_offense(<<~RUBY) + \t\t\t\t\t\t\t\t\t\t\t\t1 + ^^^^^^^^^^^^^ Line is too long. [25/10] + RUBY + end + end + context 'when AllowURI option is enabled' do let(:cop_config) { { 'Max' => 80, 'AllowURI' => true } } diff --git a/spec/rubocop/cop/style/if_unless_modifier_spec.rb b/spec/rubocop/cop/style/if_unless_modifier_spec.rb index c3900846ee2..848dbd73554 100644 --- a/spec/rubocop/cop/style/if_unless_modifier_spec.rb +++ b/spec/rubocop/cop/style/if_unless_modifier_spec.rb @@ -517,7 +517,7 @@ def f end end - context 'with disabled Layout/IndentationStyle cop' do + context 'with tabs used for indentation' do shared_examples 'with tabs indentation' do let(:source) do # Empty lines should make no difference. @@ -563,13 +563,7 @@ def f context 'with Layout/IndentationStyle: IndentationWidth config' do let(:config) do RuboCop::Config.new( - 'Layout/IndentationWidth' => { - 'Width' => 1 - }, - 'Layout/IndentationStyle' => { - 'Enabled' => false, - 'IndentationWidth' => 2 - }, + 'Layout/IndentationStyle' => { 'IndentationWidth' => 2 }, 'Layout/LineLength' => { 'Max' => 10 + 12 } # 12 is indentation ) end @@ -580,26 +574,8 @@ def f context 'with Layout/IndentationWidth: Width config' do let(:config) do RuboCop::Config.new( - 'Layout/IndentationWidth' => { - 'Width' => 1 - }, - 'Layout/IndentationStyle' => { - 'Enabled' => false - }, - 'Layout/LineLength' => { 'Max' => 10 + 6 } # 6 is indentation - ) - end - - it_behaves_like 'with tabs indentation' - end - - context 'without any IndentationWidth config' do - let(:config) do - RuboCop::Config.new( - 'Layout/IndentationStyle' => { - 'Enabled' => false - }, - 'Layout/LineLength' => { 'Max' => 10 + 12 } # 12 is indentation + 'Layout/IndentationWidth' => { 'Width' => 3 }, + 'Layout/LineLength' => { 'Max' => 10 + 18 } # 18 is indentation ) end