Merge pull request #571 from pirj/time-arithmetic

Add `Rails/DurationArithmetic` cop

Author: koic

CHANGELOG.md

@@ -482,3 +482,4 @@
 [@skryukov]: https://github.com/skryukov
 [@johnsyweb]: https://github.com/johnsyweb
 [@theunraveler]: https://github.com/theunraveler
+[@pirj]: https://github.com/pirj

changelog/new_duration_arithmetic_cop.md

@@ -0,0 +1 @@
+* [#571](https://github.com/rubocop/rubocop-rails/issues/571): Add `Rails/DurationArithmetic` cop. ([@pirj][])

config/default.yml

@@ -245,6 +245,12 @@ Rails/DelegateAllowBlank:
   Enabled: true
   VersionAdded: '0.44'
 
+Rails/DurationArithmetic:
+  Description: 'Do not use duration as arithmetic operand with `Time.current`.'
+  StyleGuide: 'https://rails.rubystyle.guide#duration-arithmetic'
+  Enabled: pending
+  VersionAdded: '2.13'
+
 Rails/DynamicFindBy:
   Description: 'Use `find_by` instead of dynamic `find_by_*`.'
   StyleGuide: 'https://rails.rubystyle.guide#find_by'

docs/modules/ROOT/pages/cops.adoc

@@ -39,6 +39,7 @@ based on the https://rails.rubystyle.guide/[Rails Style Guide].
 * xref:cops_rails.adoc#railsdefaultscope[Rails/DefaultScope]
 * xref:cops_rails.adoc#railsdelegate[Rails/Delegate]
 * xref:cops_rails.adoc#railsdelegateallowblank[Rails/DelegateAllowBlank]
+* xref:cops_rails.adoc#railsdurationarithmetic[Rails/DurationArithmetic]
 * xref:cops_rails.adoc#railsdynamicfindby[Rails/DynamicFindBy]
 * xref:cops_rails.adoc#railseagerevaluationlogmessage[Rails/EagerEvaluationLogMessage]
 * xref:cops_rails.adoc#railsenumhash[Rails/EnumHash]

docs/modules/ROOT/pages/cops_rails.adoc

@@ -1206,6 +1206,42 @@ delegate :foo, to: :bar, allow_blank: true
 delegate :foo, to: :bar, allow_nil: true
 ----
 
+== Rails/DurationArithmetic
+
+|===
+| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed
+
+| Pending
+| Yes
+| Yes
+| 2.13
+| -
+|===
+
+This cop checks if a duration is added to or subtracted from `Time.current`.
+
+=== Examples
+
+[source,ruby]
+----
+# bad
+Time.current - 1.minute
+Time.current + 2.days
+
+# good - using relative would make it harder to express and read
+Date.yesterday + 3.days
+created_at - 1.minute
+3.days - 1.hour
+
+# good
+1.minute.ago
+2.days.from_now
+----
+
+=== References
+
+* https://rails.rubystyle.guide#duration-arithmetic
+
 == Rails/DynamicFindBy
 
 |===

lib/rubocop/cop/rails/duration_arithmetic.rb

@@ -0,0 +1,97 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Rails
+      # This cop checks if a duration is added to or subtracted from `Time.current`.
+      #
+      # @example
+      #   # bad
+      #   Time.current - 1.minute
+      #   Time.current + 2.days
+      #
+      #   # good - using relative would make it harder to express and read
+      #   Date.yesterday + 3.days
+      #   created_at - 1.minute
+      #   3.days - 1.hour
+      #
+      #   # good
+      #   1.minute.ago
+      #   2.days.from_now
+      class DurationArithmetic < Base
+        extend AutoCorrector
+
+        MSG = 'Do not add or subtract duration.'
+
+        RESTRICT_ON_SEND = %i[+ -].freeze
+
+        DURATIONS = Set[:second, :seconds, :minute, :minutes, :hour, :hours,
+                        :day, :days, :week, :weeks, :fortnight, :fortnights]
+
+        # @!method duration_arithmetic_argument?(node)
+        #   Match duration subtraction or addition with current time.
+        #
+        #   @example source that matches
+        #     Time.current - 1.hour
+        #
+        #   @example source that matches
+        #     ::Time.zone.now + 1.hour
+        #
+        #   @param node [RuboCop::AST::Node]
+        #   @yield operator and duration
+        def_node_matcher :duration_arithmetic_argument?, <<~PATTERN
+          (send #time_current? ${ :+ :- } $#duration?)
+        PATTERN
+
+        # @!method duration?(node)
+        #   Match a literal Duration
+        #
+        #   @example source that matches
+        #     1.hour
+        #
+        #   @example source that matches
+        #     9.5.weeks
+        #
+        #   @param node [RuboCop::AST::Node]
+        #   @return [Boolean] true if matches
+        def_node_matcher :duration?, '(send { int float (send nil _) } DURATIONS)'
+
+        # @!method time_current?(node)
+        #   Match Time.current
+        #
+        #   @example source that matches
+        #     Time.current
+        #
+        #   @example source that matches
+        #     ::Time.zone.now
+        #
+        #   @param node [RuboCop::AST::Node]
+        #   @return [Boolean] true if matches
+        def_node_matcher :time_current?, <<~PATTERN
+          {
+            (send (const _ :Time) :current)
+            (send (send (const _ :Time) :zone) :now)
+          }
+        PATTERN
+
+        def on_send(node)
+          duration_arithmetic_argument?(node) do |*operation|
+            add_offense(node) do |corrector|
+              corrector.replace(node.source_range, corrected_source(*operation))
+            end
+          end
+        end
+
+        private
+
+        def corrected_source(operator, duration)
+          if operator == :-
+            "#{duration.source}.ago"
+          else
+            "#{duration.source}.from_now"
+          end
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rails_cops.rb

@@ -28,6 +28,7 @@
 require_relative 'rails/default_scope'
 require_relative 'rails/delegate'
 require_relative 'rails/delegate_allow_blank'
+require_relative 'rails/duration_arithmetic'
 require_relative 'rails/dynamic_find_by'
 require_relative 'rails/eager_evaluation_log_message'
 require_relative 'rails/enum_hash'

spec/rubocop/cop/rails/duration_arithmetic_spec.rb

@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Rails::DurationArithmetic, :config do
+  it 'registers an offense and corrects' do
+    expect_offense(<<~RUBY)
+      Time.zone.now - 1.minute
+      ^^^^^^^^^^^^^^^^^^^^^^^^ Do not add or subtract duration.
+      Time.current + 2.days
+      ^^^^^^^^^^^^^^^^^^^^^ Do not add or subtract duration.
+      ::Time.current + 1.hour
+      ^^^^^^^^^^^^^^^^^^^^^^^ Do not add or subtract duration.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      1.minute.ago
+      2.days.from_now
+      1.hour.from_now
+    RUBY
+  end
+
+  it 'does not register an offense for two duration operands' do
+    expect_no_offenses(<<~RUBY)
+      3.days - 1.hour
+      3.days + 1.hour
+    RUBY
+  end
+
+  it 'does not register an offense if the left operand is non current time' do
+    expect_no_offenses(<<~RUBY)
+      5.hours + Time.current # will raise
+      Date.yesterday + 3.days
+      created_at - 1.minute
+    RUBY
+  end
+end