Directly save artifacts in DSL.

9 files changed, 47 insertions, 24 deletions

diff --git a/Library/Homebrew/cask/lib/hbc/artifact.rb b/Library/Homebrew/cask/lib/hbc/artifact.rb
index cb15ec051..505f9ec13 100644
--- a/Library/Homebrew/cask/lib/hbc/artifact.rb
+++ b/Library/Homebrew/cask/lib/hbc/artifact.rb
@@ -59,10 +59,5 @@ module Hbc
       PostflightBlock,
       Zap,
     ].freeze
-
-    def self.for_cask(cask)
-      odebug "Determining which artifacts are present in Cask #{cask}"
-      CLASSES.flat_map { |klass| klass.for_cask(cask) }
-    end
   end
 end
diff --git a/Library/Homebrew/cask/lib/hbc/artifact/abstract_artifact.rb b/Library/Homebrew/cask/lib/hbc/artifact/abstract_artifact.rb
index fa8fc2996..331e098e0 100644
--- a/Library/Homebrew/cask/lib/hbc/artifact/abstract_artifact.rb
+++ b/Library/Homebrew/cask/lib/hbc/artifact/abstract_artifact.rb
@@ -1,6 +1,7 @@
 module Hbc
   module Artifact
     class AbstractArtifact
+      include Comparable
       extend Predicable
 
       def self.english_name
@@ -19,8 +20,35 @@ module Hbc
         @dirmethod ||= "#{dsl_key}dir".to_sym
       end
 
-      def self.for_cask(cask)
-        cask.artifacts[self].to_a
+      def <=>(other)
+        @@sort_order ||= [ # rubocop:disable Style/ClassVars
+          PreflightBlock,
+          Uninstall,
+          NestedContainer,
+          Installer,
+          App,
+          Suite,
+          Artifact,        # generic 'artifact' stanza
+          Colorpicker,
+          Pkg,
+          Prefpane,
+          Qlplugin,
+          Dictionary,
+          Font,
+          Service,
+          StageOnly,
+          Binary,
+          InputMethod,
+          InternetPlugin,
+          AudioUnitPlugin,
+          VstPlugin,
+          Vst3Plugin,
+          ScreenSaver,
+          PostflightBlock,
+          Zap,
+        ]
+
+        (@@sort_order.index(self.class) <=> @@sort_order.index(other.class)).to_i
       end
 
       # TODO: this sort of logic would make more sense in dsl.rb, or a
diff --git a/Library/Homebrew/cask/lib/hbc/audit.rb b/Library/Homebrew/cask/lib/hbc/audit.rb
index 2d8e4e4d7..5b5a7b63d 100644
--- a/Library/Homebrew/cask/lib/hbc/audit.rb
+++ b/Library/Homebrew/cask/lib/hbc/audit.rb
@@ -218,7 +218,7 @@ module Hbc
     end
 
     def check_generic_artifacts
-      cask.artifacts[Hbc::Artifact::Artifact].each do |artifact|
+      cask.artifacts.select { |a| a.is_a?(Hbc::Artifact::Artifact) }.each do |artifact|
         unless artifact.target.absolute?
           add_error "target must be absolute path for #{artifact.class.english_name} #{artifact.source}"
         end
diff --git a/Library/Homebrew/cask/lib/hbc/cli/info.rb b/Library/Homebrew/cask/lib/hbc/cli/info.rb
index 2b4db9530..f12fe5564 100644
--- a/Library/Homebrew/cask/lib/hbc/cli/info.rb
+++ b/Library/Homebrew/cask/lib/hbc/cli/info.rb
@@ -77,11 +77,11 @@ module Hbc
 
       def self.artifact_info(cask)
         ohai "Artifacts"
-        DSL::ORDINARY_ARTIFACT_CLASSES.flat_map { |klass| klass.for_cask(cask) }
-                                      .select { |artifact| artifact.respond_to?(:install_phase) }
-                                      .each do |artifact|
-                                        puts artifact.to_s
-                                      end
+        cask.artifacts.each do |artifact|
+          next unless artifact.respond_to?(:install_phase)
+          next unless DSL::ORDINARY_ARTIFACT_CLASSES.include?(artifact.class)
+          puts artifact.to_s
+        end
       end
     end
   end
diff --git a/Library/Homebrew/cask/lib/hbc/cli/list.rb b/Library/Homebrew/cask/lib/hbc/cli/list.rb
index 32415af8a..4b6a25d01 100644
--- a/Library/Homebrew/cask/lib/hbc/cli/list.rb
+++ b/Library/Homebrew/cask/lib/hbc/cli/list.rb
@@ -30,7 +30,7 @@ module Hbc
       end
 
       def self.list_artifacts(cask)
-        Artifact.for_cask(cask).group_by(&:class).each do |klass, artifacts|
+        cask.artifacts.group_by(&:class).each do |klass, artifacts|
           next unless klass.respond_to?(:english_description)
           ohai klass.english_description, artifacts.map(&:summarize_installed)
         end
