Merge pull request #3267 from reitermarkus/refactoring

Refactor DSL and Artifact classes.

Library/Homebrew/cask/lib/hbc/artifact.rb

@@ -25,44 +25,5 @@ require "hbc/artifact/zap"
 
 module Hbc
   module Artifact
-    # NOTE: Order is important here!
-    #
-    # The `uninstall` stanza should be run first, as it may
-    # depend on other artifacts still being installed.
-    #
-    # We want to extract nested containers before we
-    # handle any other artifacts.
-    #
-    CLASSES = [
-      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,
-    ].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

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,43 @@ module Hbc
         @dirmethod ||= "#{dsl_key}dir".to_sym
       end
 
-      def self.for_cask(cask)
+      def <=>(other)
-        cask.artifacts[dsl_key].to_a
+        return unless other.class < AbstractArtifact
+        return 0 if self.class == other.class
+
+        @@sort_order ||= [ # rubocop:disable Style/ClassVars
+          PreflightBlock,
+          # The `uninstall` stanza should be run first, as it may
+          # depend on other artifacts still being installed.
+          Uninstall,
+          # We want to extract nested containers before we
+          # handle any other artifacts.
+          NestedContainer,
+          Installer,
+          [
+            App,
+            Suite,
+            Artifact,
+            Colorpicker,
+            Prefpane,
+            Qlplugin,
+            Dictionary,
+            Font,
+            Service,
+            InputMethod,
+            InternetPlugin,
+            AudioUnitPlugin,
+            VstPlugin,
+            Vst3Plugin,
+            ScreenSaver,
+          ],
+          Binary,
+          Pkg,
+          PostflightBlock,
+          Zap,
+        ].each_with_index.flat_map { |classes, i| [*classes].map { |c| [c, i] } }.to_h
+
+        (@@sort_order[self.class] <=> @@sort_order[other.class]).to_i
       end
 
       # TODO: this sort of logic would make more sense in dsl.rb, or a

Library/Homebrew/cask/lib/hbc/artifact/abstract_flight_block.rb

@@ -11,12 +11,6 @@ module Hbc
         dsl_key.to_s.prepend("uninstall_").to_sym
       end
 
-      def self.for_cask(cask)
-        [dsl_key, uninstall_dsl_key].flat_map do |key|
-          [*cask.artifacts[key]].map { |block| new(cask, key => block) }
-        end
-      end
-
       attr_reader :directives
 
       def initialize(cask, **directives)

Library/Homebrew/cask/lib/hbc/audit.rb

@@ -218,7 +218,7 @@ module Hbc
     end
 
     def check_generic_artifacts
-      cask.artifacts[: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

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) }
+        cask.artifacts.each do |artifact|
-                                      .select { |artifact| artifact.respond_to?(:install_phase) }
+          next unless artifact.respond_to?(:install_phase)
-                                      .each do |artifact|
+          next unless DSL::ORDINARY_ARTIFACT_CLASSES.include?(artifact.class)
-                                        puts artifact.to_s
+          puts artifact.to_s
-                                      end
+        end
       end
     end
   end

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

Library/Homebrew/cask/lib/hbc/container/base.rb

@@ -20,7 +20,7 @@ module Hbc
 
         unless children.count == 1 &&
                !nested_container.directory? &&
-               @cask.artifacts[:nested_container].empty? &&
+               @cask.artifacts.none? { |a| a.is_a?(Artifact::NestedContainer) } &&
                extract_nested_container(nested_container)
 
           children.each do |src|

Library/Homebrew/cask/lib/hbc/dsl.rb

@@ -43,7 +43,7 @@ module Hbc
       Artifact::Zap,
     ].freeze
 
