diff --git a/Library/Homebrew/rubocops/patches_cop.rb b/Library/Homebrew/rubocops/patches_cop.rb index b272833821b4ddde1bd48a87c71c5f315cad9c9e..7ee1a127a3adabc04407630e1059f3ce91502090 100644 --- a/Library/Homebrew/rubocops/patches_cop.rb +++ b/Library/Homebrew/rubocops/patches_cop.rb @@ -25,12 +25,22 @@ module RuboCop def patch_problems(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/}, %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}/ + if patch_url !~ /[a-fA-F0-9]{40}/ problem <<-EOS.undent.chomp GitHub/Gist patches should specify a revision: #{patch_url} diff --git a/Library/Homebrew/test/rubocops/patches_cop_spec.rb b/Library/Homebrew/test/rubocops/patches_cop_spec.rb index 092782bfb10a9ffc45cfcdefeedc0dd9d7774fa7..28915a1ec9a987cec14df41bbabcb49fb6bd63ab 100644 --- a/Library/Homebrew/test/rubocops/patches_cop_spec.rb +++ b/Library/Homebrew/test/rubocops/patches_cop_spec.rb @@ -28,11 +28,11 @@ describe RuboCop::Cop::FormulaAudit::Patches do end EOS - expected_offenses = [{ message: "Use the patch DSL instead of defining a 'patches' method", - severity: :convention, - line: 4, - column: 2, - source: source }] + 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) @@ -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://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://github.com/dlang/dub/pull/1221.patch", ] patch_urls.each do |patch_url| source = <<-EOS.undent @@ -61,55 +62,64 @@ describe RuboCop::Cop::FormulaAudit::Patches do EOS inspect_source(cop, source) - if patch_url =~ %r{/raw\.github\.com/} - expected_offenses = [{ message: <<-EOS.undent.chomp, - GitHub/Gist patches should specify a revision: - #{patch_url} - EOS - severity: :convention, - line: 5, - column: 12, - source: source }] + expected_offense = if patch_url =~ %r{/raw\.github\.com/} + [{ message: <<-EOS.undent.chomp, + GitHub/Gist patches should specify a revision: + #{patch_url} + EOS + severity: :convention, + line: 5, + column: 12, + source: source }] elsif patch_url =~ %r{macports/trunk} - expected_offenses = [{ message: <<-EOS.undent.chomp, - MacPorts patches should specify a revision instead of trunk: - #{patch_url} - EOS - severity: :convention, - line: 5, - column: 33, - source: source }] + [{ message: <<-EOS.undent.chomp, + MacPorts patches should specify a revision instead of trunk: + #{patch_url} + EOS + severity: :convention, + line: 5, + column: 33, + source: source }] elsif patch_url =~ %r{^http://trac\.macports\.org} - expected_offenses = [{ message: <<-EOS.undent.chomp, - Patches from MacPorts Trac should be https://, not http: - #{patch_url} - EOS - severity: :convention, - line: 5, - column: 5, - source: source }] + [{ message: <<-EOS.undent.chomp, + Patches from MacPorts Trac should be https://, not http: + #{patch_url} + EOS + severity: :convention, + line: 5, + column: 5, + source: source }] elsif patch_url =~ %r{^http://bugs\.debian\.org} - expected_offenses = [{ message: <<-EOS.undent.chomp, - Patches from Debian should be https://, not http: - #{patch_url} - EOS - severity: :convention, - line: 5, - column: 5, - source: source }] + [{ message: <<-EOS.undent.chomp, + Patches from Debian should be https://, not http: + #{patch_url} + EOS + severity: :convention, + line: 5, + column: 5, + source: source }] elsif patch_url =~ %r{https?://patch-diff\.githubusercontent\.com/raw/(.+)/(.+)/pull/(.+)\.(?:diff|patch)} - expected_offenses = [{ message: <<-EOS.undent, - use GitHub pull request URLs: - https://github.com/foo/foo-bar/pull/100.patch - Rather than patch-diff: - https://patch-diff.githubusercontent.com/raw/foo/foo-bar/pull/100.patch - EOS - severity: :convention, - line: 5, - column: 5, - source: source }] + [{ message: <<-EOS.undent, + use GitHub pull request URLs: + https://github.com/foo/foo-bar/pull/100.patch + Rather than patch-diff: + https://patch-diff.githubusercontent.com/raw/foo/foo-bar/pull/100.patch + EOS + severity: :convention, + line: 5, + column: 5, + 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 - expected_offenses.zip([cop.offenses.last]).each do |expected, actual| + expected_offense.zip([cop.offenses.last]).each do |expected, actual| expect_offense(expected, actual) end end @@ -130,19 +140,19 @@ describe RuboCop::Cop::FormulaAudit::Patches do end EOS - expected_offenses = [{ message: "Use the patch DSL instead of defining a 'patches' method", - severity: :convention, - line: 4, - column: 2, - source: source }, - { message: <<-EOS.undent.chomp, - Patches from MacPorts Trac should be https://, not http: - http://trac.macports.org/export/68507/trunk/dports/net/trafshow/files/ - EOS - severity: :convention, - line: 8, - column: 26, - source: source }] + expected_offenses = [{ message: "Use the patch DSL instead of defining a 'patches' method", + severity: :convention, + line: 4, + column: 2, + source: source }, + { message: <<-EOS.undent.chomp, + Patches from MacPorts Trac should be https://, not http: + http://trac.macports.org/export/68507/trunk/dports/net/trafshow/files/ + EOS + severity: :convention, + line: 8, + column: 26, + source: source }] inspect_source(cop, source) @@ -174,55 +184,55 @@ describe RuboCop::Cop::FormulaAudit::Patches do EOS inspect_source(cop, source) - if patch_url =~ %r{/raw\.github\.com/} - expected_offenses = [{ message: <<-EOS.undent.chomp, - GitHub/Gist patches should specify a revision: - #{patch_url} - EOS - severity: :convention, - line: 5, - column: 16, - source: source }] + expected_offense = if patch_url =~ %r{/raw\.github\.com/} + [{ message: <<-EOS.undent.chomp, + GitHub/Gist patches should specify a revision: + #{patch_url} + EOS + severity: :convention, + line: 5, + column: 16, + source: source }] elsif patch_url =~ %r{macports/trunk} - expected_offenses = [{ message: <<-EOS.undent.chomp, - MacPorts patches should specify a revision instead of trunk: - #{patch_url} - EOS - severity: :convention, - line: 5, - column: 37, - source: source }] + [{ message: <<-EOS.undent.chomp, + MacPorts patches should specify a revision instead of trunk: + #{patch_url} + EOS + severity: :convention, + line: 5, + column: 37, + source: source }] elsif patch_url =~ %r{^http://trac\.macports\.org} - expected_offenses = [{ message: <<-EOS.undent.chomp, - Patches from MacPorts Trac should be https://, not http: - #{patch_url} - EOS - severity: :convention, - line: 5, - column: 9, - source: source }] + [{ message: <<-EOS.undent.chomp, + Patches from MacPorts Trac should be https://, not http: + #{patch_url} + EOS + severity: :convention, + line: 5, + column: 9, + source: source }] elsif patch_url =~ %r{^http://bugs\.debian\.org} - expected_offenses = [{ message: <<-EOS.undent.chomp, - Patches from Debian should be https://, not http: - #{patch_url} - EOS - severity: :convention, - line: 5, - column: 9, - source: source }] + [{ message: <<-EOS.undent.chomp, + Patches from Debian should be https://, not http: + #{patch_url} + EOS + severity: :convention, + line: 5, + column: 9, + source: source }] elsif patch_url =~ %r{https?://patch-diff\.githubusercontent\.com/raw/(.+)/(.+)/pull/(.+)\.(?:diff|patch)} - expected_offenses = [{ message: <<-EOS.undent, - use GitHub pull request URLs: - https://github.com/foo/foo-bar/pull/100.patch - Rather than patch-diff: - https://patch-diff.githubusercontent.com/raw/foo/foo-bar/pull/100.patch - EOS - severity: :convention, - line: 5, - column: 9, - source: source }] + [{ message: <<-EOS.undent, + use GitHub pull request URLs: + https://github.com/foo/foo-bar/pull/100.patch + Rather than patch-diff: + https://patch-diff.githubusercontent.com/raw/foo/foo-bar/pull/100.patch + EOS + severity: :convention, + line: 5, + column: 9, + source: source }] 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) end end