Merge pull request #20134 from Homebrew/homebrew_core_cask_name_audit

audit: ensure that official formula and cask names don't conflict.

.github/workflows/tests.yml

@@ -166,7 +166,7 @@ jobs:
         id: set-up-homebrew
         uses: Homebrew/actions/setup-homebrew@main
         with:
-          core: false
+          core: true
           cask: true
           test-bot: false
 

Library/Homebrew/cask/audit.rb

@@ -27,7 +27,7 @@ module Cask
 
     sig {
       params(
-        cask: ::Cask::Cask, download: T::Boolean, quarantine: T::Boolean, token_conflicts: T.nilable(T::Boolean),
+        cask: ::Cask::Cask, download: T::Boolean, quarantine: T::Boolean,
         online: T.nilable(T::Boolean), strict: T.nilable(T::Boolean), signing: T.nilable(T::Boolean),
         new_cask: T.nilable(T::Boolean), only: T::Array[String], except: T::Array[String]
       ).void
@@ -35,14 +35,13 @@ module Cask
     def initialize(
       cask,
       download: false, quarantine: false,
-      token_conflicts: nil, online: nil, strict: nil, signing: nil,
+      online: nil, strict: nil, signing: nil,
       new_cask: nil, only: [], except: []
     )
-      # `new_cask` implies `online`, `token_conflicts`, `strict` and `signing`
+      # `new_cask` implies `online`, `strict` and `signing`
       online = new_cask if online.nil?
       strict = new_cask if strict.nil?
       signing = new_cask if signing.nil?
-      token_conflicts = new_cask if token_conflicts.nil?
 
       # `online` and `signing` imply `download`
       download ||= online || signing
@@ -53,7 +52,6 @@ module Cask
       @strict = strict
       @signing = signing
       @new_cask = new_cask
-      @token_conflicts = token_conflicts
       @only = only
       @except = except
     end
@@ -70,9 +68,6 @@ module Cask
     sig { returns(T::Boolean) }
     def strict? = !!@strict
 
-    sig { returns(T::Boolean) }
-    def token_conflicts? = !!@token_conflicts
-
     sig { returns(::Cask::Audit) }
     def run!
       only_audits = @only
@@ -430,15 +425,10 @@ module Cask
 
     sig { void }
     def audit_token_conflicts
-      return unless token_conflicts?
-
       Homebrew.with_no_api_env do
         return unless core_formula_names.include?(cask.token)
 
-        add_error(
+        add_error("cask token conflicts with an existing homebrew/core formula: #{Formatter.url(core_formula_url)}")
-          "possible duplicate, cask token conflicts with Homebrew core formula: #{Formatter.url(core_formula_url)}",
-          strict_only: true,
-        )
       end
     end
 

Library/Homebrew/cask/auditor.rb

@@ -11,17 +11,17 @@ module Cask
       params(
         cask: ::Cask::Cask, audit_download: T::Boolean, audit_online: T.nilable(T::Boolean),
         audit_strict: T.nilable(T::Boolean), audit_signing: T.nilable(T::Boolean),
-        audit_token_conflicts: T.nilable(T::Boolean), audit_new_cask: T.nilable(T::Boolean), quarantine: T::Boolean,
+        audit_new_cask: T.nilable(T::Boolean), quarantine: T::Boolean,
         any_named_args: T::Boolean, language: T.nilable(String), only: T::Array[String], except: T::Array[String]
       ).returns(T::Set[String])
     }
     def self.audit(
       cask, audit_download: false, audit_online: nil, audit_strict: nil, audit_signing: nil,
-      audit_token_conflicts: nil, audit_new_cask: nil, quarantine: false, any_named_args: false, language: nil,
+      audit_new_cask: nil, quarantine: false, any_named_args: false, language: nil,
       only: [], except: []
     )
       new(
-        cask, audit_download:, audit_online:, audit_strict:, audit_signing:, audit_token_conflicts:,
+        cask, audit_download:, audit_online:, audit_strict:, audit_signing:,
         audit_new_cask:, quarantine:, any_named_args:, language:, only:, except:
       ).audit
     end
