Support SE-0166 Swift Archival & Serialization #43

norio-nomura · 2017-05-07T12:52:08Z

Support SE-0166 Swift Archival & Serialization
Fixes #38

~~This uses local built toolchain https://github.com/norio-nomura/swift-dev/releases/tag/20170501a~~

TODOs for merging:

~~[ ] Add text encoding detection to YAMLDecoder.decode(_:from:)~~
Revise throwing errors on failing Encode and Decode
Add more tests
Support first snapshot of Swift 4.0

jpsim · 2017-05-08T20:32:37Z

This is very exciting!

jpsim · 2017-05-11T16:32:09Z

Tests/YamsTests/EncoderTests.swift

-            if let expectedYAML = yaml {
-                XCTAssertEqual(expectedYAML, payload, "Produced YAML not identical to expected YAML.")
+            if let expectedYAML = yamlString {
+                let producedYAML = String(data: payload, encoding: .utf8)! // swiftlint:disable:this force_unwrapping


You can probably skip the force unwrap here as I think that XCTAssertEqual can take arguments of (String?, String)

Yup. But I prefer failing output as

XCTAssertEqual failed: ("{} ") is not equal to ("{ } ") - Produced YAML not identical to expected YAML.

to

XCTAssertEqual failed: ("Optional("{}\n")") is not equal to ("Optional("{ }\n")") - Produced YAML not identical to expected YAML.

Especially in long strings, I feel the former is easier to see.

ianpartridge · 2017-09-12T11:27:36Z

Tests/YamsTests/EncoderTests.swift

+        // MARK: - Date Strategy Tests
+        func testEncodingDate() {
+        #if os(Linux)
+            print("'Date' does not conform to 'Codable' on Linux yet.")


By the way, this is now fixed 🎉

norio-nomura · 2017-10-25T06:28:09Z

Sorry for being late.
This is ready for review.

jpsim

First round of review.

jpsim · 2017-10-25T19:30:36Z

Sources/Yams/Constructor.swift

@@ -185,50 +187,94 @@ extension Double: ScalarConstructible {
    }
 }

+extension Float: ScalarConstructible {


Do you think it'd be possible to generalize this to the FloatingPoint protocol? I ran into several issues while trying to do it, but if you can find a way, seems like that'd be pretty powerful.

jpsim · 2017-10-25T19:31:47Z

Sources/Yams/Constructor.swift

+        var scalar = node.scalar!.string
+        switch scalar {
+        case ".inf", ".Inf", ".INF", "+.inf", "+.Inf", "+.INF":
+            return Float.infinity


Can omit the type here and below: (return .infinity, return -.infinity, return .nan). Same with the Double implementation.

jpsim · 2017-10-25T19:32:45Z

Sources/Yams/Constructor.swift

+    }
+#else
+    fileprivate init?(_ value: String) {
+    self.init(value, radix: 10)


indentation

jpsim · 2017-10-25T19:45:46Z

Sources/Yams/Encoder.swift

+
+    // MARK: - CodingKey for `_UnkeyedEncodingContainer` and `superEncoders`
+
+    struct _YAMLEncodingKey: CodingKey { // swiftlint:disable:this type_name


This shares an implementation with _YAMLDecodingKey. Why not merge them into a _YAMLCodingKey?

jpsim · 2017-10-25T19:56:35Z

Sources/Yams/Representer.swift

+    }
+}
+
+extension Double: ScalarRepresentableCustomizedForCodable {


You should be able to combine these two implementations:

diff --git a/Sources/Yams/Representer.swift b/Sources/Yams/Representer.swift index 09e05f9..c79eb44 100644 --- a/Sources/Yams/Representer.swift +++ b/Sources/Yams/Representer.swift @@ -225,23 +225,25 @@ extension Date: ScalarRepresentableCustomizedForCodable { } } -extension Double: ScalarRepresentableCustomizedForCodable { +extension Double: ScalarRepresentableCustomizedForCodable {} +extension Float: ScalarRepresentableCustomizedForCodable {} + +extension FloatingPoint where Self: CVarArg { public func representedForCodable() -> Node { - #if os(macOS) || os(iOS) || os(watchOS) || os(tvOS) - return Node(formattedStringForCodable, Tag(.float)) - #else +#if os(Linux) return Node(doubleFormatter.string(for: self)!.replacingOccurrences(of: "+-", with: "-"), Tag(.float)) - #endif +#else + return Node(formattedStringForCodable, Tag(.float)) +#endif } private var formattedStringForCodable: String { // Since `NumberFormatter` creates a string with insufficient precision for Decode, // it uses with `String(format:...)` - #if os(macOS) || os(iOS) || os(watchOS) || os(tvOS) +#if os(Linux) + let DBL_DECIMAL_DIG = 9 +#endif let string = String(format: "%.*g", DBL_DECIMAL_DIG, self) - #else - let string = String(format: "%.*g", 9, self) - #endif // "%*.g" does not use scientific notation if the exponent is less than –4. // So fallback to using `NumberFormatter` if string does not uses scientific notation. guard string.lazy.suffix(5).contains("e") else { @@ -251,30 +253,4 @@ extension Double: ScalarRepresentableCustomizedForCodable { } } -extension Float: ScalarRepresentableCustomizedForCodable { - public func representedForCodable() -> Node { - #if os(macOS) || os(iOS) || os(watchOS) || os(tvOS) - return Node(formattedStringForCodable, Tag(.float)) - #else - return Node(floatFormatter.string(for: self)!.replacingOccurrences(of: "+-", with: "-"), Tag(.float)) - #endif - } - - private var formattedStringForCodable: String { - // Since `NumberFormatter` creates a string with insufficient precision for Decode, - // it uses with `String(format:...)` - #if os(macOS) || os(iOS) || os(watchOS) || os(tvOS) - let string = String(format: "%.*g", FLT_DECIMAL_DIG, self) - #else - let string = String(format: "%.*g", 9, self) - #endif - // "%*.g" does not use scientific notation if the exponent is less than –4. - // So fallback to using `NumberFormatter` if string does not uses scientific notation. - guard string.lazy.suffix(6).contains("e") else { - return floatFormatter.string(for: self)!.replacingOccurrences(of: "+-", with: "-") - } - return string - } -} - #endif

Thanks for patch.
By applying this, I noticed that my commits has mistake:

+#if os(Linux) + let DBL_DECIMAL_DIG = 9 +#endif

DBL_DECIMAL_DIG should be 17.
That caused errors that fixed on 587c382.

jpsim · 2017-10-25T19:58:07Z

Tests/YamsTests/EncoderTests.swift

+            _testRoundTrip(of: numbers, expectedYAML: "- 4\n- 8\n- 15\n- 16\n- 23\n- 42\n")
+        }
+
+        func testEncodingTopLevelStructuredSingleClass() {


This test is flaky on Linux in my testing, where the order of keys doesn't appear to be stable across invocations.

EncoderTests.testEncodingTopLevelStructuredSingleClass : XCTAssertEqual failed: ("localhost: relative: http://127.0.0.1 Apple: relative: http://apple.com ") is not equal to ("Apple: relative: http://apple.com localhost: relative: http://127.0.0.1 ") - Produced YAML not identical to expected YAML.

I'll add sortedKeys option to Emitter.Options.

This will be PR against master.

jpsim · 2017-10-25T19:59:31Z

I should say that this is super exciting. I think we'll be able to point to this to demonstrate a good implementation of Swift 4's custom encoders when this merges.

norio-nomura · 2017-10-26T06:17:40Z

Thank you for reviews!

FYI, I opened a custom encoder implementation of Swift 4 that I created when refactoring this PR.
https://github.com/norio-nomura/ObjectEncoder
That uses Any, [Any] and [String: Any] as payload instead of Yams.Node.

…der API

Double and Float are now encoded with sufficient precision for decoding.

…ted only on Apple platforms

Some types already be changed.

Decoding 'Date' has issue on Linux with nanoseconds. https://bugs.swift.org/browse/SR-6223

For stabilizing the test on Linux.

norio-nomura · 2017-10-30T01:12:06Z

I don't have any further points of review

Is this approved?

jpsim · 2017-10-30T03:25:13Z

Yes! 🎉

norio-nomura · 2017-10-30T03:28:30Z

Thanks! 🙏

norio-nomura force-pushed the nn-SE-0166 branch from 2f16213 to f1a3ee4 Compare May 7, 2017 12:57

norio-nomura mentioned this pull request May 8, 2017

Foundation Encoders swiftlang/swift#9005

Merged

jpsim reviewed May 11, 2017

View reviewed changes

norio-nomura force-pushed the nn-SE-0166 branch 2 times, most recently from 2715695 to 4b80558 Compare June 10, 2017 00:50

norio-nomura force-pushed the nn-SE-0166 branch 4 times, most recently from 390b289 to f87861f Compare June 26, 2017 10:47

norio-nomura force-pushed the nn-SE-0166 branch 2 times, most recently from 108899e to bd6ca73 Compare July 28, 2017 06:48

norio-nomura mentioned this pull request Aug 1, 2017

Add configurations to Rules.md realm/SwiftLint#1653

Closed

norio-nomura force-pushed the nn-SE-0166 branch from 087a6fe to 8cbfbf9 Compare August 3, 2017 00:52

norio-nomura force-pushed the nn-SE-0166 branch 2 times, most recently from aa8763f to 6ab910a Compare August 11, 2017 23:49

ianpartridge reviewed Sep 12, 2017

View reviewed changes

norio-nomura mentioned this pull request Oct 20, 2017

Add Emitter.Options #57

Merged

norio-nomura force-pushed the nn-SE-0166 branch 6 times, most recently from 7f64904 to 42727ed Compare October 25, 2017 01:05

norio-nomura changed the title ~~[WIP][Experimental] Support SE-0166 Swift Archival & Serialization~~ Support SE-0166 Swift Archival & Serialization Oct 25, 2017

norio-nomura mentioned this pull request Oct 25, 2017

Add negative Int support to other formats in addition to canonical #59

Merged

jpsim requested changes Oct 25, 2017

View reviewed changes

norio-nomura added 21 commits October 27, 2017 12:32

Remove unneeded method

2171c92

Inline some simple methods

b15a46a

Add missing Decoder's codingPath tests

1546ee8

Added tests to encode/decode types directly supported by Encoder/Deco…

46d9913

…der API

Fix decoding max of UnsignedInteger

d0f2102

Fix encoding Double and Float

dfbff63

Double and Float are now encoded with sufficient precision for decoding.

Disable round trip test with single empty string

795b9bb

Encoding/Decoding Double and Float with full precision are suppor…

a8c730e

…ted only on Apple platforms

Fix compilation issue on Swift 3.x

f2001b9

Apply patch created by jpsim

20476bd

DBL_DECIMAL_DIG = 17

d0b0f1f

Unify ScalarConstructible implementations of Double and Float

2b97792

Remove Substring.sexagesimal()

3524624

Merge _YAMLDecodingKey and _YAMLEncodingKey

d1f55a1

Change _YAML prefix of coders types to _ other than _YAMLCodingKey

21e59d2

Some types already be changed.

Update message about decoding 'Date' on Linux

fd9bc03

Decoding 'Date' has issue on Linux with nanoseconds. https://bugs.swift.org/browse/SR-6223

Apply patch created by jpsim

c856831

Fix compilation issue on Swift 3.x

1608f12

Move swiftlint:disable to the actual actual position of violation

e316e16

Fix wording

0690a07

Use sortKeys option on testEncodingTopLevelStructuredSingleClass()

697b6d7

For stabilizing the test on Linux.

norio-nomura force-pushed the nn-SE-0166 branch from b3ab975 to 697b6d7 Compare October 27, 2017 03:33

jpsim mentioned this pull request Oct 30, 2017

[CI] Move to CircleCI #64

Closed

jpsim approved these changes Oct 30, 2017

View reviewed changes

norio-nomura merged commit 1c69640 into master Oct 30, 2017

norio-nomura deleted the nn-SE-0166 branch October 30, 2017 03:28

norio-nomura mentioned this pull request May 5, 2022

[SR-4861] SingleValueEncodingContainer and SingleValueDecodingContainer should have encode<T>(_:) and decode<T>(_:) swiftlang/swift-corelibs-foundation#3860

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support SE-0166 Swift Archival & Serialization #43

Support SE-0166 Swift Archival & Serialization #43

norio-nomura commented May 7, 2017 •

edited

Loading

jpsim commented May 8, 2017

jpsim May 11, 2017

norio-nomura May 11, 2017

ianpartridge Sep 12, 2017

norio-nomura commented Oct 25, 2017

jpsim left a comment

jpsim Oct 25, 2017

norio-nomura Oct 26, 2017

jpsim Oct 25, 2017

jpsim Oct 25, 2017

jpsim Oct 25, 2017

jpsim Oct 25, 2017

norio-nomura Oct 25, 2017

jpsim Oct 25, 2017

norio-nomura Oct 26, 2017

norio-nomura Oct 26, 2017

jpsim commented Oct 25, 2017

norio-nomura commented Oct 26, 2017

norio-nomura commented Oct 30, 2017

jpsim commented Oct 30, 2017

norio-nomura commented Oct 30, 2017


		// MARK: - CodingKey for `_UnkeyedEncodingContainer` and `superEncoders`

		struct _YAMLEncodingKey: CodingKey { // swiftlint:disable:this type_name

Support SE-0166 Swift Archival & Serialization #43

Support SE-0166 Swift Archival & Serialization #43

Conversation

norio-nomura commented May 7, 2017 • edited Loading

jpsim commented May 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

norio-nomura commented Oct 25, 2017

jpsim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpsim commented Oct 25, 2017

norio-nomura commented Oct 26, 2017

norio-nomura commented Oct 30, 2017

jpsim commented Oct 30, 2017

norio-nomura commented Oct 30, 2017

norio-nomura commented May 7, 2017 •

edited

Loading