From 087c1ca8d660f5604f9e9f31e3c4c18cb98d91bd Mon Sep 17 00:00:00 2001 From: Gautham Goli Date: Sat, 12 Aug 2017 20:50:43 +0530 Subject: [PATCH] audit: Port rules from line_problems to rubocop part 4(WIP-3) --- Library/Homebrew/dev-cmd/audit.rb | 42 ---------------- Library/Homebrew/rubocops/lines_cop.rb | 68 ++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 42 deletions(-) diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index 4195cdf2b1..f8236e6a60 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/Library/Homebrew/dev-cmd/audit.rb @@ -824,50 +824,8 @@ class FormulaAuditor problem "\"(#{Regexp.last_match(1)}...#{Regexp.last_match(2)})\" should be \"(#{Regexp.last_match(3).downcase}+...)\"" end - if line =~ /((man)\s*\+\s*(['"])(man[1-8])(['"]))/ - problem "\"#{Regexp.last_match(1)}\" should be \"#{Regexp.last_match(4)}\"" - end - - # Prefer formula path shortcuts in strings - if line =~ %r[(\#\{prefix\}/(bin|include|libexec|lib|sbin|share|Frameworks))] - problem "\"#{Regexp.last_match(1)}\" should be \"\#{#{Regexp.last_match(2).downcase}}\"" - end - - if line =~ %r[((\#\{prefix\}/share/man/|\#\{man\}/)(man[1-8]))] - problem "\"#{Regexp.last_match(1)}\" should be \"\#{#{Regexp.last_match(3)}}\"" - end - - if line =~ %r[((\#\{share\}/(man)))[/'"]] - problem "\"#{Regexp.last_match(1)}\" should be \"\#{#{Regexp.last_match(3)}}\"" - end - - if line =~ %r[(\#\{prefix\}/share/(info|man))] - problem "\"#{Regexp.last_match(1)}\" should be \"\#{#{Regexp.last_match(2)}}\"" - end - - if line =~ /depends_on\s+['"](.+)['"]\s+=>\s+:(lua|perl|python|ruby)(\d*)/ - 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 - problem "Use separate make calls" if line.include?("make && make") - # 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)}\"" - end - - if line =~ %r{(system|ENV\[.+\]\s?=)\s?['"](/usr/bin/)?((g|llvm-g|clang)\+\+)['" ]} - problem "Use \"\#{ENV.cxx}\" instead of hard-coding \"#{Regexp.last_match(3)}\"" - end - - if line =~ /system\s+['"](env|export)(\s+|['"])/ - problem "Use ENV instead of invoking '#{Regexp.last_match(1)}' to modify the environment" - end - - if line =~ /ARGV\.(?!(debug\?|verbose\?|value[\(\s]))/ - problem "Use build instead of ARGV to check options" - end - if line =~ /JAVA_HOME/i && !formula.requirements.map(&:class).include?(JavaRequirement) problem "Use `depends_on :java` to set JAVA_HOME" end diff --git a/Library/Homebrew/rubocops/lines_cop.rb b/Library/Homebrew/rubocops/lines_cop.rb index 4c357f6ab3..32d05fb229 100644 --- a/Library/Homebrew/rubocops/lines_cop.rb +++ b/Library/Homebrew/rubocops/lines_cop.rb @@ -88,6 +88,66 @@ module RuboCop end end + [:debug?, :verbose?, :value].each do |m| + find_instance_method_call(body_node, :ARGV, m) do + problem "Use build instead of ARGV to check options" + end + end + + find_instance_method_call(body_node, :man, :+) do |m| + next unless match = regex_match_group(parameters(m).first, %r{man[1-8]}) + problem "\"#{m.source}\" should be \"#{match[1]}\"" + end + + # Avoid hard-coding compilers + find_every_method_call_by_name(body_node, :system).each do |m| + param = parameters(m).first + if match = regex_match_group(param, %r{(/usr/bin/)?(gcc|llvm-gcc|clang)\s?}) + problem "Use \"\#{ENV.cc}\" instead of hard-coding \"#{match[3]}\"" + elsif match = regex_match_group(param, %r{(/usr/bin/)?((g|llvm-g|clang)\+\+)\s?}) + problem "Use \"\#{ENV.cxx}\" instead of hard-coding \"#{match[3]}\"" + end + end + + find_instance_method_call(body_node, :ENV, :[]=) do |m| + param = parameters(m)[1] + if match = regex_match_group(param, %r{(/usr/bin/)?(gcc|llvm-gcc|clang)\s?}) + problem "Use \"\#{ENV.cc}\" instead of hard-coding \"#{match[3]}\"" + elsif match = regex_match_group(param, %r{(/usr/bin/)?((g|llvm-g|clang)\+\+)\s?}) + problem "Use \"\#{ENV.cxx}\" instead of hard-coding \"#{match[3]}\"" + end + end + + # Prefer formula path shortcuts in strings + formula_path_strings(body_node, :prefix) do |p| + next unless match = regex_match_group(p, %r{(/(man))[/'"]}) + problem "\"\#\{prefix}#{match[1]}\" should be \"\#{#{match[3]}}\"" + end + + formula_path_strings(body_node, :share) do |p| + if match = regex_match_group(p, %r{/(bin|include|libexec|lib|sbin|share|Frameworks)}i) + problem "\"\#\{prefix}#{match[1]}\" should be \"\#{#{match[1].downcase}}\"" + end + if match = regex_match_group(p, %r{((/share/man/|\#\{man\}/)(man[1-8]))}) + problem "\"\#\{prefix}#{match[1]}\" should be \"\#{#{match[3]}}\"" + end + if match = regex_match_group(p, %r{(/share/(info|man))}) + problem "\"\#\{prefix}#{match[1]}\" should be \"\#{#{match[2]}}\"" + end + end + + find_every_method_call_by_name(body_node, :depends_on) do |m| + key, value = destructure_hash(paramters(m).first) + next unless key.str_type? + next unless match = regex_match_group(value, %r{(lua|perl|python|ruby)(\d*)}) + problem "#{match[1]} modules should be vendored rather than use deprecated #{m.source}`" + end + + find_every_method_call_by_name(body_node, :system).each do |m| + next unless match = regex_match_group(parameters(m).first, %r{(env|export)(\s+)?}) + problem "Use ENV instead of invoking '#{match[1]}' to modify the environment" + end + find_every_method_call_by_name(body_node, :depends_on).each do |m| next unless modifier?(m) dep, option = hash_dep(m) @@ -281,6 +341,14 @@ module RuboCop $(hash (pair $(str _) (array $(str _) ...)))} EOS + def_node_search :destructure_hash, <<-EOS.undent + (hash (pair $_ $_)) + EOS + + def_node_matcher :formula_path_strings, <<-EOS.undent + (dstr (begin (send nil %1)) $(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