Merge pull request #206 from dvandersluis/issue/205

[Fix #205] Update `Performance/ConstantRegexp` to allow memoized regexps
rubocop · Jan 12, 2021 · 4e26952 · 4e26952
2 parents 7a29c60 + 5fdd214
commit 4e26952
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 14 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## master (unreleased)
 
+### Changes
+
+* [#205](https://github.com/rubocop-hq/rubocop-performance/issues/205): Update `Performance/ConstantRegexp` to allow memoized regexps. ([@dvandersluis][])
+
 ## 1.9.2 (2021-01-01)
 
 ### Bug fixes

diff --git a/config/default.yml b/config/default.yml
@@ -80,6 +80,7 @@ Performance/ConstantRegexp:
   Description: 'Finds regular expressions with dynamic components that are all constants.'
   Enabled: pending
   VersionAdded: '1.9'
+  VersionChanged: <<next>>
 
 Performance/Count:
   Description: >-

diff --git a/docs/modules/ROOT/pages/cops_performance.adoc b/docs/modules/ROOT/pages/cops_performance.adoc
@@ -453,15 +453,15 @@ array.sort_by { |a| a[:foo] }
 | Yes
 | Yes
 | 1.9
-| -
+| <<next>>
 |===
 
 This cop finds regular expressions with dynamic components that are all constants.
 
 Ruby allocates a new Regexp object every time it executes a code containing such
-a regular expression. It is more efficient to extract it into a constant
-or add an `/o` option to perform `#{}` interpolation only once and reuse that
-Regexp object.
+a regular expression. It is more efficient to extract it into a constant,
+memoize it, or add an `/o` option to perform `#{}` interpolation only once and
+reuse that Regexp object.
 
 === Examples
 
@@ -482,6 +482,11 @@ end
 def tokens(pattern)
   pattern.scan(TOKEN).reject { |token| token.match?(/\A#{SEPARATORS}\Z/o) }
 end
+
+# good
+def separators
+  @separators ||= /\A#{SEPARATORS}\Z/
+end
 ----
 
 == Performance/Count

diff --git a/lib/rubocop/cop/performance/constant_regexp.rb b/lib/rubocop/cop/performance/constant_regexp.rb
@@ -6,9 +6,9 @@ module Performance
       # This cop finds regular expressions with dynamic components that are all constants.
       #
       # Ruby allocates a new Regexp object every time it executes a code containing such
-      # a regular expression. It is more efficient to extract it into a constant
-      # or add an `/o` option to perform `#{}` interpolation only once and reuse that
-      # Regexp object.
+      # a regular expression. It is more efficient to extract it into a constant,
+      # memoize it, or add an `/o` option to perform `#{}` interpolation only once and
+      # reuse that Regexp object.
       #
       # @example
       #
@@ -28,13 +28,18 @@ module Performance
       #     pattern.scan(TOKEN).reject { |token| token.match?(/\A#{SEPARATORS}\Z/o) }
       #   end
       #
+      #   # good
+      #   def separators
+      #     @separators ||= /\A#{SEPARATORS}\Z/
+      #   end
+      #
       class ConstantRegexp < Base
         extend AutoCorrector
 
-        MSG = 'Extract this regexp into a constant or append an `/o` option to its options.'
+        MSG = 'Extract this regexp into a constant, memoize it, or append an `/o` option to its options.'
 
         def on_regexp(node)
-          return if within_const_assignment?(node) ||
+          return if within_allowed_assignment?(node) ||
                     !include_interpolated_const?(node) ||
                     node.single_interpolation?
 
@@ -45,8 +50,8 @@ def on_regexp(node)
 
         private
 
-        def within_const_assignment?(node)
-          node.each_ancestor(:casgn).any?
+        def within_allowed_assignment?(node)
+          node.each_ancestor(:casgn, :or_asgn).any?
         end
 
         def_node_matcher :regexp_escape?, <<~PATTERN

diff --git a/spec/rubocop/cop/performance/constant_regexp_spec.rb b/spec/rubocop/cop/performance/constant_regexp_spec.rb
@@ -6,7 +6,7 @@
   it 'registers an offense and corrects when regexp contains interpolated constant' do
     expect_offense(<<~RUBY)
       str.match?(/\A\#{CONST}/)
-                 ^^^^^^^^^^^ Extract this regexp into a constant or append an `/o` option to its options.
+                 ^^^^^^^^^^^ Extract this regexp into a constant, memoize it, or append an `/o` option to its options.
     RUBY
 
     expect_correction(<<~RUBY)
@@ -17,7 +17,7 @@
   it 'registers an offense and corrects when regexp contains multiple interpolated constants' do
     expect_offense(<<~RUBY)
       str.match?(/\A\#{CONST1}something\#{CONST2}\z/)
-                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Extract this regexp into a constant or append an `/o` option to its options.
+                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Extract this regexp into a constant, memoize it, or append an `/o` option to its options.
     RUBY
 
     expect_correction(<<~RUBY)
@@ -28,7 +28,7 @@
   it 'registers an offense and corrects when regexp contains `Regexp.escape` on constant' do
     expect_offense(<<~RUBY)
       str.match?(/\A\#{CONST1}something\#{Regexp.escape(CONST2)}\z/)
-                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Extract this regexp into a constant or append an `/o` option to its options.
+                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Extract this regexp into a constant, memoize it, or append an `/o` option to its options.
     RUBY
 
     expect_correction(<<~RUBY)
@@ -48,6 +48,12 @@
     RUBY
   end
 
+  it 'does not register an offense when the regexp is memoized' do
+    expect_no_offenses(<<~RUBY)
+      @regexp ||= /\#{ANOTHER_CONST}/
+    RUBY
+  end
+
   it 'does not register an offense when regexp has `/o` option' do
     expect_no_offenses(<<~RUBY)
       str.match?(/\#{CONST}/o)