Address feedback

- style nits
- better comments for tests that are not idempotent
- moved appdir placeholder constant to global.rb

Library/Homebrew/cask/cask.rb

@@ -20,8 +20,6 @@ module Cask
     extend APIHashable
     include Metadata
 
-    APPDIR_PLACEHOLDER = "$APPDIR"
-
     # TODO: can be removed when API JSON is regenerated with HOMEBREW_PREFIX_PLACEHOLDER.
     HOMEBREW_OLD_PREFIX_PLACEHOLDER = "$(brew --prefix)"
 

Library/Homebrew/cask/cask_loader.rb

@@ -331,7 +331,7 @@ module Cask
         string.to_s
               .gsub(HOMEBREW_HOME_PLACEHOLDER, Dir.home)
               .gsub(HOMEBREW_PREFIX_PLACEHOLDER, HOMEBREW_PREFIX)
-              .gsub(Cask::APPDIR_PLACEHOLDER, appdir)
+              .gsub(HOMEBREW_CASK_APPDIR_PLACEHOLDER, appdir)
               .gsub(Cask::HOMEBREW_OLD_PREFIX_PLACEHOLDER, HOMEBREW_PREFIX)
       end
 

Library/Homebrew/cask/dsl.rb

@@ -375,7 +375,7 @@ module Cask
 
     # @api public
     def appdir
-      return Cask::APPDIR_PLACEHOLDER if Cask.generating_hash?
+      return HOMEBREW_CASK_APPDIR_PLACEHOLDER if Cask.generating_hash?
 
       cask.config.appdir
     end

Library/Homebrew/global.rb

@@ -60,6 +60,7 @@ HOMEBREW_LINUX_DEFAULT_REPOSITORY = ENV.fetch("HOMEBREW_LINUX_DEFAULT_REPOSITORY
 HOMEBREW_PREFIX_PLACEHOLDER = "$HOMEBREW_PREFIX"
 # Needs a leading slash to avoid `File.expand.path` complaining about non-absolute home.
 HOMEBREW_HOME_PLACEHOLDER = "/$HOME"
+HOMEBREW_CASK_APPDIR_PLACEHOLDER = "$APPDIR"
 
 HOMEBREW_MACOS_NEWEST_UNSUPPORTED = ENV.fetch("HOMEBREW_MACOS_NEWEST_UNSUPPORTED").freeze
 HOMEBREW_MACOS_OLDEST_SUPPORTED = ENV.fetch("HOMEBREW_MACOS_OLDEST_SUPPORTED").freeze

Library/Homebrew/service.rb

@@ -46,7 +46,8 @@ module Homebrew
     }
     def run(command = nil, macos: nil, linux: nil)
       # Save parameters for serialization
-      @run_params ||= command || { macos: macos, linux: linux }.compact
+      @run_params ||= command
+      @run_params ||= { macos: macos, linux: linux }.compact
 
       command ||= on_system_conditional(macos: macos, linux: linux)
       case T.unsafe(command)
@@ -554,11 +555,11 @@ module Homebrew
           api_hash["run"].to_h do |key, array|
             [
               key.to_sym,
-              array.map { |value| replace_placeholders(value) },
+              array.map(&method(:replace_placeholders)),
             ]
           end
         when Array
-          api_hash["run"].map { |value| replace_placeholders(value) }
+          api_hash["run"].map(&method(:replace_placeholders))
         end
 
       hash[:keep_alive] = api_hash["keep_alive"].transform_keys(&:to_sym) if api_hash.key?("keep_alive")

Library/Homebrew/test/cask/cask_spec.rb

@@ -337,7 +337,7 @@ describe Cask::Cask, :cask do
       expect(JSON.pretty_generate(h["variations"])).to eq expected_sha256_variations.strip
     end
 
-    # @note The calls to `APIHashable.generating_hash!` and `APIHashable.generated_hash!`
+    # @note The calls to `Cask.generating_hash!` and `Cask.generated_hash!`
     #   are not idempotent so they can only be used in one test.
     it "returns the correct hash placeholders" do
       described_class.generating_hash!

Library/Homebrew/test/service_spec.rb

@@ -928,6 +928,8 @@ describe Homebrew::Service do
       }
     end
 
+    # @note The calls to `Formula.generating_hash!` and `Formula.generated_hash!`
+    #   are not idempotent so they can only be used in one test.
     it "replaces local paths with placeholders" do
       f = stub_formula do
         service do