Replace Rails Blank/Present cops

Library/.rubocop.yml

@@ -51,6 +51,11 @@ FormulaAuditStrict:
 Homebrew:
   Enabled: true
 
+Homebrew/Blank:
+  Exclude:
+    # Core extensions are not available here:
+    - "Homebrew/rubocops/**/*"
+
 # only used internally
 Homebrew/MoveToExtendOS:
   Enabled: false
@@ -209,17 +214,10 @@ Rails:
     - "Homebrew/rubocops/**/*"
 
 # These relate to ActiveSupport and not other parts of Rails.
-Rails/Blank:
-  Enabled: true
 Rails/CompactBlank:
   Enabled: true
 Rails/Presence:
   Enabled: true
-Rails/Present:
-  Enabled: true
-  Exclude:
-    # `present?` is defined as `!blank?` wihin this file
-    - "Homebrew/extend/blank.rb"
 Rails/SafeNavigationWithBlank:
   Enabled: true
 
@@ -354,10 +352,11 @@ Style/HashAsLastArrayItem:
 
 Style/InverseMethods:
   Exclude:
-    # Some files are used in contexts without core extensions:
+    # Core extensions are not available here:
     - "Homebrew/standalone/init.rb"
     - "Homebrew/rubocops/**/*"
   InverseMethods:
+    :blank?: :present?
     :exclude?: :include?
 
 Style/InvertibleUnlessCondition:
@@ -369,6 +368,7 @@ Style/InvertibleUnlessCondition:
     :zero?:
     # Don't require non-standard `exclude?` (for now at least) - it's not available in every file
     :include?:
+    :blank?: :present?
 
 Style/MutableConstant:
   # would rather freeze too much than too little

Library/Homebrew/rubocops/all.rb

@@ -2,8 +2,10 @@
 # frozen_string_literal: true
 
 require_relative "../extend/array"
+require_relative "blank"
 require_relative "io_read"
 require_relative "move_to_extend_os"
+require_relative "present"
 require_relative "shell_commands"
 
 # formula audit cops

Library/Homebrew/rubocops/blank.rb

@@ -0,0 +1,74 @@
+# typed: true
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Homebrew
+      # Checks for code that can be written with simpler conditionals
+      # using `Object#blank?`.
+      #
+      # @safety
+      #   This cop is unsafe autocorrection, because `' '.empty?` returns false,
+      #   but `' '.blank?` returns true. Therefore, autocorrection is not compatible
+      #   if the receiver is a non-empty blank string, tab, or newline meta characters.
+      #
+      # @example
+      #   # Converts usages of `nil? || empty?` to `blank?`
+      #
+      #   # bad
+      #   foo.nil? || foo.empty?
+      #   foo == nil || foo.empty?
+      #
+      #   # good
+      #   foo.blank?
+      class Blank < Base
+        extend AutoCorrector
+
+        MSG_NIL_OR_EMPTY = "Use `%<prefer>s` instead of `%<current>s`."
+        RESTRICT_ON_SEND = [:!].freeze
+
+        # `(send nil $_)` is not actually a valid match for an offense. Nodes
+        # that have a single method call on the left hand side
+        # (`bar || foo.empty?`) will blow up when checking
+        # `(send (:nil) :== $_)`.
+        def_node_matcher :nil_or_empty?, <<~PATTERN
+          (or
+              {
+                (send $_ :!)
+                (send $_ :nil?)
+                (send $_ :== nil)
+                (send nil :== $_)
+              }
+              {
+                (send $_ :empty?)
+                (send (send (send $_ :empty?) :!) :!)
+              }
+          )
+        PATTERN
+
+        def on_or(node)
+          nil_or_empty?(node) do |var1, var2|
+            return if var1 != var2
+
+            message = format(MSG_NIL_OR_EMPTY, prefer: replacement(var1), current: node.source)
+            add_offense(node, message: message) do |corrector|
+              autocorrect(corrector, node)
+            end
+          end
+        end
+
+        private
+
+        def autocorrect(corrector, node)
+          variable1, _variable2 = nil_or_empty?(node)
+          range = node.source_range
+          corrector.replace(range, replacement(variable1))
+        end
+
+        def replacement(node)
+          node.respond_to?(:source) ? "#{node.source}.blank?" : "blank?"
+        end
+      end
+    end
+  end
+end

