Refactor Cask `Artifact`s.

11 files changed, 16 insertions, 100 deletions

diff --git a/Library/Homebrew/cask/lib/hbc/artifact/base.rb b/Library/Homebrew/cask/lib/hbc/artifact/base.rb
index b53c13f08..d925ff340 100644
--- a/Library/Homebrew/cask/lib/hbc/artifact/base.rb
+++ b/Library/Homebrew/cask/lib/hbc/artifact/base.rb
@@ -27,10 +27,6 @@ module Hbc
 
       attr_reader :force
 
-      def zap_phase
-        odebug "Nothing to do. The #{self.class.artifact_name} artifact has no zap phase."
-      end
-
       # TODO: this sort of logic would make more sense in dsl.rb, or a
       #       constructor called from dsl.rb, so long as that isn't slow.
       def self.read_script_arguments(arguments, stanza, default_arguments = {}, override_arguments = {}, key = nil)
diff --git a/Library/Homebrew/cask/lib/hbc/artifact/installer.rb b/Library/Homebrew/cask/lib/hbc/artifact/installer.rb
index b64b00fe3..55e8d38c0 100644
--- a/Library/Homebrew/cask/lib/hbc/artifact/installer.rb
+++ b/Library/Homebrew/cask/lib/hbc/artifact/installer.rb
@@ -3,16 +3,6 @@ require "hbc/artifact/base"
 module Hbc
   module Artifact
     class Installer < Base
-      # TODO: for backward compatibility, removeme
-      def install
-        install_phase
-      end
-
-      # TODO: for backward compatibility, removeme
-      def uninstall
-        uninstall_phase
-      end
-
       def install_phase
         @cask.artifacts[self.class.artifact_dsl_key].each do |artifact|
           if artifact.manual
@@ -36,10 +26,6 @@ module Hbc
           end
         end
       end
-
-      def uninstall_phase
-        odebug "Nothing to do. The #{self.class.artifact_dsl_key} artifact has no uninstall phase."
-      end
     end
   end
 end
diff --git a/Library/Homebrew/cask/lib/hbc/artifact/nested_container.rb b/Library/Homebrew/cask/lib/hbc/artifact/nested_container.rb
index 107640797..45f23fe37 100644
--- a/Library/Homebrew/cask/lib/hbc/artifact/nested_container.rb
+++ b/Library/Homebrew/cask/lib/hbc/artifact/nested_container.rb
@@ -7,10 +7,6 @@ module Hbc
         @cask.artifacts[:nested_container].each { |container| extract(container) }
       end
 
-      def uninstall_phase
-        # no need to take action; is removed after extraction
-      end
-
       def extract(container_relative_path)
         source = @cask.staged_path.join(container_relative_path)
         container = Container.for_path(source, @command)
diff --git a/Library/Homebrew/cask/lib/hbc/artifact/pkg.rb b/Library/Homebrew/cask/lib/hbc/artifact/pkg.rb
index 0569d2a86..6d6362d46 100644
--- a/Library/Homebrew/cask/lib/hbc/artifact/pkg.rb
+++ b/Library/Homebrew/cask/lib/hbc/artifact/pkg.rb
@@ -36,10 +36,6 @@ module Hbc
         @cask.artifacts[:pkg].each { |pkg_description| run_installer(pkg_description) }
       end
 
-      def uninstall_phase
-        # Do nothing. Must be handled explicitly by a separate :uninstall stanza.
-      end
-
       def run_installer(pkg_description)
         load_pkg_description pkg_description
         ohai "Running installer for #{@cask}; your password may be necessary."
diff --git a/Library/Homebrew/cask/lib/hbc/artifact/stage_only.rb b/Library/Homebrew/cask/lib/hbc/artifact/stage_only.rb
index 594c5bef9..1122c1d02 100644
--- a/Library/Homebrew/cask/lib/hbc/artifact/stage_only.rb
+++ b/Library/Homebrew/cask/lib/hbc/artifact/stage_only.rb
@@ -6,14 +6,6 @@ module Hbc
       def self.artifact_dsl_key
         :stage_only
       end
-
-      def install_phase
-        # do nothing
-      end
-
-      def uninstall_phase
-        # do nothing
-      end
     end
   end
 end
diff --git a/Library/Homebrew/cask/lib/hbc/artifact/uninstall_base.rb b/Library/Homebrew/cask/lib/hbc/artifact/uninstall_base.rb
index ccb06a9ab..e18e9c311 100644
--- a/Library/Homebrew/cask/lib/hbc/artifact/uninstall_base.rb
+++ b/Library/Homebrew/cask/lib/hbc/artifact/uninstall_base.rb
@@ -54,15 +54,11 @@ module Hbc
         path_strings - undeletable
       end
 
-      def install_phase
-        odebug "Nothing to do. The uninstall artifact has no install phase."
-      end
-
       def uninstall_phase
         dispatch_uninstall_directives
       end
 
-      def dispatch_uninstall_directives(expand_tilde = true)
+      def dispatch_uninstall_directives(expand_tilde: true)
         directives_set = @cask.artifacts[stanza]
         ohai "Running #{stanza} process for #{@cask}; your password may be necessary"
 
diff --git a/Library/Homebrew/cask/lib/hbc/artifact/zap.rb b/Library/Homebrew/cask/lib/hbc/artifact/zap.rb
index 503ea35c4..b31e2ef11 100644
--- a/Library/Homebrew/cask/lib/hbc/artifact/zap.rb
+++ b/Library/Homebrew/cask/lib/hbc/artifact/zap.rb
@@ -3,17 +3,8 @@ require "hbc/artifact/uninstall_base"
 module Hbc
   module Artifact
     class Zap < UninstallBase
