audit: Port audit_legacy_patches method to rubocop and add tests

This commit is contained in:
Gautham Goli 2017-05-25 22:21:23 +05:30
parent 3fe32053a7
commit 2e82754776
3 changed files with 181 additions and 10 deletions

View File

@ -817,16 +817,6 @@ class FormulaAuditor
end
end
def audit_legacy_patches
return unless formula.respond_to?(:patches)
legacy_patches = Patch.normalize_legacy_patches(formula.patches).grep(LegacyPatch)
return if legacy_patches.empty?
problem "Use the patch DSL instead of defining a 'patches' method"
legacy_patches.each { |p| patch_problems(p) }
end
def patch_problems(patch)
case patch.url
when /raw\.github\.com/, %r{gist\.github\.com/raw}, %r{gist\.github\.com/.+/raw},

View File

@ -0,0 +1,53 @@
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
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
end
end
end
end

View File

@ -0,0 +1,128 @@
require "rubocop"
require "rubocop/rspec/support"
require_relative "../../extend/string"
require_relative "../../rubocops/legacy_patches_cop"
describe RuboCop::Cop::FormulaAudit::LegacyPatches do
subject(:cop) { described_class.new }
context "When auditing legacy patches" do
it "When there is no legacy patch" do
source = <<-EOS.undent
class Foo < Formula
url 'http://example.com/foo-1.0.tgz'
end
EOS
inspect_source(cop, source)
expect(cop.offenses).to eq([])
end
it "Formula with `def patches`" do
source = <<-EOS.undent
class Foo < Formula
homepage "ftp://example.com/foo"
url "http://example.com/foo-1.0.tgz"
def patches
DATA
end
end
EOS
expected_offenses = [{ message: "Use the patch DSL instead of defining a 'patches' method",
severity: :convention,
line: 4,
column: 2,
source: source }]
inspect_source(cop, source)
expected_offenses.zip(cop.offenses).each do |expected, actual|
expect_offense(expected, actual)
end
end
it "Patch URLs" do
patch_urls = [
"https://raw.github.com/mogaal/sendemail",
"https://mirrors.ustc.edu.cn/macports/trunk/",
"http://trac.macports.org/export/102865/trunk/dports/mail/uudeview/files/inews.c.patch",
"http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;filename=patch-libunac1.txt;att=1;bug=623340",
]
patch_urls.each do |patch_url|
source = <<-EOS.undent
class Foo < Formula
homepage "ftp://example.com/foo"
url "http://example.com/foo-1.0.tgz"
def patches
"#{patch_url}"
end
end
EOS
inspect_source(cop, source)
if patch_url =~ %r{/raw\.github\.com/}
expected_offenses = [{ message: "GitHub/Gist patches should specify a revision:\n#{patch_url}",
severity: :convention,
line: 5,
column: 12,
source: source }]
elsif patch_url =~ %r{macports/trunk}
expected_offenses = [{ message: "MacPorts patches should specify a revision instead of trunk:\n#{patch_url}",
severity: :convention,
line: 5,
column: 33,
source: source }]
elsif patch_url =~ %r{^http://trac\.macports\.org}
expected_offenses = [{ message: "Patches from MacPorts Trac should be https://, not http:\n#{patch_url}",
severity: :convention,
line: 5,
column: 5,
source: source }]
elsif patch_url =~ %r{^http://bugs\.debian\.org}
expected_offenses = [{ message: "Patches from Debian should be https://, not http:\n#{patch_url}",
severity: :convention,
line: 5,
column: 5,
source: source }]
end
expected_offenses.zip([cop.offenses.last]).each do |expected, actual|
expect_offense(expected, actual)
end
end
end
it "Formula with nested `def patches`" do
source = <<-EOS.undent
class Foo < Formula
homepage "ftp://example.com/foo"
url "http://example.com/foo-1.0.tgz"
def patches
files = %w[patch-domain_resolver.c patch-colormask.c patch-trafshow.c patch-trafshow.1 patch-configure]
{
:p0 =>
files.collect{|p| "http://trac.macports.org/export/68507/trunk/dports/net/trafshow/files/\#{p}"}
}
end
end
EOS
expected_offenses = [{ message: "Use the patch DSL instead of defining a 'patches' method",
severity: :convention,
line: 4,
column: 2,
source: source },
{ message: "Patches from MacPorts Trac should be https://, not http:\n"\
"http://trac.macports.org/export/68507/trunk/dports/net/trafshow/files/",
severity: :convention,
line: 8,
column: 26,
source: source }]
inspect_source(cop, source)
expected_offenses.zip(cop.offenses).each do |expected, actual|
expect_offense(expected, actual)
end
end
end
end