admin/brew
1
0
Fork 0
Code Issues Packages Projects Releases 6 Wiki Activity

audit: Port rules from line_problems to rubocop part 4(WIP-2)

Browse Source
This commit is contained in:
Gautham Goli 2017-08-10 19:57:53 +05:30
parent a92e1eda27
commit 4295a4ca78
4 changed files with 151 additions and 90 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
Library/.rubocop.yml
View File

@ -118,6 +118,10 @@ Style/Documentation:
Style/Encoding:
Enabled: true
# use spaces for indentation; detect tabs
Style/Tab:
Enabled: true
# dashes in filenames are typical
Style/FileName:
Regex: !ruby/regexp /^[\w\@\-\+\.]+(\.rb)?$/

87
Library/Homebrew/dev-cmd/audit.rb
View File

@ -849,23 +849,8 @@ class FormulaAuditor
problem "#{Regexp.last_match(2)} modules should be vendored rather than use deprecated `depends_on \"#{Regexp.last_match(1)}\" => :#{Regexp.last_match(2)}#{Regexp.last_match(3)}`"
end
if line =~ /depends_on\s+['"](.+)['"]\s+=>\s+(.*)/
dep = Regexp.last_match(1)
Regexp.last_match(2).split(" ").map do |o|
break if ["if", "unless"].include?(o)
next unless o =~ /^\[?['"](.*)['"]/
problem "Dependency #{dep} should not use option #{Regexp.last_match(1)}"
end
end
if line =~ /if\s+ARGV\.include\?\s+'--(HEAD|devel)'/
problem "Use \"if build.#{Regexp.last_match(1).downcase}?\" instead"
end
problem "Use separate make calls" if line.include?("make && make")
problem "Use spaces instead of tabs for indentation" if line =~ /^[ ]*\t/
# Avoid hard-coding compilers
if line =~ %r{(system|ENV\[.+\]\s?=)\s?['"](/usr/bin/)?(gcc|llvm-gcc|clang)['" ]}
problem "Use \"\#{ENV.cc}\" instead of hard-coding \"#{Regexp.last_match(3)}\""
@ -879,74 +864,14 @@ class FormulaAuditor
problem "Use ENV instead of invoking '#{Regexp.last_match(1)}' to modify the environment"
end
if line =~ /version == ['"]HEAD['"]/
problem "Use 'build.head?' instead of inspecting 'version'"
end
if line =~ /build\.include\?[\s\(]+['"]\-\-(.*)['"]/
problem "Reference '#{Regexp.last_match(1)}' without dashes"
end
if line =~ /build\.include\?[\s\(]+['"]with(out)?-(.*)['"]/
problem "Use build.with#{Regexp.last_match(1)}? \"#{Regexp.last_match(2)}\" instead of build.include? 'with#{Regexp.last_match(1)}-#{Regexp.last_match(2)}'"
end
if line =~ /build\.with\?[\s\(]+['"]-?-?with-(.*)['"]/
problem "Don't duplicate 'with': Use `build.with? \"#{Regexp.last_match(1)}\"` to check for \"--with-#{Regexp.last_match(1)}\""
end
if line =~ /build\.without\?[\s\(]+['"]-?-?without-(.*)['"]/
problem "Don't duplicate 'without': Use `build.without? \"#{Regexp.last_match(1)}\"` to check for \"--without-#{Regexp.last_match(1)}\""
end
if line =~ /unless build\.with\?(.*)/
problem "Use if build.without?#{Regexp.last_match(1)} instead of unless build.with?#{Regexp.last_match(1)}"
end
if line =~ /unless build\.without\?(.*)/
problem "Use if build.with?#{Regexp.last_match(1)} instead of unless build.without?#{Regexp.last_match(1)}"
end
if line =~ /(not\s|!)\s*build\.with?\?/
problem "Don't negate 'build.with?': use 'build.without?'"
end
if line =~ /(not\s|!)\s*build\.without?\?/
problem "Don't negate 'build.without?': use 'build.with?'"
end
if line =~ /ARGV\.(?!(debug\?|verbose\?|value[\(\s]))/
problem "Use build instead of ARGV to check options"
end
if line.include?("MACOS_VERSION")
problem "Use MacOS.version instead of MACOS_VERSION"
end
if line.include?("MACOS_FULL_VERSION")
problem "Use MacOS.full_version instead of MACOS_FULL_VERSION"
end
if line =~ /^def (\w+).*$/
problem "Define method #{Regexp.last_match(1).inspect} in the class body, not at the top-level"
end
if line.include?("ENV.fortran") && !formula.requirements.map(&:class).include?(FortranRequirement)
problem "Use `depends_on :fortran` instead of `ENV.fortran`"
end
if line =~ /JAVA_HOME/i && !formula.requirements.map(&:class).include?(JavaRequirement)
problem "Use `depends_on :java` to set JAVA_HOME"
end
if line =~ /depends_on :(.+) (if.+|unless.+)$/
conditional_dep_problems(Regexp.last_match(1).to_sym, Regexp.last_match(2), $&)
end
if line =~ /depends_on ['"](.+)['"] (if.+|unless.+)$/
conditional_dep_problems(Regexp.last_match(1), Regexp.last_match(2), $&)
end
return unless @strict
problem "`#{Regexp.last_match(1)}` in formulae is deprecated" if line =~ /(env :(std|userpaths))/
@ -993,18 +918,6 @@ class FormulaAuditor
EOS
end
def conditional_dep_problems(dep, condition, line)
quoted_dep = quote_dep(dep)
dep = Regexp.escape(dep.to_s)
case condition
when /if build\.include\? ['"]with-#{dep}['"]$/, /if build\.with\? ['"]#{dep}['"]$/
problem %Q(Replace #{line.inspect} with "depends_on #{quoted_dep} => :optional")
when /unless build\.include\? ['"]without-#{dep}['"]$/, /unless build\.without\? ['"]#{dep}['"]$/
problem %Q(Replace #{line.inspect} with "depends_on #{quoted_dep} => :recommended")
end
end
def quote_dep(dep)
dep.is_a?(Symbol) ? dep.inspect : "'#{dep}'"
end

25
Library/Homebrew/rubocops/extend/formula_cop.rb
View File

@ -55,6 +55,7 @@ module RuboCop
# Returns all string nodes among the descendants of given node
def find_strings(node)
return [] if node.nil?
return node if node.str_type?
node.each_descendant(:str)
end
@ -100,7 +101,9 @@ module RuboCop
def find_method_with_args(node, method_name, *args)
methods = find_every_method_call_by_name(node, method_name)
methods.each do |method|
yield method if parameters_passed?(method, *args)
next unless parameters_passed?(method, *args)
return true unless block_given?
yield method
end
end
@ -114,6 +117,7 @@ module RuboCop
next unless method.receiver && (method.receiver.const_name == instance || method.receiver.method_name == instance)
@offense_source_range = method.source_range
@offensive_node = method
return true unless block_given?
yield method
end
end
@ -165,6 +169,20 @@ module RuboCop
type_match && name_match
end
# Find CONSTANTs in the source
# if block given, yield matching nodes
def find_const(node, const_name)
return if node.nil?
node.each_child_node(:const) do |const_node|
next if const_node.const_name != const_name
@offensive_node = const_node
@offense_source_range = const_node.source_range
yield const_node if block_given?
return true
end
nil
end
# To compare node with appropriate Ruby variable
def node_equals?(node, var)
node == Parser::CurrentRuby.parse(var.inspect)
@ -204,11 +222,12 @@ module RuboCop
end
# Returns a method definition node with method_name
def find_method_def(node, method_name)
# Returns first method def if method_name is nil
def find_method_def(node, method_name = nil)
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
next unless method_name == def_method_name || method_name.nil?
@offensive_node = def_node
@offense_source_range = def_node.source_range
return def_node

125
Library/Homebrew/rubocops/lines_cop.rb
View File

@ -88,6 +88,59 @@ module RuboCop
end
end
find_every_method_call_by_name(body_node, :depends_on).each do |m|
next unless modifier?(m)
dep, option = hash_dep(m)
next if dep.nil? || option.nil?
problem "Dependency #{string_content(dep)} should not use option #{string_content(option)}"
end
find_instance_method_call(body_node, :version, :==) do |m|
next unless parameters_passed?(m, "HEAD")
problem "Use 'build.head?' instead of inspecting 'version'"
end
find_instance_method_call(body_node, :ENV, :fortran) do
next if depends_on?(:fortran)
problem "Use `depends_on :fortran` instead of `ENV.fortran`"
end
find_instance_method_call(body_node, :ARGV, :include?) do |m|
param = parameters(m).first
next unless match = regex_match_group(param, %r{--(HEAD|devel)})
problem "Use \"if build.#{match[1].downcase}?\" instead"
end
find_const(body_node, :MACOS_VERSION) do
problem "Use MacOS.version instead of MACOS_VERSION"
end
find_const(body_node, :MACOS_FULL_VERSION) do
problem "Use MacOS.full_version instead of MACOS_FULL_VERSION"
end
dependency(body_node) do |m|
# handle symbols and shit: WIP
next unless modifier?(m.parent)
dep = parameters(m).first
condition = m.parent.condition
if (condition.if? && condition.method_name == :include? && parameters_passed(condition, /with-#{string_content(dep)}$/))||
(condition.if? && condition.method_name == :with? && parameters_passed?(condition, /#{string_content(dep)}$/))
problem "Replace #{m.parent.source} with #{dep.source} => :optional"
end
if (condition.unless? && condition.method_name == :include? && parameters_passed?(condition, /without-#{string_content(dep)}$/))||
(condition.unless? && condition.method_name == :without? && parameters_passed?(condition, /#{string_content(dep)}$/))
problem "Replace #{m.parent.source} with #{dep.source} => :recommended"
end
end
find_every_method_call_by_name(body_node, :depends_on).each do |m|
next unless modifier?(m.parent)
dep = parameters(m).first
next if dep.hash_type?
condition = m.parent.node_parts
end
find_method_with_args(body_node, :fails_with, :llvm) do
problem "'fails_with :llvm' is now a no-op so should be removed"
end
@ -154,9 +207,81 @@ module RuboCop
problem "Use the `#{method}` Ruby method instead of `#{m.source}`"
end
if find_method_def(@processed_source.ast)
problem "Define method #{method_name(@offensive_node)} in the class body, not at the top-level"
end
find_instance_method_call(body_node, :build, :without?) do |m|
next unless unless_modifier?(m.parent)
correct = m.source.gsub("out?", "?").gsub("unless", "if")
problem "Use #{correct} instead of unless #{m.source}"
end
find_instance_method_call(body_node, :build, :with?) do |m|
next unless unless_modifier?(m.parent)
correct = m.source.gsub("?", "out?").gsub("unless", "if")
problem "Use #{correct} instead of unless #{m.source}"
end
find_instance_method_call(body_node, :build, :with?) do |m|
next unless negation?(m)
problem "Don't negate 'build.with?': use 'build.without?'"
end
find_instance_method_call(body_node, :build, :without?) do |m|
next unless negation?(m)
problem "Don't negate 'build.without?': use 'build.with?'"
end
find_instance_method_call(body_node, :build, :without?) do |m|
arg = parameters(m).first
next unless match = regex_match_group(arg, %r{-?-?without-(.*)})
problem "Don't duplicate 'without': Use `build.without? \"#{match[1]}\"` to check for \"--without-#{match[1]}\""
end
find_instance_method_call(body_node, :build, :with?) do |m|
arg = parameters(m).first
next unless match = regex_match_group(arg, %r{-?-?with-(.*)})
problem "Don't duplicate 'with': Use `build.with? \"#{match[1]}\"` to check for \"--with-#{match[1]}\""
end
find_instance_method_call(body_node, :build, :include?) do |m|
arg = parameters(m).first
next unless match = regex_match_group(arg, %r{with(out)?-(.*)})
problem "Use build.with#{match[1]}? \"#{match[2]}\" instead of build.include? 'with#{match[1]}-#{match[2]}'"
end
find_instance_method_call(body_node, :build, :include?) do |m|
arg = parameters(m).first
next unless match = regex_match_group(arg, %r{\-\-(.*)})
problem "Reference '#{match[1]}' without dashes"
end
end
def unless_modifier?(node)
node.modifier_form? && node.unless?
end
def modifier?(node)
node.modifier_form?
end
def_node_search :condition, <<-EOS.undent
(send (send nil :build) $_ $({str sym} _))
EOS
def_node_search :dependency, <<-EOS.undent
(send nil :depends_on ({str sym} _))
EOS
# Match depends_on with hash as argument
def_node_search :hash_dep, <<-EOS.undent
{$(hash (pair $(str _) $(str _)))
$(hash (pair $(str _) (array $(str _) ...)))}
EOS
def_node_matcher :negation?, '(send ... :!)'
# This is Pattern Matching method for AST
# Takes the AST node as argument and yields matching node if block given
# Else returns boolean for the match