Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Tweak audits #5583

Merged
merged 1 commit into from
Jan 22, 2019
Merged

Tweak audits #5583

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
59 changes: 25 additions & 34 deletions Library/Homebrew/dev-cmd/audit.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ def audit
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
Expand Down Expand Up @@ -405,7 +405,6 @@ def audit_deps
@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
Expand Down Expand Up @@ -434,7 +433,7 @@ def audit_deps

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 " \
Expand All @@ -443,6 +442,7 @@ def audit_deps
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?
Expand All @@ -463,19 +463,17 @@ def audit_deps
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
Expand Down Expand Up @@ -529,6 +527,8 @@ def audit_keg_only_style

def audit_postgresql
return unless formula.name == "postgresql"
return unless @core_tap

major_version = formula.version
.to_s
.split(".")
Expand Down Expand Up @@ -602,20 +602,16 @@ def audit_bottle_disabled
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

Expand All @@ -635,8 +631,7 @@ def audit_github_repository
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
Expand All @@ -647,13 +642,8 @@ def audit_github_repository
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
Expand Down Expand Up @@ -698,11 +688,11 @@ def audit_specs
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
Expand Down Expand Up @@ -922,10 +912,6 @@ def line_problems(line, _lineno)

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
Expand All @@ -944,13 +930,18 @@ def line_problems(line, _lineno)
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
Expand Down
19 changes: 10 additions & 9 deletions Library/Homebrew/rubocops/options.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ class Options < FormulaCop
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|
Expand All @@ -33,21 +36,19 @@ def audit_formula(_node, _class_node, _parent_class_node, body_node)
problem "Use '--with#{Regexp.last_match(1)}-test' instead of '--#{option}'."\
" Migrate '--#{option}' with `deprecated_option`."
end

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
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 OPTION if method_called_ever?(body_node, :option)
end
end
end
end
Expand Down
16 changes: 5 additions & 11 deletions Library/Homebrew/test/rubocops/options_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class Foo < Formula
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'
Expand All @@ -35,7 +35,7 @@ class Foo < Formula
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'
Expand All @@ -44,19 +44,13 @@ class Foo < Formula
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
Expand Down