Comparing Time object with an integer doesn't throw an error

[tgxworld]

Steps to reproduce

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem "rails"
  # If you want to test against edge Rails replace the previous line with this:
  # gem "rails", github: "rails/rails", branch: "main"
end

require "active_support"
require "active_support/core_ext/time"
require "minitest/autorun"

class BugTest < Minitest::Test
  def test_stuff
    # This would have raise an argument error in Ruby for trying to compare a Time object with an Integer
    assert Time.now >= Time.now.to_i - 999_999

    assert Time.now.to_i >= Time.now.to_i - 999_999
  end
end

Expected behavior

I'm not sure but I would expect one of the following:

1. Raise an argument error like what Ruby does when trying to compare a Time object with an Integer
2. Convert the Time object to an Integer before comparing
3. Convert the Integer to a Time object before comparing

Actual behavior

No errors are raised and the code will always evaluate to false.

System configuration

Rails version: 7.1.3

Ruby version: 3.2.2

[maxfelsher]

I tried to follow the method chain for Time#<=>, since it's used for >= comparisons. In Rails, that method is defined as #compare_with_coercion:

rails/activesupport/lib/active_support/core_ext/time/calculations.rb

Lines 315 to 328 in c7551d0

	# Layers additional behavior on Time#<=> so that DateTime and ActiveSupport::TimeWithZone instances
	# can be chronologically compared with a Time
	def compare_with_coercion(other)
	# we're avoiding Time#to_datetime and Time#to_time because they're expensive
	if other.class == Time
	compare_without_coercion(other)
	elsif other.is_a?(Time)
	compare_without_coercion(other.to_time)
	else
	to_datetime <=> other
	end
	end
	alias_method :compare_without_coercion, :<=>
	alias_method :<=>, :compare_with_coercion

The integer value on the right side of the comparison isn't Time-ish, so the Time object on the left side gets converted to a DateTime and then compared to the integer value. DateTime#<=> checks if the object on the right side responds to #to_datetime and goes up the inheritance chain if not (which is the case here):

rails/activesupport/lib/active_support/core_ext/date_time/calculations.rb

Lines 206 to 214 in c7551d0

	# Layers additional behavior on DateTime#<=> so that Time and
	# ActiveSupport::TimeWithZone instances can be compared with a DateTime.
	def <=>(other)
	if other.respond_to? :to_datetime
	super other.to_datetime rescue nil
	else
	super
	end
	end

The super-method in this case is Date#<=>. There's a method in Rails for this, but it ends up calling the native #<=> method for DateTime (which is defined on Date) if the object on the right side isn't a Time:

rails/activesupport/lib/active_support/core_ext/date/calculations.rb

Lines 151 to 160 in c7551d0

	# Allow Date to be compared with Time by converting to DateTime and relying on the <=> from there.
	def compare_with_coercion(other)
	if other.is_a?(Time)
	to_datetime <=> other
	else
	compare_without_coercion(other)
	end
	end
	alias_method :compare_without_coercion, :<=>
	alias_method :<=>, :compare_with_coercion

Here's the twist. While I didn't read through the C code for the native Date#<=>, https://ruby-doc.org/3.2.2/exts/date/Date.html#method-i-3C-3D-3E specifically notes that when comparing to a numeric value, the #ajd method is called on the Date and the resulting value is compared to the numeric value on the right side. That #ajd method is the "astronomical Julian day number"; I wouldn't have been able to tell you what that meant before today, but the important thing is that it's a count of days (since about 6700 years ago) and nowadays is between 2_000_000 and 3_000_000. But the integer on the right side of the comparison came from calling Time#to_i, and that's the number of seconds since January 1, 1970, currently a bit over 1_700_000_000. So you have to subtract a very large number from Time.now.to_i in order for Time.now to compare as greater. And for what it's worth, this also affects DateTime comparisons to integers.

I haven't checked to see how far back this behavior goes.

[maxfelsher]

To follow up on this a bit and lend support to the original statement of the issue, I think it makes sense that for any Time instances t1 and t2, if t1 > t2 evaluates to true and t1.to_i > t2.to_i evaluates to true, then t1 > t2.to_i should either evaluate to true or raise an exception. Instead, due to unexpected coercion, t1 > t2.to_i is only true if t2 is on or before 1970-01-29 (the latest day that included a time when the number of seconds since the Unix epoch was less than the astronomical Julian day number) or t1 is very far in the future. All of this goes for DateTime instances as well.

In terms of solving this, I'm not sure whether it makes more sense to fix it in Time or DateTime. Fixing it in DateTime should also address the issue in Time (due to the partial delegation of Time#<=> to DateTime#<=>) but it isn't clear to me what the solution would be for DateTime. I'm assuming that there isn't much interest in making DateTime#<=> coerce the object into Unix time when comparing to a numeric value, since that could cause the method to return a completely different value just by adding Active Support to a project. Alternatively, the DateTime#to_i method in Active Support could be changed to return the astronomical Julian day number instead of Unix time, which would be more conceptually consistent with Ruby. But that method has returned the Unix time in Rails for almost 15 years, and DateTime#to_f has returned a comparable number for even longer. And since Ruby considers DateTime deprecated, maybe it makes sense to just fix the issue in Time and not worry about DateTime?