[GR-18163] Fix Time.new with String argument and handle nanoseconds correctly

PullRequest: truffleruby/4521

Author: andrykonchin

CHANGELOG.md

@@ -6,6 +6,7 @@ New features:
 Bug fixes:
 
 * Fix `Range#cover?` on begin-less ranges and non-integer values (@nirvdrum, @rwstauner).
+* Fix `Time.new` with String argument and handle nanoseconds correctly (#3836, @andrykonchin).
 
 Compatibility:
 

spec/ruby/core/time/new_spec.rb

@@ -524,6 +524,36 @@ def obj.to_int; 3; end
       Time.new("2021-12-25 00:00:00.123456789876 +09:00", precision: 3r).subsec.should == 0.123r
     end
 
+    it "returns Time with correct subseconds when given seconds fraction is shorted than 6 digits" do
+      Time.new("2020-12-25T00:56:17.123 +09:00").nsec.should == 123000000
+      Time.new("2020-12-25T00:56:17.123 +09:00").usec.should == 123000
+      Time.new("2020-12-25T00:56:17.123 +09:00").subsec.should == 0.123
+    end
+
+    it "returns Time with correct subseconds when given seconds fraction is milliseconds" do
+      Time.new("2020-12-25T00:56:17.123456 +09:00").nsec.should == 123456000
+      Time.new("2020-12-25T00:56:17.123456 +09:00").usec.should == 123456
+      Time.new("2020-12-25T00:56:17.123456 +09:00").subsec.should == 0.123456
+    end
+
+    it "returns Time with correct subseconds when given seconds fraction is longer that 6 digits but shorted than 9 digits" do
+      Time.new("2020-12-25T00:56:17.12345678 +09:00").nsec.should == 123456780
+      Time.new("2020-12-25T00:56:17.12345678 +09:00").usec.should == 123456
+      Time.new("2020-12-25T00:56:17.12345678 +09:00").subsec.should == 0.12345678
+    end
+
+    it "returns Time with correct subseconds when given seconds fraction is nanoseconds" do
+      Time.new("2020-12-25T00:56:17.123456789 +09:00").nsec.should == 123456789
+      Time.new("2020-12-25T00:56:17.123456789 +09:00").usec.should == 123456
+      Time.new("2020-12-25T00:56:17.123456789 +09:00").subsec.should == 0.123456789
+    end
+
+    it "returns Time with correct subseconds when given seconds fraction is longer than 9 digits" do
+      Time.new("2020-12-25T00:56:17.123456789876 +09:00").nsec.should == 123456789
+      Time.new("2020-12-25T00:56:17.123456789876 +09:00").usec.should == 123456
+      Time.new("2020-12-25T00:56:17.123456789876 +09:00").subsec.should == 0.123456789
+    end
+
     ruby_version_is ""..."3.3" do
       it "raise TypeError is can't convert precision keyword argument into Integer" do
         -> {

spec/ruby/core/time/shared/time_params.rb

@@ -179,6 +179,10 @@
     }.should raise_error(ArgumentError, "argument out of range")
   end
 
+  it "raises ArgumentError when given 8 arguments" do
+    -> { Time.send(@method, *[0]*8) }.should raise_error(ArgumentError)
+  end
+
   it "raises ArgumentError when given 9 arguments" do
     -> { Time.send(@method, *[0]*9) }.should raise_error(ArgumentError)
   end

src/main/java/org/truffleruby/core/time/TimeNodes.java

@@ -532,7 +532,7 @@ RubyTime timeSFromArray(
                 Object utcoffset) {
             final RubyLanguage language = getLanguage();
 
-            if (nsec < 0 || nsec > 999999999 ||
+            if (nsec < 0 || nsec > 999_999_999 ||
                     sec < 0 || sec > 60 || // MRI accepts sec=60, whether it is a leap second or not
                     min < 0 || min > 59 ||
                     hour < 0 || hour > 23 ||

src/main/ruby/truffleruby/core/time.rb

@@ -421,11 +421,11 @@ def new(year = undefined, month = undefined, day = nil, hour = nil, minute = nil
       elsif Primitive.is_a?(year, String) && month_undefined
         Truffle::TimeOperations.new_from_string(self, year, **options)
       elsif Primitive.nil? utc_offset
-        Truffle::TimeOperations.compose(self, :local, year, month, day, hour, minute, second)
+        Truffle::TimeOperations.compose(self, :local, year, month, day, hour, minute, second, nil, nil)
       elsif utc_offset == :std
-        Truffle::TimeOperations.compose(self, :local, second, minute, hour, day, month, year, nil, nil, false, nil)
+        Truffle::TimeOperations.compose(self, :local, year, month, day, hour, minute, second, nil, false)
       elsif utc_offset == :dst
-        Truffle::TimeOperations.compose(self, :local, second, minute, hour, day, month, year, nil, nil, true, nil)
+        Truffle::TimeOperations.compose(self, :local, year, month, day, hour, minute, second, nil, true)
       else
         if utc_offset_in_utc?(utc_offset)
           utc_offset = :utc
@@ -435,7 +435,7 @@ def new(year = undefined, month = undefined, day = nil, hour = nil, minute = nil
           utc_offset = Truffle::TimeOperations.calculate_utc_offset_with_timezone_object(utc_offset, :local_to_utc, as_utc) ||
             Truffle::TimeOperations.coerce_to_utc_offset(utc_offset)
         end
-        result = Truffle::TimeOperations.compose(self, utc_offset, year, month, day, hour, minute, second)
+        result = Truffle::TimeOperations.compose(self, utc_offset, year, month, day, hour, minute, second, nil, nil)
         Truffle::TimeOperations.set_zone_if_object(result, zone)
         result
       end
@@ -464,12 +464,12 @@ def now(**options)
     end
 
     def local(*args)
-      Truffle::TimeOperations.compose(self, :local, *args)
+      Truffle::TimeOperations.compose_dual_signature(self, :local, *args)
     end
     alias_method :mktime, :local
 
     def gm(*args)
-      Truffle::TimeOperations.compose(self, :utc, *args)
+      Truffle::TimeOperations.compose_dual_signature(self, :utc, *args)
     end
     alias_method :utc, :gm
   end

src/main/ruby/truffleruby/core/truffle/time_operations.rb

@@ -17,19 +17,30 @@ module TimeOperations
 
     # Handles Time methods .utc, .gm. .local, .mktime dual signature:
     # - year, month, day, hour, min, sec, usec
-    # - sec, min, hour, day, month, year
-    def self.compose(time_class, utc_offset, p1, p2 = nil, p3 = nil, p4 = nil, p5 = nil, p6 = nil, p7 = nil,
-                yday = undefined, is_dst = undefined, tz = undefined)
+    # - sec, min, hour, day, month, year, dummy, dummy, dummy, dummy
+    def self.compose_dual_signature(time_class, utc_offset, p1, p2 = nil, p3 = nil, p4 = nil, p5 = nil, p6 = nil, p7 = nil,
+                     yday = undefined, is_dst = undefined, tz = undefined)
       if Primitive.undefined?(tz)
         unless Primitive.undefined?(is_dst)
           raise ArgumentError, 'wrong number of arguments (9 for 1..8)'
         end
 
-        year, month, mday, hour, min, sec, usec, is_dst = p1, p2, p3, p4, p5, p6, p7, -1
+        year, month, mday, hour, min, sec, usec, is_dst = p1, p2, p3, p4, p5, p6, p7, nil
       else
-        year, month, mday, hour, min, sec, usec, is_dst = p6, p5, p4, p3, p2, p1, 0, is_dst ? 1 : 0
+        year, month, mday, hour, min, sec, usec, is_dst = p6, p5, p4, p3, p2, p1, 0, is_dst
       end
 
+      nsec = nil
+      if Primitive.is_a?(usec, String)
+        nsec = usec.to_i * 1000
+      elsif usec
+        nsec = (usec * 1000).to_i
+      end
+
+      compose(time_class, utc_offset, year, month, mday, hour, min, sec, nsec, is_dst)
+    end
+
+    def self.compose(time_class, utc_offset, year, month, mday, hour, min, sec, nsec, is_dst)
       if Primitive.is_a?(month, String) or month.respond_to?(:to_str)
         month = StringValue(month)
         month = MonthValue[month.upcase] || month.to_i
@@ -50,23 +61,16 @@ def self.compose(time_class, utc_offset, p1, p2 = nil, p3 = nil, p4 = nil, p5 =
       hour  = Primitive.rb_num2int(hour  || 0)
       min   = Primitive.rb_num2int(min   || 0)
 
-      nsec = nil
-      if Primitive.is_a?(usec, String)
-        nsec = usec.to_i * 1000
-      elsif usec
-        nsec = (usec * 1000).to_i
-      end
-
       case utc_offset
       when :utc
-        is_dst = -1
+        is_dst = nil
         is_utc = true
         utc_offset = nil
       when :local
         is_utc = false
         utc_offset = nil
       else
-        is_dst = -1
+        is_dst = nil
         is_utc = false
       end
 
@@ -88,10 +92,12 @@ def self.compose(time_class, utc_offset, p1, p2 = nil, p3 = nil, p4 = nil, p5 =
       end
 
       nsec ||= 0
+      dst_code = is_dst ? 1 : (Primitive.nil?(is_dst) ? -1 : 0)
 
-      Primitive.time_s_from_array(time_class, sec, min, hour, mday, month, year, nsec, is_dst, is_utc, utc_offset)
+      Primitive.time_s_from_array(time_class, sec, min, hour, mday, month, year, nsec, dst_code, is_utc, utc_offset)
     end
 
+    # MRI: time_init_parse()
     def self.new_from_string(time_class, str, **options)
       raise ArgumentError, 'time string should have ASCII compatible encoding' unless str.encoding.ascii_compatible?
 
@@ -103,10 +109,13 @@ def self.new_from_string(time_class, str, **options)
                [ T] (?<hour> \d{2})
                   : (?<min>  \d{2})
                   : (?<sec>  \d{2})
-                  (?:\. (?<usec> \d+) )?
+                  (?:\. (?<subsec> \d+) )?
               (?:\s* (?<offset>\S+))?
              )?\z/x =~ str
-        return self.compose(time_class, self.utc_offset_for_compose(offset || options[:in]), year, month, mday, hour, min, sec, usec)
+
+        nsec = subsec[0...9].ljust(9, '0').to_i if subsec
+        utc_offset = self.utc_offset_for_compose(offset || options[:in])
+        return self.compose(time_class, utc_offset, year, month, mday, hour, min, sec, nsec, nil)
       end
 
       raise ArgumentError, "can't parse: #{str.inspect}"