[Fix #8283] Fix length calculation for Style/IfUnlessModifier

CHANGELOG.md

@@ -35,6 +35,9 @@
 * [#8422](https://github.com/rubocop-hq/rubocop/issues/8422): Fix an error for `Lint/SelfAssignment` when using or-assignment for constant. ([@koic][])
 * [#8423](https://github.com/rubocop-hq/rubocop/issues/8423): Fix an error for `Style/SingleArgumentDig` when without a receiver. ([@koic][])
 * [#8424](https://github.com/rubocop-hq/rubocop/issues/8424): Fix an error for `Lint/IneffectiveAccessModifier` when there is `begin...end` before a method definition. ([@koic][])
+* [#8006](https://github.com/rubocop-hq/rubocop/issues/8006): Fix line length calculation for `Style/IfUnlessModifier` to correctly take into account code before the if condition when considering conversation to a single-line form. ([@dsavochkin][])
+* [#8283](https://github.com/rubocop-hq/rubocop/issues/8283): Fix line length calculation for `Style/IfUnlessModifier` to correctly take into account a comment on the first line when considering conversation to a single-line form. ([@dsavochkin][])
+* [#8226](https://github.com/rubocop-hq/rubocop/issues/8226): Fix `Style/IfUnlessModifier` to add parentheses when converting if-end condition inside an array or a hash to a single-line form. ([@dsavochkin][])
 
 ### Changes
 
@@ -4736,3 +4739,4 @@
 [@knejad]: https://github.com/knejad
 [@iamravitejag]: https://github.com/iamravitejag
 [@volfgox]: https://github.com/volfgox
+[@dsavochkin]: https://github.com/dmytro-savochkin

lib/rubocop/cop/mixin/statement_modifier.rb

@@ -36,14 +36,41 @@ def non_eligible_condition?(condition)
       def modifier_fits_on_single_line?(node)
         return true unless max_line_length
 
-        length_in_modifier_form(node, node.condition) <= max_line_length
+        length_in_modifier_form(node) <= max_line_length
       end
 
-      def length_in_modifier_form(node, cond)
+      def length_in_modifier_form(node)
         keyword = node.loc.keyword
-        indentation = keyword.source_line[/^\s*/]
-        line_length("#{indentation}#{node.body.source} #{keyword.source} " \
-                    "#{cond.source}")
+        prefix = keyword.source_line[0...keyword.column]
+        expression = to_modifier_form(node)
+        line_length("#{prefix}#{expression}")
+      end
+
+      def to_modifier_form(node)
+        expression = [node.body.source,
+                      node.keyword,
+                      node.condition.source].compact.join(' ')
+        parenthesized = parenthesize?(node) ? "(#{expression})" : expression
+        [parenthesized, first_line_comment(node)].compact.join(' ')
+      end
+
+      def first_line_comment(node)
+        comment =
+          processed_source.find_comment { |c| c.loc.line == node.loc.line }
+
+        comment ? comment.loc.expression.source : nil
+      end
+
+      def parenthesize?(node)
+        # Parenthesize corrected expression if changing to modifier-if form
+        # would change the meaning of the parent expression
+        # (due to the low operator precedence of modifier-if)
+        parent = node.parent
+        return false if parent.nil?
+        return true if parent.assignment? || parent.operator_keyword?
+        return true if %i[array pair].include?(parent.type)
+
+        node.parent.send_type? && !node.parent.parenthesized?
       end
 
       def max_line_length

lib/rubocop/cop/style/if_unless_modifier.rb

@@ -148,26 +148,6 @@ def another_statement_on_same_line?(node)
             sibling.source_range.first_line == line_no
         end
 
-        def parenthesize?(node)
-          # Parenthesize corrected expression if changing to modifier-if form
-          # would change the meaning of the parent expression
-          # (due to the low operator precedence of modifier-if)
-          parent = node.parent
-          return false if parent.nil?
-          return true if parent.assignment? || parent.operator_keyword?
-
-          node.parent.send_type? && !node.parent.parenthesized?
-        end
-
-        def to_modifier_form(node)
-          expression = [node.body.source,
-                        node.keyword,
-                        node.condition.source,
-                        first_line_comment(node)].compact.join(' ')
-
-          parenthesize?(node) ? "(#{expression})" : expression
-        end
-
         def to_normal_form(node)
           indentation = ' ' * node.source_range.column
           <<~RUBY.chomp

spec/rubocop/cop/style/if_unless_modifier_spec.rb

@@ -598,4 +598,146 @@ def f
       RUBY
     end
   end
+
+  context 'when if-end condition is assigned to a variable' do
+    context 'with variable being on the same line' do
+      let(:source) do
+        <<~RUBY
+          x = if a
+            #{'b' * body_length}
+          end
+        RUBY
+      end
+
+      context 'when it is short enough to fit on a single line' do
+        let(:body_length) { 69 }
+
+        it 'corrects it to the single-line form' do
+          corrected = autocorrect_source(source)
+          expect(corrected).to eq "x = (#{'b' * body_length} if a)\n"
+        end
+      end
+
+      context 'when it is not short enough to fit on a single line' do
+        let(:body_length) { 70 }
+
+        it 'accepts it in the multiline form' do
+          expect_no_offenses(source)
+        end
+      end
+    end
+
+    context 'with variable being on the previous line' do
+      let(:source) do
+        <<~RUBY
+          x =
+            if a
+              #{'b' * body_length}
+            end
+        RUBY
+      end
+
+      context 'when it is short enough to fit on a single line' do
+        let(:body_length) { 71 }
+
+        it 'corrects it to the single-line form' do
+          corrected = autocorrect_source(source)
+          expect(corrected).to eq "x =\n  (#{'b' * body_length} if a)\n"
+        end
+      end
+
+      context 'when it is not short enough to fit on a single line' do
+        let(:body_length) { 72 }
+
+        it 'accepts it in the multiline form' do
+          expect_no_offenses(source)
+        end
+      end
+    end
+  end
+
+  context 'when if-end condition is an element of an array' do
+    let(:source) do
+      <<~RUBY
+        [
+          if a
+            #{'b' * body_length}
+          end
+        ]
+      RUBY
+    end
+
+    context 'when short enough to fit on a single line' do
+      let(:body_length) { 71 }
+
+      it 'corrects it to the single-line form' do
+        corrected = autocorrect_source(source)
+        expect(corrected).to eq "[\n  (#{'b' * body_length} if a)\n]\n"
+      end
+    end
+
+    context 'when not short enough to fit on a single line' do
+      let(:body_length) { 72 }
+
+      it 'accepts it in the multiline form' do
+        expect_no_offenses(source)
+      end
+    end
+  end
+
+  context 'when if-end condition is a value in a hash' do
+    let(:source) do
+      <<~RUBY
+        {
+          x: if a
+               #{'b' * body_length}
+             end
+        }
+      RUBY
+    end
+
+    context 'when it is short enough to fit on a single line' do
+      let(:body_length) { 68 }
+
+      it 'corrects it to the single-line form' do
+        corrected = autocorrect_source(source)
+        expect(corrected).to eq "{\n  x: (#{'b' * body_length} if a)\n}\n"
+      end
+    end
+
+    context 'when it is not short enough to fit on a single line' do
+      let(:body_length) { 69 }
+
+      it 'accepts it in the multiline form' do
+        expect_no_offenses(source)
+      end
+    end
+  end
+
+  context 'when if-end condition has a first line comment' do
+    let(:source) do
+      <<~RUBY
+        if foo # #{'c' * comment_length}
+          bar
+        end
+      RUBY
+    end
+
+    context 'when it is short enough to fit on a single line' do
+      let(:comment_length) { 67 }
+
+      it 'corrects it to the single-line form' do
+        corrected = autocorrect_source(source)
+        expect(corrected).to eq "bar if foo # #{'c' * comment_length}\n"
+      end
+    end
+
+    context 'when it is not short enough to fit on a single line' do
+      let(:comment_length) { 68 }
+
+      it 'accepts it in the multiline form' do
+        expect_no_offenses(source)
+      end
+    end
+  end
 end