Eagerly initialize formula specs

Declarations of dependencies, options, and resources in the DSL only
apply to specs that have already been initialized. For example, given
this snippet:

  url ...
  sha1 ...

  depends_on 'foo'

  devel do
    url ...
    sha1 ...
  end

The dependency 'foo' will be recorded for the stable spec, but not the
devel spec, since it was not initialized prior to the call to
depends_on.

While it is considered best practice to declare all specs (stable,
devel, head, and bottle) prior to other declarations, there is nothing
that enforces this ordering, so when it happens it can be confusing and
hard to debug.

To prevent this, we can initialize all specs up front. This comes with
a performance penalty for commands that load all formulae into memory,
but that is probably outweighed by what we gain in correctness.

Fixes #23425.

2 files changed, 36 insertions, 23 deletions

diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb
index af585e361..59f815553 100644
--- a/Library/Homebrew/formula.rb
+++ b/Library/Homebrew/formula.rb
@@ -56,7 +56,6 @@ class Formula
 
   def set_spec(name)
     spec = self.class.send(name)
-    return if spec.nil?
     if block_given? && yield(spec) || !spec.url.nil?
       spec.owner = self
       instance_variable_set("@#{name}", spec)
@@ -656,69 +655,57 @@ class Formula
     attr_rw :plist_startup, :plist_manual
 
     def specs
-      @specs ||= []
-    end
-
-    def create_spec(klass)
-      spec = klass.new
-      specs << spec
-      spec
+      @specs ||= [stable, devel, head, bottle].freeze
     end
 
     def url val, specs={}
-      @stable ||= create_spec(SoftwareSpec)
-      @stable.url(val, specs)
+      stable.url(val, specs)
     end
 
     def version val=nil
-      @stable ||= create_spec(SoftwareSpec)
-      @stable.version(val)
+      stable.version(val)
     end
 
     def mirror val
-      @stable ||= create_spec(SoftwareSpec)
-      @stable.mirror(val)
+      stable.mirror(val)
     end
 
     Checksum::TYPES.each do |cksum|
       class_eval <<-EOS, __FILE__, __LINE__ + 1
         def #{cksum}(val)
-          @stable ||= create_spec(SoftwareSpec)
-          @stable.#{cksum}(val)
+          stable.#{cksum}(val)
         end
       EOS
     end
 
     def build
-      @stable ||= create_spec(SoftwareSpec)
-      @stable.build
+      stable.build
     end
 
     def stable &block
+      @stable ||= SoftwareSpec.new
       return @stable unless block_given?
-      @stable ||= create_spec(SoftwareSpec)
       @stable.instance_eval(&block)
     end
 
     def bottle *, &block
+      @bottle ||= Bottle.new
       return @bottle unless block_given?
-      @bottle ||= create_spec(Bottle)
       @bottle.instance_eval(&block)
       @bottle.version = @stable.version
     end
 
     def devel &block
+      @devel ||= SoftwareSpec.new
       return @devel unless block_given?
-      @devel ||= create_spec(SoftwareSpec)
       @devel.instance_eval(&block)
     end
 
     def head val=nil, specs={}, &block
+      @head ||= HeadSoftwareSpec.new
       if block_given?
-        @head ||= create_spec(HeadSoftwareSpec)
         @head.instance_eval(&block)
       elsif val
-        @head ||= create_spec(HeadSoftwareSpec)
         @head.url(val, specs)
       else
         @head
diff --git a/Library/Homebrew/test/test_formula.rb b/Library/Homebrew/test/test_formula.rb
index bc1e703eb..00f929e4b 100644
--- a/Library/Homebrew/test/test_formula.rb
+++ b/Library/Homebrew/test/test_formula.rb
@@ -212,4 +212,30 @@ class FormulaTests < Test::Unit::TestCase
   ensure
     path.unlink
   end
+
+  def test_class_specs_are_always_initialized
+    f = formula { url 'foo-1.0' }
+
+    %w{stable devel head bottle}.each do |spec|
+      assert_kind_of SoftwareSpec, f.class.send(spec)
+    end
+  end
+
+  def test_incomplete_instance_specs_are_not_accessible
+    f = formula { url 'foo-1.0' }
+
+    %w{devel head bottle}.each { |spec| assert_nil f.send(spec) }
+  end
+
+  def test_honors_attributes_declared_before_specs
+    f = formula do
+      url 'foo-1.0'
+      depends_on 'foo'
+      devel { url 'foo-1.1' }
+    end
+
+    %w{stable devel head bottle}.each do |spec|
+      assert_equal 'foo', f.class.send(spec).deps.first.name
+    end
+  end
 end