Skip to content

Commit

Permalink
Merge pull request #2769 from MikeMcQuaid/rubocop-no-perl-backrefs
Browse files Browse the repository at this point in the history
rubocop: don’t allow Perl regex backrefs.
  • Loading branch information
MikeMcQuaid authored Jun 12, 2017
2 parents 22c431d + 7a0aff1 commit ed9942f
Show file tree
Hide file tree
Showing 21 changed files with 83 additions and 87 deletions.
4 changes: 0 additions & 4 deletions Library/.rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -172,10 +172,6 @@ Style/PercentLiteralDelimiters:
'%W': '[]'
'%x': '()'

# we prefer Perl-style regex back references
Style/PerlBackrefs:
Enabled: false

Style/RaiseArgs:
EnforcedStyle: exploded

Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cmd/info.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def print_json

def github_remote_path(remote, path)
if remote =~ %r{^(?:https?://|git(?:@|://))github\.com[:/](.+)/(.+?)(?:\.git)?$}
"https://github.com/#{$1}/#{$2}/blob/master/#{path}"
"https://github.com/#{Regexp.last_match(1)}/#{Regexp.last_match(2)}/blob/master/#{path}"
else
"#{remote}/#{path}"
end
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cmd/install.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def install
if name !~ HOMEBREW_TAP_FORMULA_REGEX && name !~ HOMEBREW_CASK_TAP_CASK_REGEX
next
end
tap = Tap.fetch($1, $2)
tap = Tap.fetch(Regexp.last_match(1), Regexp.last_match(2))
tap.install unless tap.installed?
end
end
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cmd/search.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def search

def query_regexp(query)
case query
when %r{^/(.*)/$} then Regexp.new($1)
when %r{^/(.*)/$} then Regexp.new(Regexp.last_match(1))
else /.*#{Regexp.escape(query)}.*/i
end
rescue RegexpError
Expand Down
106 changes: 53 additions & 53 deletions Library/Homebrew/dev-cmd/audit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ def audit_options

next unless o.name =~ /^with(out)?-(?:checks?|tests)$/
unless formula.deps.any? { |d| d.name == "check" && (d.optional? || d.recommended?) }
problem "Use '--with#{$1}-test' instead of '--#{o.name}'. Migrate '--#{o.name}' with `deprecated_option`."
problem "Use '--with#{Regexp.last_match(1)}-test' instead of '--#{o.name}'. Migrate '--#{o.name}' with `deprecated_option`."
end
end

Expand Down Expand Up @@ -722,7 +722,7 @@ def audit_specs
stable = formula.stable
case stable && stable.url
when /[\d\._-](alpha|beta|rc\d)/
matched = $1
matched = Regexp.last_match(1)
version_prefix = stable.version.to_s.sub(/\d+$/, "")
return if unstable_whitelist.include?([formula.name, version_prefix])
problem "Stable version URLs should not contain #{matched}"
Expand Down Expand Up @@ -837,7 +837,7 @@ def patch_problems(patch)
when %r{https?://patch-diff\.githubusercontent\.com/raw/(.+)/(.+)/pull/(.+)\.(?:diff|patch)}
problem <<-EOS.undent
use GitHub pull request URLs:
https://github.com/#{$1}/#{$2}/pull/#{$3}.patch
https://github.com/#{Regexp.last_match(1)}/#{Regexp.last_match(2)}/pull/#{Regexp.last_match(3)}.patch
Rather than patch-diff:
#{patch.url}
EOS
Expand Down Expand Up @@ -875,7 +875,7 @@ def audit_lines

def line_problems(line, _lineno)
if line =~ /<(Formula|AmazonWebServicesFormula|ScriptFileFormula|GithubGistFormula)/
problem "Use a space in class inheritance: class Foo < #{$1}"
problem "Use a space in class inheritance: class Foo < #{Regexp.last_match(1)}"
end

# Commented-out cmake support from default template
Expand All @@ -899,52 +899,52 @@ def line_problems(line, _lineno)
# FileUtils is included in Formula
# encfs modifies a file with this name, so check for some leading characters
if line =~ %r{[^'"/]FileUtils\.(\w+)}
problem "Don't need 'FileUtils.' before #{$1}."
problem "Don't need 'FileUtils.' before #{Regexp.last_match(1)}."
end

# Check for long inreplace block vars
if line =~ /inreplace .* do \|(.{2,})\|/
problem "\"inreplace <filenames> do |s|\" is preferred over \"|#{$1}|\"."
problem "\"inreplace <filenames> do |s|\" is preferred over \"|#{Regexp.last_match(1)}|\"."
end

# Check for string interpolation of single values.
if line =~ /(system|inreplace|gsub!|change_make_var!).*[ ,]"#\{([\w.]+)\}"/
problem "Don't need to interpolate \"#{$2}\" with #{$1}"
problem "Don't need to interpolate \"#{Regexp.last_match(2)}\" with #{Regexp.last_match(1)}"
end

# Check for string concatenation; prefer interpolation
if line =~ /(#\{\w+\s*\+\s*['"][^}]+\})/
problem "Try not to concatenate paths in string interpolation:\n #{$1}"
problem "Try not to concatenate paths in string interpolation:\n #{Regexp.last_match(1)}"
end

# Prefer formula path shortcuts in Pathname+
if line =~ %r{\(\s*(prefix\s*\+\s*(['"])(bin|include|libexec|lib|sbin|share|Frameworks)[/'"])}
problem "\"(#{$1}...#{$2})\" should be \"(#{$3.downcase}+...)\""
problem "\"(#{Regexp.last_match(1)}...#{Regexp.last_match(2)})\" should be \"(#{Regexp.last_match(3).downcase}+...)\""
end

if line =~ /((man)\s*\+\s*(['"])(man[1-8])(['"]))/
problem "\"#{$1}\" should be \"#{$4}\""
problem "\"#{Regexp.last_match(1)}\" should be \"#{Regexp.last_match(4)}\""
end

# Prefer formula path shortcuts in strings
if line =~ %r[(\#\{prefix\}/(bin|include|libexec|lib|sbin|share|Frameworks))]
problem "\"#{$1}\" should be \"\#{#{$2.downcase}}\""
problem "\"#{Regexp.last_match(1)}\" should be \"\#{#{Regexp.last_match(2).downcase}}\""
end

if line =~ %r[((\#\{prefix\}/share/man/|\#\{man\}/)(man[1-8]))]
problem "\"#{$1}\" should be \"\#{#{$3}}\""
problem "\"#{Regexp.last_match(1)}\" should be \"\#{#{Regexp.last_match(3)}}\""
end

if line =~ %r[((\#\{share\}/(man)))[/'"]]
problem "\"#{$1}\" should be \"\#{#{$3}}\""
problem "\"#{Regexp.last_match(1)}\" should be \"\#{#{Regexp.last_match(3)}}\""
end

if line =~ %r[(\#\{prefix\}/share/(info|man))]
problem "\"#{$1}\" should be \"\#{#{$2}}\""
problem "\"#{Regexp.last_match(1)}\" should be \"\#{#{Regexp.last_match(2)}}\""
end

if line =~ /depends_on :(automake|autoconf|libtool)/
problem ":#{$1} is deprecated. Usage should be \"#{$1}\""
problem ":#{Regexp.last_match(1)} is deprecated. Usage should be \"#{Regexp.last_match(1)}\""
end

if line =~ /depends_on :apr/
Expand All @@ -954,23 +954,23 @@ def line_problems(line, _lineno)
problem ":tex is deprecated" if line =~ /depends_on :tex/

if line =~ /depends_on\s+['"](.+)['"]\s+=>\s+:(lua|perl|python|ruby)(\d*)/
problem "#{$2} modules should be vendored rather than use deprecated `depends_on \"#{$1}\" => :#{$2}#{$3}`"
problem "#{Regexp.last_match(2)} modules should be vendored rather than use deprecated `depends_on \"#{Regexp.last_match(1)}\" => :#{Regexp.last_match(2)}#{Regexp.last_match(3)}`"
end

if line =~ /depends_on\s+['"](.+)['"]\s+=>\s+(.*)/
dep = $1
$2.split(" ").map do |o|
dep = Regexp.last_match(1)
Regexp.last_match(2).split(" ").map do |o|
break if ["if", "unless"].include?(o)
next unless o =~ /^\[?['"](.*)['"]/
problem "Dependency #{dep} should not use option #{$1}"
problem "Dependency #{dep} should not use option #{Regexp.last_match(1)}"
end
end

# Commented-out depends_on
problem "Commented-out dep #{$1}" if line =~ /#\s*depends_on\s+(.+)\s*$/
problem "Commented-out dep #{Regexp.last_match(1)}" if line =~ /#\s*depends_on\s+(.+)\s*$/

if line =~ /if\s+ARGV\.include\?\s+'--(HEAD|devel)'/
problem "Use \"if build.#{$1.downcase}?\" instead"
problem "Use \"if build.#{Regexp.last_match(1).downcase}?\" instead"
end

problem "Use separate make calls" if line.include?("make && make")
Expand All @@ -983,15 +983,15 @@ def line_problems(line, _lineno)

# Avoid hard-coding compilers
if line =~ %r{(system|ENV\[.+\]\s?=)\s?['"](/usr/bin/)?(gcc|llvm-gcc|clang)['" ]}
problem "Use \"\#{ENV.cc}\" instead of hard-coding \"#{$3}\""
problem "Use \"\#{ENV.cc}\" instead of hard-coding \"#{Regexp.last_match(3)}\""
end

if line =~ %r{(system|ENV\[.+\]\s?=)\s?['"](/usr/bin/)?((g|llvm-g|clang)\+\+)['" ]}
problem "Use \"\#{ENV.cxx}\" instead of hard-coding \"#{$3}\""
problem "Use \"\#{ENV.cxx}\" instead of hard-coding \"#{Regexp.last_match(3)}\""
end

if line =~ /system\s+['"](env|export)(\s+|['"])/
problem "Use ENV instead of invoking '#{$1}' to modify the environment"
problem "Use ENV instead of invoking '#{Regexp.last_match(1)}' to modify the environment"
end

if formula.name != "wine" && line =~ /ENV\.universal_binary/
Expand All @@ -1007,27 +1007,27 @@ def line_problems(line, _lineno)
end

if line =~ /build\.include\?[\s\(]+['"]\-\-(.*)['"]/
problem "Reference '#{$1}' without dashes"
problem "Reference '#{Regexp.last_match(1)}' without dashes"
end

if line =~ /build\.include\?[\s\(]+['"]with(out)?-(.*)['"]/
problem "Use build.with#{$1}? \"#{$2}\" instead of build.include? 'with#{$1}-#{$2}'"
problem "Use build.with#{Regexp.last_match(1)}? \"#{Regexp.last_match(2)}\" instead of build.include? 'with#{Regexp.last_match(1)}-#{Regexp.last_match(2)}'"
end

if line =~ /build\.with\?[\s\(]+['"]-?-?with-(.*)['"]/
problem "Don't duplicate 'with': Use `build.with? \"#{$1}\"` to check for \"--with-#{$1}\""
problem "Don't duplicate 'with': Use `build.with? \"#{Regexp.last_match(1)}\"` to check for \"--with-#{Regexp.last_match(1)}\""
end

if line =~ /build\.without\?[\s\(]+['"]-?-?without-(.*)['"]/
problem "Don't duplicate 'without': Use `build.without? \"#{$1}\"` to check for \"--without-#{$1}\""
problem "Don't duplicate 'without': Use `build.without? \"#{Regexp.last_match(1)}\"` to check for \"--without-#{Regexp.last_match(1)}\""
end

if line =~ /unless build\.with\?(.*)/
problem "Use if build.without?#{$1} instead of unless build.with?#{$1}"
problem "Use if build.without?#{Regexp.last_match(1)} instead of unless build.with?#{Regexp.last_match(1)}"
end

if line =~ /unless build\.without\?(.*)/
problem "Use if build.with?#{$1} instead of unless build.without?#{$1}"
problem "Use if build.with?#{Regexp.last_match(1)} instead of unless build.without?#{Regexp.last_match(1)}"
end

if line =~ /(not\s|!)\s*build\.with?\?/
Expand Down Expand Up @@ -1071,7 +1071,7 @@ def line_problems(line, _lineno)
end

if line =~ /^def (\w+).*$/
problem "Define method #{$1.inspect} in the class body, not at the top-level"
problem "Define method #{Regexp.last_match(1).inspect} in the class body, not at the top-level"
end

if line.include?("ENV.fortran") && !formula.requirements.map(&:class).include?(FortranRequirement)
Expand All @@ -1083,20 +1083,20 @@ def line_problems(line, _lineno)
end

if line =~ /depends_on :(.+) (if.+|unless.+)$/
conditional_dep_problems($1.to_sym, $2, $&)
conditional_dep_problems(Regexp.last_match(1).to_sym, Regexp.last_match(2), $&)
end

if line =~ /depends_on ['"](.+)['"] (if.+|unless.+)$/
conditional_dep_problems($1, $2, $&)
conditional_dep_problems(Regexp.last_match(1), Regexp.last_match(2), $&)
end

if line =~ /(Dir\[("[^\*{},]+")\])/
problem "#{$1} is unnecessary; just use #{$2}"
problem "#{Regexp.last_match(1)} is unnecessary; just use #{Regexp.last_match(2)}"
end

if line =~ /system (["'](#{FILEUTILS_METHODS})["' ])/o
system = $1
method = $2
system = Regexp.last_match(1)
method = Regexp.last_match(2)
problem "Use the `#{method}` Ruby method instead of `system #{system}`"
end

Expand All @@ -1114,7 +1114,7 @@ def line_problems(line, _lineno)
end

if line =~ /system\s+['"](otool|install_name_tool|lipo)/ && formula.name != "cctools"
problem "Use ruby-macho instead of calling #{$1}"
problem "Use ruby-macho instead of calling #{Regexp.last_match(1)}"
end

if formula.tap.to_s == "homebrew/core"
Expand All @@ -1125,29 +1125,29 @@ def line_problems(line, _lineno)
end

if line =~ /((revision|version_scheme)\s+0)/
problem "'#{$1}' should be removed"
problem "'#{Regexp.last_match(1)}' should be removed"
end

return unless @strict

problem "`#{$1}` in formulae is deprecated" if line =~ /(env :(std|userpaths))/
problem "`#{Regexp.last_match(1)}` in formulae is deprecated" if line =~ /(env :(std|userpaths))/

if line =~ /system ((["'])[^"' ]*(?:\s[^"' ]*)+\2)/
bad_system = $1
bad_system = Regexp.last_match(1)
unless %w[| < > & ; *].any? { |c| bad_system.include? c }
good_system = bad_system.gsub(" ", "\", \"")
problem "Use `system #{good_system}` instead of `system #{bad_system}` "
end
end

problem "`#{$1}` is now unnecessary" if line =~ /(require ["']formula["'])/
problem "`#{Regexp.last_match(1)}` is now unnecessary" if line =~ /(require ["']formula["'])/

if line =~ %r{#\{share\}/#{Regexp.escape(formula.name)}[/'"]}
problem "Use \#{pkgshare} instead of \#{share}/#{formula.name}"
end

return unless line =~ %r{share(\s*[/+]\s*)(['"])#{Regexp.escape(formula.name)}(?:\2|/)}
problem "Use pkgshare instead of (share#{$1}\"#{formula.name}\")"
problem "Use pkgshare instead of (share#{Regexp.last_match(1)}\"#{formula.name}\")"
end

def audit_reverse_migration
Expand Down Expand Up @@ -1297,7 +1297,7 @@ def audit_checksum

def audit_download_strategy
if url =~ %r{^(cvs|bzr|hg|fossil)://} || url =~ %r{^(svn)\+http://}
problem "Use of the #{$&} scheme is deprecated, pass `:using => :#{$1}` instead"
problem "Use of the #{$&} scheme is deprecated, pass `:using => :#{Regexp.last_match(1)}` instead"
end

url_strategy = DownloadStrategyDetector.detect(url)
Expand Down Expand Up @@ -1341,7 +1341,7 @@ def audit_download_strategy
def audit_urls
# Check GNU urls; doesn't apply to mirrors
if url =~ %r{^(?:https?|ftp)://ftpmirror.gnu.org/(.*)}
problem "Please use \"https://ftp.gnu.org/gnu/#{$1}\" instead of #{url}."
problem "Please use \"https://ftp.gnu.org/gnu/#{Regexp.last_match(1)}\" instead of #{url}."
end

if mirrors.include?(url)
Expand Down Expand Up @@ -1375,11 +1375,11 @@ def audit_urls
%r{^http://(?:[^/]*\.)?mirrorservice\.org/}
problem "Please use https:// for #{p}"
when %r{^http://search\.mcpan\.org/CPAN/(.*)}i
problem "#{p} should be `https://cpan.metacpan.org/#{$1}`"
problem "#{p} should be `https://cpan.metacpan.org/#{Regexp.last_match(1)}`"
when %r{^(http|ftp)://ftp\.gnome\.org/pub/gnome/(.*)}i
problem "#{p} should be `https://download.gnome.org/#{$2}`"
problem "#{p} should be `https://download.gnome.org/#{Regexp.last_match(2)}`"
when %r{^git://anonscm\.debian\.org/users/(.*)}i
problem "#{p} should be `https://anonscm.debian.org/git/users/#{$1}`"
problem "#{p} should be `https://anonscm.debian.org/git/users/#{Regexp.last_match(1)}`"
end
end

Expand All @@ -1389,7 +1389,7 @@ def audit_urls
when %r{^ftp://ftp\.mirrorservice\.org}
problem "Please use https:// for #{p}"
when %r{^ftp://ftp\.cpan\.org/pub/CPAN(.*)}i
problem "#{p} should be `http://search.cpan.org/CPAN#{$1}`"
problem "#{p} should be `http://search.cpan.org/CPAN#{Regexp.last_match(1)}`"
end
end

Expand All @@ -1403,7 +1403,7 @@ def audit_urls
next unless p =~ %r{^https?://.*\b(sourceforge|sf)\.(com|net)}

if p =~ /(\?|&)use_mirror=/
problem "Don't use #{$1}use_mirror in SourceForge urls (url is #{p})."
problem "Don't use #{Regexp.last_match(1)}use_mirror in SourceForge urls (url is #{p})."
end

if p.end_with?("/download")
Expand Down Expand Up @@ -1433,7 +1433,7 @@ def audit_urls
problem <<-EOS.undent
Please use a secure mirror for Debian URLs.
We recommend:
https://mirrors.ocf.berkeley.edu/debian/#{$1}
https://mirrors.ocf.berkeley.edu/debian/#{Regexp.last_match(1)}
EOS
end

Expand Down Expand Up @@ -1486,7 +1486,7 @@ def audit_urls
next unless u =~ %r{https?://codeload\.github\.com/(.+)/(.+)/(?:tar\.gz|zip)/(.+)}
problem <<-EOS.undent
use GitHub archive URLs:
https://github.com/#{$1}/#{$2}/archive/#{$3}.tar.gz
https://github.com/#{Regexp.last_match(1)}/#{Regexp.last_match(2)}/archive/#{Regexp.last_match(3)}.tar.gz
Rather than codeload:
#{u}
EOS
Expand All @@ -1495,7 +1495,7 @@ def audit_urls
# Check for Maven Central urls, prefer HTTPS redirector over specific host
urls.each do |u|
next unless u =~ %r{https?://(?:central|repo\d+)\.maven\.org/maven2/(.+)$}
problem "#{u} should be `https://search.maven.org/remotecontent?filepath=#{$1}`"
problem "#{u} should be `https://search.maven.org/remotecontent?filepath=#{Regexp.last_match(1)}`"
end

if name == "curl" && !urls.find { |u| u.start_with?("http://") }
Expand All @@ -1506,7 +1506,7 @@ def audit_urls
if @strict
urls.each do |p|
next unless p =~ %r{^https?://pypi.python.org/(.*)}
problem "#{p} should be `https://files.pythonhosted.org/#{$1}`"
problem "#{p} should be `https://files.pythonhosted.org/#{Regexp.last_match(1)}`"
end
end

Expand Down
Loading

0 comments on commit ed9942f

Please sign in to comment.