-    ACTIVATABLE_ARTIFACT_TYPES = (ORDINARY_ARTIFACT_CLASSES.map(&:dsl_key) - [:stage_only]).freeze
+    ACTIVATABLE_ARTIFACT_CLASSES = ORDINARY_ARTIFACT_CLASSES - [Artifact::StageOnly]
 
     ARTIFACT_BLOCK_CLASSES = [
       Artifact::PreflightBlock,
@@ -71,11 +71,12 @@ module Hbc
       :version,
       :appdir,
       *ORDINARY_ARTIFACT_CLASSES.map(&:dsl_key),
-      *ACTIVATABLE_ARTIFACT_TYPES,
+      *ACTIVATABLE_ARTIFACT_CLASSES.map(&:dsl_key),
       *ARTIFACT_BLOCK_CLASSES.flat_map { |klass| [klass.dsl_key, klass.uninstall_dsl_key] },
     ].freeze
 
-    attr_reader :token, :cask
+    attr_reader :cask, :token
+
     def initialize(cask)
       @cask = cask
       @token = cask.token
@@ -175,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[:nested_container] << Artifact::NestedContainer.new(cask, container.nested)
+              artifacts.add(Artifact::NestedContainer.new(cask, container.nested))
             end
           end
         end
@@ -218,7 +219,7 @@ module Hbc
     end
 
     def artifacts
-      @artifacts ||= Hash.new { |hash, key| hash[key] = Set.new }
+      @artifacts ||= SortedSet.new
     end
 
     def caskroom_path
@@ -250,15 +251,13 @@ module Hbc
     end
 
     ORDINARY_ARTIFACT_CLASSES.each do |klass|
-      type = klass.dsl_key
+      define_method(klass.dsl_key) do |*args|
-
-      define_method(type) do |*args|
         begin
-          if [*artifacts.keys, type].include?(:stage_only) && (artifacts.keys & ACTIVATABLE_ARTIFACT_TYPES).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[type].add(klass.from_args(cask, *args))
+          artifacts.add(klass.from_args(cask, *args))
         rescue CaskInvalidError
           raise
         rescue StandardError => e
@@ -270,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[dsl_key] << block
+          artifacts.add(klass.new(cask, dsl_key => block))
         end
       end
     end

Library/Homebrew/cask/lib/hbc/dsl/appcast.rb

@@ -3,17 +3,17 @@ require "hbc/system_command"
 module Hbc
   class DSL
     class Appcast
-      attr_reader :parameters, :checkpoint
+      attr_reader :uri, :checkpoint, :parameters
 
-      def initialize(uri, parameters = {})
+      def initialize(uri, **parameters)
-        @parameters     = parameters
+        @uri        = URI(uri)
-        @uri            = URI(uri)
+        @parameters = parameters
-        @checkpoint     = @parameters[:checkpoint]
+        @checkpoint = parameters[:checkpoint]
       end
 
       def calculate_checkpoint
         curl_executable, *args = curl_args(
-          "--compressed", "--location", "--fail", @uri,
+          "--compressed", "--location", "--fail", uri,
           user_agent: :fake
         )
         result = SystemCommand.run(curl_executable, args: args, print_stderr: false)
@@ -30,11 +30,11 @@ module Hbc
       end
 
       def to_yaml
-        [@uri, @parameters].to_yaml
+        [uri, parameters].to_yaml
       end
 
       def to_s
-        @uri.to_s
+        uri.to_s
       end
     end
   end

Library/Homebrew/cask/lib/hbc/dsl/base.rb

@@ -10,14 +10,14 @@ module Hbc
 
       def_delegators :@cask, :token, :version, :caskroom_path, :staged_path, :appdir, :language
 
-      def system_command(executable, options = {})
+      def system_command(executable, **options)
-        @command.run!(executable, options)
+        @command.run!(executable, **options)
       end
 
       def method_missing(method, *)
         if method
           underscored_class = self.class.name.gsub(/([[:lower:]])([[:upper:]][[:lower:]])/, '\1_\2').downcase
-          section = underscored_class.downcase.split("::").last
+          section = underscored_class.split("::").last
           Utils.method_missing_message(method, @cask.to_s, section)
           nil
         else

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"

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[: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)

Library/Homebrew/test/cask/artifact/alt_target_spec.rb

