Skip to content

Commit

Permalink
Enforce finalized sorbet methods
Browse files Browse the repository at this point in the history
  • Loading branch information
dduugg committed Nov 22, 2024
1 parent f0c746a commit f84ba4e
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 6 deletions.
10 changes: 9 additions & 1 deletion Library/Homebrew/formula.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ class Formula
extend Cachable
extend Attrable
extend APIHashable
extend T::Helpers

abstract!

SUPPORTED_NETWORK_ACCESS_PHASES = [:build, :test, :postinstall].freeze
private_constant :SUPPORTED_NETWORK_ACCESS_PHASES
Expand Down Expand Up @@ -1596,7 +1599,11 @@ def patch

# Yields |self,staging| with current working directory set to the uncompressed tarball
# where staging is a {Mktemp} staging context.
def brew(fetch: true, keep_tmp: false, debug_symbols: false, interactive: false)
sig(:final) {
params(fetch: T::Boolean, keep_tmp: T::Boolean, debug_symbols: T::Boolean, interactive: T::Boolean,
_blk: T.proc.params(arg0: Formula, arg1: Mktemp).void).void
}
def brew(fetch: true, keep_tmp: false, debug_symbols: false, interactive: false, &_blk)
@prefix_returns_versioned_prefix = true
active_spec.fetch if fetch
stage(interactive:, debug_symbols:) do |staging|
Expand Down Expand Up @@ -3346,6 +3353,7 @@ def method_added(method)

case method
when :brew
# This method has a `:final` sig, but we want to enforce it even when the sorbet runtime is disabled.
raise "You cannot override Formula#brew in class #{name}"
when :test
define_method(:test_defined?) { true }
Expand Down
9 changes: 5 additions & 4 deletions Library/Homebrew/requirement.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,15 @@
class Requirement
include Dependable
extend Cachable
extend T::Helpers

# This base class enforces no constraints on its own.
# Individual subclasses use the `satisfy` DSL to define those constraints.
abstract!

attr_reader :name, :cask, :download

def initialize(tags = [])
# Only allow instances of subclasses. This base class enforces no constraints on its own.
# Individual subclasses use the `satisfy` DSL to define those constraints.
raise "Do not call `Requirement.new' directly without a subclass." unless self.class < Requirement

@cask = self.class.cask
@download = self.class.download
tags.each do |tag|
Expand Down
4 changes: 3 additions & 1 deletion Library/Homebrew/standalone/sorbet.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
# In the future we should consider not doing this monkey patch,
# if assured that there is no performance hit from removing this.
# There are mechanisms to achieve a middle ground (`default_checked_level`).
unless ENV["HOMEBREW_SORBET_RUNTIME"]
if ENV["HOMEBREW_SORBET_RUNTIME"]
T::Configuration.enable_final_checks_on_hooks
else
# Redefine `T.let`, etc. to make the `checked` parameter default to `false` rather than `true`.
# @private
module TNoChecks
Expand Down
5 changes: 5 additions & 0 deletions Library/Homebrew/test/formula_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@
expect { klass.new }.to raise_error(ArgumentError)
end

specify "formula instantiation without a subclass" do
expect { described_class.new(name, path, spec) }
.to raise_error(RuntimeError, "Do not call `Formula.new' directly without a subclass.")
end

context "when in a Tap" do
let(:tap) { Tap.fetch("foo", "bar") }
let(:path) { (tap.path/"Formula/#{name}.rb") }
Expand Down
18 changes: 18 additions & 0 deletions Library/Homebrew/test/requirement_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@

let(:klass) { Class.new(described_class) }

describe "base class" do
it "raises an error when instantiated" do
expect { described_class.new }
.to raise_error(RuntimeError, "Requirement is declared as abstract; it cannot be instantiated")
end
end

describe "#tags" do
subject(:req) { klass.new(tags) }

Expand Down Expand Up @@ -52,6 +59,17 @@
describe "#fatal is omitted" do
it { is_expected.not_to be_fatal }
end

describe "in subclasses" do
it "raises an error when instantiated" do
expect do
Class.new(described_class) do
def fatal? = false
end
end
.to raise_error(RuntimeError, /The method `fatal\?` on #{described_class.name} was declared as final/)
end
end
end

describe "#satisfied?" do
Expand Down

0 comments on commit f84ba4e

Please sign in to comment.