Skip to content

Fix DateFormatter TimeZone Setter #1704

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

Merged
merged 4 commits into from
Oct 11, 2018
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion CoreFoundation/NumberDate.subproj/CFTimeZone.c
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,7 @@ static CFTimeZoneRef __CFTimeZoneCreateSystem(void) {
size_t zoneInfoDirLen = CFStringGetLength(__tzZoneInfo);
if (strncmp(linkbuf, tzZoneInfo, zoneInfoDirLen) == 0) {
name = CFStringCreateWithBytes(kCFAllocatorSystemDefault, (uint8_t *)linkbuf + zoneInfoDirLen,
strlen(linkbuf) - zoneInfoDirLen + 1, kCFStringEncodingUTF8, false);
strlen(linkbuf) - zoneInfoDirLen, kCFStringEncodingUTF8, false);
} else {
name = CFStringCreateWithBytes(kCFAllocatorSystemDefault, (uint8_t *)linkbuf, strlen(linkbuf), kCFStringEncodingUTF8, false);
}
Expand Down
6 changes: 3 additions & 3 deletions Foundation/DateFormatter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -176,13 +176,13 @@ open class DateFormatter : Formatter {
/*@NSCopying*/ internal var _timeZone: TimeZone? { willSet { _reset() } }
open var timeZone: TimeZone! {
get {
guard let timeZone = _timeZone else {
guard let tz = _timeZone else {
return (CFDateFormatterCopyProperty(_cfObject, kCFDateFormatterTimeZone) as! NSTimeZone)._swiftObject
}
return timeZone
return tz
}
set {
_timeZone = timeZone
_timeZone = newValue
}
}

Expand Down
27 changes: 18 additions & 9 deletions TestFoundation/TestCalendar.swift
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,16 @@ class TestNSDateComponents: XCTestCase {
// 2286-11-20 17:46:41
let date4 = Date(timeIntervalSince1970: 10_000_000_001)

let diff1 = Calendar.current.dateComponents([.month, .year, .day], from: date1, to: date2)
// The date components below assume UTC/GMT time zone.
guard let timeZone = TimeZone(abbreviation: "UTC") else {
XCTFail("Unable to create UTC TimeZone for Test")
return
}

var calendar = Calendar.current
calendar.timeZone = timeZone

let diff1 = calendar.dateComponents([.month, .year, .day], from: date1, to: date2)
XCTAssertEqual(diff1.year, 1)
XCTAssertEqual(diff1.month, 5)
XCTAssertEqual(diff1.isLeapMonth, false)
Expand All @@ -350,35 +359,35 @@ class TestNSDateComponents: XCTestCase {
XCTAssertNil(diff1.calendar)
XCTAssertNil(diff1.timeZone)

let diff2 = Calendar.current.dateComponents([.weekOfMonth], from: date2, to: date1)
let diff2 = calendar.dateComponents([.weekOfMonth], from: date2, to: date1)
XCTAssertEqual(diff2.weekOfMonth, -76)
XCTAssertEqual(diff2.isLeapMonth, false)

let diff3 = Calendar.current.dateComponents([.weekday], from: date2, to: date1)
let diff3 = calendar.dateComponents([.weekday], from: date2, to: date1)
XCTAssertEqual(diff3.weekday, -536)
XCTAssertEqual(diff3.isLeapMonth, false)

let diff4 = Calendar.current.dateComponents([.weekday, .weekOfMonth], from: date1, to: date2)
let diff4 = calendar.dateComponents([.weekday, .weekOfMonth], from: date1, to: date2)
XCTAssertEqual(diff4.weekday, 4)
XCTAssertEqual(diff4.weekOfMonth, 76)
XCTAssertEqual(diff4.isLeapMonth, false)

let diff5 = Calendar.current.dateComponents([.weekday, .weekOfYear], from: date1, to: date2)
let diff5 = calendar.dateComponents([.weekday, .weekOfYear], from: date1, to: date2)
XCTAssertEqual(diff5.weekday, 4)
XCTAssertEqual(diff5.weekOfYear, 76)
XCTAssertEqual(diff5.isLeapMonth, false)

let diff6 = Calendar.current.dateComponents([.month, .weekOfMonth], from: date1, to: date2)
let diff6 = calendar.dateComponents([.month, .weekOfMonth], from: date1, to: date2)
XCTAssertEqual(diff6.month, 17)
XCTAssertEqual(diff6.weekOfMonth, 2)
XCTAssertEqual(diff6.isLeapMonth, false)

let diff7 = Calendar.current.dateComponents([.weekOfYear, .weekOfMonth], from: date2, to: date1)
let diff7 = calendar.dateComponents([.weekOfYear, .weekOfMonth], from: date2, to: date1)
XCTAssertEqual(diff7.weekOfYear, -76)
XCTAssertEqual(diff7.weekOfMonth, 0)
XCTAssertEqual(diff7.isLeapMonth, false)

let diff8 = Calendar.current.dateComponents([.era, .quarter, .year, .month, .day, .hour, .minute, .second, .nanosecond, .calendar, .timeZone], from: date2, to: date3)
let diff8 = calendar.dateComponents([.era, .quarter, .year, .month, .day, .hour, .minute, .second, .nanosecond, .calendar, .timeZone], from: date2, to: date3)
XCTAssertEqual(diff8.era, 0)
XCTAssertEqual(diff8.year, 315)
XCTAssertEqual(diff8.quarter, 0)
Expand All @@ -392,7 +401,7 @@ class TestNSDateComponents: XCTestCase {
XCTAssertNil(diff8.calendar)
XCTAssertNil(diff8.timeZone)

let diff9 = Calendar.current.dateComponents([.era, .quarter, .year, .month, .day, .hour, .minute, .second, .nanosecond, .calendar, .timeZone], from: date4, to: date3)
let diff9 = calendar.dateComponents([.era, .quarter, .year, .month, .day, .hour, .minute, .second, .nanosecond, .calendar, .timeZone], from: date4, to: date3)
XCTAssertEqual(diff9.era, 0)
XCTAssertEqual(diff9.year, 0)
XCTAssertEqual(diff9.quarter, 0)
Expand Down
52 changes: 52 additions & 0 deletions TestFoundation/TestDateFormatter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ class TestDateFormatter: XCTestCase {
("test_dateFormatString", test_dateFormatString),
("test_setLocaleToNil", test_setLocaleToNil),
("test_setTimeZoneToNil", test_setTimeZoneToNil),
("test_setTimeZone", test_setTimeZone),
("test_expectedTimeZone", test_expectedTimeZone),
]
}

Expand Down Expand Up @@ -356,4 +358,54 @@ class TestDateFormatter: XCTestCase {
// Time zone should go back to the system one.
XCTAssertEqual(f.timeZone, NSTimeZone.system)
}

func test_setTimeZone() {
// Test two different time zones. Should ensure that if one
// happens to be TimeZone.current, we still get a valid test.
let newYork = TimeZone(identifier: "America/New_York")!
let losAngeles = TimeZone(identifier: "America/Los_Angeles")!

XCTAssertNotEqual(newYork, losAngeles)

// Case 1: New York
let f = DateFormatter()
f.timeZone = newYork
XCTAssertEqual(f.timeZone, newYork)

// Case 2: Los Angeles
f.timeZone = losAngeles
XCTAssertEqual(f.timeZone, losAngeles)
}

func test_expectedTimeZone() {
let gmt = TimeZone(abbreviation: DEFAULT_TIMEZONE)
let newYork = TimeZone(identifier: "America/New_York")!
let losAngeles = TimeZone(identifier: "America/Los_Angeles")!

XCTAssertNotEqual(newYork, losAngeles)

let now = Date()

let f = DateFormatter()
f.dateFormat = "z"
f.locale = Locale(identifier: "en_US_POSIX")

// Case 1: TimeZone.current
// This case can catch some issues that cause TimeZone.current to be
// treated like GMT, but it doesn't work if TimeZone.current is GMT.
// If you do find an issue like this caused by this first case,
// it would benefit from a more specific test that fails when
// TimeZone.current is GMT as well.
// (ex. TestTimeZone.test_systemTimeZoneName)
f.timeZone = TimeZone.current
XCTAssertEqual(f.string(from: now), TimeZone.current.abbreviation())

// Case 2: New York
f.timeZone = newYork
XCTAssertEqual(f.string(from: now), newYork.abbreviation())

// Case 3: Los Angeles
f.timeZone = losAngeles
XCTAssertEqual(f.string(from: now), losAngeles.abbreviation())
}
}
16 changes: 16 additions & 0 deletions TestFoundation/TestTimeZone.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//

import CoreFoundation

class TestTimeZone: XCTestCase {

static var allTests: [(String, (TestTimeZone) -> () throws -> Void)] {
Expand All @@ -29,6 +31,7 @@ class TestTimeZone: XCTestCase {

("test_customMirror", test_tz_customMirror),
("test_knownTimeZones", test_knownTimeZones),
("test_systemTimeZoneName", test_systemTimeZoneName),
]
}

Expand Down Expand Up @@ -198,4 +201,17 @@ class TestTimeZone: XCTestCase {
XCTAssertNotNil(TimeZone(identifier: tz), "Cant instantiate valid timeZone: \(tz)")
}
}

func test_systemTimeZoneName() {
// Ensure that the system time zone creates names the same way as creating them with an identifier.
// If it isn't the same, bugs in DateFormat can result, but in this specific case, the bad length
// is only visible to CoreFoundation APIs, and the Swift versions hide it, making it hard to detect.
let timeZone = CFTimeZoneCopySystem()
let timeZoneName = CFTimeZoneGetName(timeZone)

let createdTimeZone = TimeZone(identifier: TimeZone.current.identifier)!

XCTAssertEqual(CFStringGetLength(timeZoneName), TimeZone.current.identifier.count)
XCTAssertEqual(CFStringGetLength(timeZoneName), createdTimeZone.identifier.count)
}
}