Library/Homebrew/rubocops/present.rb

@@ -0,0 +1,76 @@
+# typed: true
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Homebrew
+      # Checks for code that can be written with simpler conditionals
+      # using `Object#present?`.
+      #
+      # @example
+      #   # Converts usages of `!nil? && !empty?` to `present?`
+      #
+      #   # bad
+      #   !foo.nil? && !foo.empty?
+      #
+      #   # bad
+      #   foo != nil && !foo.empty?
+      #
+      #   # good
+      #   foo.present?
+      class Present < Base
+        extend AutoCorrector
+
+        MSG_EXISTS_AND_NOT_EMPTY = "Use `%<prefer>s` instead of `%<current>s`."
+
+        def_node_matcher :exists_and_not_empty?, <<~PATTERN
+          (and
+              {
+                (send (send $_ :nil?) :!)
+                (send (send $_ :!) :!)
+                (send $_ :!= nil)
+                $_
+              }
+              {
+                (send (send $_ :empty?) :!)
+              }
+          )
+        PATTERN
+
+        def on_and(node)
+          exists_and_not_empty?(node) do |var1, var2|
+            return if var1 != var2
+
+            message = format(MSG_EXISTS_AND_NOT_EMPTY, prefer: replacement(var1), current: node.source)
+
+            add_offense(node, message: message) do |corrector|
+              autocorrect(corrector, node)
+            end
+          end
+        end
+
+        def on_or(node)
+          exists_and_not_empty?(node) do |var1, var2|
+            return if var1 != var2
+
+            add_offense(node, message: MSG_EXISTS_AND_NOT_EMPTY) do |corrector|
+              autocorrect(corrector, node)
+            end
+          end
+        end
+
+        def autocorrect(corrector, node)
+          variable1, _variable2 = exists_and_not_empty?(node)
+          range = node.source_range
+          corrector.replace(range, replacement(variable1))
+        end
+
+        private
+
+        def replacement(node)
+          node.respond_to?(:source) ? "#{node.source}.present?" : "present?"
+        end
+      end
+    end
+  end
+end

Library/Homebrew/startup/bootsnap.rb

@@ -2,7 +2,7 @@
 # frozen_string_literal: true
 
 # Disable Rails cops, as we haven't required active_support yet.
-# rubocop:disable Rails
+# rubocop:disable Rails, Homebrew/Blank
 homebrew_bootsnap_enabled = ENV["HOMEBREW_NO_BOOTSNAP"].nil? && !ENV["HOMEBREW_BOOTSNAP"].nil?
 
 # we need some development tools to build bootsnap native code
@@ -49,4 +49,4 @@
     $stderr.puts "Error: HOMEBREW_BOOTSNAP could not `require \"bootsnap\"`!\n\n"
   end
 end
-# rubocop:enable Rails
+# rubocop:enable Rails, Homebrew/Blank

Library/Homebrew/test/rubocops/blank_spec.rb

