diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index 6d869a077a..7b5befaa0f 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/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) diff --git a/Library/Homebrew/rubocops/extend/formula_cop.rb b/Library/Homebrew/rubocops/extend/formula_cop.rb index e42d6ee191..4be0c0fe30 100644 --- a/Library/Homebrew/rubocops/extend/formula_cop.rb +++ b/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? diff --git a/Library/Homebrew/rubocops/urls_cop.rb b/Library/Homebrew/rubocops/urls_cop.rb index 94f049aed3..676e735238 100644 --- a/Library/Homebrew/rubocops/urls_cop.rb +++ b/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) - 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 + def autocorrect(node) + lambda do |corrector| + url_string_node = parameters(node).first + url = string_content(url_string_node) + match = regex_match_group(url_string_node, @pypi_pattern) + correction = node.source.sub(url, "https://files.pythonhosted.org/#{match[1]}") + corrector.insert_before(node.source_range, correction) + corrector.remove(node.source_range) end end end diff --git a/Library/Homebrew/test/rubocops/urls_cop_spec.rb b/Library/Homebrew/test/rubocops/urls_cop_spec.rb index 733732bd01..280da6314c 100644 --- a/Library/Homebrew/test/rubocops/urls_cop_spec.rb +++ b/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