Merge pull request #2975 from GauthamGoli/audit_urls_rubocop_strict

audit: Port audit_urls strict rules to rubocop, add tests, autocorrect

Library/Homebrew/dev-cmd/audit.rb

@@ -1278,14 +1278,6 @@ class ResourceAuditor
       problem "should always include at least one HTTP url"
     end
 
-    # Check pypi urls
-    if @strict
-      urls.each do |p|
-        next unless p =~ %r{^https?://pypi.python.org/(.*)}
-        problem "#{p} should be `https://files.pythonhosted.org/#{Regexp.last_match(1)}`"
-      end
-    end
-
     return unless @online
     urls.each do |url|
       next if !@strict && mirrors.include?(url)

Library/Homebrew/rubocops/extend/formula_cop.rb

@@ -37,6 +37,20 @@ module RuboCop
         match_object
       end
 
+      # Yields to block when there is a match
+      # Parameters: urls : Array of url/mirror method call nodes
+      #             regex: regex pattern to match urls
+      def audit_urls(urls, regex)
+        urls.each do |url_node|
+          url_string_node = parameters(url_node).first
+          url_string = string_content(url_string_node)
+          match_object = regex_match_group(url_string_node, regex)
+          next unless match_object
+          offending_node(url_string_node.parent)
+          yield match_object, url_string
+        end
+      end
+
       # Returns all string nodes among the descendants of given node
       def find_strings(node)
         return [] if node.nil?

Library/Homebrew/rubocops/urls_cop.rb

@@ -188,17 +188,32 @@ module RuboCop
             problem "#{url} should be `https://search.maven.org/remotecontent?filepath=#{match[1]}`"
           end
         end
+      end
+    end
+    module FormulaAuditStrict
+      class PyPiUrls < FormulaCop
+        def audit_formula(_node, _class_node, _parent_class_node, body_node)
+          urls = find_every_method_call_by_name(body_node, :url)
+          mirrors = find_every_method_call_by_name(body_node, :mirror)
+          urls += mirrors
+
+          # Check pypi urls
+          @pypi_pattern = %r{^https?://pypi.python.org/(.*)}
+          audit_urls(urls, @pypi_pattern) do |match, url|
+            problem "#{url} should be `https://files.pythonhosted.org/#{match[1]}`"
+          end
+        end
 
         private
 
-        def audit_urls(urls, regex)
+        def autocorrect(node)
-          urls.each do |url_node|
+          lambda do |corrector|
-            url_string_node = parameters(url_node).first
+            url_string_node = parameters(node).first
-            url_string = string_content(url_string_node)
+            url = string_content(url_string_node)
-            match_object = regex_match_group(url_string_node, regex)
+            match = regex_match_group(url_string_node, @pypi_pattern)
-            next unless match_object
+            correction = node.source.sub(url, "https://files.pythonhosted.org/#{match[1]}")
-            offending_node(url_string_node.parent)
+            corrector.insert_before(node.source_range, correction)
-            yield match_object, url_string
+            corrector.remove(node.source_range)
           end
         end
       end

Library/Homebrew/test/rubocops/urls_cop_spec.rb

@@ -182,3 +182,46 @@ describe RuboCop::Cop::FormulaAudit::Urls do
     end
   end
 end
+
+describe RuboCop::Cop::FormulaAuditStrict::PyPiUrls do
+  subject(:cop) { described_class.new }
+
+  context "When auditing urls" do
+    it "with pypi offenses" do
+      formulas = [{
+        "url" => "https://pypi.python.org/packages/source/foo/foo-0.1.tar.gz",
+        "msg" => "https://pypi.python.org/packages/source/foo/foo-0.1.tar.gz should be `https://files.pythonhosted.org/packages/source/foo/foo-0.1.tar.gz`",
+        "col" => 2,
+        "corrected_url" =>"https://files.pythonhosted.org/packages/source/foo/foo-0.1.tar.gz",
+      }]
+      formulas.each do |formula|
+        source = <<-EOS.undent
+          class Foo < Formula
+            desc "foo"
+            url "#{formula["url"]}"
+          end
+        EOS
+        corrected_source = <<-EOS.undent
+          class Foo < Formula
+            desc "foo"
+            url "#{formula["corrected_url"]}"
+          end
+        EOS
+        expected_offenses = [{ message: formula["msg"],
+                               severity: :convention,
+                               line: 3,
+                               column: formula["col"],
+                               source: source }]
+
+        inspect_source(cop, source)
+        # Check for expected offenses
+        expected_offenses.zip(cop.offenses.reverse).each do |expected, actual|
+          expect_offense(expected, actual)
+        end
+        # Check for expected auto corrected source
+        new_source = autocorrect_source(cop, source)
+        expect(new_source).to eq(corrected_source)
+      end
+    end
+  end
+end