diff options
| -rw-r--r-- | Library/.rubocop.yml | 4 | ||||
| -rw-r--r-- | Library/Homebrew/cmd/style.rb | 12 | ||||
| -rw-r--r-- | Library/Homebrew/dev-cmd/audit.rb | 22 | ||||
| -rw-r--r-- | Library/Homebrew/rubocops/bottle_block_cop.rb | 2 | ||||
| -rw-r--r-- | Library/Homebrew/rubocops/extend/formula_cop.rb | 354 | ||||
| -rw-r--r-- | Library/Homebrew/rubocops/formula_desc_cop.rb | 2 | ||||
| -rw-r--r-- | Library/Homebrew/test/rubocops/bottle_block_cop_spec.rb | 2 | ||||
| -rw-r--r-- | Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb | 2 | ||||
| -rw-r--r-- | docs/Manpage.md | 4 | ||||
| -rw-r--r-- | manpages/brew.1 | 6 |
10 files changed, 219 insertions, 191 deletions
diff --git a/Library/.rubocop.yml b/Library/.rubocop.yml index 276e4aabf..07f9cc84f 100644 --- a/Library/.rubocop.yml +++ b/Library/.rubocop.yml @@ -8,10 +8,10 @@ AllCops: require: ./Homebrew/rubocops.rb -Homebrew/CorrectBottleBlock: +FormulaAuditStrict/CorrectBottleBlock: Enabled: true -Homebrew/FormulaDesc: +FormulaAuditStrict/FormulaDesc: Enabled: true Homebrew/FormulaComponentsOrder: diff --git a/Library/Homebrew/cmd/style.rb b/Library/Homebrew/cmd/style.rb index d89c9b72f..4410f0feb 100644 --- a/Library/Homebrew/cmd/style.rb +++ b/Library/Homebrew/cmd/style.rb @@ -56,8 +56,18 @@ module Homebrew ] args << "--auto-correct" if fix + if options[:exclude].eql?(:FormulaAuditStrict) && !(options.key?(:except) || options.key?(:only)) + args << "--except" << :FormulaAuditStrict + end + + if options[:except] + cops_to_exclude = options[:except].select { |cop| RuboCop::Cop::Cop.registry.names.include?(cop) } + args << "--except" << cops_to_exclude.join(" ") unless cops_to_exclude.empty? + end + if options[:only] - args << "--only" << RuboCop::Cop::Cop.registry.with_department(options[:only]).names.join(" ") + cops_to_include = options[:only].select { |cop| RuboCop::Cop::Cop.registry.names.include?(cop) } + args << "--only" << cops_to_include.join(" ") unless cops_to_include.empty? end if files.nil? diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index 2f4b37096..e96d4cf4c 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/Library/Homebrew/dev-cmd/audit.rb @@ -27,6 +27,10 @@ #: #: If `--except` is passed, the methods named `audit_<method>` will not be run. #: +#: If `--only-cops` is passed, only the mentioned cops' violations would be checked. +#: +#: If `--except-cops` is passed, the mentioned cops' 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. @@ -69,16 +73,22 @@ module Homebrew files = ARGV.resolved_formulae.map(&:path) end + 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 simulataneously!" + 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) + else + options = { fix: ARGV.flag?("--fix"), realpath: true, exclude: :FormulaAuditStrict } end - if !strict - options = { fix: ARGV.flag?("--fix"), realpath: true, only: :Homebrew } - style_results = check_style_json(files, options) - end + options[:only] = only_cops unless only_cops.empty? + options[:except] = except_cops unless except_cops.empty? + # 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 } diff --git a/Library/Homebrew/rubocops/bottle_block_cop.rb b/Library/Homebrew/rubocops/bottle_block_cop.rb index 141d87b35..2d6289cbb 100644 --- a/Library/Homebrew/rubocops/bottle_block_cop.rb +++ b/Library/Homebrew/rubocops/bottle_block_cop.rb @@ -2,7 +2,7 @@ require_relative "./extend/formula_cop" 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 diff --git a/Library/Homebrew/rubocops/extend/formula_cop.rb b/Library/Homebrew/rubocops/extend/formula_cop.rb index 11e5d93d3..4120be6ef 100644 --- a/Library/Homebrew/rubocops/extend/formula_cop.rb +++ b/Library/Homebrew/rubocops/extend/formula_cop.rb @@ -1,213 +1,211 @@ module RuboCop module Cop - module Homebrew - class FormulaCop < Cop - @registry = Cop.registry - - # This method is called by RuboCop and is the main entry point - def on_class(node) - file_path = processed_source.buffer.name - return unless file_path_allowed?(file_path) - class_node, parent_class_node, body = *node - return unless formula_class?(parent_class_node) - return unless respond_to?(:audit_formula) - @formula_name = class_name(class_node) - audit_formula(node, class_node, parent_class_node, body) - end + class FormulaCop < Cop + @registry = Cop.registry + + # This method is called by RuboCop and is the main entry point + def on_class(node) + file_path = processed_source.buffer.name + return unless file_path_allowed?(file_path) + class_node, parent_class_node, body = *node + return unless formula_class?(parent_class_node) + return unless respond_to?(:audit_formula) + @formula_name = class_name(class_node) + audit_formula(node, class_node, parent_class_node, body) + end - # Checks for regex match of pattern in the node and - # Sets the appropriate instance variables to report the match - def regex_match_group(node, pattern) - string_repr = string_content(node) - match_object = string_repr.match(pattern) - return unless match_object - node_begin_pos = start_column(node) - line_begin_pos = line_start_column(node) - @column = node_begin_pos + match_object.begin(0) - line_begin_pos + 1 - @length = match_object.to_s.length - @line_no = line_number(node) - @source_buf = source_buffer(node) - @offense_source_range = source_range(@source_buf, @line_no, @column, @length) - @offensive_node = node - match_object - end + # Checks for regex match of pattern in the node and + # Sets the appropriate instance variables to report the match + def regex_match_group(node, pattern) + string_repr = string_content(node) + match_object = string_repr.match(pattern) + return unless match_object + node_begin_pos = start_column(node) + line_begin_pos = line_start_column(node) + @column = node_begin_pos + match_object.begin(0) - line_begin_pos + 1 + @length = match_object.to_s.length + @line_no = line_number(node) + @source_buf = source_buffer(node) + @offense_source_range = source_range(@source_buf, @line_no, @column, @length) + @offensive_node = node + match_object + end - # Returns method_node matching method_name - def find_node_method_by_name(node, method_name) - return if node.nil? - node.each_child_node(:send) do |method_node| - next unless method_node.method_name == method_name - @offensive_node = method_node - @offense_source_range = method_node.source_range - return method_node - end - # If not found then, parent node becomes the offensive node - @offensive_node = node.parent - @offense_source_range = node.parent.source_range - nil - end + # Returns method_node matching method_name + def find_node_method_by_name(node, method_name) + return if node.nil? + node.each_child_node(:send) do |method_node| + next unless method_node.method_name == method_name + @offensive_node = method_node + @offense_source_range = method_node.source_range + return method_node + end + # If not found then, parent node becomes the offensive node + @offensive_node = node.parent + @offense_source_range = node.parent.source_range + nil + end - # Returns an array of method call nodes matching method_name inside node - def find_method_calls_by_name(node, method_name) - return if node.nil? - node.each_child_node(:send).select { |method_node| method_name == method_node.method_name } - end + # Returns an array of method call nodes matching method_name inside node + def find_method_calls_by_name(node, method_name) + return if node.nil? + node.each_child_node(:send).select { |method_node| method_name == method_node.method_name } + end - # Returns a block named block_name inside node - def find_block(node, block_name) - return if node.nil? - node.each_child_node(:block) do |block_node| - next if block_node.method_name != block_name - @offensive_node = block_node - @offense_source_range = block_node.source_range - return block_node - end - # If not found then, parent node becomes the offensive node - @offensive_node = node.parent - @offense_source_range = node.parent.source_range - nil - end + # Returns a block named block_name inside node + def find_block(node, block_name) + return if node.nil? + node.each_child_node(:block) do |block_node| + next if block_node.method_name != block_name + @offensive_node = block_node + @offense_source_range = block_node.source_range + return block_node + end + # If not found then, parent node becomes the offensive node + @offensive_node = node.parent + @offense_source_range = node.parent.source_range + nil + end - # Returns an array of block nodes named block_name inside node - def find_blocks(node, block_name) - return if node.nil? - node.each_child_node(:block).select { |block_node| block_name == block_node.method_name } - end + # Returns an array of block nodes named block_name inside node + def find_blocks(node, block_name) + return if node.nil? + node.each_child_node(:block).select { |block_node| block_name == block_node.method_name } + end - # Returns a method definition node with method_name - def find_method_def(node, method_name) - return if node.nil? - node.each_child_node(:def) do |def_node| - def_method_name = method_name(def_node) - next unless method_name == def_method_name - @offensive_node = def_node - @offense_source_range = def_node.source_range - return def_node - end - # If not found then, parent node becomes the offensive node - @offensive_node = node.parent - @offense_source_range = node.parent.source_range - nil - end + # Returns a method definition node with method_name + def find_method_def(node, method_name) + return if node.nil? + node.each_child_node(:def) do |def_node| + def_method_name = method_name(def_node) + next unless method_name == def_method_name + @offensive_node = def_node + @offense_source_range = def_node.source_range + return def_node + end + # If not found then, parent node becomes the offensive node + @offensive_node = node.parent + @offense_source_range = node.parent.source_range + nil + end - # Check if a method is called inside a block - def method_called_in_block?(node, method_name) - block_body = node.children[2] - block_body.each_child_node(:send) do |call_node| - next unless call_node.method_name == method_name - @offensive_node = call_node - @offense_source_range = call_node.source_range - return true - end - false + # Check if a method is called inside a block + def method_called_in_block?(node, method_name) + block_body = node.children[2] + block_body.each_child_node(:send) do |call_node| + next unless call_node.method_name == method_name + @offensive_node = call_node + @offense_source_range = call_node.source_range + return true end + false + end - # Check if method_name is called among the direct children nodes in the given node - def method_called?(node, method_name) - node.each_child_node(:send) do |call_node| - next unless call_node.method_name == method_name - @offensive_node = call_node - @offense_source_range = call_node.source_range - return true - end - false + # Check if method_name is called among the direct children nodes in the given node + def method_called?(node, method_name) + node.each_child_node(:send) do |call_node| + next unless call_node.method_name == method_name + @offensive_node = call_node + @offense_source_range = call_node.source_range + return true end + false + end - # Checks for precedence, returns the first pair of precedence violating nodes - def check_precedence(first_nodes, next_nodes) - next_nodes.each do |each_next_node| - first_nodes.each do |each_first_node| - if component_precedes?(each_first_node, each_next_node) - return [each_first_node, each_next_node] - end + # Checks for precedence, returns the first pair of precedence violating nodes + def check_precedence(first_nodes, next_nodes) + next_nodes.each do |each_next_node| + first_nodes.each do |each_first_node| + if component_precedes?(each_first_node, each_next_node) + return [each_first_node, each_next_node] end end - nil end + nil + end - # If first node does not precede next_node, sets appropriate instance variables for reporting - def component_precedes?(first_node, next_node) - return false if line_number(first_node) < line_number(next_node) - @offense_source_range = first_node.source_range - @offensive_node = first_node - true - end + # If first node does not precede next_node, sets appropriate instance variables for reporting + def component_precedes?(first_node, next_node) + return false if line_number(first_node) < line_number(next_node) + @offense_source_range = first_node.source_range + @offensive_node = first_node + true + end - # Returns the array of arguments of the method_node - def parameters(method_node) - return unless method_node.send_type? - method_node.method_args - end + # Returns the array of arguments of the method_node + def parameters(method_node) + return unless method_node.send_type? + method_node.method_args + end - # Returns the begin position of the node's line in source code - def line_start_column(node) - node.source_range.source_buffer.line_range(node.loc.line).begin_pos - end + # Returns the begin position of the node's line in source code + def line_start_column(node) + node.source_range.source_buffer.line_range(node.loc.line).begin_pos + end - # Returns the begin position of the node in source code - def start_column(node) - node.source_range.begin_pos - end + # Returns the begin position of the node in source code + def start_column(node) + node.source_range.begin_pos + end - # Returns the line number of the node - def line_number(node) - node.loc.line - end + # Returns the line number of the node + def line_number(node) + node.loc.line + end - # Returns the class node's name, nil if not a class node - def class_name(node) - @offensive_node = node - @offense_source_range = node.source_range - node.const_name - end + # Returns the class node's name, nil if not a class node + def class_name(node) + @offensive_node = node + @offense_source_range = node.source_range + node.const_name + end - # Returns the method name for a def node - def method_name(node) - node.children[0] if node.def_type? - end + # Returns the method name for a def node + def method_name(node) + node.children[0] if node.def_type? + end - # Returns the node size in the source code - def size(node) - node.source_range.size - end + # Returns the node size in the source code + def size(node) + node.source_range.size + end - # Returns the block length of the block node - def block_size(block) - block_length(block) - end + # Returns the block length of the block node + def block_size(block) + block_length(block) + end - # Source buffer is required as an argument to report style violations - def source_buffer(node) - node.source_range.source_buffer - end + # Source buffer is required as an argument to report style violations + def source_buffer(node) + node.source_range.source_buffer + end - # Returns the string representation if node is of type str - def string_content(node) - node.str_content if node.type == :str - end + # Returns the string representation if node is of type str + def string_content(node) + node.str_content if node.type == :str + end - # Returns printable component name - def format_component(component_node) - return component_node.method_name if component_node.send_type? || component_node.block_type? - method_name(component_node) if component_node.def_type? - end + # Returns printable component name + def format_component(component_node) + return component_node.method_name if component_node.send_type? || component_node.block_type? + method_name(component_node) if component_node.def_type? + end - def problem(msg) - add_offense(@offensive_node, @offense_source_range, msg) - end + def problem(msg) + add_offense(@offensive_node, @offense_source_range, msg) + end - private + private - def formula_class?(parent_class_node) - parent_class_node && parent_class_node.const_name == "Formula" - end + def formula_class?(parent_class_node) + parent_class_node && parent_class_node.const_name == "Formula" + end - def file_path_allowed?(file_path) - paths_to_exclude = [%r{/Library/Homebrew/compat/}, - %r{/Library/Homebrew/test/}] - return true if file_path.nil? # file_path is nil when source is directly passed to the cop eg., in specs - file_path !~ Regexp.union(paths_to_exclude) - end + def file_path_allowed?(file_path) + paths_to_exclude = [%r{/Library/Homebrew/compat/}, + %r{/Library/Homebrew/test/}] + return true if file_path.nil? # file_path is nil when source is directly passed to the cop eg., in specs + file_path !~ Regexp.union(paths_to_exclude) end end end diff --git a/Library/Homebrew/rubocops/formula_desc_cop.rb b/Library/Homebrew/rubocops/formula_desc_cop.rb index 7d69a48e7..d72fa53cc 100644 --- a/Library/Homebrew/rubocops/formula_desc_cop.rb +++ b/Library/Homebrew/rubocops/formula_desc_cop.rb @@ -3,7 +3,7 @@ require_relative "../extend/string" module RuboCop module Cop - module Homebrew + module FormulaAuditStrict # This cop audits `desc` in Formulae # # - Checks for existence of `desc` diff --git a/Library/Homebrew/test/rubocops/bottle_block_cop_spec.rb b/Library/Homebrew/test/rubocops/bottle_block_cop_spec.rb index 5be2d6cf5..a7ae0c6ac 100644 --- a/Library/Homebrew/test/rubocops/bottle_block_cop_spec.rb +++ b/Library/Homebrew/test/rubocops/bottle_block_cop_spec.rb @@ -3,7 +3,7 @@ require "rubocop/rspec/support" require_relative "../../extend/string" require_relative "../../rubocops/bottle_block_cop" -describe RuboCop::Cop::Homebrew::CorrectBottleBlock do +describe RuboCop::Cop::FormulaAuditStrict::CorrectBottleBlock do subject(:cop) { described_class.new } context "When auditing Bottle Block" do diff --git a/Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb b/Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb index 04c4c27da..7a3026703 100644 --- a/Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb +++ b/Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb @@ -3,7 +3,7 @@ require "rubocop/rspec/support" require_relative "../../extend/string" require_relative "../../rubocops/formula_desc_cop" -describe RuboCop::Cop::Homebrew::FormulaDesc do +describe RuboCop::Cop::FormulaAuditStrict::FormulaDesc do subject(:cop) { described_class.new } context "When auditing formula desc" do diff --git a/docs/Manpage.md b/docs/Manpage.md index 1f8906c8a..4e173668a 100644 --- a/docs/Manpage.md +++ b/docs/Manpage.md @@ -635,6 +635,10 @@ With `--verbose` or `-v`, many commands print extra debugging information. Note If `--except` is passed, the methods named `audit_`method`` will not be run. + If `--only-cops` is passed, only the mentioned cops' violations would be checked. + + If `--except-cops` is passed, the mentioned cops' 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. diff --git a/manpages/brew.1 b/manpages/brew.1 index 34cd7c5ed..e4b17e3b9 100644 --- a/manpages/brew.1 +++ b/manpages/brew.1 @@ -662,6 +662,12 @@ If \fB\-\-only\fR is passed, only the methods named \fBaudit_<method>\fR will be If \fB\-\-except\fR is passed, the methods named \fBaudit_<method>\fR will not be run\. . .IP +If \fB\-\-only\-cops\fR is passed, only the mentioned cops\' violations would be checked\. +. +.IP +If \fB\-\-except\-cops\fR is passed, the mentioned cops\' checks would be skipped\. +. +.IP \fBaudit\fR exits with a non\-zero status if any errors are found\. This is useful, for instance, for implementing pre\-commit hooks\. . .TP |