Wrap rubocop specific code into methods inside FormulaCop
This commit is contained in:
parent
febc108598
commit
a693ca332e
@ -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:
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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)
|
||||
desc_call = find_node_method_by_name(body, :desc)
|
||||
|
||||
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
|
||||
|
||||
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
|
||||
|
||||
@ -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)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user