Merge pull request rubocop#482 from jdelStrother/relative-date

Fix a false-positive with Rails/RelativeDateConstant
koic · May 11, 2021 · 3683d92 · 3683d92
2 parents c0715c6 + e0e1c9a
commit 3683d92
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 20 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 ### Bug fixes
 
+* [#482](https://github.com/rubocop/rubocop-rails/pull/482): Fix a false positive for `Rails/RelativeDateConstant` when assigning (hashes/arrays/etc)-containing procs to a constant. ([@jdelStrother][])
 * [#419](https://github.com/rubocop/rubocop-rails/issues/419): Fix an error for `Rails/UniqueValidationWithoutIndex` when using a unique index and `check_constraint` that has `nil` first argument. ([@koic][])
 
 ## 2.10.1 (2021-05-06)
@@ -382,3 +383,4 @@
 [@leonp1991]: https://github.com/leonp1991
 [@drenmi]: https://github.com/drenmi
 [@rhymes]: https://github.com/rhymes
+[@jdelStrother]: https://github.com/jdelStrother
diff --git a/lib/rubocop/cop/rails/relative_date_constant.rb b/lib/rubocop/cop/rails/relative_date_constant.rb
@@ -33,16 +33,12 @@ class RelativeDateConstant < Base
 
         MSG = 'Do not assign `%<method_name>s` to constants as it ' \
               'will be evaluated only once.'
-        RELATIVE_DATE_METHODS = %i[since from_now after ago until before yesterday tomorrow].freeze
+        RELATIVE_DATE_METHODS = %i[since from_now after ago until before yesterday tomorrow].to_set.freeze
 
         def on_casgn(node)
-          return if node.children[2]&.block_type?
-
-          node.each_descendant(:send) do |send_node|
-            relative_date?(send_node) do |method_name|
-              add_offense(node, message: message(method_name)) do |corrector|
-                autocorrect(corrector, node)
-              end
+          nested_relative_date(node) do |method_name|
+            add_offense(node, message: message(method_name)) do |corrector|
+              autocorrect(corrector, node)
             end
           end
         end
@@ -55,7 +51,7 @@ def on_masgn(node)
           lhs.children.zip(rhs.children).each do |(name, value)|
             next unless name.casgn_type?
 
-            relative_date?(value) do |method_name|
+            nested_relative_date(value) do |method_name|
               add_offense(offense_range(name, value), message: message(method_name)) do |corrector|
                 autocorrect(corrector, node)
               end
@@ -64,7 +60,7 @@ def on_masgn(node)
         end
 
         def on_or_asgn(node)
-          relative_date_or_assignment?(node) do |method_name|
+          relative_date_or_assignment(node) do |method_name|
             add_offense(node, message: format(MSG, method_name: method_name))
           end
         end
@@ -93,20 +89,22 @@ def offense_range(name, value)
           range_between(name.loc.expression.begin_pos, value.loc.expression.end_pos)
         end
 
-        def relative_date_method?(method_name)
-          RELATIVE_DATE_METHODS.include?(method_name)
+        def nested_relative_date(node, &callback)
+          return if node.block_type?
+
+          node.each_child_node do |child|
+            nested_relative_date(child, &callback)
+          end
+
+          relative_date(node, &callback)
         end
 
-        def_node_matcher :relative_date_or_assignment?, <<~PATTERN
-          (:or_asgn (casgn _ _) (send _ $#relative_date_method?))
+        def_node_matcher :relative_date_or_assignment, <<~PATTERN
+          (:or_asgn (casgn _ _) (send _ $RELATIVE_DATE_METHODS))
         PATTERN
 
-        def_node_matcher :relative_date?, <<~PATTERN
-          {
-            ({erange irange} _ (send _ $#relative_date_method?))
-            ({erange irange} (send _ $#relative_date_method?) _)
-            (send _ $#relative_date_method?)
-          }
+        def_node_matcher :relative_date, <<~PATTERN
+          (send _ $RELATIVE_DATE_METHODS)
         PATTERN
       end
     end

diff --git a/spec/rubocop/cop/rails/relative_date_constant_spec.rb b/spec/rubocop/cop/rails/relative_date_constant_spec.rb
@@ -26,6 +26,17 @@ class SomeClass
       RUBY
     end
 
+    it 'accepts a nested proc' do
+      expect_no_offenses(<<~RUBY)
+        class SomeClass
+          EXPIRIES = {
+            yearly: Proc.new { 1.year.ago },
+            monthly: Proc.new { 1.month.ago }
+          }
+        end
+      RUBY
+    end
+
     it 'registers an offense for ActiveSupport::Duration.since' do
       expect_offense(<<~RUBY)
         class SomeClass