@@ -0,0 +1,108 @@
+# frozen_string_literal: true
+
+require "rubocops/blank"
+
+describe RuboCop::Cop::Homebrew::Blank do
+  subject(:cop) { described_class.new }
+
+  shared_examples "offense" do |source, correction, message|
+    it "registers an offense and corrects" do
+      expect_offense(<<~RUBY, source: source, message: message)
+        #{source}
+        ^{source} #{message}
+      RUBY
+
+      expect_correction(<<~RUBY)
+        #{correction}
+      RUBY
+    end
+  end
+
+  it "accepts checking nil?" do
+    expect_no_offenses("foo.nil?")
+  end
+
+  it "accepts checking empty?" do
+    expect_no_offenses("foo.empty?")
+  end
+
+  it "accepts checking nil? || empty? on different objects" do
+    expect_no_offenses("foo.nil? || bar.empty?")
+  end
+
+  # Bug: https://github.com/rubocop/rubocop/issues/4171
+  it "does not break when RHS of `or` is a naked falsiness check" do
+    expect_no_offenses("foo.empty? || bar")
+  end
+
+  it "does not break when LHS of `or` is a naked falsiness check" do
+    expect_no_offenses("bar || foo.empty?")
+  end
+
+  # Bug: https://github.com/rubocop/rubocop/issues/4814
+  it "does not break when LHS of `or` is a send node with an argument" do
+    expect_no_offenses("x(1) || something")
+  end
+
+  context "when nil or empty" do
+    it_behaves_like "offense", "foo.nil? || foo.empty?",
+                    "foo.blank?",
+                    "Homebrew/Blank: Use `foo.blank?` instead of `foo.nil? || foo.empty?`."
+    it_behaves_like "offense", "nil? || empty?", "blank?", "Homebrew/Blank: Use `blank?` instead of `nil? || empty?`."
+    it_behaves_like "offense", "foo == nil || foo.empty?",
+                    "foo.blank?",
+                    "Homebrew/Blank: Use `foo.blank?` instead of `foo == nil || foo.empty?`."
+    it_behaves_like "offense", "nil == foo || foo.empty?",
+                    "foo.blank?",
+                    "Homebrew/Blank: Use `foo.blank?` instead of `nil == foo || foo.empty?`."
+    it_behaves_like "offense", "!foo || foo.empty?", "foo.blank?",
+                    "Homebrew/Blank: Use `foo.blank?` instead of `!foo || foo.empty?`."
+
+    it_behaves_like "offense", "foo.nil? || !!foo.empty?",
+                    "foo.blank?",
+                    "Homebrew/Blank: Use `foo.blank?` instead of `foo.nil? || !!foo.empty?`."
+    it_behaves_like "offense", "foo == nil || !!foo.empty?",
+                    "foo.blank?",
+                    "Homebrew/Blank: Use `foo.blank?` instead of " \
+                    "`foo == nil || !!foo.empty?`."
+    it_behaves_like "offense", "nil == foo || !!foo.empty?",
+                    "foo.blank?",
+                    "Homebrew/Blank: Use `foo.blank?` instead of " \
+                    "`nil == foo || !!foo.empty?`."
+  end
+
+  context "when checking all variable types" do
+    it_behaves_like "offense", "foo.bar.nil? || foo.bar.empty?",
+                    "foo.bar.blank?",
+                    "Homebrew/Blank: Use `foo.bar.blank?` instead of " \
+                    "`foo.bar.nil? || foo.bar.empty?`."
+    it_behaves_like "offense", "FOO.nil? || FOO.empty?",
+                    "FOO.blank?",
+                    "Homebrew/Blank: Use `FOO.blank?` instead of `FOO.nil? || FOO.empty?`."
+    it_behaves_like "offense", "Foo.nil? || Foo.empty?",
+                    "Foo.blank?",
+                    "Homebrew/Blank: Use `Foo.blank?` instead of `Foo.nil? || Foo.empty?`."
+    it_behaves_like "offense", "Foo::Bar.nil? || Foo::Bar.empty?",
+                    "Foo::Bar.blank?",
+                    "Homebrew/Blank: Use `Foo::Bar.blank?` instead of " \
+                    "`Foo::Bar.nil? || Foo::Bar.empty?`."
+    it_behaves_like "offense", "@foo.nil? || @foo.empty?",
+                    "@foo.blank?",
+                    "Homebrew/Blank: Use `@foo.blank?` instead of `@foo.nil? || @foo.empty?`."
+    it_behaves_like "offense", "$foo.nil? || $foo.empty?",
+                    "$foo.blank?",
+                    "Homebrew/Blank: Use `$foo.blank?` instead of `$foo.nil? || $foo.empty?`."
+    it_behaves_like "offense", "@@foo.nil? || @@foo.empty?",
+                    "@@foo.blank?",
+                    "Homebrew/Blank: Use `@@foo.blank?` instead of " \
+                    "`@@foo.nil? || @@foo.empty?`."
+    it_behaves_like "offense", "foo[bar].nil? || foo[bar].empty?",
+                    "foo[bar].blank?",
+                    "Homebrew/Blank: Use `foo[bar].blank?` instead of " \
+                    "`foo[bar].nil? || foo[bar].empty?`."
+    it_behaves_like "offense", "foo(bar).nil? || foo(bar).empty?",
+                    "foo(bar).blank?",
+                    "Homebrew/Blank: Use `foo(bar).blank?` instead of " \
+                    "`foo(bar).nil? || foo(bar).empty?`."
+  end
+end