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.

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

audit: Allow skipping/selective running of cops and cops refactor #2531

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 4 additions & 4 deletions Library/.rubocop.yml
Expand Up @@ -8,16 +8,16 @@ AllCops:

require: ./Homebrew/rubocops.rb

Homebrew/CorrectBottleBlock:
FormulaAuditStrict/BottleBlock:
Enabled: true

Homebrew/FormulaDesc:
FormulaAuditStrict/Desc:
Enabled: true

Homebrew/FormulaComponentsOrder:
FormulaAuditStrict/ComponentsOrder:
Enabled: true

Homebrew/ComponentsRedundancy:
FormulaAuditStrict/ComponentsRedundancy:
Enabled: true

Metrics/AbcSize:
Expand Down
41 changes: 39 additions & 2 deletions Library/Homebrew/cmd/style.rb
@@ -1,4 +1,4 @@
#: * `style` [`--fix`] [`--display-cop-names`] [<files>|<taps>|<formulae>]:
#: * `style` [`--fix`] [`--display-cop-names`] [`--only-cops=`[COP1,COP2..]|`--except-cops=`[COP1,COP2..]] [<files>|<taps>|<formulae>]:
#: Check formulae or files for conformance to Homebrew style guidelines.
#:
#: <formulae> and <files> may not be combined. If both are omitted, style will run
Expand All @@ -11,10 +11,16 @@
#: If `--display-cop-names` is passed, the RuboCop cop name for each violation
#: is included in the output.
#:
#: If `--only-cops` is passed, only the given Rubocop cop(s)' violations would be checked.
#:
#: If `--except-cops` is passed, the given Rubocop cop(s)' checks would be skipped.
#:
#: Exits with a non-zero status if any style violations are found.

require "utils"
require "json"
require "rubocop"
require_relative "../rubocops"

module Homebrew
module_function
Expand All @@ -30,7 +36,20 @@ def style
ARGV.formulae.map(&:path)
end

Homebrew.failed = check_style_and_print(target, fix: ARGV.flag?("--fix"))
only_cops = ARGV.value("only-cops").to_s.split(",")
except_cops = ARGV.value("except-cops").to_s.split(",")
if !only_cops.empty? && !except_cops.empty?
odie "--only-cops and --except-cops cannot be used simultaneously!"
end

options = { fix: ARGV.flag?("--fix") }
if !only_cops.empty?
options[:only_cops] = only_cops
elsif !except_cops.empty?
options[:except_cops] = except_cops
end

Homebrew.failed = check_style_and_print(target, options)
end

# Checks style for a list of files, printing simple RuboCop output.
Expand All @@ -54,6 +73,24 @@ def check_style_impl(files, output_type, options = {})
]
args << "--auto-correct" if fix

if options[:except_cops]
options[:except_cops].map! { |cop| RuboCop::Cop::Cop.registry.qualified_cop_name(cop, "") }
cops_to_exclude = options[:except_cops].select do |cop|
RuboCop::Cop::Cop.registry.names.include?(cop) ||
RuboCop::Cop::Cop.registry.departments.include?(cop.to_sym)
end

args << "--except" << cops_to_exclude.join(",") unless cops_to_exclude.empty?
elsif options[:only_cops]
options[:only_cops].map! { |cop| RuboCop::Cop::Cop.registry.qualified_cop_name(cop, "") }
cops_to_include = options[:only_cops].select do |cop|
RuboCop::Cop::Cop.registry.names.include?(cop) ||
RuboCop::Cop::Cop.registry.departments.include?(cop.to_sym)
end

args << "--only" << cops_to_include.join(",") unless cops_to_include.empty?
end

