Merge pull request #24315 from Homebrew/zap_audit

Require `zap` stanzas in new casks

Library/Homebrew/cask/artifact/zap.rb

@@ -7,6 +7,17 @@ module Cask
   module Artifact
     # Artifact corresponding to the `zap` stanza.
     class Zap < AbstractUninstall
+      # `:not_required` is serialised as an empty stanza, which older versions of
+      # Homebrew can still load from the API.
+      sig { params(cask: Cask, exemption: T.nilable(Symbol), directives: DirectivesType).returns(Zap) }
+      def self.from_args(cask, exemption = nil, **directives)
+        if exemption && (exemption != :not_required || directives.any?)
+          raise CaskInvalidError.new(cask, "'zap' stanza requires either directives or only ':not_required'.")
+        end
+
+        new(cask, **directives)
+      end
+
       # Removing files also waits, as `uninstall` steps may still need them, e.g. to
       # identify a keychain certificate to delete.
       sig { override.returns(T::Array[Symbol]) }

Library/Homebrew/cask/audit.rb

@@ -357,6 +357,23 @@ module Cask
       add_error "at least one installable artifact stanza is required" unless installable_artifact
     end
 
+    sig { void }
+    def audit_zap_required
+      return unless new_cask?
+      return unless cask.tap&.official?
+      return if cask.artifacts.any?(Artifact::Zap)
+
+      zap_suggestion = if Homebrew::SimulateSystem.simulating_or_running_on_linux?
+        "Add one"
+      else
+        # `brew generate-zap` only scans macOS paths.
+        "Generate one with `brew generate-zap #{cask.token}`"
+      end
+      add_error "New casks must have a `zap` stanza for every OS and architecture they support. " \
+                "#{zap_suggestion} or use `zap :not_required` only if there is nothing to zap. " \
+                "See #{Formatter.url("https://docs.brew.sh/Cask-Cookbook#stanza-zap")}"
+    end
+
     sig { void }
     def audit_description
       # Fonts seldom benefit from descriptions and requiring them disproportionately

Library/Homebrew/dev-cmd/audit.rb

@@ -266,9 +266,14 @@ module Homebrew
           path = cask.sourcefile_path
 
           errors = os_arch_combinations.flat_map do |os, arch|
-            # Linux-only casks have no stanza values for macOS, so audit them
-            # under Linux instead.
+            # Casks have no stanza values for platforms they don't support, so
+            # audit them on one they do instead. `depends_on arch` can differ
+            # between OSes, so read it on the OS being audited.
             os = :linux if os != :linux && !cask.supports_macos?
+            os = :macos if os == :linux && !cask.supports_linux?
+            cask_archs = SimulateSystem.with(os:, arch:) { Cask::CaskLoader.load(path) }
+                                       .depends_on.arch&.map { |cask_arch| cask_arch[:type] }
+            arch = OnSystem::ARCH_OPTIONS.find { cask_archs.include?(it) } if cask_archs&.exclude?(arch)
 
             SimulateSystem.with(os:, arch:) do
               odebug "Auditing Cask #{cask} on os #{os} and arch #{arch}"

Library/Homebrew/dev-cmd/generate-zap.rb

@@ -24,8 +24,8 @@ module Homebrew
           The target application should have been launched at least once so that
           preference files and caches exist on disk.
 
-          Outputs `trash`, `delete`, and `rmdir` directives suitable for pasting
-          into a cask definition.
+          Outputs `trash`, `delete` and `rmdir` directives suitable for pasting
+          into a cask definition, or `zap :not_required` if nothing is found.
         EOS
 
         switch "--name",
@@ -123,8 +123,11 @@ module Homebrew
         rmdir_paths = derive_rmdir_candidates(trash_paths + delete_paths)
 
         if trash_paths.empty? && delete_paths.empty?
-          opoo "No files found matching #{format_patterns(patterns)}."
-          puts "# No zap stanza required"
+          opoo <<~EOS
+            No files found matching #{format_patterns(patterns)}.
+            Only use `zap :not_required` if the app has been launched at least once.
+          EOS
+          puts "zap :not_required"
           return
         end
 

Library/Homebrew/rubocops/cask/uninstall_methods_order.rb

@@ -28,7 +28,7 @@ module RuboCop
           return unless [:zap, :uninstall].include?(node.method_name)
 
           hash_node = node.arguments.first
