Merge pull request #1956 from reitermarkus/refactor-artifacts

Refactor `Artifact`.

4 files changed, 47 insertions, 50 deletions

diff --git a/Library/Homebrew/cask/lib/hbc/artifact.rb b/Library/Homebrew/cask/lib/hbc/artifact.rb
index 8b4babdf6..b155a125a 100644
--- a/Library/Homebrew/cask/lib/hbc/artifact.rb
+++ b/Library/Homebrew/cask/lib/hbc/artifact.rb
@@ -27,41 +27,39 @@ module Hbc
   module Artifact
     # NOTE: order is important here, since we want to extract nested containers
     #       before we handle any other artifacts
-    def self.artifacts
-      [
-        PreflightBlock,
-        NestedContainer,
-        Installer,
-        App,
-        Suite,
-        Artifact, # generic 'artifact' stanza
-        Colorpicker,
-        Pkg,
-        Prefpane,
-        Qlplugin,
-        Dictionary,
-        Font,
-        Service,
-        StageOnly,
-        Binary,
-        InputMethod,
-        InternetPlugin,
-        AudioUnitPlugin,
-        VstPlugin,
-        Vst3Plugin,
-        ScreenSaver,
-        Uninstall,
-        PostflightBlock,
-        Zap,
-      ]
-    end
+    TYPES = [
+      PreflightBlock,
+      NestedContainer,
+      Installer,
+      App,
+      Suite,
+      Artifact, # generic 'artifact' stanza
+      Colorpicker,
+      Pkg,
+      Prefpane,
+      Qlplugin,
+      Dictionary,
+      Font,
+      Service,
+      StageOnly,
+      Binary,
+      InputMethod,
+      InternetPlugin,
+      AudioUnitPlugin,
+      VstPlugin,
+      Vst3Plugin,
+      ScreenSaver,
+      Uninstall,
+      PostflightBlock,
+      Zap,
+    ].freeze
 
-    def self.for_cask(cask)
+    def self.for_cask(cask, command: SystemCommand, force: false)
       odebug "Determining which artifacts are present in Cask #{cask}"
-      artifacts.select do |artifact|
-        odebug "Checking for artifact class #{artifact}"
-        artifact.me?(cask)
-      end
+
+      TYPES
+        .select { |klass| klass.me?(cask) }
+        .map { |klass| klass.new(cask, command: command, force: force) }
     end
   end
 end
diff --git a/Library/Homebrew/cask/lib/hbc/cli/list.rb b/Library/Homebrew/cask/lib/hbc/cli/list.rb
index 4094b3d38..e100fbd83 100644
--- a/Library/Homebrew/cask/lib/hbc/cli/list.rb
+++ b/Library/Homebrew/cask/lib/hbc/cli/list.rb
@@ -55,7 +55,7 @@ module Hbc
 
       def self.list_artifacts(cask)
         Artifact.for_cask(cask).each do |artifact|
-          summary = artifact.new(cask).summary
+          summary = artifact.summary
           ohai summary[:english_description], summary[:contents] unless summary.empty?
         end
       end
diff --git a/Library/Homebrew/cask/lib/hbc/dsl.rb b/Library/Homebrew/cask/lib/hbc/dsl.rb
index c62873665..4707ae76a 100644
--- a/Library/Homebrew/cask/lib/hbc/dsl.rb
+++ b/Library/Homebrew/cask/lib/hbc/dsl.rb
@@ -270,14 +270,17 @@ module Hbc
 
     ORDINARY_ARTIFACT_TYPES.each do |type|
       define_method(type) do |*args|
-        if type == :stage_only && args != [true]
-          raise CaskInvalidError.new(token, "'stage_only' takes a single argument: true")
-        end
-        artifacts[type] << args
-        if artifacts.key?(:stage_only) && artifacts.keys.count > 1 &&
-           !(artifacts.keys & ACTIVATABLE_ARTIFACT_TYPES).empty?
-          raise CaskInvalidError.new(token, "'stage_only' must be the only activatable artifact")
+        if type == :stage_only
+          if args != [true]
+            raise CaskInvalidError.new(token, "'stage_only' takes a single argument: true")
+          end
+
+          unless (artifacts.keys & ACTIVATABLE_ARTIFACT_TYPES).empty?
+            raise CaskInvalidError.new(token, "'stage_only' must be the only activatable artifact")
+          end
         end
+
+        artifacts[type].add(args)
       end
     end
 
diff --git a/Library/Homebrew/cask/lib/hbc/installer.rb b/Library/Homebrew/cask/lib/hbc/installer.rb
index 28f67e5fd..776a3acd2 100644
--- a/Library/Homebrew/cask/lib/hbc/installer.rb
+++ b/Library/Homebrew/cask/lib/hbc/installer.rb
@@ -133,16 +133,14 @@ module Hbc
 
     def install_artifacts
       already_installed_artifacts = []
-      options = { command: @command, force: force }
 
       odebug "Installing artifacts"
-      artifacts = Artifact.for_cask(@cask)
+      artifacts = Artifact.for_cask(@cask, command: @command, force: force)
       odebug "#{artifacts.length} artifact/s defined", artifacts
 
       artifacts.each do |artifact|
-        artifact = artifact.new(@cask, options)
         next unless artifact.respond_to?(:install_phase)
-        odebug "Installing artifact of class #{artifact}"
+        odebug "Installing artifact of class #{artifact.class}"
         artifact.install_phase
         already_installed_artifacts.unshift(artifact)
       end
@@ -150,7 +148,7 @@ module Hbc
       begin
         already_installed_artifacts.each do |artifact|
           next unless artifact.respond_to?(:uninstall_phase)
-          odebug "Reverting installation of artifact of class #{artifact}"
+          odebug "Reverting installation of artifact of class #{artifact.class}"
           artifact.uninstall_phase
         end
       ensure
@@ -319,13 +317,11 @@ module Hbc
 
     def uninstall_artifacts
       odebug "Un-installing artifacts"
-      artifacts = Artifact.for_cask(@cask)
+      artifacts = Artifact.for_cask(@cask, command: @command, force: force)
       odebug "#{artifacts.length} artifact/s defined", artifacts
       artifacts.each do |artifact|
-        options = { command: @command, force: force }
-        artifact = artifact.new(@cask, options)
         next unless artifact.respond_to?(:uninstall_phase)
-        odebug "Un-installing artifact of class #{artifact}"
+        odebug "Un-installing artifact of class #{artifact.class}"
         artifact.uninstall_phase
       end
     end