Tweak audits

- Ensure that new formulae problems aren't duplicated
- Now that homebrew/core will imminently not have options adjust the
  various audits so they make more sense (and exclude taps)
- Exclude taps from more preferential audits

Library/Homebrew/dev-cmd/audit.rb

@@ -202,7 +202,7 @@ module Homebrew
   end
 
   def format_problem_lines(problems)
-    problems.map { |p| "* #{p.chomp.gsub("\n", "\n    ")}" }
+    problems.uniq.map { |p| "* #{p.chomp.gsub("\n", "\n    ")}" }
   end
 
   class FormulaText
@@ -405,7 +405,6 @@ module Homebrew
       @specs.each do |spec|
         # Check for things we don't like to depend on.
         # We allow non-Homebrew installs whenever possible.
-        options_message = "Formulae should not have optional or recommended dependencies"
         spec.deps.each do |dep|
           begin
             dep_f = dep.to_formula
@@ -434,7 +433,7 @@ module Homebrew
 
           if @new_formula && dep_f.keg_only_reason &&
              !["openssl", "apr", "apr-util"].include?(dep.name) &&
-             (!["openblas"].include?(dep.name) || @core_tap) &&
+             !["openblas"].include?(dep.name) &&
              dep_f.keg_only_reason.reason == :provided_by_macos
             new_formula_problem(
               "Dependency '#{dep.name}' may be unnecessary as it is provided " \
@@ -443,6 +442,7 @@ module Homebrew
           end
 
           dep.options.each do |opt|
+            next if @core_tap
             next if dep_f.option_defined?(opt)
             next if dep_f.requirements.find do |r|
               if r.recommended?
@@ -463,19 +463,17 @@ module Homebrew
             problem "Dependency '#{dep.name}' is marked as :run. Remove :run; it is a no-op."
           end
 
-          next unless @new_formula
           next unless @core_tap
 
           if dep.tags.include?(:recommended) || dep.tags.include?(:optional)
-            new_formula_problem options_message
+            problem "Formulae should not have optional or recommended dependencies"
           end
         end
 
-        next unless @new_formula
         next unless @core_tap
 
         if spec.requirements.map(&:recommended?).any? || spec.requirements.map(&:optional?).any?
-          new_formula_problem options_message
+          problem "Formulae should not have optional or recommended requirements"
         end
       end
     end
@@ -529,6 +527,8 @@ module Homebrew
 
     def audit_postgresql
       return unless formula.name == "postgresql"
+      return unless @core_tap
+
       major_version = formula.version
                              .to_s
                              .split(".")
@@ -602,20 +602,16 @@ module Homebrew
       return unless formula.bottle_disabled?
       return if formula.bottle_unneeded?
 
-      if !formula.bottle_disable_reason.valid?
+      unless formula.bottle_disable_reason.valid?
         problem "Unrecognized bottle modifier"
-      else
-        bottle_disabled_whitelist = %w[
-          cryptopp
-          leafnode
-        ]
-        return if bottle_disabled_whitelist.include?(formula.name)
-
-        problem "Formulae should not use `bottle :disabled`" if @core_tap
       end
+
+      return unless @core_tap
+      problem "Formulae should not use `bottle :disabled`"
     end
 
     def audit_github_repository
+      return unless @core_tap
       return unless @online
       return unless @new_formula
 
@@ -635,8 +631,7 @@ module Homebrew
       return if metadata.nil?
 
       new_formula_problem "GitHub fork (not canonical repository)" if metadata["fork"]
-      if @core_tap &&
+      if (metadata["forks_count"] < 30) && (metadata["subscribers_count"] < 30) &&
-         (metadata["forks_count"] < 30) && (metadata["subscribers_count"] < 30) &&
          (metadata["stargazers_count"] < 75)
         new_formula_problem "GitHub repository not notable enough (<30 forks, <30 watchers and <75 stars)"
       end
@@ -647,13 +642,8 @@ module Homebrew
     end
 
     def audit_specs
-      if head_only?(formula) && formula.tap.to_s.downcase !~ %r{[-/]head-only$}
+      problem "Head-only (no stable download)" if head_only?(formula)
-        problem "Head-only (no stable download)"
+      problem "Devel-only (no stable download)" if devel_only?(formula)
-      end
-
-      if devel_only?(formula) && formula.tap.to_s.downcase !~ %r{[-/]devel-only$}
-        problem "Devel-only (no stable download)"
-      end
 
       %w[Stable Devel HEAD].each do |name|
         spec_name = name.downcase.to_sym
@@ -698,11 +688,11 @@ module Homebrew
         end
       end
 
-      if @core_tap && formula.devel
+      return unless @core_tap
-        problem "Formulae should not have a `devel` spec"
-      end
 
-      if @core_tap && formula.head
+      problem "Formulae should not have a `devel` spec" if formula.devel
+
+      if formula.head
         head_spec_message = "Formulae should not have a `HEAD` spec"
         if @new_formula
           new_formula_problem head_spec_message
@@ -922,10 +912,6 @@ module Homebrew
 
       return unless @strict
 
-      if @core_tap && line.include?("env :std")
-        problem "`env :std` in `core` formulae is deprecated"
-      end
-
       if line.include?("env :userpaths")
         problem "`env :userpaths` in formulae is deprecated"
       end
@@ -944,13 +930,18 @@ module Homebrew
         problem "Use \#{pkgshare} instead of \#{share}/#{formula.name}"
       end
 
-      if line =~ /depends_on .+ if build\.with(out)?\?\(?["']\w+["']\)?/
+      if !@core_tap && line =~ /depends_on .+ if build\.with(out)?\?\(?["']\w+["']\)?/
         problem "`Use :optional` or `:recommended` instead of `#{Regexp.last_match(0)}`"
       end
 
       return unless line =~ %r{share(\s*[/+]\s*)(['"])#{Regexp.escape(formula.name)}(?:\2|/)}
 
       problem "Use pkgshare instead of (share#{Regexp.last_match(1)}\"#{formula.name}\")"
+
+      return unless @core_tap
+
+      return unless line.include?("env :std")
+      problem "`env :std` in `core` formulae is deprecated"
     end
 
     def audit_reverse_migration

Library/Homebrew/rubocops/options.rb

@@ -8,6 +8,9 @@ module RuboCop
         DEPRECATION_MSG = "macOS has been 64-bit only since 10.6 so 32-bit options are deprecated.".freeze
         UNI_DEPRECATION_MSG = "macOS has been 64-bit only since 10.6 so universal options are deprecated.".freeze
 
+        DEP_OPTION = "Formulae should not use `deprecated_option`".freeze
+        OPTION = "Formulae should not have an `option`".freeze
+
         def audit_formula(_node, _class_node, _parent_class_node, body_node)
           option_call_nodes = find_every_method_call_by_name(body_node, :option)
           option_call_nodes.each do |option_call|
@@ -33,22 +36,20 @@ module RuboCop
             problem "Use '--with#{Regexp.last_match(1)}-test' instead of '--#{option}'."\
                     " Migrate '--#{option}' with `deprecated_option`."
           end
-        end
-      end
-    end
 
-    module NewFormulaAudit
-      class Options < FormulaCop
-        DEP_OPTION = "New formulae should not use `deprecated_option`".freeze
-        OPTION = "Formulae should not have an `option`".freeze
-
-        def audit_formula(_node, _class_node, _parent_class_node, body_node)
-          problem DEP_OPTION if method_called_ever?(body_node, :deprecated_option)
           return unless formula_tap == "homebrew-core"
 
+          problem DEP_OPTION if method_called_ever?(body_node, :deprecated_option)
           problem OPTION if method_called_ever?(body_node, :option)
         end
       end
     end
+
+    # Keep this (empty) module and class around in case we need it later to
+    # avoid deleting all the NewFormulaAudit referencing logic.
+    module NewFormulaAudit
+      class Options < FormulaCop
+      end
+    end
   end
 end

Library/Homebrew/test/rubocops/options_spec.rb

@@ -24,7 +24,7 @@ describe RuboCop::Cop::FormulaAudit::Options do
       RUBY
     end
 
-    it "with deprecated options" do
+    it "with bad option names" do
       expect_offense(<<~RUBY)
         class Foo < Formula
           url 'https://brew.sh/foo-1.0.tgz'
@@ -35,7 +35,7 @@ describe RuboCop::Cop::FormulaAudit::Options do
       RUBY
     end
 
-    it "with misc deprecated options" do
+    it "with without-check option name" do
       expect_offense(<<~RUBY)
         class Foo < Formula
           url 'https://brew.sh/foo-1.0.tgz'
@@ -44,19 +44,13 @@ describe RuboCop::Cop::FormulaAudit::Options do
         end
       RUBY
     end
-  end
-end
 
-describe RuboCop::Cop::NewFormulaAudit::Options do
+    it "with deprecated_optionss" do
-  subject(:cop) { described_class.new }
+      expect_offense(<<~RUBY, "/homebrew-core/")
-
-  context "When auditing options for a new formula" do
-    it "with deprecated options" do
-      expect_offense(<<~RUBY)
         class Foo < Formula
           url 'https://brew.sh/foo-1.0.tgz'
           deprecated_option "examples" => "with-examples"
-          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ New formulae should not use `deprecated_option`
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Formulae should not use `deprecated_option`
         end
       RUBY
     end