@@ -3,7 +3,11 @@ describe Hbc::Artifact::App, :cask do
     let(:cask) { Hbc::CaskLoader.load_from_file(TEST_FIXTURE_DIR/"cask/Casks/with-alt-target.rb") }
 
     let(:install_phase) {
-      -> { described_class.for_cask(cask).each { |artifact| artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false) } }
+      lambda do
+        cask.artifacts.select { |a| a.is_a?(described_class) }.each do |artifact|
+          artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false)
+        end
+      end
     }
 
     let(:source_path) { cask.staged_path.join("Caffeine.app") }

Library/Homebrew/test/cask/artifact/app_spec.rb

@@ -2,7 +2,7 @@ describe Hbc::Artifact::App, :cask do
   let(:cask) { Hbc::CaskLoader.load_from_file(TEST_FIXTURE_DIR/"cask/Casks/local-caffeine.rb") }
   let(:command) { Hbc::SystemCommand }
   let(:force) { false }
-  let(:app) { described_class.for_cask(cask).first }
+  let(:app) { cask.artifacts.find { |a| a.is_a?(described_class) } }
 
   let(:source_path) { cask.staged_path.join("Caffeine.app") }
   let(:target_path) { Hbc.appdir.join("Caffeine.app") }

Library/Homebrew/test/cask/artifact/binary_spec.rb

@@ -4,6 +4,7 @@ describe Hbc::Artifact::Binary, :cask do
       InstallHelper.install_without_artifacts(cask)
     end
   }
+  let(:artifacts) { cask.artifacts.select { |a| a.is_a?(described_class) } }
   let(:expected_path) { Hbc.binarydir.join("binary") }
 
   before(:each) do
@@ -26,8 +27,9 @@ describe Hbc::Artifact::Binary, :cask do
   end
 
   it "links the binary to the proper directory" do
-    described_class.for_cask(cask)
+    artifacts.each do |artifact|
-      .each { |artifact| artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false) }
+      artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false)
+    end
 
     expect(expected_path).to be_a_symlink
     expect(expected_path.readlink).to exist
@@ -46,8 +48,9 @@ describe Hbc::Artifact::Binary, :cask do
       expect(FileUtils).to receive(:chmod)
         .with("+x", cask.staged_path.join("naked_non_executable")).and_call_original
 
-      described_class.for_cask(cask)
+      artifacts.each do |artifact|
-      .each { |artifact| artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false) }
+        artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false)
+      end
 
       expect(expected_path).to be_a_symlink
       expect(expected_path.readlink).to be_executable
@@ -58,8 +61,9 @@ describe Hbc::Artifact::Binary, :cask do
     FileUtils.touch expected_path
 
     expect {
-      described_class.for_cask(cask)
+      artifacts.each do |artifact|
-        .each { |artifact| artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false) }
+        artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false)
+      end
     }.to raise_error(Hbc::CaskError)
 
     expect(expected_path).not_to be :symlink?
@@ -68,8 +72,9 @@ describe Hbc::Artifact::Binary, :cask do
   it "clobbers an existing symlink" do
     expected_path.make_symlink("/tmp")
 
-    described_class.for_cask(cask)
+    artifacts.each do |artifact|
-      .each { |artifact| artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false) }
+      artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false)
+    end
 
     expect(File.readlink(expected_path)).not_to eq("/tmp")
   end
@@ -77,8 +82,9 @@ describe Hbc::Artifact::Binary, :cask do
   it "creates parent directory if it doesn't exist" do
     FileUtils.rmdir Hbc.binarydir
 
-    described_class.for_cask(cask)
+    artifacts.each do |artifact|
-      .each { |artifact| artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false) }
+      artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false)
+    end
 
     expect(expected_path.exist?).to be true
   end
@@ -91,10 +97,12 @@ describe Hbc::Artifact::Binary, :cask do
     }
 
     it "links the binary to the proper directory" do
