Merge pull request #2478 from MikeMcQuaid/audit-skip-methods

audit: allow skipping audit methods.
This commit is contained in:
Mike McQuaid 2017-04-18 09:18:55 +01:00 committed by GitHub
commit 55c02ae774
7 changed files with 77 additions and 61 deletions

View File

@ -1,4 +1,4 @@
#: * `audit` [`--strict`] [`--fix`] [`--online`] [`--new-formula`] [`--display-cop-names`] [`--display-filename`] [<formulae>]: #: * `audit` [`--strict`] [`--fix`] [`--online`] [`--new-formula`] [`--display-cop-names`] [`--display-filename`] [`--only=`<method>|`--except=`<method] [<formulae>]:
#: Check <formulae> for Homebrew coding style violations. This should be #: Check <formulae> for Homebrew coding style violations. This should be
#: run before submitting a new formula. #: run before submitting a new formula.
#: #:
@ -23,6 +23,10 @@
#: If `--display-filename` is passed, every line of output is prefixed with the #: If `--display-filename` is passed, every line of output is prefixed with the
#: name of the file or formula being audited, to make the output easy to grep. #: name of the file or formula being audited, to make the output easy to grep.
#: #:
#: If `--only` is passed, only the methods named `audit_<method>` will be run.
#:
#: If `--except` is passed, the methods named `audit_<method>` will not be run.
#:
#: `audit` exits with a non-zero status if any errors are found. This is useful, #: `audit` exits with a non-zero status if any errors are found. This is useful,
#: for instance, for implementing pre-commit hooks. #: for instance, for implementing pre-commit hooks.
@ -728,7 +732,7 @@ class FormulaAuditor
} }
end end
spec.patches.each { |p| audit_patch(p) if p.external? } spec.patches.each { |p| patch_problems(p) if p.external? }
end end
%w[Stable Devel].each do |name| %w[Stable Devel].each do |name|
@ -864,10 +868,10 @@ class FormulaAuditor
return if legacy_patches.empty? return if legacy_patches.empty?
problem "Use the patch DSL instead of defining a 'patches' method" problem "Use the patch DSL instead of defining a 'patches' method"
legacy_patches.each { |p| audit_patch(p) } legacy_patches.each { |p| patch_problems(p) }
end end
def audit_patch(patch) def patch_problems(patch)
case patch.url case patch.url
when /raw\.github\.com/, %r{gist\.github\.com/raw}, %r{gist\.github\.com/.+/raw}, when /raw\.github\.com/, %r{gist\.github\.com/raw}, %r{gist\.github\.com/.+/raw},
%r{gist\.githubusercontent\.com/.+/raw} %r{gist\.githubusercontent\.com/.+/raw}
@ -939,7 +943,13 @@ class FormulaAuditor
problem "require \"language/go\" is unnecessary unless using `go_resource`s" problem "require \"language/go\" is unnecessary unless using `go_resource`s"
end end
def audit_line(line, _lineno) def audit_lines
text.without_patch.split("\n").each_with_index do |line, lineno|
line_problems(line, lineno+1)
end
end
def line_problems(line, _lineno)
if line =~ /<(Formula|AmazonWebServicesFormula|ScriptFileFormula|GithubGistFormula)/ if line =~ /<(Formula|AmazonWebServicesFormula|ScriptFileFormula|GithubGistFormula)/
problem "Use a space in class inheritance: class Foo < #{$1}" problem "Use a space in class inheritance: class Foo < #{$1}"
end end
@ -1142,11 +1152,11 @@ class FormulaAuditor
end end
if line =~ /depends_on :(.+) (if.+|unless.+)$/ if line =~ /depends_on :(.+) (if.+|unless.+)$/
audit_conditional_dep($1.to_sym, $2, $&) conditional_dep_problems($1.to_sym, $2, $&)
end end
if line =~ /depends_on ['"](.+)['"] (if.+|unless.+)$/ if line =~ /depends_on ['"](.+)['"] (if.+|unless.+)$/
audit_conditional_dep($1, $2, $&) conditional_dep_problems($1, $2, $&)
end end
if line =~ /(Dir\[("[^\*{},]+")\])/ if line =~ /(Dir\[("[^\*{},]+")\])/
@ -1234,7 +1244,7 @@ class FormulaAuditor
EOS EOS
end end
def audit_conditional_dep(dep, condition, line) def conditional_dep_problems(dep, condition, line)
quoted_dep = quote_dep(dep) quoted_dep = quote_dep(dep)
dep = Regexp.escape(dep.to_s) dep = Regexp.escape(dep.to_s)
@ -1250,30 +1260,26 @@ class FormulaAuditor
dep.is_a?(Symbol) ? dep.inspect : "'#{dep}'" dep.is_a?(Symbol) ? dep.inspect : "'#{dep}'"
end end
def audit_check_output(output) def problem_if_output(output)
problem(output) if output problem(output) if output
end end
def audit def audit
audit_file only_audits = ARGV.value("only").to_s.split(",")
audit_formula_name except_audits = ARGV.value("except").to_s.split(",")
audit_class if !only_audits.empty? && !except_audits.empty?
audit_specs odie "--only and --except cannot be used simulataneously!"
audit_revision_and_version_scheme end
audit_homepage
audit_bottle_spec methods.map(&:to_s).grep(/^audit_/).each do |audit_method_name|
audit_github_repository name = audit_method_name.gsub(/^audit_/, "")
audit_deps if !only_audits.empty?
audit_conflicts next unless only_audits.include?(name)
audit_options elsif !except_audits.empty?
audit_legacy_patches next if except_audits.include?(name)
audit_text end
audit_caveats send(audit_method_name)
text.without_patch.split("\n").each_with_index { |line, lineno| audit_line(line, lineno+1) } end
audit_installed
audit_prefix_has_contents
audit_reverse_migration
audit_style
end end
private private

View File

@ -67,7 +67,7 @@ module FormulaCellarChecks
checker = LinkageChecker.new(keg, formula) checker = LinkageChecker.new(keg, formula)
return unless checker.broken_dylibs? return unless checker.broken_dylibs?
audit_check_output <<-EOS.undent problem_if_output <<-EOS.undent
The installation was broken. The installation was broken.
Broken dylib links found: Broken dylib links found:
#{checker.broken_dylibs.to_a * "\n "} #{checker.broken_dylibs.to_a * "\n "}
@ -76,9 +76,9 @@ module FormulaCellarChecks
def audit_installed def audit_installed
generic_audit_installed generic_audit_installed
audit_check_output(check_shadowed_headers) problem_if_output(check_shadowed_headers)
audit_check_output(check_openssl_links) problem_if_output(check_openssl_links)
audit_check_output(check_python_framework_links(formula.lib)) problem_if_output(check_python_framework_links(formula.lib))
check_linkage check_linkage
end end
end end

View File

@ -154,17 +154,17 @@ module FormulaCellarChecks
end end
def audit_installed def audit_installed
audit_check_output(check_manpages) problem_if_output(check_manpages)
audit_check_output(check_infopages) problem_if_output(check_infopages)
audit_check_output(check_jars) problem_if_output(check_jars)
audit_check_output(check_non_libraries) problem_if_output(check_non_libraries)
audit_check_output(check_non_executables(formula.bin)) problem_if_output(check_non_executables(formula.bin))
audit_check_output(check_generic_executables(formula.bin)) problem_if_output(check_generic_executables(formula.bin))
audit_check_output(check_non_executables(formula.sbin)) problem_if_output(check_non_executables(formula.sbin))
audit_check_output(check_generic_executables(formula.sbin)) problem_if_output(check_generic_executables(formula.sbin))
audit_check_output(check_easy_install_pth(formula.lib)) problem_if_output(check_easy_install_pth(formula.lib))
audit_check_output(check_elisp_dirname(formula.share, formula.name)) problem_if_output(check_elisp_dirname(formula.share, formula.name))
audit_check_output(check_elisp_root(formula.share, formula.name)) problem_if_output(check_elisp_root(formula.share, formula.name))
end end
alias generic_audit_installed audit_installed alias generic_audit_installed audit_installed

View File

@ -858,15 +858,15 @@ class FormulaInstaller
tab.write tab.write
end end
def audit_check_output(output) def problem_if_output(output)
return unless output return unless output
opoo output opoo output
@show_summary_heading = true @show_summary_heading = true
end end
def audit_installed def audit_installed
audit_check_output(check_env_path(formula.bin)) problem_if_output(check_env_path(formula.bin))
audit_check_output(check_env_path(formula.sbin)) problem_if_output(check_env_path(formula.sbin))
super super
end end

View File

@ -303,7 +303,7 @@ describe FormulaAuditor do
end end
end end
describe "#audit_line" do describe "#line_problems" do
specify "pkgshare" do specify "pkgshare" do
fa = formula_auditor "foo", <<-EOS.undent, strict: true fa = formula_auditor "foo", <<-EOS.undent, strict: true
class Foo < Formula class Foo < Formula
@ -311,25 +311,25 @@ describe FormulaAuditor do
end end
EOS EOS
fa.audit_line 'ohai "#{share}/foo"', 3 fa.line_problems 'ohai "#{share}/foo"', 3
expect(fa.problems.shift).to eq("Use \#{pkgshare} instead of \#{share}/foo") expect(fa.problems.shift).to eq("Use \#{pkgshare} instead of \#{share}/foo")
fa.audit_line 'ohai "#{share}/foo/bar"', 3 fa.line_problems 'ohai "#{share}/foo/bar"', 3
expect(fa.problems.shift).to eq("Use \#{pkgshare} instead of \#{share}/foo") expect(fa.problems.shift).to eq("Use \#{pkgshare} instead of \#{share}/foo")
fa.audit_line 'ohai share/"foo"', 3 fa.line_problems 'ohai share/"foo"', 3
expect(fa.problems.shift).to eq('Use pkgshare instead of (share/"foo")') expect(fa.problems.shift).to eq('Use pkgshare instead of (share/"foo")')
fa.audit_line 'ohai share/"foo/bar"', 3 fa.line_problems 'ohai share/"foo/bar"', 3
expect(fa.problems.shift).to eq('Use pkgshare instead of (share/"foo")') expect(fa.problems.shift).to eq('Use pkgshare instead of (share/"foo")')
fa.audit_line 'ohai "#{share}/foo-bar"', 3 fa.line_problems 'ohai "#{share}/foo-bar"', 3
expect(fa.problems).to eq([]) expect(fa.problems).to eq([])
fa.audit_line 'ohai share/"foo-bar"', 3 fa.line_problems 'ohai share/"foo-bar"', 3
expect(fa.problems).to eq([]) expect(fa.problems).to eq([])
fa.audit_line 'ohai share/"bar"', 3 fa.line_problems 'ohai share/"bar"', 3
expect(fa.problems).to eq([]) expect(fa.problems).to eq([])
end end
@ -344,11 +344,11 @@ describe FormulaAuditor do
end end
EOS EOS
fa.audit_line 'ohai "#{share}/foolibc++"', 3 fa.line_problems 'ohai "#{share}/foolibc++"', 3
expect(fa.problems.shift) expect(fa.problems.shift)
.to eq("Use \#{pkgshare} instead of \#{share}/foolibc++") .to eq("Use \#{pkgshare} instead of \#{share}/foolibc++")
fa.audit_line 'ohai share/"foolibc++"', 3 fa.line_problems 'ohai share/"foolibc++"', 3
expect(fa.problems.shift) expect(fa.problems.shift)
.to eq('Use pkgshare instead of (share/"foolibc++")') .to eq('Use pkgshare instead of (share/"foolibc++")')
end end
@ -360,7 +360,7 @@ describe FormulaAuditor do
end end
EOS EOS
fa.audit_line "class Foo<Formula", 1 fa.line_problems "class Foo<Formula", 1
expect(fa.problems.shift) expect(fa.problems.shift)
.to eq("Use a space in class inheritance: class Foo < Formula") .to eq("Use a space in class inheritance: class Foo < Formula")
end end
@ -368,10 +368,10 @@ describe FormulaAuditor do
specify "default template" do specify "default template" do
fa = formula_auditor "foo", "class Foo < Formula; url '/foo-1.0.tgz'; end" fa = formula_auditor "foo", "class Foo < Formula; url '/foo-1.0.tgz'; end"
fa.audit_line '# system "cmake", ".", *std_cmake_args', 3 fa.line_problems '# system "cmake", ".", *std_cmake_args', 3
expect(fa.problems.shift).to eq("Commented cmake call found") expect(fa.problems.shift).to eq("Commented cmake call found")
fa.audit_line "# PLEASE REMOVE", 3 fa.line_problems "# PLEASE REMOVE", 3
expect(fa.problems.shift).to eq("Please remove default template comments") expect(fa.problems.shift).to eq("Please remove default template comments")
end end
end end

View File

@ -606,7 +606,7 @@ With `--verbose` or `-v`, many commands print extra debugging information. Note
## DEVELOPER COMMANDS ## DEVELOPER COMMANDS
* `audit` [`--strict`] [`--fix`] [`--online`] [`--new-formula`] [`--display-cop-names`] [`--display-filename`] [`formulae`]: * `audit` [`--strict`] [`--fix`] [`--online`] [`--new-formula`] [`--display-cop-names`] [`--display-filename`] [`--only=``method`|`--except=``method] [<formulae`]:
Check `formulae` for Homebrew coding style violations. This should be Check `formulae` for Homebrew coding style violations. This should be
run before submitting a new formula. run before submitting a new formula.
@ -631,6 +631,10 @@ With `--verbose` or `-v`, many commands print extra debugging information. Note
If `--display-filename` is passed, every line of output is prefixed with the If `--display-filename` is passed, every line of output is prefixed with the
name of the file or formula being audited, to make the output easy to grep. name of the file or formula being audited, to make the output easy to grep.
If `--only` is passed, only the methods named `audit_`method`` will be run.
If `--except` is passed, the methods named `audit_`method`` will not be run.
`audit` exits with a non-zero status if any errors are found. This is useful, `audit` exits with a non-zero status if any errors are found. This is useful,
for instance, for implementing pre-commit hooks. for instance, for implementing pre-commit hooks.

View File

@ -631,7 +631,7 @@ Print the version number of Homebrew to standard output and exit\.
.SH "DEVELOPER COMMANDS" .SH "DEVELOPER COMMANDS"
. .
.TP .TP
\fBaudit\fR [\fB\-\-strict\fR] [\fB\-\-fix\fR] [\fB\-\-online\fR] [\fB\-\-new\-formula\fR] [\fB\-\-display\-cop\-names\fR] [\fB\-\-display\-filename\fR] [\fIformulae\fR] \fBaudit\fR [\fB\-\-strict\fR] [\fB\-\-fix\fR] [\fB\-\-online\fR] [\fB\-\-new\-formula\fR] [\fB\-\-display\-cop\-names\fR] [\fB\-\-display\-filename\fR] [\fB\-\-only=\fR\fImethod\fR|\fB\-\-except=\fR\fImethod] [<formulae\fR]
Check \fIformulae\fR for Homebrew coding style violations\. This should be run before submitting a new formula\. Check \fIformulae\fR for Homebrew coding style violations\. This should be run before submitting a new formula\.
. .
.IP .IP
@ -656,6 +656,12 @@ If \fB\-\-display\-cop\-names\fR is passed, the RuboCop cop name for each violat
If \fB\-\-display\-filename\fR is passed, every line of output is prefixed with the name of the file or formula being audited, to make the output easy to grep\. If \fB\-\-display\-filename\fR is passed, every line of output is prefixed with the name of the file or formula being audited, to make the output easy to grep\.
. .
.IP .IP
If \fB\-\-only\fR is passed, only the methods named \fBaudit_<method>\fR will be run\.
.
.IP
If \fB\-\-except\fR is passed, the methods named \fBaudit_<method>\fR will not be run\.
.
.IP
\fBaudit\fR exits with a non\-zero status if any errors are found\. This is useful, for instance, for implementing pre\-commit hooks\. \fBaudit\fR exits with a non\-zero status if any errors are found\. This is useful, for instance, for implementing pre\-commit hooks\.
. .
.TP .TP