diff --git a/Library/Homebrew/cask/lib/hbc/container/base.rb b/Library/Homebrew/cask/lib/hbc/container/base.rb
index 4363168e7..78610a1ab 100644
--- a/Library/Homebrew/cask/lib/hbc/container/base.rb
+++ b/Library/Homebrew/cask/lib/hbc/container/base.rb
@@ -20,7 +20,7 @@ module Hbc
 
         unless children.count == 1 &&
                !nested_container.directory? &&
-               Artifact::NestedContainer.for_cask(@cask).none? &&
+               @cask.artifacts.none? { |a| a.is_a?(Artifact::NestedContainer) } &&
                extract_nested_container(nested_container)
 
           children.each do |src|
diff --git a/Library/Homebrew/cask/lib/hbc/dsl.rb b/Library/Homebrew/cask/lib/hbc/dsl.rb
index 75c1ca873..2db2c66a9 100644
--- a/Library/Homebrew/cask/lib/hbc/dsl.rb
+++ b/Library/Homebrew/cask/lib/hbc/dsl.rb
@@ -176,7 +176,7 @@ module Hbc
           DSL::Container.new(*args).tap do |container|
             # TODO: remove this backward-compatibility section after removing nested_container
             if container&.nested
-              artifacts[Artifact::NestedContainer] << Artifact::NestedContainer.new(cask, container.nested)
+              artifacts.add(Artifact::NestedContainer.new(cask, container.nested))
             end
           end
         end
@@ -219,7 +219,7 @@ module Hbc
     end
 
     def artifacts
-      @artifacts ||= Hash.new { |hash, key| hash[key] = Set.new }
+      @artifacts ||= SortedSet.new
     end
 
     def caskroom_path
@@ -253,11 +253,11 @@ module Hbc
     ORDINARY_ARTIFACT_CLASSES.each do |klass|
       define_method(klass.dsl_key) do |*args|
         begin
-          if [*artifacts.keys, klass].include?(Artifact::StageOnly) && (artifacts.keys & ACTIVATABLE_ARTIFACT_CLASSES).any?
+          if [*artifacts.map(&:class), klass].include?(Artifact::StageOnly) && (artifacts.map(&:class) & ACTIVATABLE_ARTIFACT_CLASSES).any?
             raise CaskInvalidError.new(cask, "'stage_only' must be the only activatable artifact.")
           end
 
-          artifacts[klass].add(klass.from_args(cask, *args))
+          artifacts.add(klass.from_args(cask, *args))
         rescue CaskInvalidError
           raise
         rescue StandardError => e
@@ -269,7 +269,7 @@ module Hbc
     ARTIFACT_BLOCK_CLASSES.each do |klass|
       [klass.dsl_key, klass.uninstall_dsl_key].each do |dsl_key|
         define_method(dsl_key) do |&block|
-          artifacts[klass] << klass.new(cask, dsl_key => block)
+          artifacts.add(klass.new(cask, dsl_key => block))
         end
       end
     end
diff --git a/Library/Homebrew/cask/lib/hbc/installer.rb b/Library/Homebrew/cask/lib/hbc/installer.rb
index 01aae935d..ffbbac181 100644
--- a/Library/Homebrew/cask/lib/hbc/installer.rb
+++ b/Library/Homebrew/cask/lib/hbc/installer.rb
@@ -177,7 +177,7 @@ module Hbc
       already_installed_artifacts = []
 
       odebug "Installing artifacts"
-      artifacts = Artifact.for_cask(@cask)
+      artifacts = @cask.artifacts
       odebug "#{artifacts.length} artifact/s defined", artifacts
 
       artifacts.each do |artifact|
@@ -374,7 +374,7 @@ module Hbc
 
     def uninstall_artifacts
       odebug "Un-installing artifacts"
-      artifacts = Artifact.for_cask(@cask)
+      artifacts = @cask.artifacts
 
       odebug "#{artifacts.length} artifact/s defined", artifacts
 
@@ -388,7 +388,7 @@ module Hbc
     def zap
       ohai %Q(Implied "brew cask uninstall #{@cask}")
       uninstall_artifacts
-      if (zap_stanzas = Artifact::Zap.for_cask(@cask)).empty?
+      if (zap_stanzas = @cask.artifacts.select { |a| a.is_a?(Artifact::Zap) }).empty?
         opoo "No zap stanza present for Cask '#{@cask}'"
       else
         ohai "Dispatching zap stanza"
diff --git a/Library/Homebrew/cask/lib/hbc/staged.rb b/Library/Homebrew/cask/lib/hbc/staged.rb
index 6fd3f6709..da097e0cf 100644
--- a/Library/Homebrew/cask/lib/hbc/staged.rb
+++ b/Library/Homebrew/cask/lib/hbc/staged.rb
@@ -4,7 +4,7 @@ module Hbc
       index =  0 if index == :first
       index =  1 if index == :second
       index = -1 if index == :last
-      @cask.artifacts[Artifact::App].to_a.at(index).target.join("Contents", "Info.plist")
+      @cask.artifacts.select { |a| a.is_a?(Artifact::App) }.at(index).target.join("Contents", "Info.plist")
     end
 
     def plist_exec(cmd)