diff --git a/CHANGELOG.md b/CHANGELOG.md index 42a5c979cd..b06d53b907 100644 --- 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 index dfb20d66d7..5ccf00e192 100644 --- 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: <> Performance/Count: Description: >- diff --git a/docs/modules/ROOT/pages/cops_performance.adoc b/docs/modules/ROOT/pages/cops_performance.adoc index 407211b4c7..e7adc44322 100644 --- 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 -| - +| <> |=== 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 index 09e5c0d81f..dcb55dd0fd 100644 --- 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 index 75314e65d1..c38b3c3736 100644 --- 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)