-          return if hash_node.nil? || (!hash_node.is_a?(AST::Node) && !hash_node.hash_type?)
+          return unless hash_node.is_a?(AST::HashNode)
 
           comments = processed_source.comments
 

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

@@ -4,6 +4,19 @@
 require_relative "shared_examples/uninstall_zap"
 
 RSpec.describe Cask::Artifact::Zap, :cask do
+  describe "::from_args" do
+    let(:cask) { Cask::Cask.new("zap-not-required") }
+
+    it "serialises `:not_required` as an empty stanza that older versions can load" do
+      expect(described_class.from_args(cask, :not_required).to_args).to be_empty
+    end
+
+    it "rejects `:not_required` with directives" do
+      expect { described_class.from_args(cask, :not_required, trash: "~/Library/Foo") }
+        .to raise_error(Cask::CaskInvalidError, /:not_required/)
+    end
+  end
+
   describe "directive dispatch" do
     include_examples "uninstall/zap directive dispatch"
 

Library/Homebrew/test/cask/audit_spec.rb

@@ -1780,6 +1780,72 @@ RSpec.describe Cask::Audit, :cask do
       end
     end
 
+    describe "zap required" do
+      subject(:run) { Homebrew::SimulateSystem.with(os:) { audit.run! } }
+
+      let(:only) { ["zap_required"] }
+      let(:new_cask) { true }
+      let(:os) { :sequoia }
+      let(:message) { /New casks must have a `zap` stanza/ }
+      let(:tap) { CoreCaskTap.instance }
+      let(:stanzas) { nil }
+      let(:cask) do
+        extra_stanzas = stanzas
+        Cask::Cask.new("zap-required", tap:) do
+          version "1.0"
+          sha256 :no_check
+          url "https://brew.sh/zap-required.dmg"
+          homepage "https://brew.sh"
+
+          instance_eval(&extra_stanzas) if extra_stanzas
+        end
+      end
+
+      context "when the cask has no zap stanza" do
+        it do
+          expect(run).to error_with(
+            /Generate one with `brew generate-zap zap-required` or use `zap :not_required`/,
+          )
+        end
+      end
+
+      context "when the cask is not new" do
+        let(:new_cask) { false }
+
+        it { is_expected.not_to error_with(message) }
+      end
+
+      context "when the cask is not in an official tap" do
+        let(:tap) { nil }
+
+        it { is_expected.not_to error_with(message) }
+      end
+
+      context "when the cask has `zap :not_required`" do
+        let(:stanzas) { proc { zap :not_required } }
+
+        it { is_expected.not_to error_with(message) }
+      end
+
+      context "when the cask only has a zap stanza on macOS" do
+        let(:stanzas) do
+          proc do
+            on_macos do
+              zap trash: "~/Library/Preferences/sh.brew.zap-required.plist"
+            end
+          end
+        end
+
+        it { is_expected.not_to error_with(message) }
+
+        context "when auditing Linux" do
+          let(:os) { :linux }
+
+          it { is_expected.to error_with(/Add one or use `zap :not_required`/) }
+        end
+      end
+    end
+
     describe "GitHub repository notability" do
       let(:only) { ["github_repository"] }
       let(:new_cask) { true }

Library/Homebrew/test/dev-cmd/audit_spec.rb

@@ -92,6 +92,116 @@ RSpec.describe Homebrew::DevCmd::Audit do
       end
     end
 
