Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some minor regexp matching perf improvements #16405

Merged
merged 2 commits into from
Dec 28, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion Library/Homebrew/cask/artifact/abstract_artifact.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def self.english_name
end

def self.english_article
@english_article ||= (english_name =~ /^[aeiou]/i) ? "an" : "a"
@english_article ||= /^[aeiou]/i.match?(english_name) ? "an" : "a"
end

def self.dsl_key
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cask/audit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -843,7 +843,7 @@ def get_repo_data(regex)
def bad_url_format?(regex, valid_formats_array)
return false unless cask.url.to_s.match?(regex)

valid_formats_array.none? { |format| cask.url.to_s =~ format }
valid_formats_array.none? { |format| cask.url.to_s&.match?(format) }
dduugg marked this conversation as resolved.
Show resolved Hide resolved
end

sig { returns(T::Boolean) }
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/dev-cmd/bottle.rb
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ def self.formula_ignores(formula)

# Ignore matches to go keg, because all go binaries are statically linked.
any_go_deps = formula.deps.any? do |dep|
dep.name =~ Version.formula_optionally_versioned_regex(:go)
dep.name&.match?(Version.formula_optionally_versioned_regex(:go))
dduugg marked this conversation as resolved.
Show resolved Hide resolved
end
if any_go_deps
go_regex = Version.formula_optionally_versioned_regex(:go, full: false)
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/extend/os/mac/formula_cellar_checks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def check_openssl_links
keg = Keg.new(formula.prefix)
system_openssl = keg.mach_o_files.select do |obj|
dlls = obj.dynamically_linked_libraries
dlls.any? { |dll| %r{/usr/lib/lib(crypto|ssl|tls)\..*dylib}.match dll }
dlls.any? { |dll| %r{/usr/lib/lib(crypto|ssl|tls)\..*dylib}.match? dll }
end
return if system_openssl.empty?

Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/formula.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1328,7 +1328,7 @@ def link_overwrite?(path)
self.class.link_overwrite_paths.any? do |p|
p == to_check ||
to_check.start_with?("#{p.chomp("/")}/") ||
to_check =~ /^#{Regexp.escape(p).gsub('\*', ".*?")}$/
/^#{Regexp.escape(p).gsub('\*', ".*?")}$/.match?(to_check)
end
end

Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/formula_auditor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def audit_file
!@versioned_formula &&
(versioned_formulae = formula.versioned_formulae - [formula]) &&
versioned_formulae.present?
versioned_aliases, unversioned_aliases = formula.aliases.partition { |a| a =~ /.@\d/ }
versioned_aliases, unversioned_aliases = formula.aliases.partition { |a| /.@\d/.match?(a) }
_, last_alias_version = versioned_formulae.map(&:name).last.split("@")

alias_name_major = "#{formula.name}@#{formula.version.major}"
Expand Down Expand Up @@ -715,7 +715,7 @@ def audit_specs
return unless stable.url

version = stable.version
problem "Stable: version (#{version}) is set to a string without a digit" if version.to_s !~ /\d/
problem "Stable: version (#{version}) is set to a string without a digit" unless /\d/.match?(version.to_s)

stable_version_string = version.to_s
if stable_version_string.start_with?("HEAD")
Expand Down
6 changes: 3 additions & 3 deletions Library/Homebrew/formula_text_auditor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def without_patch
end

def trailing_newline?
/\Z\n/ =~ @text
/\Z\n/.match?(@text)
end

def =~(other)
Expand All @@ -32,12 +32,12 @@ def to_s
end

def line_number(regex, skip = 0)
index = @lines.drop(skip).index { |line| line =~ regex }
index = @lines.drop(skip).index { |line| line&.match?(regex) }
index ? index + 1 : nil
end

def reverse_line_number(regex)
Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we delete this method? I think the only use was removed in 413a7e5#diff-0718ff8226ebdfd9a6b515e3c2017968bf77c4dad21e7cd6bdbd84c7d6c479c4L292

Copy link
Member

@Bo98 Bo98 Dec 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the whole class can be removed and its reference replaced with a file read as it seems the only thing called is to_s?

Generally speaking, RuboCop is the successor of this.