-      def install_phase
-        odebug "Nothing to do. The zap artifact has no install phase."
-      end
-
       def uninstall_phase
-        odebug "Nothing to do. The zap artifact has no uninstall phase."
-      end
-
-      def zap_phase
-        expand_tilde = true
-        dispatch_uninstall_directives(expand_tilde)
+        dispatch_uninstall_directives(expand_tilde: true)
       end
     end
   end
diff --git a/Library/Homebrew/cask/lib/hbc/installer.rb b/Library/Homebrew/cask/lib/hbc/installer.rb
index 465011735..28f67e5fd 100644
--- a/Library/Homebrew/cask/lib/hbc/installer.rb
+++ b/Library/Homebrew/cask/lib/hbc/installer.rb
@@ -140,15 +140,18 @@ module Hbc
       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}"
-        artifact.new(@cask, options).install_phase
+        artifact.install_phase
         already_installed_artifacts.unshift(artifact)
       end
     rescue StandardError => e
       begin
         already_installed_artifacts.each do |artifact|
+          next unless artifact.respond_to?(:uninstall_phase)
           odebug "Reverting installation of artifact of class #{artifact}"
-          artifact.new(@cask, options).uninstall_phase
+          artifact.uninstall_phase
         end
       ensure
         purge_versioned_files
@@ -319,9 +322,11 @@ module Hbc
       artifacts = Artifact.for_cask(@cask)
       odebug "#{artifacts.length} artifact/s defined", artifacts
       artifacts.each do |artifact|
-        odebug "Un-installing artifact of class #{artifact}"
         options = { command: @command, force: force }
-        artifact.new(@cask, options).uninstall_phase
+        artifact = artifact.new(@cask, options)
+        next unless artifact.respond_to?(:uninstall_phase)
+        odebug "Un-installing artifact of class #{artifact}"
+        artifact.uninstall_phase
       end
     end
 
@@ -330,7 +335,7 @@ module Hbc
       uninstall_artifacts
       if Artifact::Zap.me?(@cask)
         ohai "Dispatching zap stanza"
-        Artifact::Zap.new(@cask, command: @command).zap_phase
+        Artifact::Zap.new(@cask, command: @command).uninstall_phase
       else
         opoo "No zap stanza present for Cask '#{@cask}'"
       end
diff --git a/Library/Homebrew/cask/test/cask/artifact/pkg_test.rb b/Library/Homebrew/cask/test/cask/artifact/pkg_test.rb
index a77fb0a07..b054290ce 100644
--- a/Library/Homebrew/cask/test/cask/artifact/pkg_test.rb
+++ b/Library/Homebrew/cask/test/cask/artifact/pkg_test.rb
@@ -21,16 +21,6 @@ describe Hbc::Artifact::Pkg do
     end
   end
 
-  describe "uninstall_phase" do
-    it "does nothing, because the uninstall_phase method is a no-op" do
-      pkg = Hbc::Artifact::Pkg.new(@cask,
-                                   command: Hbc::FakeSystemCommand)
-      shutup do
-        pkg.uninstall_phase
-      end
-    end
-  end
-
   describe "choices" do
     before do
       @cask = Hbc::CaskLoader.load_from_file(TEST_FIXTURE_DIR/"cask/Casks/with-choices.rb")
diff --git a/Library/Homebrew/cask/test/cask/artifact/uninstall_test.rb b/Library/Homebrew/cask/test/cask/artifact/uninstall_test.rb
index 3fb679196..464cad345 100644
--- a/Library/Homebrew/cask/test/cask/artifact/uninstall_test.rb
+++ b/Library/Homebrew/cask/test/cask/artifact/uninstall_test.rb
@@ -13,28 +13,12 @@ describe Hbc::Artifact::Uninstall do
     end
   end
 
-  describe "install_phase" do
-    it "does nothing, because the install_phase method is a no-op" do
-      shutup do
-        uninstall_artifact.install_phase
-      end
-    end
-  end
-
-  describe "zap_phase" do
-    it "does nothing, because the zap_phase method is a no-op" do
-      shutup do
-        uninstall_artifact.zap_phase
-      end
-    end
-  end
-
   describe "uninstall_phase" do
-    subject do
+    subject {
       shutup do
         uninstall_artifact.uninstall_phase
       end
-    end
+    }
 
     describe "when using launchctl" do
       let(:cask) { Hbc::CaskLoader.load_from_file(TEST_FIXTURE_DIR/"cask/Casks/with-uninstall-launchctl.rb") }
diff --git a/Library/Homebrew/cask/test/cask/artifact/zap_test.rb b/Library/Homebrew/cask/test/cask/artifact/zap_test.rb
index 0ebd9b304..ea546728b 100644
--- a/Library/Homebrew/cask/test/cask/artifact/zap_test.rb
+++ b/Library/Homebrew/cask/test/cask/artifact/zap_test.rb
@@ -14,28 +14,12 @@ describe Hbc::Artifact::Zap do
     end
   end
 
-  describe "install_phase" do
-    it "does nothing, because the install_phase method is a no-op" do
-      shutup do
-        zap_artifact.install_phase
-      end
-    end
-  end
-
   describe "uninstall_phase" do
-    it "does nothing, because the uninstall_phase method is a no-op" do
+    subject {
       shutup do
         zap_artifact.uninstall_phase
       end
-    end
-  end
-
-  describe "zap_phase" do
-    subject do
-      shutup do
-        zap_artifact.zap_phase
-      end
-    end
+    }
 
     describe "when using launchctl" do
       let(:cask) { Hbc::CaskLoader.load_from_file(TEST_FIXTURE_DIR/"cask/Casks/with-zap-launchctl.rb") }