audit: Port patches audit code to a rubocop

3 files changed, 42 insertions, 62 deletions

diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb
index 7b5befaa0..1f07fa89e 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 4be0c0fe3..e4a9a5b45 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 e569f650e..25a580b12 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
-          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}"
+      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
-
-          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