index = @lines.reverse.index { |line| line =~ regex }
index = @lines.reverse.index { |line| line&.match?(regex) }
dduugg marked this conversation as resolved.
Show resolved Hide resolved
index ? @lines.count - index : nil
end
end
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/keg_relocate.rb
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ def self.text_matches_in_file(file, string, ignores, linked_libraries, formula_a
Utils.popen_read("strings", "-t", "x", "-", file.to_s) do |io|
until io.eof?
str = io.readline.chomp
next if ignores.any? { |i| i =~ str }
next if ignores.any? { |i| i&.match?(str) }
dduugg marked this conversation as resolved.
Show resolved Hide resolved
next unless str.match? path_regex

offset, match = str.split(" ", 2)
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/livecheck/strategy/git.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def self.tag_info(url, regex = nil)

# Isolate tag strings and filter by regex
tags = stdout.gsub(%r{^.*\trefs/tags/|\^{}$}, "").split("\n").uniq.sort
tags.select! { |t| t =~ regex } if regex
tags.select! { |t| t&.match?(regex) } if regex
dduugg marked this conversation as resolved.
Show resolved Hide resolved
tags_data[:tags] = tags

tags_data
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/patch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ def contents
path.open("rb") do |f|
loop do
line = f.gets
break if line.nil? || line =~ /^__END__$/
break if line.nil? || /^__END__$/.match?(line)
end
while (line = f.gets)
data << line
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/rubocops/extend/formula_cop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ def file_path_allowed?
paths_to_exclude = [%r{/Library/Homebrew/test/}]
return true if @file_path.nil? # file_path is nil when source is directly passed to the cop, e.g. in specs

@file_path !~ Regexp.union(paths_to_exclude)
!@file_path&.match?(Regexp.union(paths_to_exclude))
dduugg marked this conversation as resolved.
Show resolved Hide resolved
end

def on_system_methods
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/rubocops/keg_only.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def audit_formula(_node, _class_node, _parent_class_node, body_node)
reason = string_content(reason).sub(name, "")
first_word = reason.split.first

if reason =~ /\A[A-Z]/ && !reason.start_with?(*allowlist)
if /\A[A-Z]/.match?(reason) && !reason.start_with?(*allowlist)
problem "'#{first_word}' from the `keg_only` reason should be '#{first_word.downcase}'." do |corrector|
reason[0] = reason[0].downcase
corrector.replace(@offensive_node.source_range, "\"#{reason}\"")
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/rubocops/options.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def audit_formula(_node, _class_node, _parent_class_node, body_node)
option = string_content(option)
problem UNI_DEPRECATION_MSG if option == "universal"

if option !~ /with(out)?-/ &&
if !/with(out)?-/.match?(option) &&
option != "cxx11" &&
option != "universal"
problem "Options should begin with with/without. " \
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/sandbox.rb
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ def exec(*args)
logs = Utils.popen_read("syslog", *syslog_args)

# These messages are confusing and non-fatal, so don't report them.
logs = logs.lines.reject { |l| l.match(/^.*Python\(\d+\) deny file-write.*pyc$/) }.join
logs = logs.lines.grep_v(/^.*Python\(\d+\) deny file-write.*pyc$/).join

unless logs.empty?
if @logfile
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/test/cask/audit_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
describe Cask::Audit, :cask do
def include_msg?(problems, msg)
if msg.is_a?(Regexp)
Array(problems).any? { |problem| problem[:message] =~ msg }
Array(problems).any? { |problem| problem[:message]&.match?(msg) }
dduugg marked this conversation as resolved.
Show resolved Hide resolved
else
Array(problems).any? { |problem| problem[:message] == msg }
end
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/utils/bottles.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def extname_tag_rebuild(filename)

def receipt_path(bottle_file)
bottle_file_list(bottle_file).find do |line|
line =~ %r{.+/.+/INSTALL_RECEIPT.json}
%r{.+/.+/INSTALL_RECEIPT.json}.match?(line)
end
end

Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/utils/curl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@
# Strategy:
# If the `:homepage` 404s, it's a GitHub link, and we have a token then
# check the API (which does use tokens) for the repository
repo_details = url.match(%r{https?://github\.com/(?<user>[^/]+)/(?<repo>[^/]+)/?.*})
repo_details = url.match?(%r{https?://github\.com/(?<user>[^/]+)/(?<repo>[^/]+)/?.*})

Check warning on line 338 in Library/Homebrew/utils/curl.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/utils/curl.rb#L338

Added line #L338 was not covered by tests
dduugg marked this conversation as resolved.
Show resolved Hide resolved
check_github_api = url_type == SharedAudits::URL_TYPE_HOMEPAGE &&
details[:status_code] == "404" &&
repo_details &&
Expand Down