@@ -36,7 +36,7 @@ module Cask
       params(
         cask: ::Cask::Cask, audit_download: T::Boolean, audit_online: T.nilable(T::Boolean),
         audit_strict: T.nilable(T::Boolean), audit_signing: T.nilable(T::Boolean),
-        audit_token_conflicts: T.nilable(T::Boolean), audit_new_cask: T.nilable(T::Boolean), quarantine: T::Boolean,
+        audit_new_cask: T.nilable(T::Boolean), quarantine: T::Boolean,
         any_named_args: T::Boolean, language: T.nilable(String), only: T::Array[String], except: T::Array[String]
       ).void
     }
@@ -46,7 +46,6 @@ module Cask
       audit_online: nil,
       audit_strict: nil,
       audit_signing: nil,
-      audit_token_conflicts: nil,
       audit_new_cask: nil,
       quarantine: false,
       any_named_args: false,
@@ -61,7 +60,6 @@ module Cask
       @audit_strict = audit_strict
       @audit_signing = audit_signing
       @quarantine = quarantine
-      @audit_token_conflicts = audit_token_conflicts
       @any_named_args = any_named_args
       @language = language
       @only = only
@@ -127,15 +125,14 @@ module Cask
     def audit_cask_instance(cask)
       audit = Audit.new(
         cask,
-        online:          @audit_online,
+        online:     @audit_online,
-        strict:          @audit_strict,
+        strict:     @audit_strict,
-        signing:         @audit_signing,
+        signing:    @audit_signing,
-        new_cask:        @audit_new_cask,
+        new_cask:   @audit_new_cask,
-        token_conflicts: @audit_token_conflicts,
+        download:   @audit_download,
-        download:        @audit_download,
+        quarantine: @quarantine,
-        quarantine:      @quarantine,
+        only:       @only,
-        only:            @only,
+        except:     @except,
-        except:          @except,
       )
       audit.run!
     end

Library/Homebrew/dev-cmd/audit.rb

@@ -58,8 +58,10 @@ module Homebrew
                hidden:      true
         switch "--[no-]signing",
                description: "Audit for app signatures, which are required by macOS on ARM."
+        # should be odeprecated in future
         switch "--token-conflicts",
-               description: "Audit for token conflicts."
+               description: "Audit for token conflicts.",
+               hidden:      true
         flag   "--tap=",
                description: "Check formulae and casks within the given tap, specified as <user>`/`<repo>."
         switch "--fix",
@@ -251,18 +253,17 @@ module Homebrew
                 # For switches, we add `|| nil` so that `nil` will be passed
                 # instead of `false` if they aren't set.
                 # This way, we can distinguish between "not set" and "set to false".
-                audit_online:          args.online? || nil,
+                audit_online:   args.online? || nil,
-                audit_strict:          args.strict? || nil,
+                audit_strict:   args.strict? || nil,
 
                 # No need for `|| nil` for `--[no-]signing`
                 # because boolean switches are already `nil` if not passed
-                audit_signing:         args.signing?,
+                audit_signing:  args.signing?,
-                audit_new_cask:        args.new? || nil,
+                audit_new_cask: args.new? || nil,
-                audit_token_conflicts: args.token_conflicts? || nil,
+                quarantine:     true,
-                quarantine:            true,
+                any_named_args: !no_named_args,
-                any_named_args:        !no_named_args,
+                only:           args.only || [],
-                only:                  args.only || [],
+                except:         args.except || [],
-                except:                args.except || [],
               ).to_a
             end
           end.uniq

Library/Homebrew/dev-cmd/generate-cask-ci-matrix.rb

@@ -274,10 +274,7 @@ module Homebrew
                                    bitbucket_repository]
           end
 
-          if labels.include?("ci-skip-token")
+          audit_exceptions << %w[token_valid token_bad_words] if labels.include?("ci-skip-token")
-            audit_exceptions << %w[token_conflicts token_valid
-                                   token_bad_words]
-          end
 
           audit_args << "--except" << audit_exceptions.join(",") if audit_exceptions.any?
 

Library/Homebrew/formula_auditor.rb

@@ -170,9 +170,15 @@ module Homebrew
                                                                               last_word_connector: " or ")}."
       end
 
-      return unless @strict
       return unless @core_tap
 
+      if CoreCaskTap.instance.cask_tokens.include?(name)
+        problem "Formula name conflicts with an existing Homebrew/cask cask's token."
+        return
+      end
+
+      return unless @strict
+
       problem "'#{name}' is not allowed in homebrew/core." if MissingFormula.disallowed_reason(name)
 
       if Formula.aliases.include? name
@@ -185,6 +191,11 @@ module Homebrew
         return
       end
 