+    it "audits macOS-only casks under macOS when auditing all operating systems" do
+      macos_url_cask_file = tap_path/"Casks/macos-url-example.rb"
+      macos_url_cask_file.write <<~RUBY
+        cask "macos-url-example" do
+          version "1.0"
+          sha256 "0000000000000000000000000000000000000000000000000000000000000000"
+
+          on_macos do
+            url "https://example.invalid/x-\#{version}.pkg"
+          end
+
+          name "Example"
+          desc "macOS-only cask"
+          homepage "https://example.invalid/"
+
+          depends_on macos: :ventura
+
+          binary "x"
+        end
+      RUBY
+      allow(tap).to receive(:cask_files).and_return([macos_url_cask_file])
+
+      expect { described_class.new(["--tap=homebrew/test", "--os=all", "--arch=arm"]).run }
+        .not_to output.to_stdout
+    end
+
+    it "audits ARM-only casks under ARM when auditing Intel" do
+      arm_url_cask_file = tap_path/"Casks/arm-url-example.rb"
+      arm_url_cask_file.write <<~RUBY
+        cask "arm-url-example" do
+          version "1.0"
+          sha256 "0000000000000000000000000000000000000000000000000000000000000000"
+
+          on_arm do
+            url "https://example.invalid/x-\#{version}.tar.gz"
+          end
+
+          name "Example"
+          desc "ARM-only cask"
+          homepage "https://example.invalid/"
+
+          depends_on arch: :arm64
+
+          binary "x"
+        end
+      RUBY
+      allow(tap).to receive(:cask_files).and_return([arm_url_cask_file])
+
+      Homebrew::SimulateSystem.with(os: :macos, arch: :intel) do
+        expect { audit.run }.not_to output.to_stdout
+      end
+    end
+
+    it "audits casks on the architecture each operating system supports" do
+      os_arch_cask_file = tap_path/"Casks/os-arch-example.rb"
+      os_arch_cask_file.write <<~RUBY
+        cask "os-arch-example" do
+          version "1.0"
+          sha256 arm:          "0000000000000000000000000000000000000000000000000000000000000000",
+                 x86_64_linux: "1111111111111111111111111111111111111111111111111111111111111111"
+
+          url "https://example.invalid/x-\#{version}.tar.gz"
+          name "Example"
+          desc "macOS ARM and Linux Intel cask"
+          homepage "https://example.invalid/"
+
+          on_macos do
+            depends_on arch: :arm64
+          end
+
+          on_linux do
+            depends_on arch: :x86_64
+          end
+
+          binary "x"
+        end
+      RUBY
+      allow(tap).to receive(:cask_files).and_return([os_arch_cask_file])
+
+      expect { described_class.new(["--tap=homebrew/test", "--os=all", "--arch=intel"]).run }
+        .not_to output.to_stdout
+    end
+
+    it "audits Linux Intel for a cask that only requires macOS on ARM" do
+      arm_macos_cask_file = tap_path/"Casks/arm-macos-example.rb"
+      arm_macos_cask_file.write <<~RUBY
+        cask "arm-macos-example" do
+          version "1.0"
+          sha256 arm:         "0000000000000000000000000000000000000000000000000000000000000000",
+                 intel:       "1111111111111111111111111111111111111111111111111111111111111111",
+                 arm64_linux: "0000000000000000000000000000000000000000000000000000000000000000"
+
+          url "https://example.invalid/x-\#{version}.tar.gz"
+          name "Example"
+          desc "Cask without a Linux Intel checksum"
+          homepage "https://example.invalid/"
+
+          on_arm do
+            depends_on macos: :big_sur
+          end
+
+          binary "x"
+        end
+      RUBY
+      allow(tap).to receive(:cask_files).and_return([arm_macos_cask_file])
+
+      expect { described_class.new(["--tap=homebrew/test", "--os=all", "--arch=all"]).run }
+        .to output(/a sha256 stanza is required/).to_stdout
+    end
+
     it "enables API access when auditing external formulae after it was automatically disabled" do
       formula_file = tap_path/"Formula/example.rb"
       formula_file.dirname.mkpath

Library/Homebrew/test/dev-cmd/generate-zap_spec.rb

@@ -22,8 +22,8 @@ RSpec.describe Homebrew::DevCmd::GenerateZap do
     CoreCaskTap.instance.clear_cache
 
     expect { brew "generate-zap", caskfile }
-      .to output(/Scanning for files matching.*No zap stanza required/m).to_stdout
-      .and output(/No files found matching/).to_stderr
+      .to output(/Scanning for files matching.*zap :not_required/m).to_stdout
+      .and output(/No files found matching.*Only use `zap :not_required` if the app has been launched/m).to_stderr
       .and be_a_success
   end
 

Library/Homebrew/test/rubocops/cask/uninstall_methods_order_spec.rb

@@ -317,6 +317,16 @@ RSpec.describe RuboCop::Cop::Cask::UninstallMethodsOrder, :config do
         CASK
       end
     end
+
+    it "does not report an offense when no zap is required" do
+      expect_no_offenses(<<~CASK)
+        cask "foo" do
+          url "https://example.com/foo.zip"
+
+          zap :not_required
+        end
+      CASK
+    end
   end
 
   context "with both uninstall and zap blocks" do

docs/Cask-Cookbook.md

@@ -126,7 +126,7 @@ An exception is `target:` which typically consists of short lines.
 ### Required stanzas
 
 Each cask requires the identity and download stanzas below.
