formula: allow excluding deprecate! reason when disable! exists

Author: cho-m

Library/Homebrew/formula.rb

@@ -4237,12 +4237,10 @@ def pour_bottle?(only_if: nil, &block)
     # @see https://docs.brew.sh/Deprecating-Disabling-and-Removing-Formulae
     # @see DeprecateDisable::FORMULA_DEPRECATE_DISABLE_REASONS
     # @api public
-    def deprecate!(date:, because:)
+    def deprecate!(date:, because: nil)
       @deprecation_date = Date.parse(date)
-      return if @deprecation_date > Date.today
-
-      @deprecation_reason = because
-      @deprecated = true
+      @deprecated = !disabled? && @deprecation_date <= Date.today
+      @deprecation_reason = because if because
     end
 
     # Whether this {Formula} is deprecated (i.e. warns on installation).
@@ -4288,8 +4286,8 @@ def disable!(date:, because:)
       @disable_date = Date.parse(date)
 
       if @disable_date > Date.today
-        @deprecation_reason = because
-        @deprecated = true
+        @deprecation_reason ||= because
+        @deprecated = true if deprecation_date.nil?
         return
       end
 

Library/Homebrew/rubocops/deprecate_disable.rb

@@ -45,15 +45,15 @@ class DeprecateDisableReason < FormulaCop
         sig { override.params(formula_nodes: FormulaNodes).void }
         def audit_formula(formula_nodes)
           body_node = formula_nodes.body_node
+          reason_nodes = T.let([], T::Array[T.any(AST::StrNode, AST::SymbolNode)])
 
-          [:deprecate!, :disable!].each do |method|
+          [:disable!, :deprecate!].each do |method|
             node = find_node_method_by_name(body_node, method)
 
             next if node.nil?
 
-            reason_found = T.let(false, T::Boolean)
             reason(node) do |reason_node|
-              reason_found = true
+              reason_nodes << reason_node
               next if reason_node.sym_type?
 
               offending_node(reason_node)
@@ -72,7 +72,7 @@ def audit_formula(formula_nodes)
               end
             end
 
-            next if reason_found
+            next unless reason_nodes.empty?
 
             case method
             when :deprecate!
@@ -81,6 +81,18 @@ def audit_formula(formula_nodes)
               problem 'Add a reason for disabling: `disable! because: "..."`'
             end
           end
+
+          return if reason_nodes.length < 2
+
+          disable_reason_node = T.must(reason_nodes.first)
+          deprecate_reason_node = T.must(reason_nodes.second)
+          return if disable_reason_node.value != deprecate_reason_node.value
+
+          offending_node(deprecate_reason_node.parent)
+          problem "Remove deprecate reason when disable reason is identical" do |corrector|
+            range = deprecate_reason_node.parent.source_range
+            corrector.remove(range_with_surrounding_comma(range_with_surrounding_space(range:, side: :left)))
+          end
         end
 
         def_node_search :reason, <<~EOS

Library/Homebrew/test/rubocops/deprecate_disable/reason_spec.rb

@@ -324,4 +324,65 @@ class Foo < Formula
       RUBY
     end
   end
+
+  context "when auditing `deprecate!` and `disable!`" do
+    it "reports no offense if deprecate `reason` is absent" do
+      expect_no_offenses(<<~RUBY)
+        class Foo < Formula
+          url 'https://brew.sh/foo-1.0.tgz'
+          disable! date: "2021-08-28", because: :does_not_build
+          deprecate! date: "2020-08-28"
+        end
+      RUBY
+    end
+
+    it "reports offense if disable `reason` is absent`" do
+      expect_offense(<<~RUBY)
+        class Foo < Formula
+          url 'https://brew.sh/foo-1.0.tgz'
+          disable! date: "2021-08-28"
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^ FormulaAudit/DeprecateDisableReason: Add a reason for disabling: `disable! because: "..."`
+          deprecate! date: "2020-08-28", because: :does_not_build
+        end
+      RUBY
+    end
+
+    it "reports and corrects an offense if disable and deprecate `reason` are identical symbols" do
+      expect_offense(<<~RUBY)
+        class Foo < Formula
+          url 'https://brew.sh/foo-1.0.tgz'
+          disable! date: "2021-08-28", because: :does_not_build
+          deprecate! date: "2020-08-28", because: :does_not_build
+                                         ^^^^^^^^^^^^^^^^^^^^^^^^ FormulaAudit/DeprecateDisableReason: Remove deprecate reason when disable reason is identical
+        end
+      RUBY
+
+      expect_correction(<<~RUBY)
+        class Foo < Formula
+          url 'https://brew.sh/foo-1.0.tgz'
+          disable! date: "2021-08-28", because: :does_not_build
+          deprecate! date: "2020-08-28"
+        end
+      RUBY
+    end
+
+    it "reports and corrects an offense if disable and deprecate `reason` are identical strings" do
+      expect_offense(<<~RUBY)
+        class Foo < Formula
+          url 'https://brew.sh/foo-1.0.tgz'
+          disable! date: "2021-08-28", because: "is broken"
+          deprecate! date: "2020-08-28", because: "is broken"
+                                         ^^^^^^^^^^^^^^^^^^^^ FormulaAudit/DeprecateDisableReason: Remove deprecate reason when disable reason is identical
+        end
+      RUBY
+
+      expect_correction(<<~RUBY)
+        class Foo < Formula
+          url 'https://brew.sh/foo-1.0.tgz'
+          disable! date: "2021-08-28", because: "is broken"
+          deprecate! date: "2020-08-28"
+        end
+      RUBY
+    end
+  end
 end