[Rails/Blank & Rails/Present cops] new cop configs allowing return unless present?|blank? for multiple unless guard clauses

changelog/change_rails_blank_and_rails_present_cops.md

@@ -0,0 +1 @@
+* [#1327](https://github.com/rubocop/rubocop-rails/pull/1327): [`Rails/Blank` & `Rails/Present` cops] add AllowUnlessPresentAmongMultipleGuardClauses & AllowUnlessBlankAmongMultipleGuardClauses rules. ([@RDeckard][])

config/default.yml

@@ -258,6 +258,7 @@ Rails/Blank:
   NotPresent: true
   # Convert usages of `unless present?` to `if blank?`
   UnlessPresent: true
+  AllowMultipleUnlessPresentGuardClauses: false
 
 Rails/BulkChangeTable:
   Description: 'Check whether alter queries are combinable.'
@@ -792,6 +793,7 @@ Rails/Present:
   NotBlank: true
   # Convert usages of `unless blank?` to `if present?`
   UnlessBlank: true
+  AllowMultipleUnlessBlankGuardClauses: false
 
 Rails/RakeEnvironment:
   Description: 'Include `:environment` as a dependency for all Rake tasks.'

lib/rubocop/cop/rails/blank.rb

@@ -58,6 +58,39 @@ module Rails
       #   def blank?
       #     !present?
       #   end
+      #
+      # @example AllowMultipleUnlessPresentGuardClauses: false (default)
+      #
+      #   # bad
+      #   return unless foo.present?
+      #
+      #   # good
+      #   return if foo.present?
+      #
+      #   # bad
+      #   return unless foo.present?
+      #   return if bar.blank?
+      #
+      #   # bad
+      #   return unless foo.present?
+      #   return unless bar.blank?
+      #
+      # @example AllowMultipleUnlessPresentGuardClauses: true
+      #   # Allow usages of `unless present?` in multiple `unless` guard clauses
+      #
+      #   # bad
+      #   return unless foo.present?
+      #
+      #   # good
+      #   return if foo.present?
+      #
+      #   # bad
+      #   return unless foo.present?
+      #   return if bar.blank?
+      #
+      #   # good
+      #   return unless foo.present?
+      #   return unless bar.blank?
       class Blank < Base
         extend AutoCorrector
 
@@ -123,6 +156,7 @@ def on_or(node)
         def on_if(node)
           return unless cop_config['UnlessPresent']
           return unless node.unless?
+          return if allowed_multiple_unless_guard_clauses?(node)
           return if node.else? && config.for_cop('Style/UnlessElse')['Enabled']
 
           unless_present?(node) do |method_call, receiver|
@@ -151,6 +185,18 @@ def autocorrect(corrector, node)
           corrector.replace(range, replacement(variable1))
         end
 
+        def allowed_multiple_unless_guard_clauses?(node)
+          cop_config['AllowMultipleUnlessPresentGuardClauses'] && (
+            in_an_unless_guard_clause?(node) &&
+              (in_an_unless_guard_clause?(node.right_sibling) || in_an_unless_guard_clause?(node.left_sibling))
+          )
+        end
+
+        def in_an_unless_guard_clause?(node)
+          node.respond_to?(:unless?) && node.unless? &&
+            node.children.any? { _1.respond_to?(:guard_clause?) && _1.guard_clause? }
+        end
+
         def unless_condition(node, method_call)
           if node.modifier_form?
             node.loc.keyword.join(node.source_range.end)

lib/rubocop/cop/rails/present.rb

@@ -43,6 +43,39 @@ module Rails
       #
       #   # good
       #   something if foo.present?
+      #
+      # @example AllowMultipleUnlessBlankGuardClauses: false (default)
+      #
+      #   # bad
+      #   return unless foo.blank?
+      #
+      #   # good
+      #   return if foo.blank?
+      #
+      #   # bad
+      #   return unless foo.blank?
+      #   return if bar.present?
+      #
+      #   # bad
+      #   return unless foo.blank?
+      #   return unless bar.present?
+      #
+      # @example AllowMultipleUnlessBlankGuardClauses: true
+      #   # Allow usages of `unless blank?` in multiple `unless` guard clauses
+      #
+      #   # bad
+      #   return unless foo.blank?
+      #
+      #   # good
+      #   return if foo.blank?
+      #
+      #   # bad
+      #   return unless foo.blank?
+      #   return if bar.present?
+      #
+      #   # good
+      #   return unless foo.blank?
+      #   return unless bar.present?
       class Present < Base
         extend AutoCorrector
 
@@ -110,6 +143,7 @@ def on_or(node)
         def on_if(node)
           return unless cop_config['UnlessBlank']
           return unless node.unless?
+          return if allowed_multiple_unless_guard_clauses?(node)
           return if node.else? && config.for_cop('Style/UnlessElse')['Enabled']
 
           unless_blank?(node) do |method_call, receiver|
@@ -121,6 +155,8 @@ def on_if(node)
           end
         end
 
+        private
+
         def autocorrect(corrector, node)
           method_call, variable1 = unless_blank?(node)
 
@@ -135,7 +171,17 @@ def autocorrect(corrector, node)
           corrector.replace(range, replacement(variable1))
         end
 
-        private
+        def allowed_multiple_unless_guard_clauses?(node)
+          cop_config['AllowMultipleUnlessBlankGuardClauses'] && (
+            in_an_unless_guard_clause?(node) &&
+              (in_an_unless_guard_clause?(node.right_sibling) || in_an_unless_guard_clause?(node.left_sibling))
+          )
+        end
+
+        def in_an_unless_guard_clause?(node)
+          node.respond_to?(:unless?) && node.unless? &&
+            node.children.any? { _1.respond_to?(:guard_clause?) && _1.guard_clause? }
+        end
 
         def unless_condition(node, method_call)
           if node.modifier_form?

spec/rubocop/cop/rails/blank_spec.rb

@@ -186,6 +186,38 @@
           RUBY
         end
       end
+
+      context 'in a guard clause' do
+        context 'with a receiver' do
+          it 'registers an offense and corrects' do
+            expect_offense(<<~RUBY)
+              return unless foo.present?
+                     ^^^^^^^^^^^^^^^^^^^ Use `if foo.blank?` instead of `unless foo.present?`.
+              return unless bar.blank?
+            RUBY
+
+            expect_correction(<<~RUBY)
+              return if foo.blank?
+              return unless bar.blank?
+            RUBY
+          end
+        end
+
+        context 'without a receiver' do
+          it 'registers an offense and corrects' do
+            expect_offense(<<~RUBY)
+              return unless present?
+                     ^^^^^^^^^^^^^^^ Use `if blank?` instead of `unless present?`.
+              return if blank?
+            RUBY
+
+            expect_correction(<<~RUBY)
+              return if blank?
+              return if blank?
+            RUBY
+          end
+        end
+      end
     end
 
     context 'normal unless present?' do
@@ -261,6 +293,186 @@
         end
       end
     end
+
+    context 'AllowMultipleUnlessPresentGuardClauses set to true' do
+      let(:cop_config) do
+        { 'UnlessPresent' => true, 'AllowMultipleUnlessPresentGuardClauses' => true }
+      end
+
+      it 'accepts modifier if present?' do
+        expect_no_offenses('something if foo.present?')
+      end
+
+      it 'accepts modifier unless blank?' do
+        expect_no_offenses('something unless foo.blank?')
+      end
+
+      it 'accepts normal if present?' do
+        expect_no_offenses(<<~RUBY)
+          if foo.present?
+            something
+          end
+        RUBY
+      end
+
+      it 'accepts normal unless blank?' do
+        expect_no_offenses(<<~RUBY)
+          unless foo.blank?
+            something
+          end
+        RUBY
+      end
+
+      it 'accepts elsif present?' do
+        expect_no_offenses(<<~RUBY)
+          if bar.present?
+            something
+          elsif bar.present?
+            something_else
+          end
+        RUBY
+      end
+
+      context 'modifier unless' do
+        context 'with a receiver' do
+          it 'registers an offense and corrects' do
+            expect_offense(<<~RUBY)
+              something unless foo.present?
+                        ^^^^^^^^^^^^^^^^^^^ Use `if foo.blank?` instead of `unless foo.present?`.
+              something_else unless bar.blank?
+            RUBY
+
+            expect_correction(<<~RUBY)
+              something if foo.blank?
+              something_else unless bar.blank?
+            RUBY
+          end
+        end
+
+        context 'without a receiver' do
+          it 'registers an offense and corrects' do
+            expect_offense(<<~RUBY)
+              something unless present?
+                        ^^^^^^^^^^^^^^^ Use `if blank?` instead of `unless present?`.
+              something_else if blank?
+            RUBY
+
+            expect_correction(<<~RUBY)
+              something if blank?
+              something_else if blank?
+            RUBY
+          end
+        end
+
+        context 'in a guard clause' do
+          context 'with multiple `unless` guard clauses' do
+            it 'accepts unless present? in a serie of `unless` guard clauses' do
+              expect_no_offenses(<<~RUBY)
+                return unless foo.present?
+                return unless bar.blank?
+              RUBY
+            end
+          end
+
+          context 'without multiple `unless` guard clauses' do
+            it 'registers an offense and corrects' do
+              expect_offense(<<~RUBY)
+                return unless foo.present?
+                       ^^^^^^^^^^^^^^^^^^^ Use `if foo.blank?` instead of `unless foo.present?`.
+                return if bar.blank?
+              RUBY
+
+              expect_correction(<<~RUBY)
+                return if foo.blank?
+                return if bar.blank?
+              RUBY
+            end
+          end
+        end
+      end
+
+      context 'normal unless present?' do
+        it 'registers an offense and corrects' do
+          expect_offense(<<~RUBY)
+            unless foo.present?
+            ^^^^^^^^^^^^^^^^^^^ Use `if foo.blank?` instead of `unless foo.present?`.
+              something
+            end
+
+            if bar.blank?
+              something_else
+            end
+          RUBY
+
+          expect_correction(<<~RUBY)
+            if foo.blank?
+              something
+            end
+
+            if bar.blank?
+              something_else
+            end
+          RUBY
+        end
+      end
+
+      context 'unless present? with an else' do
+        context 'Style/UnlessElse disabled' do
+          let(:config) do
+            RuboCop::Config.new(
+              'Rails/Blank' => {
+                'UnlessPresent' => true
+              },
+              'Style/UnlessElse' => {
+                'Enabled' => false
+              }
+            )
+          end
+
+          it 'registers an offense and corrects' do
+            expect_offense(<<~RUBY)
+              unless foo.present?
+              ^^^^^^^^^^^^^^^^^^^ Use `if foo.blank?` instead of `unless foo.present?`.
+                something
+              else
+                something_else
+              end
+            RUBY
+
+            expect_correction(<<~RUBY)
+              if foo.blank?
+                something
+              else
+                something_else
+              end
+            RUBY
+          end
+        end
+
+        context 'Style/UnlessElse enabled' do
+          let(:config) do
+            RuboCop::Config.new(
+              'Rails/Blank' => {
+                'UnlessPresent' => true
+              },
+              'Style/UnlessElse' => {
+                'Enabled' => true
+              }
+            )
+          end
+
+          it 'does not register an offense' do
+            expect_no_offenses(<<~RUBY)
+              unless foo.present?
+                something
+              else
+                something_else
+              end
+            RUBY
+          end
+        end
+      end
+    end
   end
 
   context 'NilOrEmpty set to false' do