if files.nil?
args << "--config" << HOMEBREW_LIBRARY_PATH/".rubocop.yml"
args += [HOMEBREW_LIBRARY_PATH]
Expand Down
33 changes: 26 additions & 7 deletions Library/Homebrew/dev-cmd/audit.rb
@@ -1,4 +1,4 @@
#: * `audit` [`--strict`] [`--fix`] [`--online`] [`--new-formula`] [`--display-cop-names`] [`--display-filename`] [`--only=`<method>|`--except=`<method] [<formulae>]:
#: * `audit` [`--strict`] [`--fix`] [`--online`] [`--new-formula`] [`--display-cop-names`] [`--display-filename`] [`--only=`<method>|`--except=`<method>] [`--only-cops=`[COP1,COP2..]|`--except-cops=`[COP1,COP2..]] [<formulae>]:
#: Check <formulae> for Homebrew coding style violations. This should be
#: run before submitting a new formula.
#:
Expand Down Expand Up @@ -27,6 +27,10 @@
#:
#: If `--except` is passed, the methods named `audit_<method>` will not be run.
#:
#: If `--only-cops` is passed, only the given Rubocop cop(s)' violations would be checked.
#:
#: If `--except-cops` is passed, the given Rubocop cop(s)' checks would be skipped.
#:
#: `audit` exits with a non-zero status if any errors are found. This is useful,
#: for instance, for implementing pre-commit hooks.

Expand Down Expand Up @@ -69,15 +73,30 @@ def audit
files = ARGV.resolved_formulae.map(&:path)
end

if strict
options = { fix: ARGV.flag?("--fix"), realpath: true }
# Check style in a single batch run up front for performance
style_results = check_style_json(files, options)
only_cops = ARGV.value("only-cops").to_s.split(",")
except_cops = ARGV.value("except-cops").to_s.split(",")
if !only_cops.empty? && !except_cops.empty?
odie "--only-cops and --except-cops cannot be used simultaneously!"
elsif (!only_cops.empty? || !except_cops.empty?) && strict
odie "--only-cops/--except-cops and --strict cannot be used simultaneously"
end

options = { fix: ARGV.flag?("--fix"), realpath: true }

if !only_cops.empty?
options[:only_cops] = only_cops
elsif !except_cops.empty?
options[:except_cops] = except_cops
elsif !strict
options[:except_cops] = [:FormulaAuditStrict]
end

# Check style in a single batch run up front for performance
style_results = check_style_json(files, options)

ff.each do |f|
options = { new_formula: new_formula, strict: strict, online: online }
options[:style_offenses] = style_results.file_offenses(f.path) if strict
options[:style_offenses] = style_results.file_offenses(f.path)
fa = FormulaAuditor.new(f, options)
fa.audit

Expand Down Expand Up @@ -1257,7 +1276,7 @@ def audit
only_audits = ARGV.value("only").to_s.split(",")
except_audits = ARGV.value("except").to_s.split(",")
if !only_audits.empty? && !except_audits.empty?
odie "--only and --except cannot be used simulataneously!"
odie "--only and --except cannot be used simultaneously!"
end

methods.map(&:to_s).grep(/^audit_/).each do |audit_method_name|
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/rubocops/bottle_block_cop.rb
Expand Up @@ -2,12 +2,12 @@

module RuboCop
module Cop
module Homebrew
module FormulaAuditStrict
# This cop audits `bottle` block in Formulae
#
# - `rebuild` should be used instead of `revision` in `bottle` block

class CorrectBottleBlock < FormulaCop
class BottleBlock < FormulaCop
MSG = "Use rebuild instead of revision in bottle block".freeze

def audit_formula(_node, _class_node, _parent_class_node, formula_class_body_node)
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/rubocops/components_order_cop.rb
Expand Up @@ -2,12 +2,12 @@

module RuboCop
module Cop
module Homebrew
module FormulaAuditStrict
# This cop checks for correct order of components in a Formula
#
# - component_precedence_list has component hierarchy in a nested list
# where each sub array contains components' details which are at same precedence level
class FormulaComponentsOrder < FormulaCop
class ComponentsOrder < FormulaCop
def audit_formula(_node, _class_node, _parent_class_node, formula_class_body_node)
component_precedence_list = [
[{ name: :include, type: :method_call }],
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/rubocops/components_redundancy_cop.rb
Expand Up @@ -2,7 +2,7 @@

module RuboCop
module Cop
module Homebrew
module FormulaAuditStrict
# This cop checks if redundant components are present and other component errors
#
# - `url|checksum|mirror` should be inside `stable` block
Expand Down