From 4fa1355be0bbf0de469925fce9aeff9ce841cd82 Mon Sep 17 00:00:00 2001 From: botantony Date: Mon, 14 Apr 2025 13:46:05 +0200 Subject: [PATCH] deprecate!/disable!: remove non-typed `replacement` field Signed-off-by: botantony Co-authored-by: Mike McQuaid --- Library/Homebrew/cask/cask.rb | 70 ++++++++++--------- Library/Homebrew/cask/dsl.rb | 30 ++++---- Library/Homebrew/deprecate_disable.rb | 15 ++-- Library/Homebrew/formula.rb | 53 +++----------- Library/Homebrew/sorbet/rbi/dsl/cask/cask.rbi | 6 -- Library/Homebrew/sorbet/rbi/dsl/formula.rbi | 6 -- .../Homebrew/test/deprecate_disable_spec.rb | 57 ++++----------- .../support/fixtures/cask/everything.json | 6 +- 8 files changed, 81 insertions(+), 162 deletions(-) diff --git a/Library/Homebrew/cask/cask.rb b/Library/Homebrew/cask/cask.rb index b57147a8b7..a48af3acb2 100644 --- a/Library/Homebrew/cask/cask.rb +++ b/Library/Homebrew/cask/cask.rb @@ -361,40 +361,42 @@ module Cask def to_h { - "token" => token, - "full_token" => full_name, - "old_tokens" => old_tokens, - "tap" => tap&.name, - "name" => name, - "desc" => desc, - "homepage" => homepage, - "url" => url, - "url_specs" => url_specs, - "version" => version, - "installed" => installed_version, - "installed_time" => install_time&.to_i, - "bundle_version" => bundle_long_version, - "bundle_short_version" => bundle_short_version, - "outdated" => outdated?, - "sha256" => sha256, - "artifacts" => artifacts_list, - "caveats" => (Tty.strip_ansi(caveats) unless caveats.empty?), - "depends_on" => depends_on, - "conflicts_with" => conflicts_with, - "container" => container&.pairs, - "auto_updates" => auto_updates, - "deprecated" => deprecated?, - "deprecation_date" => deprecation_date, - "deprecation_reason" => deprecation_reason, - "deprecation_replacement" => deprecation_replacement, - "disabled" => disabled?, - "disable_date" => disable_date, - "disable_reason" => disable_reason, - "disable_replacement" => disable_replacement, - "tap_git_head" => tap_git_head, - "languages" => languages, - "ruby_source_path" => ruby_source_path, - "ruby_source_checksum" => ruby_source_checksum, + "token" => token, + "full_token" => full_name, + "old_tokens" => old_tokens, + "tap" => tap&.name, + "name" => name, + "desc" => desc, + "homepage" => homepage, + "url" => url, + "url_specs" => url_specs, + "version" => version, + "installed" => installed_version, + "installed_time" => install_time&.to_i, + "bundle_version" => bundle_long_version, + "bundle_short_version" => bundle_short_version, + "outdated" => outdated?, + "sha256" => sha256, + "artifacts" => artifacts_list, + "caveats" => (Tty.strip_ansi(caveats) unless caveats.empty?), + "depends_on" => depends_on, + "conflicts_with" => conflicts_with, + "container" => container&.pairs, + "auto_updates" => auto_updates, + "deprecated" => deprecated?, + "deprecation_date" => deprecation_date, + "deprecation_reason" => deprecation_reason, + "deprecation_replacement_formula" => deprecation_replacement_formula, + "deprecation_replacement_cask" => deprecation_replacement_cask, + "disabled" => disabled?, + "disable_date" => disable_date, + "disable_reason" => disable_reason, + "disable_replacement_formula" => disable_replacement_formula, + "disable_replacement_cask" => disable_replacement_cask, + "tap_git_head" => tap_git_head, + "languages" => languages, + "ruby_source_path" => ruby_source_path, + "ruby_source_checksum" => ruby_source_checksum, } end diff --git a/Library/Homebrew/cask/dsl.rb b/Library/Homebrew/cask/dsl.rb index 0947d8d0a3..4de0e32c32 100644 --- a/Library/Homebrew/cask/dsl.rb +++ b/Library/Homebrew/cask/dsl.rb @@ -90,14 +90,12 @@ module Cask :deprecated?, :deprecation_date, :deprecation_reason, - :deprecation_replacement, :deprecation_replacement_formula, :deprecation_replacement_cask, :disable!, :disabled?, :disable_date, :disable_reason, - :disable_replacement, :disable_replacement_formula, :disable_replacement_cask, :discontinued?, # TODO: remove once discontinued? is removed (4.5.0) @@ -114,9 +112,8 @@ module Cask include OnSystem::MacOSAndLinux - attr_reader :cask, :token, :deprecation_date, :deprecation_reason, :deprecation_replacement, - :deprecation_replacement_formula, :deprecation_replacement_caks, :disable_date, - :disable_reason, :disable_replacement, :disable_replacement_formula, + attr_reader :cask, :token, :deprecation_date, :deprecation_reason, :deprecation_replacement_formula, + :deprecation_replacement_cask, :disable_date, :disable_reason, :disable_replacement_formula, :disable_replacement_cask, :on_system_block_min_os def initialize(cask) @@ -533,8 +530,8 @@ module Cask # # @api public def deprecate!(date:, because:, replacement: nil, replacement_formula: nil, replacement_cask: nil) - if replacement_formula && replacement_cask - raise ArgumentError, "replacement_formula and replacement_cask specified!" + if [replacement, replacement_formula, replacement_cask].filter_map(&:presence).length > 1 + raise ArgumentError, "more than one of replacement, replacement_formula and/or replacement_cask specified!" end # TODO: deprecate in >= 4.5.0 @@ -550,9 +547,8 @@ module Cask return if @deprecation_date > Date.today @deprecation_reason = because - @deprecation_replacement = replacement - @deprecation_replacement_formula = replacement_formula - @deprecation_replacement_cask = replacement_cask + @deprecation_replacement_formula = replacement_formula.presence || replacement + @deprecation_replacement_cask = replacement_cask.presence || replacement @deprecated = true end @@ -562,8 +558,8 @@ module Cask # # @api public def disable!(date:, because:, replacement: nil, replacement_formula: nil, replacement_cask: nil) - if replacement_formula && replacement_cask - raise ArgumentError, "replacement_formula and replacement_cask specified!" + if [replacement, replacement_formula, replacement_cask].filter_map(&:presence).length > 1 + raise ArgumentError, "more than one of replacement, replacement_formula and/or replacement_cask specified!" end # TODO: deprecate in >= 4.5.0 @@ -579,17 +575,15 @@ module Cask if @disable_date > Date.today @deprecation_reason = because - @deprecation_replacement = replacement - @deprecation_replacement_formula = replacement_formula - @deprecation_replacement_cask = replacement_cask + @deprecation_replacement_formula = replacement_formula.presence || replacement + @deprecation_replacement_cask = replacement_cask.presence || replacement @deprecated = true return end @disable_reason = because - @disable_replacement = replacement - @disable_replacement_formula = replacement_formula - @disable_replacement_cask = replacement_cask + @disable_replacement_formula = replacement_formula.presence || replacement + @disable_replacement_cask = replacement_cask.presence || replacement @disabled = true end diff --git a/Library/Homebrew/deprecate_disable.rb b/Library/Homebrew/deprecate_disable.rb index 7bc9e72d6e..c09e95f30c 100644 --- a/Library/Homebrew/deprecate_disable.rb +++ b/Library/Homebrew/deprecate_disable.rb @@ -43,18 +43,17 @@ module DeprecateDisable sig { params( - formula: T.nilable(String), - cask: T.nilable(String), - not_typed: T.nilable(String), + formula: T.nilable(String), + cask: T.nilable(String), ).returns(T.nilable(String)) } - def replacement_with_type(formula, cask, not_typed) - if formula + def replacement_with_type(formula, cask) + if formula && formula == cask + formula + elsif formula "--formula #{formula}" elsif cask "--cask #{cask}" - else - not_typed end end @@ -98,13 +97,11 @@ module DeprecateDisable replacement_with_type( formula_or_cask.disable_replacement_formula, formula_or_cask.disable_replacement_cask, - formula_or_cask.disable_replacement, ) elsif formula_or_cask.deprecated? replacement_with_type( formula_or_cask.deprecation_replacement_formula, formula_or_cask.deprecation_replacement_cask, - formula_or_cask.deprecation_replacement, ) end diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index db30b17da8..1eeed5aa13 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -1477,13 +1477,6 @@ class Formula # @see .deprecate! delegate deprecation_reason: :"self.class" - # The replacement for this deprecated {Formula}. - # Returns `nil` if no replacement is specified or the formula is not deprecated. - # @!method deprecation_replacement - # @return [String] - # @see .deprecate! - delegate deprecation_replacement: :"self.class" - # The replacement formula for this deprecated {Formula}. # Returns `nil` if no replacement is specified or the formula is not deprecated. # @!method deprecation_replacement_formula @@ -1519,13 +1512,6 @@ class Formula # @see .disable! delegate disable_reason: :"self.class" - # The replacement for this disabled {Formula}. - # Returns `nil` if no replacement is specified or the formula is not disabled. - # @!method disable_replacement - # @return [String] - # @see .disable! - delegate disable_replacement: :"self.class" - # The replacement formula for this disabled {Formula}. # Returns `nil` if no replacement is specified or the formula is not disabled. # @!method disable_replacement_formula @@ -2560,13 +2546,11 @@ class Formula "deprecated" => deprecated?, "deprecation_date" => deprecation_date, "deprecation_reason" => deprecation_reason, - "deprecation_replacement" => deprecation_replacement, "deprecation_replacement_formula" => deprecation_replacement_formula, "deprecation_replacement_cask" => deprecation_replacement_cask, "disabled" => disabled?, "disable_date" => disable_date, "disable_reason" => disable_reason, - "disable_replacement" => disable_replacement, "disable_replacement_formula" => disable_replacement_formula, "disable_replacement_cask" => disable_replacement_cask, "post_install_defined" => post_install_defined?, @@ -4370,8 +4354,8 @@ class Formula ).void } def deprecate!(date:, because:, replacement: nil, replacement_formula: nil, replacement_cask: nil) - if replacement_formula && replacement_cask - raise ArgumentError, "replacement_formula and replacement_cask specified!" + if [replacement, replacement_formula, replacement_cask].filter_map(&:presence).length > 1 + raise ArgumentError, "more than one of replacement, replacement_formula and/or replacement_cask specified!" end # TODO: deprecate in >= 4.5.0 @@ -4387,9 +4371,8 @@ class Formula return if T.must(@deprecation_date) > Date.today @deprecation_reason = T.let(because, T.any(NilClass, String, Symbol)) - @deprecation_replacement = T.let(replacement, T.nilable(String)) - @deprecation_replacement_formula = T.let(replacement_formula, T.nilable(String)) - @deprecation_replacement_cask = T.let(replacement_cask, T.nilable(String)) + @deprecation_replacement_formula = T.let(replacement_formula.presence || replacement, T.nilable(String)) + @deprecation_replacement_cask = T.let(replacement_cask.presence || replacement, T.nilable(String)) T.must(@deprecated = T.let(true, T.nilable(T::Boolean))) end @@ -4415,13 +4398,6 @@ class Formula sig { returns(T.any(NilClass, String, Symbol)) } attr_reader :deprecation_reason - # The replacement for a deprecated {Formula}. - # - # @return [nil] if no replacement was provided or the formula is not deprecated. - # @see .deprecate! - sig { returns(T.nilable(String)) } - attr_reader :deprecation_replacement - # The replacement formula for a deprecated {Formula}. # # @return [nil] if no replacement was provided or the formula is not deprecated. @@ -4471,8 +4447,8 @@ class Formula ).void } def disable!(date:, because:, replacement: nil, replacement_formula: nil, replacement_cask: nil) - if replacement_formula && replacement_cask - raise ArgumentError, "replacement_formula and replacement_cask specified!" + if [replacement, replacement_formula, replacement_cask].filter_map(&:presence).length > 1 + raise ArgumentError, "more than one of replacement, replacement_formula and/or replacement_cask specified!" end # TODO: deprecate in >= 4.5.0 @@ -4488,17 +4464,15 @@ class Formula if T.must(@disable_date) > Date.today @deprecation_reason = T.let(because, T.any(NilClass, String, Symbol)) - @deprecation_replacement = T.let(replacement, T.nilable(String)) - @deprecation_replacement_formula = T.let(replacement_formula, T.nilable(String)) - @deprecation_replacement_cask = T.let(replacement_cask, T.nilable(String)) + @deprecation_replacement_formula = T.let(replacement_formula.presence || replacement, T.nilable(String)) + @deprecation_replacement_cask = T.let(replacement_cask.presence || replacement, T.nilable(String)) @deprecated = T.let(true, T.nilable(T::Boolean)) return end @disable_reason = T.let(because, T.nilable(T.any(String, Symbol))) - @disable_replacement = T.let(replacement, T.nilable(String)) - @disable_replacement_formula = T.let(replacement_formula, T.nilable(String)) - @disable_replacement_cask = T.let(replacement_cask, T.nilable(String)) + @disable_replacement_formula = T.let(replacement_formula.presence || replacement, T.nilable(String)) + @disable_replacement_cask = T.let(replacement_cask.presence || replacement, T.nilable(String)) @disabled = T.let(true, T.nilable(T::Boolean)) end @@ -4525,13 +4499,6 @@ class Formula sig { returns(T.any(NilClass, String, Symbol)) } attr_reader :disable_reason - # The replacement for a disabled {Formula}. - # Returns `nil` if no reason was provided or the formula is not disabled. - # - # @see .disable! - sig { returns(T.nilable(String)) } - attr_reader :disable_replacement - # The replacement formula for a disabled {Formula}. # Returns `nil` if no reason was provided or the formula is not disabled. # diff --git a/Library/Homebrew/sorbet/rbi/dsl/cask/cask.rbi b/Library/Homebrew/sorbet/rbi/dsl/cask/cask.rbi index 831a01c677..78c80320e5 100644 --- a/Library/Homebrew/sorbet/rbi/dsl/cask/cask.rbi +++ b/Library/Homebrew/sorbet/rbi/dsl/cask/cask.rbi @@ -66,9 +66,6 @@ class Cask::Cask sig { params(args: T.untyped, block: T.untyped).returns(T.untyped) } def deprecation_reason(*args, &block); end - sig { params(args: T.untyped, block: T.untyped).returns(T.untyped) } - def deprecation_replacement(*args, &block); end - sig { params(args: T.untyped, block: T.untyped).returns(T.untyped) } def deprecation_replacement_formula(*args, &block); end @@ -90,9 +87,6 @@ class Cask::Cask sig { params(args: T.untyped, block: T.untyped).returns(T.untyped) } def disable_reason(*args, &block); end - sig { params(args: T.untyped, block: T.untyped).returns(T.untyped) } - def disable_replacement(*args, &block); end - sig { params(args: T.untyped, block: T.untyped).returns(T.untyped) } def disable_replacement_formula(*args, &block); end diff --git a/Library/Homebrew/sorbet/rbi/dsl/formula.rbi b/Library/Homebrew/sorbet/rbi/dsl/formula.rbi index c0e149d99b..86118a7373 100644 --- a/Library/Homebrew/sorbet/rbi/dsl/formula.rbi +++ b/Library/Homebrew/sorbet/rbi/dsl/formula.rbi @@ -51,9 +51,6 @@ class Formula sig { params(args: T.untyped, block: T.untyped).returns(T.untyped) } def deprecation_reason(*args, &block); end - sig { params(args: T.untyped, block: T.untyped).returns(T.untyped) } - def deprecation_replacement(*args, &block); end - sig { params(args: T.untyped, block: T.untyped).returns(T.untyped) } def deprecation_replacement_formula(*args, &block); end @@ -72,9 +69,6 @@ class Formula sig { params(args: T.untyped, block: T.untyped).returns(T.untyped) } def disable_reason(*args, &block); end - sig { params(args: T.untyped, block: T.untyped).returns(T.untyped) } - def disable_replacement(*args, &block); end - sig { params(args: T.untyped, block: T.untyped).returns(T.untyped) } def disable_replacement_formula(*args, &block); end diff --git a/Library/Homebrew/test/deprecate_disable_spec.rb b/Library/Homebrew/test/deprecate_disable_spec.rb index 819191261c..0110e6e209 100644 --- a/Library/Homebrew/test/deprecate_disable_spec.rb +++ b/Library/Homebrew/test/deprecate_disable_spec.rb @@ -7,55 +7,52 @@ RSpec.describe DeprecateDisable do let(:disable_date) { deprecate_date >> DeprecateDisable::REMOVE_DISABLED_TIME_WINDOW } let(:deprecated_formula) do instance_double(Formula, deprecated?: true, disabled?: false, deprecation_reason: :does_not_build, - deprecation_replacement: nil, deprecation_replacement_formula: nil, - deprecation_replacement_cask: nil, deprecation_date: nil, disable_date: nil) + deprecation_replacement_formula: nil, deprecation_replacement_cask: nil, + deprecation_date: nil, disable_date: nil) end let(:deprecated_formula_with_date) do instance_double(Formula, deprecated?: true, disabled?: false, deprecation_reason: :does_not_build, - deprecation_replacement: nil, deprecation_replacement_formula: nil, - deprecation_replacement_cask: nil, deprecation_date: deprecate_date, disable_date: nil) + deprecation_replacement_formula: nil, deprecation_replacement_cask: nil, + deprecation_date: deprecate_date, disable_date: nil) end let(:disabled_formula) do instance_double(Formula, deprecated?: false, disabled?: true, disable_reason: "is broken", - deprecation_replacement: nil, deprecation_replacement_formula: nil, - deprecation_replacement_cask: nil, disable_replacement: nil, + deprecation_replacement_formula: nil, deprecation_replacement_cask: nil, disable_replacement_formula: nil, disable_replacement_cask: nil, deprecation_date: nil, disable_date: nil) end let(:disabled_formula_with_date) do instance_double(Formula, deprecated?: false, disabled?: true, disable_reason: :does_not_build, - deprecation_replacement: nil, deprecation_replacement_formula: nil, - deprecation_replacement_cask: nil, disable_replacement: nil, + deprecation_replacement_formula: nil, deprecation_replacement_cask: nil, disable_replacement_formula: nil, disable_replacement_cask: nil, deprecation_date: nil, disable_date: disable_date) end let(:deprecated_cask) do instance_double(Cask::Cask, deprecated?: true, disabled?: false, deprecation_reason: :discontinued, - deprecation_replacement: nil, deprecation_replacement_formula: nil, - deprecation_replacement_cask: nil, deprecation_date: nil, disable_date: nil) + deprecation_replacement_formula: nil, deprecation_replacement_cask: nil, + deprecation_date: nil, disable_date: nil) end let(:disabled_cask) do instance_double(Cask::Cask, deprecated?: false, disabled?: true, disable_reason: nil, - deprecation_replacement: nil, deprecation_replacement_formula: nil, - deprecation_replacement_cask: nil, disable_replacement: nil, + deprecation_replacement_formula: nil, deprecation_replacement_cask: nil, disable_replacement_formula: nil, disable_replacement_cask: nil, deprecation_date: nil, disable_date: nil) end let(:deprecated_formula_with_replacement) do instance_double(Formula, deprecated?: true, disabled?: false, deprecation_reason: :does_not_build, - deprecation_replacement: "foo", deprecation_date: nil, disable_date: nil) + deprecation_date: nil, disable_date: nil) end let(:disabled_formula_with_replacement) do instance_double(Formula, deprecated?: false, disabled?: true, disable_reason: "is broken", - disable_replacement: "bar", deprecation_date: nil, disable_date: nil) + deprecation_date: nil, disable_date: nil) end let(:deprecated_cask_with_replacement) do instance_double(Cask::Cask, deprecated?: true, disabled?: false, deprecation_reason: :discontinued, - deprecation_replacement: "baz", deprecation_date: nil, disable_date: nil) + deprecation_date: nil, disable_date: nil) end let(:disabled_cask_with_replacement) do instance_double(Cask::Cask, deprecated?: false, disabled?: true, disable_reason: nil, - disable_replacement: "qux", deprecation_date: nil, disable_date: nil) + deprecation_date: nil, disable_date: nil) end before do @@ -136,13 +133,6 @@ RSpec.describe DeprecateDisable do .to eq "disabled!" end - it "returns a replacement message for a deprecated formula" do - allow(deprecated_formula_with_replacement).to receive_messages(deprecation_replacement_formula: nil, - deprecation_replacement_cask: nil) - expect(described_class.message(deprecated_formula_with_replacement)) - .to eq "deprecated because it does not build!\nReplacement:\n brew install foo\n" - end - it "returns a replacement formula message for a deprecated formula" do allow(deprecated_formula_with_replacement).to receive_messages(deprecation_replacement_formula: "foo", deprecation_replacement_cask: nil) @@ -157,13 +147,6 @@ RSpec.describe DeprecateDisable do .to eq "deprecated because it does not build!\nReplacement:\n brew install --cask foo\n" end - it "returns a replacement message for a disabled formula" do - allow(disabled_formula_with_replacement).to receive_messages(disable_replacement_formula: nil, - disable_replacement_cask: nil) - expect(described_class.message(disabled_formula_with_replacement)) - .to eq "disabled because it is broken!\nReplacement:\n brew install bar\n" - end - it "returns a replacement formula message for a disabled formula" do allow(disabled_formula_with_replacement).to receive_messages(disable_replacement_formula: "bar", disable_replacement_cask: nil) @@ -178,13 +161,6 @@ RSpec.describe DeprecateDisable do .to eq "disabled because it is broken!\nReplacement:\n brew install --cask bar\n" end - it "returns a replacement message for a deprecated cask" do - allow(deprecated_cask_with_replacement).to receive_messages(deprecation_replacement_formula: nil, - deprecation_replacement_cask: nil) - expect(described_class.message(deprecated_cask_with_replacement)) - .to eq "deprecated because it is discontinued upstream!\nReplacement:\n brew install baz\n" - end - it "returns a replacement formula message for a deprecated cask" do allow(deprecated_cask_with_replacement).to receive_messages(deprecation_replacement_formula: "baz", deprecation_replacement_cask: nil) @@ -199,13 +175,6 @@ RSpec.describe DeprecateDisable do .to eq "deprecated because it is discontinued upstream!\nReplacement:\n brew install --cask baz\n" end - it "returns a replacement message for a disabled cask" do - allow(disabled_cask_with_replacement).to receive_messages(disable_replacement_formula: nil, - disable_replacement_cask: nil) - expect(described_class.message(disabled_cask_with_replacement)) - .to eq "disabled!\nReplacement:\n brew install qux\n" - end - it "returns a replacement formula message for a disabled cask" do allow(disabled_cask_with_replacement).to receive_messages(disable_replacement_formula: "qux", disable_replacement_cask: nil) diff --git a/Library/Homebrew/test/support/fixtures/cask/everything.json b/Library/Homebrew/test/support/fixtures/cask/everything.json index b247e82a05..d3d981258f 100644 --- a/Library/Homebrew/test/support/fixtures/cask/everything.json +++ b/Library/Homebrew/test/support/fixtures/cask/everything.json @@ -95,11 +95,13 @@ "deprecated": false, "deprecation_date": null, "deprecation_reason": null, - "deprecation_replacement": null, + "deprecation_replacement_formula": null, + "deprecation_replacement_cask": null, "disabled": false, "disable_date": null, "disable_reason": null, - "disable_replacement": null, + "disable_replacement_formula": null, + "disable_replacement_cask": null, "tap_git_head": "abcdef1234567890abcdef1234567890abcdef12", "languages": [ "en",