diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index 02968fd412..a5afaf6789 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/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 && - (metadata["forks_count"] < 30) && (metadata["subscribers_count"] < 30) && + if (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)" - end - - if devel_only?(formula) && formula.tap.to_s.downcase !~ %r{[-/]devel-only$} - problem "Devel-only (no stable download)" - end + problem "Head-only (no stable download)" if head_only?(formula) + problem "Devel-only (no stable download)" if devel_only?(formula) %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 - problem "Formulae should not have a `devel` spec" - end + return unless @core_tap - 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 diff --git a/Library/Homebrew/rubocops/options.rb b/Library/Homebrew/rubocops/options.rb index a5eaac00fd..a5a0c5e870 100644 --- a/Library/Homebrew/rubocops/options.rb +++ b/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 diff --git a/Library/Homebrew/test/rubocops/options_spec.rb b/Library/Homebrew/test/rubocops/options_spec.rb index 440a56fd96..a2eac92d8f 100644 --- a/Library/Homebrew/test/rubocops/options_spec.rb +++ b/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 - subject(:cop) { described_class.new } - - context "When auditing options for a new formula" do - it "with deprecated options" do - expect_offense(<<~RUBY) + it "with deprecated_optionss" do + expect_offense(<<~RUBY, "/homebrew-core/") 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