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

Performance/ConstantRegexp and Performance/RegexpMatch interact badly when autocorrecting #351

Closed
pvande opened this issue Apr 5, 2023 · 0 comments · Fixed by #354
Closed
Labels
bug Something isn't working

Comments

@pvande
Copy link

pvande commented Apr 5, 2023

When both the ConstantRegexp and RegexpMatch cops have violations on the same line, the ConstantRegexp cop may make multiple corrections on the same line, breaking existing code.


Steps to reproduce the problem

Given this configuration:

# .rubocop.yml
require:
  - rubocop-performance

AllCops:
  DisabledByDefault: true

Performance/RegexpMatch:
  Enabled: true
Performance/ConstantRegexp:
  Enabled: true

And given this source file:

CONSTANT = "12345"

if content =~ /#{CONSTANT}.*\n/
  thing
end

Rubocop describes the following offenses:

# rubocop test.rb
Inspecting 1 file
C

Offenses:

test.rb:3:4: C: [Correctable] Performance/RegexpMatch: Use match? instead of =~ when MatchData is not used.
if content =~ /#{CONSTANT}.*\n/
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
test.rb:3:15: C: [Correctable] Performance/ConstantRegexp: Extract this regexp into a constant, memoize it, or append an /o option to its options.
if content =~ /#{CONSTANT}.*\n/
              ^^^^^^^^^^^^^^^^^

1 file inspected, 2 offenses detected, 2 offenses autocorrectable

This is all as expected.

Running this with the --autocorrect/-a flag, however, generates a different report.

# rubocop -a test.rb
Inspecting 1 file
C

Offenses:

test.rb:3:4: C: [Corrected] Performance/ConstantRegexp: Extract this regexp into a constant, memoize it, or append an /o option to its options.
if /#{CONSTANT}.*\n/.match?(contento)
   ^^^^^^^^^^^^^^^^^
test.rb:3:4: C: [Corrected] Performance/RegexpMatch: Use match? instead of =~ when MatchData is not used.
if content =~ /#{CONSTANT}.*\n/
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
test.rb:3:15: C: [Corrected] Performance/ConstantRegexp: Extract this regexp into a constant, memoize it, or append an /o option to its options.
if content =~ /#{CONSTANT}.*\n/
              ^^^^^^^^^^^^^^^^^

1 file inspected, 3 offenses detected, 3 offenses corrected

Note that the bottom-most entry reports an issue pre-RegexpMatch, and the top-most entry reports post-RegexpMatch. Also note that the top-most entry is matching against contento (not content). The resulting file is as follows:

CONSTANT = "12345"

if /#{CONSTANT}.*\n/o.match?(contento)
  thing
end

Since the content variable has been changed to contento, the autocorrected file has semantically changed.

RuboCop version

# rubocop -V
1.49.0 (using Parser 3.2.2.0, rubocop-ast 1.28.0, running on ruby 3.2.0) [aarch64-linux-musl]
  - rubocop-performance 1.16.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants