audit: check for conflicts with core formula names (#23631)

* audit: add check for formula token conflicts

* audit_modified_casks: check new casks for token conflicts

* Style fixes

lib/hbc/audit.rb

@@ -7,12 +7,17 @@ class Hbc::Audit
 
   attr_reader :cask, :download
 
-  def initialize(cask, download = false, command = Hbc::SystemCommand)
+  def initialize(cask, download: false, check_token_conflicts: false, command: Hbc::SystemCommand)
     @cask = cask
     @download = download
+    @check_token_conflicts = check_token_conflicts
     @command = command
   end
 
+  def check_token_conflicts?
+    @check_token_conflicts
+  end
+
   def run!
     check_required_stanzas
     check_version
@@ -20,6 +25,7 @@ class Hbc::Audit
     check_appcast
     check_url
     check_generic_artifacts
+    check_token_conflicts
     check_download
     self
   rescue StandardError => e
@@ -181,6 +187,24 @@ class Hbc::Audit
     end
   end
 
+  def check_token_conflicts
+    return unless check_token_conflicts?
+    return unless core_formula_names.include?(cask.token)
+    add_warning "possible duplicate, cask token conflicts with Homebrew core formula: #{core_formula_url}"
+  end
+
+  def core_tap
+    @core_tap ||= CoreTap.instance
+  end
+
+  def core_formula_names
+    core_tap.formula_names
+  end
+
+  def core_formula_url
+    "#{core_tap.default_remote}/blob/master/Formula/#{cask.token}.rb"
+  end
+
   def check_download
     return unless download && cask.url
     odebug "Auditing download"

lib/hbc/auditor.rb

@@ -1,7 +1,8 @@
 class Hbc::Auditor
-  def self.audit(cask, audit_download: false)
+  def self.audit(cask, audit_download: false, check_token_conflicts: false)
     download = audit_download && Hbc::Download.new(cask)
-    audit = Hbc::Audit.new(cask, download)
+    audit = Hbc::Audit.new(cask, download:              download,
+                                 check_token_conflicts: check_token_conflicts)
     audit.run!
     puts audit.summary
     audit.success?

lib/hbc/cli/audit.rb

@@ -22,13 +22,18 @@ class Hbc::CLI::Audit < Hbc::CLI::Base
 
   def audit(cask)
     odebug "Auditing Cask #{cask}"
-    @auditor.audit(cask, audit_download: audit_download?)
+    @auditor.audit(cask, audit_download:        audit_download?,
+                         check_token_conflicts: check_token_conflicts?)
   end
 
   def audit_download?
     @args.include?("--download")
   end
 
+  def check_token_conflicts?
+    @args.include?("--token-conflicts")
+  end
+
   def casks_to_audit
     if cask_tokens.empty?
       Hbc.all
@@ -38,7 +43,7 @@ class Hbc::CLI::Audit < Hbc::CLI::Base
   end
 
   def cask_tokens
-    @cask_tokens ||= @args.reject { |a| a == "--download" }
+    @cask_tokens ||= self.class.cask_tokens_from(@args)
   end
 
   def self.needs_init?

lib/hbc/cli/internal_audit_modified_casks.rb

@@ -76,6 +76,13 @@ class Hbc::CLI::InternalAuditModifiedCasks < Hbc::CLI::InternalUseBase
     @modified_cask_files = out.split("\n")
   end
 
+  def added_cask_files
+    return @added_cask_files if defined? @added_cask_files
+    out = git(*%w[diff --name-only --diff-filter=A], commit_range,
+              "--", "#{cask_dir}/*.rb")
+    @added_cask_files = out.split("\n")
+  end
+
   def modified_casks
     return @modified_casks if defined? @modified_casks
     @modified_casks = modified_cask_files.map { |f| Hbc.load(f) }
@@ -89,7 +96,9 @@ class Hbc::CLI::InternalAuditModifiedCasks < Hbc::CLI::InternalUseBase
 
   def audit(cask, cask_file)
     audit_download = audit_download?(cask, cask_file)
-    success = Hbc::Auditor.audit(cask, audit_download: audit_download)
+    check_token_conflicts = added_cask_files.include?(cask_file)
+    success = Hbc::Auditor.audit(cask, audit_download:        audit_download,
+                                       check_token_conflicts: check_token_conflicts)
     failed_casks << cask unless success
   end
 

spec/cask/audit_spec.rb

@@ -6,8 +6,13 @@ describe Hbc::Audit do
 
   let(:cask) { instance_double(Hbc::Cask) }
   let(:download) { false }
+  let(:check_token_conflicts) { false }
   let(:fake_system_command) { class_double(Hbc::SystemCommand) }
-  let(:audit) { Hbc::Audit.new(cask, download, fake_system_command) }
+  let(:audit) {
+    Hbc::Audit.new(cask, download:              download,
+                         check_token_conflicts: check_token_conflicts,
+                         command:               fake_system_command)
+  }
 
   describe "#result" do
     subject { audit.result }
@@ -263,6 +268,25 @@ describe Hbc::Audit do
       end
     end
 
+    describe "token conflicts" do
+      let(:cask_token) { "with-binary" }
+      let(:check_token_conflicts) { true }
+
+      before do
+        expect(audit).to receive(:core_formula_names).and_return(formula_names)
+      end
+
+      context "when cask token conflicts with a core formula" do
+        let(:formula_names) { %w[with-binary other-formula] }
+        it { should warn_with(%r{possible duplicate}) }
+      end
+
+      context "when cask token does not conflict with a core formula" do
+        let(:formula_names) { %w[other-formula] }
+        it { should_not warn_with(%r{possible duplicate}) }
+      end
+    end
+
     describe "audit of downloads" do
       let(:cask_token) { "with-binary" }
       let(:cask) { Hbc.load(cask_token) }

test/cask/cli/audit_test.rb

@@ -15,7 +15,7 @@ describe Hbc::CLI::Audit do
     it "audits specified Casks if tokens are given" do
       cask_token = "nice-app"
       Hbc.expects(:load).with(cask_token).returns(cask)
-      auditor.expects(:audit).with(cask, audit_download: false)
+      auditor.expects(:audit).with(cask, audit_download: false, check_token_conflicts: false)
 
       run_audit([cask_token], auditor)
     end
@@ -25,7 +25,7 @@ describe Hbc::CLI::Audit do
     it "does not download the Cask per default" do
       Hbc.stubs(load: cask)
 
-      auditor.expects(:audit).with(cask, audit_download: false)
+      auditor.expects(:audit).with(cask, audit_download: false, check_token_conflicts: false)
 
       run_audit(["casktoken"], auditor)
     end
@@ -33,12 +33,30 @@ describe Hbc::CLI::Audit do
     it "download a Cask if --download flag is set" do
       Hbc.stubs(load: cask)
 
-      auditor.expects(:audit).with(cask, audit_download: true)
+      auditor.expects(:audit).with(cask, audit_download: true, check_token_conflicts: false)
 
       run_audit(["casktoken", "--download"], auditor)
     end
   end
 
+  describe "rules for checking token conflicts" do
+    it "does not check for token conflicts per default" do
+      Hbc.stubs(load: cask)
+
+      auditor.expects(:audit).with(cask, audit_download: false, check_token_conflicts: false)
+
+      run_audit(["casktoken"], auditor)
+    end
+
+    it "checks for token conflicts if --token-conflicts flag is set" do
+      Hbc.stubs(load: cask)
+
+      auditor.expects(:audit).with(cask, audit_download: false, check_token_conflicts: true)
+
+      run_audit(["casktoken", "--token-conflicts"], auditor)
+    end
+  end
+
   def run_audit(args, auditor)
     Hbc::CLI::Audit.new(args, auditor).run
   end