-      Hbc::Artifact::App.for_cask(cask)
+      cask.artifacts.select { |a| a.is_a?(Hbc::Artifact::App) }.each do |artifact|
-        .each { |artifact| artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false) }
+        artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false)
-      described_class.for_cask(cask)
+      end
-        .each { |artifact| artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false) }
+      artifacts.each do |artifact|
+        artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false)
+      end
 
       expect(expected_path).to be_a_symlink
       expect(expected_path.readlink).to exist

Library/Homebrew/test/cask/artifact/generic_artifact_spec.rb

@@ -2,7 +2,11 @@ describe Hbc::Artifact::Artifact, :cask do
   let(:cask) { Hbc::CaskLoader.load_from_file(TEST_FIXTURE_DIR/"cask/Casks/with-generic-artifact.rb") }
 
   let(:install_phase) {
-    -> { described_class.for_cask(cask).each { |artifact| artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false) } }
+    lambda do
+      cask.artifacts.select { |a| a.is_a?(described_class) }.each do |artifact|
+        artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false)
+      end
+    end
   }
 
   let(:source_path) { cask.staged_path.join("Caffeine.app") }

Library/Homebrew/test/cask/artifact/nested_container_spec.rb

@@ -5,8 +5,9 @@ describe Hbc::Artifact::NestedContainer, :cask do
         InstallHelper.install_without_artifacts(c)
       end
 
-      described_class.for_cask(cask)
+      cask.artifacts.select { |a| a.is_a?(described_class) }.each do |artifact|
-        .each { |artifact| artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false) }
+        artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false)
+      end
 
       expect(cask.staged_path.join("MyNestedApp.app")).to be_a_directory
     end

Library/Homebrew/test/cask/artifact/pkg_spec.rb

@@ -8,7 +8,7 @@ describe Hbc::Artifact::Pkg, :cask do
 
   describe "install_phase" do
     it "runs the system installer on the specified pkgs" do
-      pkg = described_class.for_cask(cask).first
+      pkg = cask.artifacts.find { |a| a.is_a?(described_class) }
 
       expect(fake_system_command).to receive(:run!).with(
         "/usr/sbin/installer",
@@ -25,7 +25,7 @@ describe Hbc::Artifact::Pkg, :cask do
     let(:cask) { Hbc::CaskLoader.load_from_file(TEST_FIXTURE_DIR/"cask/Casks/with-choices.rb") }
 
     it "passes the choice changes xml to the system installer" do
-      pkg = described_class.for_cask(cask).first
+      pkg = cask.artifacts.find { |a| a.is_a?(described_class) }
 
       file = double(path: Pathname.new("/tmp/choices.xml"))
 

Library/Homebrew/test/cask/artifact/postflight_block_spec.rb

@@ -11,8 +11,9 @@ describe Hbc::Artifact::PostflightBlock, :cask do
         end
       end
 
-      described_class.for_cask(cask)
+      cask.artifacts.select { |a| a.is_a?(described_class) }.each do |artifact|
-        .each { |artifact| artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false) }
+        artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false)
+      end
 
       expect(called).to be true
       expect(yielded_arg).to be_kind_of(Hbc::DSL::Postflight)
@@ -31,8 +32,9 @@ describe Hbc::Artifact::PostflightBlock, :cask do
         end
       end
 
-      described_class.for_cask(cask)
+      cask.artifacts.select { |a| a.is_a?(described_class) }.each do |artifact|
-        .each { |artifact| artifact.uninstall_phase(command: Hbc::NeverSudoSystemCommand, force: false) }
+        artifact.uninstall_phase(command: Hbc::NeverSudoSystemCommand, force: false)
+      end
 
       expect(called).to be true
       expect(yielded_arg).to be_kind_of(Hbc::DSL::UninstallPostflight)

Library/Homebrew/test/cask/artifact/preflight_block_spec.rb

@@ -11,8 +11,9 @@ describe Hbc::Artifact::PreflightBlock, :cask do
         end
       end
 
-      described_class.for_cask(cask)
+      cask.artifacts.select { |a| a.is_a?(described_class) }.each do |artifact|
-        .each { |artifact| artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false) }
+        artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false)
+      end
 
       expect(called).to be true
       expect(yielded_arg).to be_kind_of Hbc::DSL::Preflight
