Some minor regexp matching perf improvements #16405

dduugg · 2023-12-27T21:17:23Z

Have you followed the guidelines in our Contributing document?
Have you checked to ensure there aren't other open Pull Requests for the same change?
Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes? Here's an example.
Have you successfully run brew style with your changes locally?
Have you successfully run brew typecheck with your changes locally?
Have you successfully run brew tests with your changes locally?

This fixes some inefficient regexp matching (creating unused captures) that Performance/RegexpMatch fails to catch.

Bo98

Apart from the utils/curl.rb change, LGTM.

I have some nits in regards to safe navigation.

Library/Homebrew/utils/curl.rb

Library/Homebrew/rubocops/extend/formula_cop.rb

Library/Homebrew/cask/audit.rb

Library/Homebrew/dev-cmd/bottle.rb

Library/Homebrew/formula_text_auditor.rb

Library/Homebrew/keg_relocate.rb

Library/Homebrew/livecheck/strategy/git.rb

Library/Homebrew/test/cask/audit_spec.rb

Bo98

Thanks!

dduugg · 2023-12-28T00:26:10Z

Library/Homebrew/formula_text_auditor.rb

@@ -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)


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

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.

Some minor regexp match perf improvements

3abbf44

Bo98 reviewed Dec 27, 2023

View reviewed changes

Code review changes

caf8259

Bo98 approved these changes Dec 27, 2023

View reviewed changes

dduugg commented Dec 28, 2023

View reviewed changes

Bo98 merged commit 84c2360 into Homebrew:master Dec 28, 2023
24 checks passed

github-actions bot added the outdated PR was locked due to age label Jan 28, 2024

github-actions bot locked as resolved and limited conversation to collaborators Jan 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Some minor regexp matching perf improvements #16405

Some minor regexp matching perf improvements #16405

dduugg commented Dec 27, 2023 •

edited

Bo98 left a comment

Bo98 left a comment

dduugg Dec 28, 2023

Bo98 Dec 28, 2023 •

edited

Some minor regexp matching perf improvements #16405

Some minor regexp matching perf improvements #16405

Conversation

dduugg commented Dec 27, 2023 • edited

Bo98 left a comment

Choose a reason for hiding this comment

Bo98 left a comment

Choose a reason for hiding this comment

dduugg Dec 28, 2023

Choose a reason for hiding this comment

Bo98 Dec 28, 2023 • edited

Choose a reason for hiding this comment

dduugg commented Dec 27, 2023 •

edited

Bo98 Dec 28, 2023 •

edited