Skip to content
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.

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

Support timezone object argument to Time.{at,new} and Time#{getlocal,localtime} #3740

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ Compatibility:
* Add `Dir.for_fd` (#3681, @andrykonchin).
* Add `Dir.fchdir` (#3681, @andrykonchin).
* Add `Dir#chdir` (#3681, @andrykonchin).
* Support Timezone argument to `Time.{new,at}` and `Time#{getlocal,localtime}` (#1717, @patricklinpl, @manefz, @rwstauner).

Performance:

Expand Down
3 changes: 2 additions & 1 deletion spec/ruby/core/time/getlocal_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
t = Time.gm(2007, 1, 9, 12, 0, 0).getlocal(3630)
t.should == Time.new(2007, 1, 9, 13, 0, 30, 3630)
t.utc_offset.should == 3630
t.zone.should be_nil
end

platform_is_not :windows do
Expand Down Expand Up @@ -128,7 +129,7 @@ def zone.local_to_utc(time)

-> {
Time.utc(2000, 1, 1, 12, 0, 0).getlocal(zone)
}.should raise_error(TypeError, /can't convert \w+ into an exact number/)
}.should raise_error(TypeError)
end

it "does not raise exception if timezone does not implement #local_to_utc method" do
Expand Down
11 changes: 11 additions & 0 deletions spec/ruby/core/time/localtime_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,17 @@
end
end

describe "with an argument that responds to #utc_to_local" do
it "coerces using #utc_to_local" do
o = mock('string')
o.should_receive(:utc_to_local).and_return(Time.new(2007, 1, 9, 13, 0, 0, 3600))
t = Time.gm(2007, 1, 9, 12, 0, 0)
t.localtime(o)
t.should == Time.new(2007, 1, 9, 13, 0, 0, 3600)
t.utc_offset.should == 3600
end
end

it "raises ArgumentError if the String argument is not of the form (+|-)HH:MM" do
t = Time.now
-> { t.localtime("3600") }.should raise_error(ArgumentError)
Expand Down
2 changes: 1 addition & 1 deletion spec/ruby/core/time/new_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ def zone.utc_to_local(time)

-> {
Time.new(2000, 1, 1, 12, 0, 0, zone)
}.should raise_error(TypeError, /can't convert \w+ into an exact number/)
}.should raise_error(TypeError)
end

it "does not raise exception if timezone does not implement #utc_to_local method" do
Expand Down
1 change: 0 additions & 1 deletion spec/tags/core/time/at_tags.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
fails:Time.at :in keyword argument could be a timezone object
fails:Time.at passed non-Time, non-Numeric with an argument that responds to #to_r needs for the argument to respond to #to_int too
4 changes: 0 additions & 4 deletions spec/tags/core/time/getlocal_tags.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,5 @@ fails:Time#getlocal returns a Time with a UTC offset of the specified number of
fails:Time#getlocal with an argument that responds to #to_r coerces using #to_r
fails:Time#getlocal raises ArgumentError if the argument represents a value less than or equal to -86400 seconds
fails:Time#getlocal raises ArgumentError if the argument represents a value greater than or equal to 86400 seconds
fails:Time#getlocal with a timezone argument returns a Time in the timezone
fails:Time#getlocal with a timezone argument accepts timezone argument that must have #local_to_utc and #utc_to_local methods
fails:Time#getlocal with a timezone argument raises TypeError if timezone does not implement #utc_to_local method
fails:Time#getlocal with a timezone argument does not raise exception if timezone does not implement #local_to_utc method
fails:Time#getlocal with a timezone argument subject's class implements .find_timezone method calls .find_timezone to build a time object if passed zone name as a timezone argument
fails:Time#getlocal with a timezone argument subject's class implements .find_timezone method does not call .find_timezone if passed any not string/numeric/timezone timezone argument
1 change: 0 additions & 1 deletion spec/tags/core/time/minus_tags.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
fails:Time#- maintains subseconds precision
fails:Time#- maintains precision
fails:Time#- zone is a timezone object preserves time zone
12 changes: 0 additions & 12 deletions spec/tags/core/time/new_tags.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,12 @@ fails:Time.new with a utc_offset argument returns a Time with a UTC offset of th
fails:Time.new with a utc_offset argument with an argument that responds to #to_r coerces using #to_r
fails:Time.new with a utc_offset argument raises ArgumentError if the argument represents a value less than or equal to -86400 seconds
fails:Time.new with a utc_offset argument raises ArgumentError if the argument represents a value greater than or equal to 86400 seconds
fails:Time.new with a timezone argument returns a Time in the timezone
fails:Time.new with a timezone argument accepts timezone argument that must have #local_to_utc and #utc_to_local methods
fails:Time.new with a timezone argument raises TypeError if timezone does not implement #local_to_utc method
fails:Time.new with a timezone argument does not raise exception if timezone does not implement #utc_to_local method
fails:Time.new with a timezone argument the #abbr method is used by '%Z' in #strftime
fails:Time.new with a timezone argument returned value by #utc_to_local and #local_to_utc methods could be Time instance
fails:Time.new with a timezone argument returned value by #utc_to_local and #local_to_utc methods could be Time subclass instance
fails:Time.new with a timezone argument returned value by #utc_to_local and #local_to_utc methods could be any object with #to_i method
fails:Time.new with a timezone argument returned value by #utc_to_local and #local_to_utc methods could have any #zone and #utc_offset because they are ignored
fails:Time.new with a timezone argument returned value by #utc_to_local and #local_to_utc methods leads to raising Argument error if difference between argument and result is too large
fails:Time.new with a timezone argument Time-like argument of #utc_to_local and #local_to_utc methods implements subset of Time methods
fails:Time.new with a timezone argument Time-like argument of #utc_to_local and #local_to_utc methods has attribute values the same as a Time object in UTC
fails:Time.new with a timezone argument #name method uses the optional #name method for marshaling
fails:Time.new with a timezone argument #name method cannot marshal Time if #name method isn't implemented
fails:Time.new with a timezone argument subject's class implements .find_timezone method calls .find_timezone to build a time object at loading marshaled data
fails:Time.new with a timezone argument subject's class implements .find_timezone method calls .find_timezone to build a time object if passed zone name as a timezone argument
fails:Time.new with a timezone argument subject's class implements .find_timezone method does not call .find_timezone if passed any not string/numeric/timezone timezone argument
fails:Time.new with a timezone argument :in keyword argument could be a timezone object
fails: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:Time.new with a timezone argument Time.new with a String argument converts precision keyword argument into Integer if is not nil
fails:Time.new with a timezone argument Time.new with a String argument raise TypeError is can't convert precision keyword argument into Integer
Expand Down
1 change: 0 additions & 1 deletion spec/tags/core/time/plus_tags.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
fails:Time#+ maintains subseconds precision
fails:Time#+ zone is a timezone object preserves time zone
1 change: 0 additions & 1 deletion spec/tags/library/time/to_time_tags.txt

This file was deleted.

5 changes: 2 additions & 3 deletions src/main/java/org/truffleruby/core/time/TimeNodes.java
Original file line number Diff line number Diff line change
Expand Up @@ -409,9 +409,8 @@ Object timeZone(RubyTime time) {

@Primitive(name = "time_set_zone")
public abstract static class TimeSetZoneNode extends PrimitiveArrayArgumentsNode {
@Specialization(guards = "strings.isRubyString(this, zone)", limit = "1")
Object timeSetZone(RubyTime time, Object zone,
@Cached RubyStringLibrary strings) {
@Specialization
Object timeSetZone(RubyTime time, Object zone) {
time.zone = zone;
return zone;
}
Expand Down
43 changes: 29 additions & 14 deletions src/main/ruby/truffleruby/core/time.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@
class Time
include Comparable

# Time#to_time is defined in date_core.c but it's just `return self`
# so we can make it available for use without having to `require "date"`.
alias_method :to_time, :itself

def inspect
str = strftime('%Y-%m-%d %H:%M:%S')

Expand Down Expand Up @@ -107,13 +111,15 @@ def getlocal(offset = nil)
def zone
zone = Primitive.time_zone(self)

if zone && zone.ascii_only?
zone.encode Encoding::US_ASCII
elsif zone && Encoding.default_internal
zone.encode Encoding.default_internal
else
zone
if zone && Primitive.is_a?(zone, String)
if zone.ascii_only?
return zone.encode Encoding::US_ASCII
elsif Encoding.default_internal
return zone.encode Encoding.default_internal
end
end

zone
end

# Random number for hash codes. Stops hashes for similar values in
Expand Down Expand Up @@ -166,10 +172,11 @@ def getgm
end
alias_method :getutc, :getgm

def localtime(offset = nil)
def localtime(zone_or_offset = nil)
offset = zone_or_offset
if offset
to_utc = Time.send(:utc_offset_in_utc?, offset)
offset = Truffle::Type.coerce_to_utc_offset(offset)
offset = Truffle::Type.coerce_to_utc_offset(offset, self, :utc_to_local)
end

# the only cases when #localtime is allowed for a frozen time -
Expand All @@ -183,7 +190,9 @@ def localtime(offset = nil)
if to_utc
Primitive.time_utctime(self)
else
Primitive.time_localtime(self, offset)
result = Primitive.time_localtime(self, offset)
Truffle::TimeOperations.set_zone_if_object(result, zone_or_offset)
result
end
end

Expand Down Expand Up @@ -347,8 +356,6 @@ class << self
def at(sec, sub_sec = undefined, unit = undefined, **kwargs)
# **kwargs is used here because 'in' is a ruby keyword
timezone = kwargs[:in]
offset = timezone ? Truffle::Type.coerce_to_utc_offset(timezone) : nil
is_utc = utc_offset_in_utc?(timezone) if offset

result = if Primitive.undefined?(sub_sec)
if Primitive.is_a?(sec, Time)
Expand All @@ -362,10 +369,15 @@ def at(sec, sub_sec = undefined, unit = undefined, **kwargs)
Primitive.time_at self, sec.to_i, ns
end
end

offset = timezone ? Truffle::Type.coerce_to_utc_offset(timezone, result, :utc_to_local) : nil
is_utc = utc_offset_in_utc?(timezone) if offset

if result && offset
result = is_utc ? Primitive.time_utctime(result) : Primitive.time_localtime(result, offset)
end
if result
Truffle::TimeOperations.set_zone_if_object(result, timezone)
return result
end

Expand Down Expand Up @@ -426,9 +438,12 @@ def new(year = undefined, month = undefined, day = nil, hour = nil, minute = nil
if utc_offset_in_utc?(utc_offset)
utc_offset = :utc
else
utc_offset = Truffle::Type.coerce_to_utc_offset(utc_offset)
zone = utc_offset
utc_offset = Truffle::Type.coerce_to_utc_offset(utc_offset, Time.utc(year, month, day, hour, minute, second), :local_to_utc)
end
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)
Truffle::TimeOperations.set_zone_if_object(result, zone)
result
end
end

Expand All @@ -442,7 +457,7 @@ def now(**options)
in_timezone = options[:in]

if in_timezone
utc_offset = Truffle::Type.coerce_to_utc_offset(in_timezone)
utc_offset = Truffle::Type.coerce_to_utc_offset(in_timezone, time_now, :utc_to_local)
is_utc = utc_offset_in_utc?(in_timezone)
is_utc ? Primitive.time_utctime(time_now) : Primitive.time_localtime(time_now, utc_offset)
else
Expand Down
9 changes: 9 additions & 0 deletions src/main/ruby/truffleruby/core/truffle/time_operations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -121,5 +121,14 @@ def self.utc_offset_for_compose(utc_offset)
Truffle::Type.coerce_to_utc_offset(utc_offset)
end
end

# When a timezone object is used (via its utc_to_local or local_to_utc methods)
# the resulting time object gets its zone set to be the object
# (but this isn't done when the zone is just an integer offset or string abbreviation).
def self.set_zone_if_object(time, zone)
return if Primitive.nil?(zone) || Primitive.is_a?(zone, Integer) || Primitive.is_a?(zone, String)

Primitive.time_set_zone(time, zone)
end
end
end
10 changes: 9 additions & 1 deletion src/main/ruby/truffleruby/core/type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -481,11 +481,19 @@ def self.coerce_to_exact_num(obj)
end
end

def self.coerce_to_utc_offset(offset)
def self.coerce_to_utc_offset(offset, time = nil, conversion_method = nil)
offset = String.try_convert(offset) || offset

if Primitive.is_a? offset, String
offset = Truffle::Type.coerce_string_to_utc_offset(offset)
elsif conversion_method == :local_to_utc && Primitive.respond_to?(offset, :local_to_utc, false)
time ||= Time.now
as_utc = offset.local_to_utc(time.getutc)
offset = time.to_i - as_utc.to_i
elsif conversion_method == :utc_to_local && Primitive.respond_to?(offset, :utc_to_local, false)
time ||= Time.now
as_local = offset.utc_to_local(time.getutc)
offset = as_local.utc_offset.nonzero? || as_local.to_i - time.to_i
else
offset = Truffle::Type.coerce_to_exact_num(offset)
end
Expand Down
Loading