From 43cec1000088d885504d05d7bbbcaa514b6e008a Mon Sep 17 00:00:00 2001 From: Gautham Goli Date: Sun, 30 Jul 2017 16:02:06 +0530 Subject: [PATCH] audit: Port patches audit code to a rubocop --- Library/Homebrew/dev-cmd/audit.rb | 31 --------------- .../Homebrew/rubocops/extend/formula_cop.rb | 34 ++++++++++++++++ .../Homebrew/rubocops/legacy_patches_cop.rb | 39 ++++--------------- 3 files changed, 42 insertions(+), 62 deletions(-) diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index 7b5befaa0f..1f07fa89ea 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/Library/Homebrew/dev-cmd/audit.rb @@ -630,7 +630,6 @@ class FormulaAuditor end next if spec.patches.empty? - spec.patches.each { |p| patch_problems(p) if p.external? } next unless @new_formula problem "New formulae should not require patches to build. Patches should be submitted and accepted upstream first." end @@ -786,36 +785,6 @@ class FormulaAuditor end end - def patch_problems(patch) - case patch.url - when %r{https?://github\.com/.+/.+/(?:commit|pull)/[a-fA-F0-9]*.(?:patch|diff)} - unless patch.url =~ /\?full_index=\w+$/ - problem <<-EOS.undent - GitHub patches should use the full_index parameter: - #{patch.url}?full_index=1 - EOS - end - when /raw\.github\.com/, %r{gist\.github\.com/raw}, %r{gist\.github\.com/.+/raw}, - %r{gist\.githubusercontent\.com/.+/raw} - unless patch.url =~ /[a-fA-F0-9]{40}/ - problem "GitHub/Gist patches should specify a revision:\n#{patch.url}" - end - when %r{https?://patch-diff\.githubusercontent\.com/raw/(.+)/(.+)/pull/(.+)\.(?:diff|patch)} - problem <<-EOS.undent - use GitHub pull request URLs: - https://github.com/#{Regexp.last_match(1)}/#{Regexp.last_match(2)}/pull/#{Regexp.last_match(3)}.patch?full_index=1 - Rather than patch-diff: - #{patch.url} - EOS - when %r{macports/trunk} - problem "MacPorts patches should specify a revision instead of trunk:\n#{patch.url}" - when %r{^http://trac\.macports\.org} - problem "Patches from MacPorts Trac should be https://, not http:\n#{patch.url}" - when %r{^http://bugs\.debian\.org} - problem "Patches from Debian should be https://, not http:\n#{patch.url}" - end - end - def audit_text bin_names = Set.new bin_names << formula.name diff --git a/Library/Homebrew/rubocops/extend/formula_cop.rb b/Library/Homebrew/rubocops/extend/formula_cop.rb index 4be0c0fe30..e4a9a5b455 100644 --- a/Library/Homebrew/rubocops/extend/formula_cop.rb +++ b/Library/Homebrew/rubocops/extend/formula_cop.rb @@ -380,6 +380,40 @@ module RuboCop add_offense(@offensive_node, @offense_source_range, msg) end + def patch_problems(patch) + patch_url = string_content(patch) + gh_patch_patterns = Regexp.union([%r{/raw\.github\.com/}, + %r{gist\.github\.com/raw}, + %r{gist\.github\.com/.+/raw}, + %r{gist\.githubusercontent\.com/.+/raw}]) + if regex_match_group(patch, gh_patch_patterns) + unless patch_url =~ /[a-fA-F0-9]{40}/ + problem "GitHub/Gist patches should specify a revision:\n#{patch_url}" + end + end + + gh_patch_diff_pattern = %r{https?://patch-diff\.githubusercontent\.com/raw/(.+)/(.+)/pull/(.+)\.(?:diff|patch)} + if match_obj = regex_match_group(patch, gh_patch_diff_pattern) + problem <<-EOS.undent + use GitHub pull request URLs: + https://github.com/#{match_obj[1]}/#{match_obj[2]}/pull/#{match_ojb[3]}.patch + Rather than patch-diff: + #{patch_url} + EOS + end + + if regex_match_group(patch, %r{macports/trunk}) + problem "MacPorts patches should specify a revision instead of trunk:\n#{patch_url}" + end + + if regex_match_group(patch, %r{^http://trac\.macports\.org}) + problem "Patches from MacPorts Trac should be https://, not http:\n#{patch_url}" + end + + return unless regex_match_group(patch, %r{^http://bugs\.debian\.org}) + problem "Patches from Debian should be https://, not http:\n#{patch_url}" + end + private def formula_class?(node) diff --git a/Library/Homebrew/rubocops/legacy_patches_cop.rb b/Library/Homebrew/rubocops/legacy_patches_cop.rb index e569f650e1..25a580b12a 100644 --- a/Library/Homebrew/rubocops/legacy_patches_cop.rb +++ b/Library/Homebrew/rubocops/legacy_patches_cop.rb @@ -13,39 +13,16 @@ module RuboCop problem "Use the patch DSL instead of defining a 'patches' method" legacy_patches.each { |p| patch_problems(p) } end + end - def patch_problems(patch) - patch_url = string_content(patch) - gh_patch_patterns = Regexp.union([%r{/raw\.github\.com/}, - %r{gist\.github\.com/raw}, - %r{gist\.github\.com/.+/raw}, - %r{gist\.githubusercontent\.com/.+/raw}]) - if regex_match_group(patch, gh_patch_patterns) - unless patch_url =~ /[a-fA-F0-9]{40}/ - problem "GitHub/Gist patches should specify a revision:\n#{patch_url}" - end + class ExternalPatches < FormulaCop + def audit_formula(_node, _class_node, _parent_class_node, body) + patches = find_all_blocks(body, :patch) + patches.each do |patch_block| + url_node = find_node_method_by_name(patch_block, :url) + url_string = parameters(url_node).first + patch_problems(url_string) end - - gh_patch_diff_pattern = %r{https?://patch-diff\.githubusercontent\.com/raw/(.+)/(.+)/pull/(.+)\.(?:diff|patch)} - if match_obj = regex_match_group(patch, gh_patch_diff_pattern) - problem <<-EOS.undent - use GitHub pull request URLs: - https://github.com/#{match_obj[1]}/#{match_obj[2]}/pull/#{match_ojb[3]}.patch - Rather than patch-diff: - #{patch_url} - EOS - end - - if regex_match_group(patch, %r{macports/trunk}) - problem "MacPorts patches should specify a revision instead of trunk:\n#{patch_url}" - end - - if regex_match_group(patch, %r{^http://trac\.macports\.org}) - problem "Patches from MacPorts Trac should be https://, not http:\n#{patch_url}" - end - - return unless regex_match_group(patch, %r{^http://bugs\.debian\.org}) - problem "Patches from Debian should be https://, not http:\n#{patch_url}" end end end