Skip to content

Time fixes #8877

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
enebo merged 9 commits into from
Jun 26, 2025
Merged

Time fixes #8877

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e021fc6
Fix some obvious mistakes in parsing code. Add comment on current state
enebo Apr 28, 2025
8891dc8
Merge branch 'master' into time_fix
enebo Jun 23, 2025
b9a3846
Time init fixing bad logic and naming mistakes and address spec failures
enebo Jun 23, 2025
7786b91
Fix more Time.new+ specs
enebo Jun 24, 2025
1c0a5a2
Remove unused method and make all numbers use _ for clarity
enebo Jun 24, 2025
d927e32
time args processes usecs and not nsecs
enebo Jun 25, 2025
87ccaff
timezone matching for inspect/to_s/zone
enebo Jun 25, 2025
3aa4242
Merge branch 'master' into time_fix
enebo Jun 26, 2025
33bc5f9
Use common constant instead of listing it twice
enebo Jun 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 27 additions & 9 deletions core/src/main/java/org/jruby/RubyTime.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ public class RubyTime extends RubyObject {
private static final BigDecimal ONE_MILLION_BD = BigDecimal.valueOf(1000000);
private static final BigDecimal ONE_BILLION_BD = BigDecimal.valueOf(1000000000);
public static final int TIME_SCALE = 1_000_000_000;
public static final int TIME_SCALE_DIGITS = 9;

private DateTime dt;
private long nsec;
Expand Down Expand Up @@ -629,7 +630,11 @@ public RubyTime localtime(ThreadContext context) {
public RubyTime localtime(ThreadContext context, IRubyObject arg) {
final DateTimeZone zone = getTimeZoneFromUtcOffset(context, arg);

if (zone == null) throw invalidUTCOffset(context);
if (zone == null) {
throw invalidUTCOffset(context);
} else if (zone == DateTimeZone.UTC) {
return gmtime(context);
}

return adjustTimeZone(context, zone, true);
}
Expand Down Expand Up @@ -1780,10 +1785,8 @@ private IRubyObject initializeNow(ThreadContext context, IRubyObject zone) {
this.zone = zone;
dtz = DateTimeZone.UTC;
} else {
dtz = getTimeZoneFromUtcOffset(context, zone);
if (dtz != null) {
this.setIsTzRelative(true);
} else {
dtz = handleUTCDateTimeZone(context, zone);
if (dtz == null) {
this.zone = findTimezone(context, zone);
maybeZoneObj = true;
dtz = DateTimeZone.UTC;
Expand Down Expand Up @@ -1833,6 +1836,23 @@ private IRubyObject initializeNow(ThreadContext context, IRubyObject zone) {
return context.nil;
}

private DateTimeZone handleUTCDateTimeZone(ThreadContext context, IRubyObject zone) {
var dtz = getTimeZoneFromUtcOffset(context, zone);

// FIXME: At some point UTC started returning "UTC" for #zone. This is a form-fit and
// we need to resolve how we store info about zone and joda time vs how MRI stores the
// same info.
if (dtz != null) {
if (!(zone instanceof RubyString &&
(zone.asJavaString().equals("Z") ||
zone.asJavaString().equals("UTC") ||
zone.asJavaString().equals("-00:00")))) {
this.setIsTzRelative(true);
}
}
return dtz;
}

@JRubyMethod(name = "initialize", optional = 8, checkArity = false, visibility = PRIVATE, keywords = true)
public IRubyObject initialize(ThreadContext context, IRubyObject[] args) {
boolean keywords = hasKeywords(ThreadContext.resetCallInfo(context));
Expand Down Expand Up @@ -1927,10 +1947,8 @@ public IRubyObject initialize(ThreadContext context, IRubyObject year, IRubyObje
this.zone = zone;
dtz = DateTimeZone.UTC;
} else {
dtz = getTimeZoneFromUtcOffset(context, zone);
if (dtz != null) {
this.setIsTzRelative(true);
} else {
dtz = handleUTCDateTimeZone(context, zone);
if (dtz == null) {
this.zone = findTimezone(context, zone);
maybeZoneObj = true;
dtz = DateTimeZone.UTC;
Expand Down
22 changes: 13 additions & 9 deletions core/src/main/java/org/jruby/util/RubyTimeParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@
import org.jruby.runtime.ThreadContext;
import org.jruby.runtime.builtin.IRubyObject;

import static org.jruby.RubyTime.TIME_SCALE_DIGITS;
import static org.jruby.api.Convert.*;
import static org.jruby.api.Convert.asFixnum;
import static org.jruby.api.Create.newString;
import static org.jruby.api.Error.argumentError;
import static org.jruby.util.RubyStringBuilder.str;

public class RubyTimeParser {
public static final int TIME_SCALE_NUMDIGITS = 10;
public static final long SIZE_MAX = Long.MAX_VALUE;


Expand Down Expand Up @@ -84,7 +84,6 @@ public IRubyObject parse(ThreadContext context, RubyTime self, RubyString str, I
IRubyObject invalidSecs = context.runtime.newString(new ByteList(bytes, timePart, ptr - timePart + 1, true));
throw argumentError(context, str(context.runtime, "subsecond expected after dot: ", invalidSecs));
}
ndigits = digits;
subsec = parseInt(context, false);
if (subsec.isNil()) break;
while (!isEOS() && isDigit(0)) ptr++;
Expand All @@ -105,12 +104,18 @@ public IRubyObject parse(ThreadContext context, RubyTime self, RubyString str, I
throw argumentError(context, "no time information");
}
if (!subsec.isNil()) {
if (ndigits < TIME_SCALE_NUMDIGITS) {
int mul = (int) Math.pow(10, TIME_SCALE_NUMDIGITS - ndigits);
subsec = asFixnum(context, ((RubyInteger) subsec).asLong(context) * mul);
} else if (ndigits > TIME_SCALE_NUMDIGITS) {
int mul = (int) Math.pow(10, ndigits - TIME_SCALE_NUMDIGITS);
// FIXME: Our time args processing later will examine all subseconds as-if they are microseconds so
// this will calculate as nanoseconds and make a rational to adjust it back to microseconds. I am
// not sure if time args should change or perhaps all other paths in should give subseconds in nanoseconds
if (ndigits < TIME_SCALE_DIGITS) {
int mul = (int) Math.pow(10, TIME_SCALE_DIGITS - ndigits);
var value = ((RubyInteger) subsec).asLong(context) * mul;
subsec = RubyRational.newRational(context.runtime, value, 1000);
} else if (ndigits > TIME_SCALE_DIGITS) {
int mul = (int) Math.pow(10, ndigits - TIME_SCALE_DIGITS - 3/*1000*/);
subsec = RubyRational.newRational(context.runtime, ((RubyInteger) subsec).asLong(context), mul);
} else if (subsec instanceof RubyInteger) {
subsec = RubyRational.newRational(context.runtime, ((RubyInteger) subsec).asLong(context), 1000);
}
}

Expand Down Expand Up @@ -229,14 +234,13 @@ private int expectTwoDigits(ThreadContext context, String label, int max) {

private IRubyObject parseInt(ThreadContext context, boolean parseSign) {
int sign = 1;
ndigits = 0;
if (parseSign) {
eatSpace();
byte signByte = peek();
if (signByte == '+') {
ndigits++;
advance();
} else if (signByte == '-') {
ndigits++;
advance();
sign = -1;
}
Expand Down
85 changes: 42 additions & 43 deletions core/src/main/java/org/jruby/util/time/TimeArgs.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import org.joda.time.IllegalFieldValueException;
import org.jruby.RubyBoolean;
import org.jruby.RubyFloat;
import org.jruby.RubyInteger;
import org.jruby.RubyNumeric;
import org.jruby.RubyRational;
import org.jruby.RubyString;
Expand All @@ -14,7 +15,9 @@
import org.jruby.runtime.ThreadContext;
import org.jruby.runtime.builtin.IRubyObject;

import static org.jruby.RubyRational.rationalCanonicalize;
import static org.jruby.RubyTime.TIME_SCALE;
import static org.jruby.RubyTime.TIME_SCALE_DIGITS;
import static org.jruby.api.Convert.asFixnum;
import static org.jruby.api.Convert.toDouble;
import static org.jruby.api.Convert.toInt;
Expand All @@ -26,7 +29,7 @@ public class TimeArgs {
private final int day;
private final int hour;
private final int minute;
private final int second;
private final int second; // -1 represents no explicit second paraameter
private final IRubyObject secondObj;
private final IRubyObject usecObj;
private final boolean dst;
Expand Down Expand Up @@ -115,68 +118,64 @@ public void initializeTime(ThreadContext context, RubyTime time, DateTimeZone dt
instant = chrono.days().add(instant, this.day - 1);
instant = chrono.hours().add(instant, this.hour);
instant = chrono.minutes().add(instant, this.minute);
instant = chrono.seconds().add(instant, this.second);

IRubyObject usecObj = this.usecObj;
IRubyObject secondObj = this.secondObj;

long millis = 0;
long nanos = 0;
int secondsInRational = -1;

if (usecObj.isNil()) {
if (!secondObj.isNil()) {
if (secondObj instanceof RubyRational rat) {
if (rat.isNegativeNumber(context)) throw argumentError(context, "argument out of range.");

RubyRational nsec = (RubyRational) rat.op_mul(context, asFixnum(context, 1_000_000_000));

long fullNanos = nsec.asLong(context);
long fullMillis = fullNanos / 1_000_000;

nanos = fullNanos - fullMillis * 1_000_000;
millis = fullMillis % 1000;
if (secondObj instanceof RubyRational subSecond) {
if (subSecond.isNegativeNumber(context)) throw argumentError(context, "argument out of range");

var numerator = subSecond.getNumerator().asLong(context);
var denominator = subSecond.getDenominator().asLong(context);
if (numerator > denominator) {
secondsInRational = (int) (numerator / (double) denominator);
numerator = numerator % denominator;
subSecond = RubyRational.newRational(context.runtime, numerator, denominator);
}
var subSeconds = subSecond.asDouble(context) * TIME_SCALE;

millis = (long) subSeconds / 1_000_000;
nanos = (long) subSeconds % 1_000_000;
} else {
double secs = toDouble(context, secondObj);

if (secs < 0 || secs >= TIME_SCALE) throw argumentError(context, "argument out of range.");
if (secs < 0 || secs >= TIME_SCALE) throw argumentError(context, "argument out of range");

millis = (int) (secs * 1000) % 1000;
nanos = ((long) (secs * TIME_SCALE) % 1000000);
millis = (int) (secs * 1000) % 1_000;
nanos = ((long) (secs * TIME_SCALE) % 1_000_000);
}
}
} else if (usecObj instanceof RubyRational rat) {
if (rat.isNegativeNumber(context)) throw argumentError(context, "argument out of range.");

RubyRational nsec = (RubyRational) rat.op_mul(context, asFixnum(context, 1000));
} else if (usecObj instanceof RubyRational subSecond) {
if (subSecond.isNegativeNumber(context)) throw argumentError(context, "argument out of range");

long tmpNanos = (long) nsec.asDouble(context);
var subSeconds = subSecond.asDouble(context) * 1_000;

millis = tmpNanos / 1_000_000;
nanos = tmpNanos % 1_000_000;
millis = (long) subSeconds / 1_000_000;
nanos = (long) subSeconds % 1_000_000;
} else if (usecObj instanceof RubyFloat flo) {
if (flo.isNegativeNumber(context)) throw argumentError(context, "argument out of range.");
if (flo.isNegativeNumber(context)) throw argumentError(context, "argument out of range");

double micros = flo.asDouble(context);

millis = (long) (micros / 1000);
nanos = (long) Math.rint((micros * 1000) % 1_000_000);
millis = (long) (micros / 1_000);
nanos = (long) Math.rint((micros * 1_000) % 1_000_000);
} else {
int usec = parseIntArg(context, usecObj).isNil() ? 0 : toInt(context, usecObj);
int subSeconds = parseIntArg(context, usecObj).isNil() ? 0 : toInt(context, usecObj);
if (subSeconds < 0 || subSeconds >= 1_000_000) throw argumentError(context, "argument out of range");

if (usec < 0 || usec >= TIME_SCALE / 1000) throw argumentError(context, "argument out of range.");

int usecPart = usec % 1000;
int msecPart = usec / 1000;

if (usec < 0) {
msecPart -= 1;
usecPart += 1000;
}

nanos = 1000 * usecPart;
millis = msecPart;
double micros = subSeconds;
millis = (long) (micros / 1_000);
nanos = (long) Math.rint((micros * 1_000) % 1_000_000);
}

// We need to know if we passed in explicit 0 for second or omitted it as a param
if (this.second == -1) {
instant = chrono.seconds().add(instant, secondsInRational != -1 ? secondsInRational : 0);
} else {
instant = chrono.seconds().add(instant, this.second);
}
instant = chrono.millis().add(instant, millis);

try {
Expand Down Expand Up @@ -267,7 +266,7 @@ private void validateDayHourMinuteSecond(ThreadContext context) {
throw argumentError(context, "argument out of range for minute");
}

if ((second < 0 || second > 60) || (hour == 24 && second > 0)) {
if (second != -1 && (second < 0 || second > 60) || (hour == 24 && second > 0)) {
throw argumentError(context, "argument out of range");
}
}
Expand Down
5 changes: 0 additions & 5 deletions spec/tags/ruby/core/time/new_tags.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,11 @@ fails(not implemented, jruby/jruby#6161):Time.new with a timezone argument subje
fails(only during full spec run):Time.new with a utc_offset argument raises ArgumentError if the String argument is not of the form (+|-)HH:MM
fails:Time.new with a timezone argument returned value by #utc_to_local and #local_to_utc methods cannot have arbitrary #utc_offset if it is an instance of Time
fails(https://github.com/jruby/jruby/issues/8736):Time.new with a timezone argument :in keyword argument allows omitting minor arguments
fails(https://github.com/jruby/jruby/issues/8736):Time.new with a timezone argument Time.new with a String argument parses an ISO-8601 like format
fails(https://github.com/jruby/jruby/issues/8736):Time.new with a timezone argument Time.new with a String argument accepts precision keyword argument and truncates specified digits of sub-second part
fails(https://github.com/jruby/jruby/issues/8736):Time.new with a timezone argument Time.new with a String argument converts precision keyword argument into Integer if is not nil
fails(https://github.com/jruby/jruby/issues/8736):Time.new with a timezone argument Time.new with a String argument raises ArgumentError if date/time parts values are not valid
fails(https://github.com/jruby/jruby/issues/8736):Time.new with a timezone argument Time.new with a String argument raises ArgumentError if utc offset parts are not valid
fails(https://github.com/jruby/jruby/issues/8736):Time.new with a timezone argument Time.new with a String argument raises ArgumentError if string doesn't start with year
fails(https://github.com/jruby/jruby/issues/8736):Time.new with a timezone argument Time.new with a String argument raises ArgumentError when there are leading space characters
fails(https://github.com/jruby/jruby/issues/8736):Time.new with a timezone argument Time.new with a String argument raises ArgumentError when there are trailing whitespaces
fails:Time.new with a timezone argument Time.new with a String argument returns Time with correct subseconds when given seconds fraction is shorted than 6 digits
fails:Time.new with a timezone argument Time.new with a String argument returns Time with correct subseconds when given seconds fraction is milliseconds
fails:Time.new with a timezone argument Time.new with a String argument returns Time with correct subseconds when given seconds fraction is longer that 6 digits but shorted than 9 digits
fails:Time.new with a timezone argument Time.new with a String argument returns Time with correct subseconds when given seconds fraction is nanoseconds
fails:Time.new with a timezone argument Time.new with a String argument returns Time with correct subseconds when given seconds fraction is longer than 9 digits
3 changes: 0 additions & 3 deletions spec/tags/ruby/core/time/utc_tags.txt
View file
Open in desktop

This file was deleted.

Loading