Refactor legacy_patches_cop to merge external patch audit rules in a single cop and add tests

3 files changed, 62 insertions, 64 deletions

diff --git a/Library/Homebrew/rubocops/extend/formula_cop.rb b/Library/Homebrew/rubocops/extend/formula_cop.rb
index e4a9a5b45..4be0c0fe3 100644
--- a/Library/Homebrew/rubocops/extend/formula_cop.rb
+++ b/Library/Homebrew/rubocops/extend/formula_cop.rb
@@ -380,40 +380,6 @@ 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
deleted file mode 100644
index 25a580b12..000000000
--- a/Library/Homebrew/rubocops/legacy_patches_cop.rb
+++ /dev/null
@@ -1,30 +0,0 @@
-require_relative "./extend/formula_cop"
-require_relative "../extend/string"
-
-module RuboCop
-  module Cop
-    module FormulaAudit
-      # This cop checks for and audits legacy patches in Formulae
-      class LegacyPatches < FormulaCop
-        def audit_formula(_node, _class_node, _parent_class_node, body)
-          patches_node = find_method_def(body, :patches)
-          return if patches_node.nil?
-          legacy_patches = find_strings(patches_node)
-          problem "Use the patch DSL instead of defining a 'patches' method"
-          legacy_patches.each { |p| patch_problems(p) }
-        end
-      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
-        end
-      end
-    end
-  end
-end
diff --git a/Library/Homebrew/rubocops/patches_cop.rb b/Library/Homebrew/rubocops/patches_cop.rb
new file mode 100644
index 000000000..fb14d8acc
--- /dev/null
+++ b/Library/Homebrew/rubocops/patches_cop.rb
@@ -0,0 +1,62 @@
+require_relative "./extend/formula_cop"
+require_relative "../extend/string"
+
+module RuboCop
+  module Cop
+    module FormulaAudit
+      # This cop audits patches in Formulae
+      class Patches < FormulaCop
+        def audit_formula(_node, _class_node, _parent_class_node, body)
+          external_patches = find_all_blocks(body, :patch)
+          external_patches.each do |patch_block|
+            url_node = find_every_method_call_by_name(patch_block, :url).first
+            url_string = parameters(url_node).first
+            patch_problems(url_string)
+          end
+
+          patches_node = find_method_def(body, :patches)
+          return if patches_node.nil?
+          legacy_patches = find_strings(patches_node)
+          problem "Use the patch DSL instead of defining a 'patches' method"
+          legacy_patches.each { |p| patch_problems(p) }
+        end
+
+        private
+
+        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_obj[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
+  end
+end