Merge pull request #7926 from jonas054/7885_IfUnlessModifier_with_tab…

…_indentation

[Fix #7885] Re-use LineLengthHelp for statement modifiers

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
 

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)

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

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,39 +36,21 @@ 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
         return unless config.for_cop('Layout/LineLength')['Enabled']
 
         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

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 } }
 

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