@@ -31,8 +32,9 @@ describe Hbc::Artifact::PreflightBlock, :cask do
         end
       end
 
-      described_class.for_cask(cask)
+      cask.artifacts.select { |a| a.is_a?(described_class) }.each do |artifact|
-        .each { |artifact| artifact.uninstall_phase(command: Hbc::NeverSudoSystemCommand, force: false) }
+        artifact.uninstall_phase(command: Hbc::NeverSudoSystemCommand, force: false)
+      end
 
       expect(called).to be true
       expect(yielded_arg).to be_kind_of Hbc::DSL::UninstallPreflight

Library/Homebrew/test/cask/artifact/suite_spec.rb

@@ -2,7 +2,11 @@ describe Hbc::Artifact::Suite, :cask do
   let(:cask) { Hbc::CaskLoader.load_from_file(TEST_FIXTURE_DIR/"cask/Casks/with-suite.rb") }
 
   let(:install_phase) {
-    -> { described_class.for_cask(cask).each { |artifact| artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false) } }
+    lambda do
+      cask.artifacts.select { |a| a.is_a?(described_class) }.each do |artifact|
+        artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false)
+      end
+    end
   }
 
   let(:target_path) { Hbc.appdir.join("Caffeine") }

Library/Homebrew/test/cask/artifact/two_apps_correct_spec.rb

@@ -3,7 +3,11 @@ describe Hbc::Artifact::App, :cask do
     let(:cask) { Hbc::CaskLoader.load_from_file(TEST_FIXTURE_DIR/"cask/Casks/with-two-apps-correct.rb") }
 
     let(:install_phase) {
-      -> { described_class.for_cask(cask).each { |artifact| artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false) } }
+      lambda do
+        cask.artifacts.select { |a| a.is_a?(described_class) }.each do |artifact|
+          artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false)
+        end
+      end
     }
 
     let(:source_path_mini) { cask.staged_path.join("Caffeine Mini.app") }

Library/Homebrew/test/cask/artifact/uninstall_no_zap_spec.rb

@@ -2,7 +2,7 @@ describe Hbc::Artifact::Zap, :cask do
   let(:cask) { Hbc::CaskLoader.load_from_file(TEST_FIXTURE_DIR/"cask/Casks/with-installable.rb") }
 
   let(:zap_artifact) {
-    described_class.for_cask(cask).first
+    cask.artifacts.find { |a| a.is_a?(described_class) }
   }
 
   before(:each) do

Library/Homebrew/test/cask/artifact/uninstall_zap_shared_examples.rb

@@ -1,6 +1,6 @@
 shared_examples "#uninstall_phase or #zap_phase" do
   let(:artifact_dsl_key) { described_class.dsl_key }
-  let(:artifact) { described_class.for_cask(cask).first }
+  let(:artifact) { cask.artifacts.find { |a| a.is_a?(described_class) } }
   let(:fake_system_command) { Hbc::FakeSystemCommand }
 
   subject { artifact.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command) }

Library/Homebrew/test/cask/cli/list_spec.rb

@@ -72,8 +72,9 @@ describe Hbc::CLI::List, :cask do
     it "lists the installed files for those Casks" do
       casks.each(&InstallHelper.method(:install_without_artifacts_with_caskfile))
 
