bugfix: make uninstall :script accept a hash

The indentation in CONTRIBUTING.md implies that :script accepts a hash.
But that is not the case. Instead :script accesses the entire hash defined
by uninstall.  Unrelated install keys such as :quit leak through to
@command.run! when attempting to exec :script.  Result: contrary to docs,
:script cannot be combined with other uninstall keys.

This PR makes uninstall :script accept a hash.  When :args or :input are not
needed, :script can still accept a plain string, so many Casks require no
alteration.

In addition
- adds key :executable, required when using hash argument to :script
- keys to :script are validated and sanitized before being passed to
  @command.run!
- adds :quit to with-installable.rb test to protect against regression
  on leaky keys

CONTRIBUTING.md

@@ -215,7 +215,8 @@ many features to help properly remove a Cask-installed application.
 These features are utilized via a hash argument to `uninstall` with any number
 of the following keys:
 
-* `:script` (string) - relative path to an uninstall script to be run via sudo
+* `:script` (string or hash) - relative path to an uninstall script to be run via sudo; use hash if args are needed
+  - `:executable` - relative path to an uninstall script to be run via sudo (required for hash)
   - `:args` - array of arguments to the uninstall script
   - `:input` - array of lines of input to be sent to `stdin` of the script
 * `:launchctl` (string or array) - ids of launchctl services to remove

Casks/boxcryptor-classic.rb

@@ -4,5 +4,5 @@ class BoxcryptorClassic < Cask
   version 'latest'
   no_checksum
   install 'Install Boxcryptor Classic.pkg'
-  uninstall :script => 'Uninstall.command', :args => %w[--unattended]
+  uninstall :script => { :executable => 'Uninstall.command', :args => %w[--unattended] }
 end

Casks/totalfinder.rb

@@ -4,6 +4,8 @@ class Totalfinder < Cask
   version '1.5.6'
   sha1 'ce0e2fe7fc3bd7b98622c95e2bb73e9fb2f55546'
   install 'TotalFinder.pkg'
-  uninstall :script => 'TotalFinder Uninstaller.app/Contents/MacOS/TotalFinder Uninstaller',
-            :args => %w[--headless]
+  uninstall :script => {
+    :executable => 'TotalFinder Uninstaller.app/Contents/MacOS/TotalFinder Uninstaller',
+    :args => %w[--headless]
+  }
 end

Casks/totalterminal.rb

@@ -4,6 +4,8 @@ class Totalterminal < Cask
   version '1.4.6'
   sha1 'd0163f80f4c993b05952cef84b55aabaa37af0cc'
   install 'TotalTerminal.pkg'
-  uninstall :script => 'TotalTerminal Uninstaller.app/Contents/MacOS/TotalTerminal Uninstaller',
-            :args => %w[--headless]
+  uninstall :script => {
+    :executable => 'TotalTerminal Uninstaller.app/Contents/MacOS/TotalTerminal Uninstaller',
+    :args => %w[--headless]
+  }
 end

Casks/vagrant.rb

@@ -4,5 +4,5 @@ class Vagrant < Cask
   version '1.4.3'
   sha1 '78d20fbe44704d91b7958af57308eb0fde147ed4'
   install 'Vagrant.pkg'
-  uninstall :script => 'uninstall.tool', :input => %w[Yes]
+  uninstall :script => { :executable => 'uninstall.tool', :input => %w[Yes] }
 end

Casks/virtualbox.rb

@@ -4,5 +4,5 @@ class Virtualbox < Cask
   version '4.3.6-91406'
   sha1 'b268e52552449304ba8021687645a6f9d7d18920'
   install 'VirtualBox.pkg'
-  uninstall :script => 'VirtualBox_Uninstall.tool', :args => %w[--unattended]
+  uninstall :script => { :executable => 'VirtualBox_Uninstall.tool', :args => %w[--unattended] }
 end

lib/cask/artifact/pkg.rb

@@ -4,6 +4,33 @@ class Cask::Artifact::Pkg < Cask::Artifact::Base
     :install
   end
 
+  def self.read_script_arguments(uninstall_options, key)
+    script_arguments = uninstall_options[key]
+
+    # backwards-compatible string value
+    if script_arguments.kind_of?(String)
+      script_arguments = { :executable => script_arguments }
+    end
+
+    # key sanity
+    permitted_keys = [:args, :input, :executable]
+    unknown_keys = script_arguments.keys - permitted_keys
+    unless unknown_keys.empty?
+      opoo "Unknown arguments to uninstall :#{key} -- :#{unknown_keys.join(", :")} (ignored). Running `brew update; brew upgrade brew-cask` will likely fix it.'"
+    end
+    script_arguments.reject! {|k,v| ! permitted_keys.include?(k)}
+
+    # extract executable
+    if script_arguments.key?(:executable)
+      executable = script_arguments.delete(:executable)
+    else
+      executable = nil
+    end
+
+    script_arguments.merge!(:sudo => true, :print => true)
+    return executable, script_arguments
+  end
+
   def install
     @cask.artifacts[:install].each { |pkg| run_installer(pkg) }
   end
@@ -30,11 +57,11 @@ class Cask::Artifact::Pkg < Cask::Artifact::Base
     end
 
     ohai "Running uninstall process for #{@cask}; your password may be necessary."
+
     if uninstall_options.key? :script
-      @command.run!(
-        @cask.destination_path.join(uninstall_options[:script]),
-        uninstall_options.merge(:sudo => true, :print => true)
-      )
+      executable, script_arguments = self.class.read_script_arguments(uninstall_options, :script)
+      raise "Error in Cask #{@cask}: uninstall :script without :executable." if executable.nil?
+      @command.run!(@cask.destination_path.join(executable), script_arguments)
     end
 
     if uninstall_options.key? :launchctl

test/cask/artifact/pkg_test.rb

@@ -27,6 +27,9 @@ describe Cask::Artifact::Pkg do
     it 'runs the specified uninstaller for the cask' do
       pkg = Cask::Artifact::Pkg.new(@cask, Cask::FakeSystemCommand)
 
+      Cask::FakeSystemCommand.stubs_command(%Q(sudo -E '/usr/bin/osascript' '-e' 'tell application "System Events" to count processes whose bundle identifier is "my.fancy.package.app"' 2>&1), '1')
+      Cask::FakeSystemCommand.stubs_command(%Q(sudo -E '/usr/bin/osascript' '-e' 'tell application id "my.fancy.package.app" to quit' 2>&1))
+
       expected_command = "sudo -E '#{@cask.destination_path/'MyFancyPkg'/'FancyUninstaller.tool'}' '--please' 2>&1"
       Cask::FakeSystemCommand.stubs_command(expected_command)
 

test/support/Casks/with-installable.rb

@@ -4,5 +4,6 @@ class WithInstallable < TestCask
   version '1.2.3'
   sha1 '8588bd8175a54b8e0a1310cc18e6567d520ab7c4'
   install 'MyFancyPkg/Fancy.pkg'
-  uninstall :script => 'MyFancyPkg/FancyUninstaller.tool', :args => %w[--please]
+  uninstall :script => { :executable => 'MyFancyPkg/FancyUninstaller.tool', :args => %w[--please] },
+            :quit => 'my.fancy.package.app'
 end