Merge pull request #3087 from GauthamGoli/patches_missing_rule_fix

audit: Add previously unported GH patch audit rule in Patches cop
This commit is contained in:
Mike McQuaid 2017-08-26 14:23:39 +01:00 committed by GitHub
commit 4d90ae2f4d
2 changed files with 127 additions and 107 deletions

View File

@ -25,12 +25,22 @@ module RuboCop
def patch_problems(patch) def patch_problems(patch)
patch_url = string_content(patch) patch_url = string_content(patch)
gh_patch_param_pattern = %r{https?://github\.com/.+/.+/(?:commit|pull)/[a-fA-F0-9]*.(?:patch|diff)}
if regex_match_group(patch, gh_patch_param_pattern)
if patch_url !~ /\?full_index=\w+$/
problem <<-EOS.undent
GitHub patches should use the full_index parameter:
#{patch_url}?full_index=1
EOS
end
end
gh_patch_patterns = Regexp.union([%r{/raw\.github\.com/}, gh_patch_patterns = Regexp.union([%r{/raw\.github\.com/},
%r{gist\.github\.com/raw}, %r{gist\.github\.com/raw},
%r{gist\.github\.com/.+/raw}, %r{gist\.github\.com/.+/raw},
%r{gist\.githubusercontent\.com/.+/raw}]) %r{gist\.githubusercontent\.com/.+/raw}])
if regex_match_group(patch, gh_patch_patterns) if regex_match_group(patch, gh_patch_patterns)
unless patch_url =~ /[a-fA-F0-9]{40}/ if patch_url !~ /[a-fA-F0-9]{40}/
problem <<-EOS.undent.chomp problem <<-EOS.undent.chomp
GitHub/Gist patches should specify a revision: GitHub/Gist patches should specify a revision:
#{patch_url} #{patch_url}

View File

@ -28,11 +28,11 @@ describe RuboCop::Cop::FormulaAudit::Patches do
end end
EOS EOS
expected_offenses = [{ message: "Use the patch DSL instead of defining a 'patches' method", expected_offenses = [{ message: "Use the patch DSL instead of defining a 'patches' method",
severity: :convention, severity: :convention,
line: 4, line: 4,
column: 2, column: 2,
source: source }] source: source }]
inspect_source(cop, source) inspect_source(cop, source)
@ -48,6 +48,7 @@ describe RuboCop::Cop::FormulaAudit::Patches do
"http://trac.macports.org/export/102865/trunk/dports/mail/uudeview/files/inews.c.patch", "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", "http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;filename=patch-libunac1.txt;att=1;bug=623340",
"https://patch-diff.githubusercontent.com/raw/foo/foo-bar/pull/100.patch", "https://patch-diff.githubusercontent.com/raw/foo/foo-bar/pull/100.patch",
"https://github.com/dlang/dub/pull/1221.patch",
] ]
patch_urls.each do |patch_url| patch_urls.each do |patch_url|
source = <<-EOS.undent source = <<-EOS.undent
@ -61,55 +62,64 @@ describe RuboCop::Cop::FormulaAudit::Patches do
EOS EOS
inspect_source(cop, source) inspect_source(cop, source)
if patch_url =~ %r{/raw\.github\.com/} expected_offense = if patch_url =~ %r{/raw\.github\.com/}
expected_offenses = [{ message: <<-EOS.undent.chomp, [{ message: <<-EOS.undent.chomp,
GitHub/Gist patches should specify a revision: GitHub/Gist patches should specify a revision:
#{patch_url} #{patch_url}
EOS EOS
severity: :convention, severity: :convention,
line: 5, line: 5,
column: 12, column: 12,
source: source }] source: source }]
elsif patch_url =~ %r{macports/trunk} elsif patch_url =~ %r{macports/trunk}
expected_offenses = [{ message: <<-EOS.undent.chomp, [{ message: <<-EOS.undent.chomp,
MacPorts patches should specify a revision instead of trunk: MacPorts patches should specify a revision instead of trunk:
#{patch_url} #{patch_url}
EOS EOS
severity: :convention, severity: :convention,
line: 5, line: 5,
column: 33, column: 33,
source: source }] source: source }]
elsif patch_url =~ %r{^http://trac\.macports\.org} elsif patch_url =~ %r{^http://trac\.macports\.org}
expected_offenses = [{ message: <<-EOS.undent.chomp, [{ message: <<-EOS.undent.chomp,
Patches from MacPorts Trac should be https://, not http: Patches from MacPorts Trac should be https://, not http:
#{patch_url} #{patch_url}
EOS EOS
severity: :convention, severity: :convention,
line: 5, line: 5,
column: 5, column: 5,
source: source }] source: source }]
elsif patch_url =~ %r{^http://bugs\.debian\.org} elsif patch_url =~ %r{^http://bugs\.debian\.org}
expected_offenses = [{ message: <<-EOS.undent.chomp, [{ message: <<-EOS.undent.chomp,
Patches from Debian should be https://, not http: Patches from Debian should be https://, not http:
#{patch_url} #{patch_url}
EOS EOS
severity: :convention, severity: :convention,
line: 5, line: 5,
column: 5, column: 5,
source: source }] source: source }]
elsif patch_url =~ %r{https?://patch-diff\.githubusercontent\.com/raw/(.+)/(.+)/pull/(.+)\.(?:diff|patch)} elsif patch_url =~ %r{https?://patch-diff\.githubusercontent\.com/raw/(.+)/(.+)/pull/(.+)\.(?:diff|patch)}
expected_offenses = [{ message: <<-EOS.undent, [{ message: <<-EOS.undent,
use GitHub pull request URLs: use GitHub pull request URLs:
https://github.com/foo/foo-bar/pull/100.patch https://github.com/foo/foo-bar/pull/100.patch
Rather than patch-diff: Rather than patch-diff:
https://patch-diff.githubusercontent.com/raw/foo/foo-bar/pull/100.patch https://patch-diff.githubusercontent.com/raw/foo/foo-bar/pull/100.patch
EOS EOS
severity: :convention, severity: :convention,
line: 5, line: 5,
column: 5, column: 5,
source: source }] source: source }]
elsif patch_url =~ %r{https?://github\.com/.+/.+/(?:commit|pull)/[a-fA-F0-9]*.(?:patch|diff)}
[{ message: <<-EOS.undent,
GitHub patches should use the full_index parameter:
#{patch_url}?full_index=1
EOS
severity: :convention,
line: 5,
column: 5,
source: source }]
end end
expected_offenses.zip([cop.offenses.last]).each do |expected, actual| expected_offense.zip([cop.offenses.last]).each do |expected, actual|
expect_offense(expected, actual) expect_offense(expected, actual)
end end
end end
@ -130,19 +140,19 @@ describe RuboCop::Cop::FormulaAudit::Patches do
end end
EOS EOS
expected_offenses = [{ message: "Use the patch DSL instead of defining a 'patches' method", expected_offenses = [{ message: "Use the patch DSL instead of defining a 'patches' method",
severity: :convention, severity: :convention,
line: 4, line: 4,
column: 2, column: 2,
source: source }, source: source },
{ message: <<-EOS.undent.chomp, { message: <<-EOS.undent.chomp,
Patches from MacPorts Trac should be https://, not http: Patches from MacPorts Trac should be https://, not http:
http://trac.macports.org/export/68507/trunk/dports/net/trafshow/files/ http://trac.macports.org/export/68507/trunk/dports/net/trafshow/files/
EOS EOS
severity: :convention, severity: :convention,
line: 8, line: 8,
column: 26, column: 26,
source: source }] source: source }]
inspect_source(cop, source) inspect_source(cop, source)
@ -174,55 +184,55 @@ describe RuboCop::Cop::FormulaAudit::Patches do
EOS EOS
inspect_source(cop, source) inspect_source(cop, source)
if patch_url =~ %r{/raw\.github\.com/} expected_offense = if patch_url =~ %r{/raw\.github\.com/}
expected_offenses = [{ message: <<-EOS.undent.chomp, [{ message: <<-EOS.undent.chomp,
GitHub/Gist patches should specify a revision: GitHub/Gist patches should specify a revision:
#{patch_url} #{patch_url}
EOS EOS
severity: :convention, severity: :convention,
line: 5, line: 5,
column: 16, column: 16,
source: source }] source: source }]
elsif patch_url =~ %r{macports/trunk} elsif patch_url =~ %r{macports/trunk}
expected_offenses = [{ message: <<-EOS.undent.chomp, [{ message: <<-EOS.undent.chomp,
MacPorts patches should specify a revision instead of trunk: MacPorts patches should specify a revision instead of trunk:
#{patch_url} #{patch_url}
EOS EOS
severity: :convention, severity: :convention,
line: 5, line: 5,
column: 37, column: 37,
source: source }] source: source }]
elsif patch_url =~ %r{^http://trac\.macports\.org} elsif patch_url =~ %r{^http://trac\.macports\.org}
expected_offenses = [{ message: <<-EOS.undent.chomp, [{ message: <<-EOS.undent.chomp,
Patches from MacPorts Trac should be https://, not http: Patches from MacPorts Trac should be https://, not http:
#{patch_url} #{patch_url}
EOS EOS
severity: :convention, severity: :convention,
line: 5, line: 5,
column: 9, column: 9,
source: source }] source: source }]
elsif patch_url =~ %r{^http://bugs\.debian\.org} elsif patch_url =~ %r{^http://bugs\.debian\.org}
expected_offenses = [{ message: <<-EOS.undent.chomp, [{ message: <<-EOS.undent.chomp,
Patches from Debian should be https://, not http: Patches from Debian should be https://, not http:
#{patch_url} #{patch_url}
EOS EOS
severity: :convention, severity: :convention,
line: 5, line: 5,
column: 9, column: 9,
source: source }] source: source }]
elsif patch_url =~ %r{https?://patch-diff\.githubusercontent\.com/raw/(.+)/(.+)/pull/(.+)\.(?:diff|patch)} elsif patch_url =~ %r{https?://patch-diff\.githubusercontent\.com/raw/(.+)/(.+)/pull/(.+)\.(?:diff|patch)}
expected_offenses = [{ message: <<-EOS.undent, [{ message: <<-EOS.undent,
use GitHub pull request URLs: use GitHub pull request URLs:
https://github.com/foo/foo-bar/pull/100.patch https://github.com/foo/foo-bar/pull/100.patch
Rather than patch-diff: Rather than patch-diff:
https://patch-diff.githubusercontent.com/raw/foo/foo-bar/pull/100.patch https://patch-diff.githubusercontent.com/raw/foo/foo-bar/pull/100.patch
EOS EOS
severity: :convention, severity: :convention,
line: 5, line: 5,
column: 9, column: 9,
source: source }] source: source }]
end end
expected_offenses.zip([cop.offenses.last]).each do |expected, actual| expected_offense.zip([cop.offenses.last]).each do |expected, actual|
expect_offense(expected, actual) expect_offense(expected, actual)
end end
end end