Skip to content

Commit

Permalink
formula: allow excluding deprecate! reason when disable! exists
Browse files Browse the repository at this point in the history
  • Loading branch information
cho-m committed Oct 30, 2024
1 parent 0e24ee2 commit c113c8e
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 11 deletions.
12 changes: 5 additions & 7 deletions Library/Homebrew/formula.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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

Expand Down
20 changes: 16 additions & 4 deletions Library/Homebrew/rubocops/deprecate_disable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -72,7 +72,7 @@ def audit_formula(formula_nodes)
end
end

next if reason_found
next unless reason_nodes.empty?

case method
when :deprecate!
Expand All @@ -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
Expand Down
61 changes: 61 additions & 0 deletions Library/Homebrew/test/rubocops/deprecate_disable/reason_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit c113c8e

Please sign in to comment.