Merge pull request #11541 from bayandin/improve-conflicts-audit

Improve conflicts audit

Library/Homebrew/formula_auditor.rb

@@ -314,15 +314,38 @@ module Homebrew
     end
 
     def audit_conflicts
-      formula.conflicts.each do |c|
+      tap = formula.tap
-        Formulary.factory(c.name)
+      formula.conflicts.each do |conflict|
+        conflicting_formula = Formulary.factory(conflict.name)
+        next if tap != conflicting_formula.tap
+
+        problem "Formula should not conflict with itself" if formula == conflicting_formula
+
+        if tap.formula_renames.key?(conflict.name) || tap.aliases.include?(conflict.name)
+          problem "Formula conflict should be declared using " \
+                  "canonical name (#{conflicting_formula.name}) instead of #{conflict.name}"
+        end
+
+        reverse_conflict_found = false
+        conflicting_formula.conflicts.each do |reverse_conflict|
+          reverse_conflict_formula = Formulary.factory(reverse_conflict.name)
+          if tap.formula_renames.key?(reverse_conflict.name) || tap.aliases.include?(reverse_conflict.name)
+            problem "Formula #{conflicting_formula.name} conflict should be declared using " \
+                    "canonical name (#{reverse_conflict_formula.name}) instead of #{reverse_conflict.name}"
+          end
+
+          reverse_conflict_found ||= reverse_conflict_formula == formula
+        end
+        unless reverse_conflict_found
+          problem "Formula #{conflicting_formula.name} should also have a conflict declared with #{formula.name}"
+        end
       rescue TapFormulaUnavailableError
         # Don't complain about missing cross-tap conflicts.
         next
       rescue FormulaUnavailableError
-        problem "Can't find conflicting formula #{c.name.inspect}."
+        problem "Can't find conflicting formula #{conflict.name.inspect}."
       rescue TapFormulaAmbiguityError, TapFormulaWithOldnameAmbiguityError
-        problem "Ambiguous conflicting formula #{c.name.inspect}."
+        problem "Ambiguous conflicting formula #{conflict.name.inspect}."
       end
     end
 

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

@@ -1139,5 +1139,60 @@ module Homebrew
         expect(fa.problems).to be_empty
       end
     end
+
+    describe "#audit_conflicts" do
+      before do
+        # We don't really test FormulaTextAuditor here
+        allow(File).to receive(:open).and_return("")
+      end
+
+      specify "it warns when conflicting with non-existing formula" do
+        foo = formula("foo") do
+          url "https://brew.sh/bar-1.0.tgz"
+
+          conflicts_with "bar"
+        end
+
+        fa = described_class.new foo
+        fa.audit_conflicts
+
+        expect(fa.problems.first[:message])
+          .to match("Can't find conflicting formula \"bar\"")
+      end
+
+      specify "it warns when conflicting with itself" do
+        foo = formula("foo") do
+          url "https://brew.sh/bar-1.0.tgz"
+
+          conflicts_with "foo"
+        end
+        stub_formula_loader foo
+
+        fa = described_class.new foo
+        fa.audit_conflicts
+
+        expect(fa.problems.first[:message])
+          .to match("Formula should not conflict with itself")
+      end
+
+      specify "it warns when another formula does not have a symmetric conflict" do
+        foo = formula("foo") do
+          url "https://brew.sh/foo-1.0.tgz"
+        end
+        stub_formula_loader foo
+
+        bar = formula("bar") do
+          url "https://brew.sh/bar-1.0.tgz"
+
+          conflicts_with "foo"
+        end
+
+        fa = described_class.new bar
+        fa.audit_conflicts
+
+        expect(fa.problems.first[:message])
+          .to match("Formula foo should also have a conflict declared with bar")
+      end
+    end
   end
 end