-      Hbc::Artifact::App.for_cask(transmission)
+      transmission.artifacts.select { |a| a.is_a?(Hbc::Artifact::App) }.each do |artifact|
-        .each { |artifact| artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false) }
+        artifact.install_phase(command: Hbc::NeverSudoSystemCommand, force: false)
+      end
 
       expect {
         described_class.run("local-transmission", "local-caffeine")

Library/Homebrew/test/cask/dsl_spec.rb

@@ -216,12 +216,12 @@ describe Hbc::DSL, :cask do
         app "Bar.app"
       end
 
-      expect(cask.artifacts[:app].map(&:to_s)).to eq(["Foo.app (App)", "Bar.app (App)"])
+      expect(cask.artifacts.map(&:to_s)).to eq(["Foo.app (App)", "Bar.app (App)"])
     end
 
     it "allow app stanzas to be empty" do
       cask = Hbc::Cask.new("cask-with-no-apps")
-      expect(cask.artifacts[:app]).to be_empty
+      expect(cask.artifacts).to be_empty
     end
   end
 
@@ -249,7 +249,7 @@ describe Hbc::DSL, :cask do
         pkg "Bar.pkg"
       end
 
-      expect(cask.artifacts[:pkg].map(&:to_s)).to eq(["Foo.pkg (Pkg)", "Bar.pkg (Pkg)"])
+      expect(cask.artifacts.map(&:to_s)).to eq(["Foo.pkg (Pkg)", "Bar.pkg (Pkg)"])
     end
   end
 
@@ -501,10 +501,10 @@ describe Hbc::DSL, :cask do
       let(:token) { "with-installer-script" }
 
       it "allows installer script to be specified" do
-        expect(cask.artifacts[:installer].first.path).to eq(Pathname("/usr/bin/true"))
+        expect(cask.artifacts.to_a[0].path).to eq(Pathname("/usr/bin/true"))
-        expect(cask.artifacts[:installer].first.args[:args]).to eq(["--flag"])
+        expect(cask.artifacts.to_a[0].args[:args]).to eq(["--flag"])
-        expect(cask.artifacts[:installer].to_a[1].path).to eq(Pathname("/usr/bin/false"))
+        expect(cask.artifacts.to_a[1].path).to eq(Pathname("/usr/bin/false"))
-        expect(cask.artifacts[:installer].to_a[1].args[:args]).to eq(["--flag"])
+        expect(cask.artifacts.to_a[1].args[:args]).to eq(["--flag"])
       end
     end
 
@@ -512,7 +512,7 @@ describe Hbc::DSL, :cask do
       let(:token) { "with-installer-manual" }
 
       it "allows installer manual to be specified" do
-        installer = cask.artifacts[:installer].first
+        installer = cask.artifacts.first
         expect(installer).to be_a(Hbc::Artifact::Installer::ManualInstaller)
         expect(installer.path).to eq(cask.staged_path.join("Caffeine.app"))
       end
@@ -524,7 +524,7 @@ describe Hbc::DSL, :cask do
       let(:token) { "stage-only" }
 
       it "allows stage_only stanza to be specified" do
-        expect(cask.artifacts[:stage_only]).not_to be_empty
+        expect(cask.artifacts).to contain_exactly a_kind_of Hbc::Artifact::StageOnly
       end
     end
 
@@ -545,12 +545,12 @@ describe Hbc::DSL, :cask do
     end
   end
 
-  describe "appdir" do
+  describe "#appdir" do
     context "interpolation of the appdir in stanzas" do
       let(:token) { "appdir-interpolation" }
 
       it "is allowed" do
-        expect(cask.artifacts[:binary].first.source).to eq(Hbc.appdir/"some/path")
+        expect(cask.artifacts.first.source).to eq(Hbc.appdir/"some/path")
       end
     end
 
@@ -563,10 +563,35 @@ describe Hbc::DSL, :cask do
           binary "#{appdir}/some/path"
         end
 
-        expect(cask.artifacts[:binary].first.source).to eq(original_appdir/"some/path")
+        expect(cask.artifacts.first.source).to eq(original_appdir/"some/path")
       ensure
         Hbc.appdir = original_appdir
       end
     end
   end
+
+  describe "#artifacts" do
+    it "sorts artifacts according to the preferable installation order" do
+      cask = Hbc::Cask.new("appdir-trailing-slash") do
+        postflight do
+          next
+        end
+
+        preflight do
+          next
+        end
+
+        binary "binary"
+
+        app "App.app"
+      end
+
+      expect(cask.artifacts.map(&:class).map(&:dsl_key)).to eq [
+        :preflight,
+        :app,
+        :binary,
+        :postflight,
+      ]
+    end
+  end
 end