-Submissions to `homebrew/cask` may require additional stanzas such as `livecheck`, `depends_on`, `uninstall` or `zap` when the package needs them.
+New submissions to `homebrew/cask` also require a [`zap`](#stanza-zap) stanza and may require additional stanzas such as `livecheck`, `depends_on` or `uninstall` when the package needs them.
 
 | name                               | multiple occurrences allowed? | value |
 | ---------------------------------- | :---------------------------: | ----- |
@@ -211,7 +211,7 @@ Allowing writes to a directory does not allow connections to sockets in it.
 | [`caveats`](#stanza-caveats)               | yes                           | String or Ruby block providing the user with cask-specific information at install time. |
 | [`livecheck`](#stanza-livecheck)           | no                            | Ruby block describing how to find updates when automatic detection is insufficient. It is not used with `version :latest` unless the block uses `skip`. |
 | [`depends_on`](#stanza-depends_on)         | yes                           | Dependencies and operating-system or architecture requirements for this cask. |
-| [`zap`](#stanza-zap)                       | no                            | Additional procedures for a more complete uninstall, including user files and shared resources. |
+| [`zap`](#stanza-zap)                       | no                            | Additional procedures for a more complete uninstall, including user files and shared resources. Required for new casks in `homebrew/cask`. |
 | [`deprecate!`](#stanza-deprecate--disable) | no                            | Date as a string in `YYYY-MM-DD` format and a string or symbol providing a reason. |
 | [`disable!`](#stanza-deprecate--disable)   | no                            | Date as a string in `YYYY-MM-DD` format and a string or symbol providing a reason. |
 | `preflight_steps`                          | yes                           | Declarative file preparation steps run before artifact installation. |
@@ -1323,6 +1323,8 @@ brew uninstall --zap --force firefox
 
 The form of the `zap` stanza follows the [`uninstall`](#stanza-uninstall) stanza. All the same directives are available. The `trash:` key is preferred over `delete:`.
 
+Use `zap :not_required` instead of directives when there is nothing to zap (see [`zap` creation](#zap-creation)).
+
 Example: [dropbox.rb](https://github.com/Homebrew/homebrew-cask/blob/974a55ade77bb4edc8bbb80ef72eec83ae0e76c0/Casks/d/dropbox.rb#L30-L68)
 
 #### `zap` creation
@@ -1335,12 +1337,15 @@ Manual creation can be facilitated with:
 * An uninstaller tool such as [AppCleaner](https://formulae.brew.sh/cask/appcleaner)
 * Inspection of the usual paths, i.e. `/Library/{'Application Support',LaunchAgents,LaunchDaemons,Frameworks,Logs,Preferences,PrivilegedHelperTools}` and `~/Library/{'Application Support',Caches,Containers,LaunchAgents,Logs,Preferences,'Saved Application State'}`
 
-If no additional files are discovered, instead of a zap stanza, include the following comment:
+If no additional files are discovered, declare that nothing needs to be zapped instead:
 
 ```ruby
-# No zap stanza required
+zap :not_required
 ```
 
+A cask that supports Linux and has its `zap` stanza inside an `on_macos` block also needs a `zap` stanza or `zap :not_required` inside its `on_linux` block.
+`brew generate-zap` only scans macOS paths, so write a Linux `zap` stanza by hand.
+
 ## Conditional statements
 
 ### Handling different system configurations

docs/Manpage.md

@@ -3524,8 +3524,8 @@ resolved from the cask's `app` artifact.
 The target application should have been launched at least once so that
 preference files and caches exist on disk.
 
-Outputs `trash`, `delete`, and `rmdir` directives suitable for pasting into a
-cask definition.
+Outputs `trash`, `delete` and `rmdir` directives suitable for pasting into a
+cask definition, or `zap :not_required` if nothing is found.
 
 `--name`
 

manpages/brew.1

@@ -2217,7 +2217,7 @@ Accepts a cask token (e\.g\. \fBfirefox\fP) or, with \fB\-\-name\fP, a raw appli
 .P
 The target application should have been launched at least once so that preference files and caches exist on disk\.
 .P
-Outputs \fBtrash\fP, \fBdelete\fP, and \fBrmdir\fP directives suitable for pasting into a cask definition\.
+Outputs \fBtrash\fP, \fBdelete\fP and \fBrmdir\fP directives suitable for pasting into a cask definition, or \fBzap :not_required\fP if nothing is found\.
 .TP
 \fB\-\-name\fP
 Treat the argument as a raw application name instead of a cask token\.