diff options
Diffstat (limited to 'Library')
| -rw-r--r-- | Library/.rubocop.yml | 7 | ||||
| -rw-r--r-- | Library/Homebrew/rubocops/bottle_block_cop.rb | 22 | ||||
| -rw-r--r-- | Library/Homebrew/rubocops/extend/formula_cop.rb | 126 | ||||
| -rw-r--r-- | Library/Homebrew/rubocops/formula_desc_cop.rb | 78 | ||||
| -rw-r--r-- | Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb | 2 |
5 files changed, 149 insertions, 86 deletions
diff --git a/Library/.rubocop.yml b/Library/.rubocop.yml index 41219f2cd..a782c1117 100644 --- a/Library/.rubocop.yml +++ b/Library/.rubocop.yml @@ -8,17 +8,10 @@ AllCops: require: ./Homebrew/rubocops.rb -Homebrew/FormulaCop: - Enabled: false - Homebrew/CorrectBottleBlock: - Include: - - '**/Taps/homebrew/**/*.rb' Enabled: true Homebrew/FormulaDesc: - Include: - - '**/Taps/homebrew/**/*.rb' Enabled: true Metrics/AbcSize: diff --git a/Library/Homebrew/rubocops/bottle_block_cop.rb b/Library/Homebrew/rubocops/bottle_block_cop.rb index 733c492e8..4d7a94461 100644 --- a/Library/Homebrew/rubocops/bottle_block_cop.rb +++ b/Library/Homebrew/rubocops/bottle_block_cop.rb @@ -11,21 +11,13 @@ module RuboCop MSG = "Use rebuild instead of revision in bottle block".freeze def audit_formula(_node, _class_node, _parent_class_node, formula_class_body_node) - check(formula_class_body_node) + bottle = find_block(formula_class_body_node, :bottle) + return if bottle.nil? || block_size(bottle).zero? + problem "Use rebuild instead of revision in bottle block" if method_called?(bottle, :revision) end private - def check(formula_class_body_node) - formula_class_body_node.each_child_node(:block) do |block_node| - next if block_length(block_node).zero? - method, _args, block_body = *block_node - _keyword, method_name = *method - next unless method_name == :bottle - check_revision?(block_body) - end - end - def autocorrect(node) lambda do |corrector| correction = node.source.sub("revision", "rebuild") @@ -33,14 +25,6 @@ module RuboCop corrector.remove(node.source_range) end end - - def check_revision?(body) - body.children.each do |method_call_node| - _receiver, method_name, _args = *method_call_node - next unless method_name == :revision - add_offense(method_call_node, :expression) - end - end end end end diff --git a/Library/Homebrew/rubocops/extend/formula_cop.rb b/Library/Homebrew/rubocops/extend/formula_cop.rb index cf5d513c9..49108bd48 100644 --- a/Library/Homebrew/rubocops/extend/formula_cop.rb +++ b/Library/Homebrew/rubocops/extend/formula_cop.rb @@ -6,16 +6,138 @@ module RuboCop def on_class(node) # This method is called by RuboCop and is the main entry point + file_path = processed_source.buffer.name + return unless file_path_allowed?(file_path) class_node, parent_class_node, body = *node - return unless a_formula_class?(parent_class_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 + def regex_match_group(node, pattern) + # Checks for regex match of pattern in the node and + # Sets the appropriate instance variables to report the match + 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 + + def find_node_method_by_name(node, method_name) + # Returns method_node matching 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 + + def find_block(node, block_name) + # Returns a block named block_name inside node + 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 + + def method_called?(node, method_name) + # Check if a method is called inside a block + 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 + + def parameters(method_node) + # Returns the array of arguments of the method_node + return unless method_node.send_type? + method_node.method_args + end + + def line_start_column(node) + # Returns the begin position of the node's line in source code + node.source_range.source_buffer.line_range(node.loc.line).begin_pos + end + + def start_column(node) + # Returns the begin position of the node in source code + node.source_range.begin_pos + end + + def line_number(node) + # Returns the line number of the node + node.loc.line + end + + def class_name(node) + # Returns the class node's name, nil if not a class node + @offensive_node = node + @offense_source_range = node.source_range + node.const_name + end + + def size(node) + # Returns the node size in the source code + node.source_range.size + end + + def block_size(block) + # Returns the block length of the block node + block_length(block) + end + + def source_buffer(node) + # Source buffer is required as an argument to report style violations + node.source_range.source_buffer + end + + def string_content(node) + # Returns the string representation if node is of type str + node.str_content if node.type == :str + end + + def problem(msg) + add_offense(@offensive_node, @offense_source_range, msg) + end + private - def a_formula_class?(parent_class_node) + 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 end end end diff --git a/Library/Homebrew/rubocops/formula_desc_cop.rb b/Library/Homebrew/rubocops/formula_desc_cop.rb index d276be5cc..7d69a48e7 100644 --- a/Library/Homebrew/rubocops/formula_desc_cop.rb +++ b/Library/Homebrew/rubocops/formula_desc_cop.rb @@ -11,72 +11,36 @@ module RuboCop # - Checks if `desc` begins with an article # - Checks for correct usage of `command-line` in `desc` # - Checks if `desc` contains the formula name - class FormulaDesc < FormulaCop - attr_accessor :formula_name, :description, :source_buffer, :line_number, :line_begin_pos, - :desc_begin_pos, :call_node - - def audit_formula(node, class_node, _parent_class_node, body) - check(node, body, class_node.const_name) - end - - private - - def check(node, body, formula_name) - body.each_child_node(:send) do |call_node| - _receiver, call_name, args = *call_node - next if call_name != :desc || args.children[0].empty? - @formula_name = formula_name - @description = args.children[0] - @source_buffer = call_node.source_range.source_buffer - @line_number = call_node.loc.line - @line_begin_pos = call_node.source_range.source_buffer.line_range(call_node.loc.line).begin_pos - @desc_begin_pos = call_node.children[2].source_range.begin_pos - @call_node = call_node + def audit_formula(_node, _class_node, _parent_class_node, body) + desc_call = find_node_method_by_name(body, :desc) - check_for_desc_length_offense - - check_for_offense(/(command ?line)/i, - "Description should use \"command-line\" instead of \"%s\"") - - check_for_offense(/^(an?)\s/i, - "Description shouldn't start with an indefinite article (%s)") - - check_for_offense(/^#{formula_name}/i, - "Description shouldn't include the formula name") - return nil + if desc_call.nil? + problem "Formula should have a desc (Description)." + return end - add_offense(node, node.source_range, "Formula should have a desc (Description).") - end - def check_for_offense(regex, offense_msg) - # This method checks if particular regex has a match within formula's desc - # If so, adds a violation - match_object = @description.match(regex) - if match_object - column = @desc_begin_pos + match_object.begin(0) - @line_begin_pos + 1 - length = match_object.to_s.length - offense_source_range = source_range(source_buffer, @line_number, column, length) - offense_msg = offense_msg % [match_object] - add_offense(@call_node, offense_source_range, offense_msg) - end - end - - def check_for_desc_length_offense - # This method checks if desc length > max_desc_length - # If so, adds a violation - desc_length = "#{@formula_name}: #{@description}".length + desc = parameters(desc_call).first + desc_length = "#{@formula_name}: #{string_content(desc)}".length max_desc_length = 80 if desc_length > max_desc_length - column = @desc_begin_pos - @line_begin_pos - length = @call_node.children[2].source_range.size - offense_source_range = source_range(source_buffer, @line_number, column, length) - desc_length_offense_msg = <<-EOS.undent + problem <<-EOS.undent Description is too long. "name: desc" should be less than #{max_desc_length} characters. - Length is calculated as #{@formula_name} + desc. (currently #{"#{@formula_name}: #{@description}".length}) + Length is calculated as #{@formula_name} + desc. (currently #{desc_length}) EOS - add_offense(@call_node, offense_source_range, desc_length_offense_msg) end + + # Check if command-line is wrongly used in formula's desc + if match = regex_match_group(desc, /(command ?line)/i) + problem "Description should use \"command-line\" instead of \"#{match}\"" + end + + if match = regex_match_group(desc, /^(an?)\s/i) + problem "Description shouldn't start with an indefinite article (#{match})" + end + + # Check if formula's name is used in formula's desc + problem "Description shouldn't include the formula name" if regex_match_group(desc, /^#{@formula_name}/i) end end end diff --git a/Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb b/Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb index cd506623f..04c4c27da 100644 --- a/Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb +++ b/Library/Homebrew/test/rubocops/formula_desc_cop_spec.rb @@ -42,7 +42,7 @@ describe RuboCop::Cop::Homebrew::FormulaDesc do expected_offenses = [{ message: msg, severity: :convention, line: 3, - column: 7, + column: 2, source: source }] inspect_source(cop, source) |