+      if CoreCaskTap.instance.cask_tokens.include?(name)
+        problem "Formula name conflicts with an existing Homebrew/cask cask's token."
+        return
+      end
+
       return if formula.core_formula?
       return unless Formula.core_names.include?(name)
 

Library/Homebrew/test/cask/audit_spec.rb

@@ -49,13 +49,11 @@ RSpec.describe Cask::Audit, :cask do
   let(:only) { [] }
   let(:except) { [] }
   let(:strict) { nil }
-  let(:token_conflicts) { nil }
   let(:signing) { nil }
   let(:audit) do
     described_class.new(cask, online:,
                               strict:,
                               new_cask:,
-                              token_conflicts:,
                               signing:,
                               only:,
                               except:)
@@ -72,10 +70,6 @@ RSpec.describe Cask::Audit, :cask do
       it "implies `strict`" do
         expect(audit).to be_strict
       end
-
-      it "implies `token_conflicts`" do
-        expect(audit.token_conflicts?).to be true
-      end
     end
 
     context "when `online` is specified" do
@@ -949,34 +943,15 @@ RSpec.describe Cask::Audit, :cask do
     end
 
     describe "token conflicts" do
-      let(:only) { ["token_conflicts"] }
       let(:cask_token) { "with-binary" }
-      let(:token_conflicts) { true }
 
       context "when cask token conflicts with a core formula" do
         let(:formula_names) { %w[with-binary other-formula] }
 
-        context "when `--strict` is passed" do
+        it "warns about conflicts" do
-          let(:strict) { true }
+          expect(audit).to receive(:core_formula_names).and_return(formula_names)
-
+          expect(run).to error_with(/cask token conflicts/)
-          it "warns about duplicates" do
-            expect(audit).to receive(:core_formula_names).and_return(formula_names)
-            expect(run).to error_with(/possible duplicate/)
-          end
         end
-
-        context "when `--strict` is not passed" do
-          it "does not warn about duplicates" do
-            expect(audit).to receive(:core_formula_names).and_return(formula_names)
-            expect(run).not_to error_with(/possible duplicate/)
-          end
-        end
-      end
-
-      context "when cask token does not conflict with a core formula" do
-        let(:formula_names) { %w[other-formula] }
-
-        it { is_expected.to pass }
       end
     end
 

docs/Cask-Cookbook.md

@@ -1321,6 +1321,7 @@ Software vendors are often inconsistent with their naming. By enforcing strict n
 * Prevent duplicate submissions
 * Minimize renaming events
 * Unambiguously boil down the name of the software into a unique identifier
+* Avoid conflicts with Homebrew/homebrew-core formulae
 
 Details of software names and brands will inevitably be lost in the conversion to a minimal token. To capture the vendor’s full name for a distribution, use the [`name`](#stanza-name) within a cask, which accepts an unrestricted UTF-8 string.
 
@@ -1362,7 +1363,9 @@ Details of software names and brands will inevitably be lost in the conversion t
 
 * If the result of this process is a generic term, such as “Macintosh Installer”, try prepending the name of the vendor or developer, followed by a hyphen. If that doesn’t work, then just create the best name you can, based on the vendor’s web page.
 
-* If the result conflicts with the name of an existing cask, make yours unique by prepending the name of the vendor or developer, followed by a hyphen. Example: [unison.rb](https://github.com/Homebrew/homebrew-cask/blob/HEAD/Casks/u/unison.rb) and [panic-unison.rb](https://github.com/Homebrew/homebrew-cask/blob/HEAD/Casks/p/panic-unison.rb).
+* If the result conflicts with the name of an existing cask or Homebrew/homebrew-core formula, make yours unique by prepending the name of the vendor or developer, followed by a hyphen. Example: [unison.rb](https://github.com/Homebrew/homebrew-cask/blob/HEAD/Casks/u/unison.rb) and [panic-unison.rb](https://github.com/Homebrew/homebrew-cask/blob/HEAD/Casks/p/panic-unison.rb).
+
+* If the result still conflicts with the name of an existing Homebrew/homebrew-core formula, adjust the name to better describe the difference by e.g. appending `-app`. Example: `appium` formula and `appium-desktop` cask, `angband` formula and `angband-app` cask.
 
 * Inevitably, there are a small number of exceptions not covered by the rules. Don’t hesitate to [use the forum](https://github.com/orgs/Homebrew/discussions) if you have a problem.