Skip to content

Commit a3d5c02

Browse files
tbkkatbkka
authored
Merge pull request apple#1051 from tbkka/tbkka/JSON-oneof-nullvalue-conformance
Fix JSON coding/decoding of NullValue WKT to resolve conformance test failures.
2 parents 42422f3 + 2d03e5e commit a3d5c02

File tree

13 files changed

+381
-6
lines changed

13 files changed

+381
-6
lines changed

‎Makefile‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ TEST_PROTOS= \
123123
Protos/unittest_swift_extension4.proto \
124124
Protos/unittest_swift_fieldorder.proto \
125125
Protos/unittest_swift_groups.proto \
126+
Protos/unittest_swift_json.proto \
126127
Protos/unittest_swift_naming.proto \
127128
Protos/unittest_swift_naming_no_prefix.proto \
128129
Protos/unittest_swift_naming_number_prefix.proto \

‎Protos/unittest_swift_json.proto‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Protos/unittest_swift_json.proto
2+
// This source file is part of the Swift.org open source project
3+
//
4+
// Copyright (c) 2020 Apple Inc. and the Swift project authors
5+
// Licensed under Apache License v2.0 with Runtime Library Exception
6+
//
7+
// See http://swift.org/LICENSE.txt for license information
8+
// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
9+
10+
syntax = "proto3";
11+
12+
package protobuf_unittest;
13+
14+
import "google/protobuf/struct.proto";
15+
16+
message SwiftJSONTest {
17+
// This case was omitted from test_messages_proto3.proto
18+
repeated google.protobuf.NullValue repeated_null_value = 318;
19+
}

‎Reference/unittest_swift_json.pb.swift‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
// DO NOT EDIT.
2+
// swift-format-ignore-file
3+
//
4+
// Generated by the Swift generator plugin for the protocol buffer compiler.
5+
// Source: unittest_swift_json.proto
6+
//
7+
// For information on using the generated types, please see the documentation:
8+
// https://github.com/apple/swift-protobuf/
9+
10+
// Protos/unittest_swift_json.proto
11+
// This source file is part of the Swift.org open source project
12+
//
13+
// Copyright (c) 2020 Apple Inc. and the Swift project authors
14+
// Licensed under Apache License v2.0 with Runtime Library Exception
15+
//
16+
// See http://swift.org/LICENSE.txt for license information
17+
// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
18+
19+
import Foundation
20+
import SwiftProtobuf
21+
22+
// If the compiler emits an error on this type, it is because this file
23+
// was generated by a version of the `protoc` Swift plug-in that is
24+
// incompatible with the version of SwiftProtobuf to which you are linking.
25+
// Please ensure that you are building against the same version of the API
26+
// that was used to generate this file.
27+
fileprivate struct _GeneratedWithProtocGenSwiftVersion: SwiftProtobuf.ProtobufAPIVersionCheck {
28+
struct _2: SwiftProtobuf.ProtobufAPIVersion_2 {}
29+
typealias Version = _2
30+
}
31+
32+
struct ProtobufUnittest_SwiftJSONTest {
33+
// SwiftProtobuf.Message conformance is added in an extension below. See the
34+
// `Message` and `Message+*Additions` files in the SwiftProtobuf library for
35+
// methods supported on all messages.
36+
37+
/// This case was omitted from test_messages_proto3.proto
38+
var repeatedNullValue: [SwiftProtobuf.Google_Protobuf_NullValue] = []
39+
40+
var unknownFields = SwiftProtobuf.UnknownStorage()
41+
42+
init() {}
43+
}
44+
45+
// MARK: - Code below here is support for the SwiftProtobuf runtime.
46+
47+
fileprivate let _protobuf_package = "protobuf_unittest"
48+
49+
extension ProtobufUnittest_SwiftJSONTest: SwiftProtobuf.Message, SwiftProtobuf._MessageImplementationBase, SwiftProtobuf._ProtoNameProviding {
50+
static let protoMessageName: String = _protobuf_package + ".SwiftJSONTest"
51+
static let _protobuf_nameMap: SwiftProtobuf._NameMap = [
52+
318: .standard(proto: "repeated_null_value"),
53+
]
54+
55+
mutating func decodeMessage<D: SwiftProtobuf.Decoder>(decoder: inout D) throws {
56+
while let fieldNumber = try decoder.nextFieldNumber() {
57+
// The use of inline closures is to circumvent an issue where the compiler
58+
// allocates stack space for every case branch when no optimizations are
59+
// enabled. https://github.com/apple/swift-protobuf/issues/1034
60+
switch fieldNumber {
61+
case 318: try { try decoder.decodeRepeatedEnumField(value: &self.repeatedNullValue) }()
62+
default: break
63+
}
64+
}
65+
}
66+
67+
func traverse<V: SwiftProtobuf.Visitor>(visitor: inout V) throws {
68+
if !self.repeatedNullValue.isEmpty {
69+
try visitor.visitPackedEnumField(value: self.repeatedNullValue, fieldNumber: 318)
70+
}
71+
try unknownFields.traverse(visitor: &visitor)
72+
}
73+
74+
static func ==(lhs: ProtobufUnittest_SwiftJSONTest, rhs: ProtobufUnittest_SwiftJSONTest) -> Bool {
75+
if lhs.repeatedNullValue != rhs.repeatedNullValue {return false}
76+
if lhs.unknownFields != rhs.unknownFields {return false}
77+
return true
78+
}
79+
}

‎Sources/Conformance/failure_list_swift.txt‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +0,0 @@
1-
Recommended.Proto3.JsonInput.NullValueInOtherOneofNewFormat.Validator
2-
Recommended.Proto3.JsonInput.NullValueInOtherOneofOldFormat.Validator

‎Sources/SwiftProtobuf/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ add_library(SwiftProtobuf
3131
Google_Protobuf_Duration+Extensions.swift
3232
Google_Protobuf_FieldMask+Extensions.swift
3333
Google_Protobuf_ListValue+Extensions.swift
34+
Google_Protobuf_NullValue+Extensions.swift
3435
Google_Protobuf_Struct+Extensions.swift
3536
Google_Protobuf_Timestamp+Extensions.swift
3637
Google_Protobuf_Value+Extensions.swift

‎Sources/SwiftProtobuf/Google_Protobuf_NullValue+Extensions.swift‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// Sources/SwiftProtobuf/Google_Protobuf_NullValue+Extensions.swift - NullValue extensions
2+
//
3+
// Copyright (c) 2014 - 2020 Apple Inc. and the project authors
4+
// Licensed under Apache License v2.0 with Runtime Library Exception
5+
//
6+
// See LICENSE.txt for license information:
7+
// https://github.com/apple/swift-protobuf/blob/master/LICENSE.txt
8+
//
9+
// -----------------------------------------------------------------------------
10+
///
11+
/// NullValue is a well-known message type that can be used to parse or encode
12+
/// JSON Null values.
13+
///
14+
// -----------------------------------------------------------------------------
15+
16+
extension Google_Protobuf_NullValue: _CustomJSONCodable {
17+
internal func encodedJSONString(options: JSONEncodingOptions) throws -> String {
18+
return "null"
19+
}
20+
internal mutating func decodeJSON(from decoder: inout JSONDecoder) throws {
21+
if decoder.scanner.skipOptionalNull() {
22+
return
23+
}
24+
}
25+
static func decodedFromJSONNull() -> Google_Protobuf_NullValue? {
26+
return .nullValue
27+
}
28+
}

‎Sources/SwiftProtobuf/JSONDecoder.swift‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,10 @@ internal struct JSONDecoder: Decoder {
469469
mutating func decodeSingularEnumField<E: Enum>(value: inout E?) throws
470470
where E.RawValue == Int {
471471
if scanner.skipOptionalNull() {
472+
if let customDecodable = E.self as? _CustomJSONCodable.Type {
473+
value = try customDecodable.decodedFromJSONNull() as? E
474+
return
475+
}
472476
value = nil
473477
return
474478
}
@@ -478,6 +482,10 @@ internal struct JSONDecoder: Decoder {
478482
mutating func decodeSingularEnumField<E: Enum>(value: inout E) throws
479483
where E.RawValue == Int {
480484
if scanner.skipOptionalNull() {
485+
if let customDecodable = E.self as? _CustomJSONCodable.Type {
486+
value = try customDecodable.decodedFromJSONNull() as! E
487+
return
488+
}
481489
value = E()
482490
return
483491
}
@@ -493,9 +501,19 @@ internal struct JSONDecoder: Decoder {
493501
if scanner.skipOptionalArrayEnd() {
494502
return
495503
}
504+
let maybeCustomDecodable = E.self as? _CustomJSONCodable.Type
496505
while true {
497-
let e: E = try scanner.nextEnumValue()
498-
value.append(e)
506+
if scanner.skipOptionalNull() {
507+
if let customDecodable = maybeCustomDecodable {
508+
let e = try customDecodable.decodedFromJSONNull() as! E
509+
value.append(e)
510+
} else {
511+
throw JSONDecodingError.illegalNull
512+
}
513+
} else {
514+
let e: E = try scanner.nextEnumValue()
515+
value.append(e)
516+
}
499517
if scanner.skipOptionalArrayEnd() {
500518
return
501519
}

‎Sources/SwiftProtobuf/JSONEncodingVisitor.swift‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,10 @@ internal struct JSONEncodingVisitor: Visitor {
155155

156156
mutating func visitSingularEnumField<E: Enum>(value: E, fieldNumber: Int) throws {
157157
try startField(for: fieldNumber)
158-
if !options.alwaysPrintEnumsAsInts, let n = value.name {
158+
if let e = value as? _CustomJSONCodable {
159+
let json = try e.encodedJSONString(options: options)
160+
encoder.append(text: json)
161+
} else if !options.alwaysPrintEnumsAsInts, let n = value.name {
159162
encoder.appendQuoted(name: n)
160163
} else {
161164
encoder.putEnumInt(value: value.rawValue)
@@ -277,10 +280,14 @@ internal struct JSONEncodingVisitor: Visitor {
277280
}
278281

279282
mutating func visitRepeatedEnumField<E: Enum>(value: [E], fieldNumber: Int) throws {
283+
let options = self.options
280284
let alwaysPrintEnumsAsInts = options.alwaysPrintEnumsAsInts
281285
try _visitRepeated(value: value, fieldNumber: fieldNumber) {
282286
(encoder: inout JSONEncoder, v: E) throws in
283-
if !alwaysPrintEnumsAsInts, let n = v.name {
287+
if let e = v as? _CustomJSONCodable {
288+
let json = try e.encodedJSONString(options: options)
289+
encoder.append(text: json)
290+
} else if !alwaysPrintEnumsAsInts, let n = v.name {
284291
encoder.appendQuoted(name: n)
285292
} else {
286293
encoder.putEnumInt(value: v.rawValue)

‎SwiftProtobuf.xcodeproj/project.pbxproj‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,10 @@
208208
9CCD5F931E008203002D1940 /* Test_TextFormat_WKT_proto3.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9CCD5F921E008203002D1940 /* Test_TextFormat_WKT_proto3.swift */; };
209209
9CCD5F941E008203002D1940 /* Test_TextFormat_WKT_proto3.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9CCD5F921E008203002D1940 /* Test_TextFormat_WKT_proto3.swift */; };
210210
9CCD5F951E008203002D1940 /* Test_TextFormat_WKT_proto3.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9CCD5F921E008203002D1940 /* Test_TextFormat_WKT_proto3.swift */; };
211+
9CE4C56824EF34F200471257 /* Google_Protobuf_NullValue+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9CE4C56724EF34F200471257 /* Google_Protobuf_NullValue+Extensions.swift */; };
212+
9CE4C56924EF34F200471257 /* Google_Protobuf_NullValue+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9CE4C56724EF34F200471257 /* Google_Protobuf_NullValue+Extensions.swift */; };
213+
9CE4C56A24EF34F200471257 /* Google_Protobuf_NullValue+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9CE4C56724EF34F200471257 /* Google_Protobuf_NullValue+Extensions.swift */; };
214+
9CE4C56B24EF34F200471257 /* Google_Protobuf_NullValue+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9CE4C56724EF34F200471257 /* Google_Protobuf_NullValue+Extensions.swift */; };
211215
9CE9CF371E32C9F8004FBED6 /* JSONScanner.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9CE9CF361E32C9F8004FBED6 /* JSONScanner.swift */; };
212216
9CEB0D681DF5E934002D80F0 /* TextFormatEncodingVisitor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9CEB0D671DF5E934002D80F0 /* TextFormatEncodingVisitor.swift */; };
213217
9CEB0D6C1DF5F921002D80F0 /* TextFormatScanner.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9CEB0D6B1DF5F921002D80F0 /* TextFormatScanner.swift */; };
@@ -738,6 +742,7 @@
738742
9CC8CAAA1EC512A0008EF45F /* generated_swift_names_fields.pb.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = generated_swift_names_fields.pb.swift; sourceTree = "<group>"; };
739743
9CC8CAAB1EC512A0008EF45F /* generated_swift_names_messages.pb.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = generated_swift_names_messages.pb.swift; sourceTree = "<group>"; };
740744
9CCD5F921E008203002D1940 /* Test_TextFormat_WKT_proto3.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Test_TextFormat_WKT_proto3.swift; sourceTree = "<group>"; };
745+
9CE4C56724EF34F200471257 /* Google_Protobuf_NullValue+Extensions.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "Google_Protobuf_NullValue+Extensions.swift"; sourceTree = "<group>"; };
741746
9CE9CF361E32C9F8004FBED6 /* JSONScanner.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = JSONScanner.swift; sourceTree = "<group>"; };
742747
9CEB0D671DF5E934002D80F0 /* TextFormatEncodingVisitor.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TextFormatEncodingVisitor.swift; sourceTree = "<group>"; };
743748
9CEB0D6B1DF5F921002D80F0 /* TextFormatScanner.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TextFormatScanner.swift; sourceTree = "<group>"; };
@@ -1032,6 +1037,7 @@
10321037
__PBXFileRef_Sources/Protobuf/Google_Protobuf_Duration_Extensions.swift /* Google_Protobuf_Duration+Extensions.swift */,
10331038
__PBXFileRef_Sources/Protobuf/Google_Protobuf_FieldMask_Extensions.swift /* Google_Protobuf_FieldMask+Extensions.swift */,
10341039
AAB979F41E7066D8003DC2F4 /* Google_Protobuf_ListValue+Extensions.swift */,
1040+
9CE4C56724EF34F200471257 /* Google_Protobuf_NullValue+Extensions.swift */,
10351041
__PBXFileRef_Sources/Protobuf/Google_Protobuf_Struct.swift /* Google_Protobuf_Struct+Extensions.swift */,
10361042
__PBXFileRef_Sources/Protobuf/Google_Protobuf_Timestamp_Extensions.swift /* Google_Protobuf_Timestamp+Extensions.swift */,
10371043
AAE0F4E21E6F5CB9005240E0 /* Google_Protobuf_Value+Extensions.swift */,
@@ -1595,6 +1601,7 @@
15951601
9C667A921E4C203D008B974F /* JSONDecodingError.swift in Sources */,
15961602
F44F93691DAD7FA500BC5B85 /* Google_Protobuf_Wrappers+Extensions.swift in Sources */,
15971603
9C667A9A1E4C203D008B974F /* TextFormatDecodingError.swift in Sources */,
1604+
9CE4C56924EF34F200471257 /* Google_Protobuf_NullValue+Extensions.swift in Sources */,
15981605
9C75F8901DDD3108005CCFF2 /* ExtensibleMessage.swift in Sources */,
15991606
);
16001607
runOnlyForDeploymentPostprocessing = 0;
@@ -1684,6 +1691,7 @@
16841691
9C75F8801DDBE0DE005CCFF2 /* Visitor.swift in Sources */,
16851692
AA05BF4A1DAEB7E400619042 /* WireFormat.swift in Sources */,
16861693
AAEA52741DA80DEA003F318F /* wrappers.pb.swift in Sources */,
1694+
9CE4C56824EF34F200471257 /* Google_Protobuf_NullValue+Extensions.swift in Sources */,
16871695
AA28A4A61DA30E5900C866D9 /* ZigZag.swift in Sources */,
16881696
);
16891697
runOnlyForDeploymentPostprocessing = 0;
@@ -1892,6 +1900,7 @@
18921900
9C667A931E4C203D008B974F /* JSONDecodingError.swift in Sources */,
18931901
F44F93881DAEA76700BC5B85 /* ExtensionFields.swift in Sources */,
18941902
9C667A9B1E4C203D008B974F /* TextFormatDecodingError.swift in Sources */,
1903+
9CE4C56A24EF34F200471257 /* Google_Protobuf_NullValue+Extensions.swift in Sources */,
18951904
9C75F8911DDD3108005CCFF2 /* ExtensibleMessage.swift in Sources */,
18961905
);
18971906
runOnlyForDeploymentPostprocessing = 0;
@@ -2100,6 +2109,7 @@
21002109
9C667A941E4C203D008B974F /* JSONDecodingError.swift in Sources */,
21012110
ECBC5C511DF6ABC500F658E8 /* wrappers.pb.swift in Sources */,
21022111
9C667A9C1E4C203D008B974F /* TextFormatDecodingError.swift in Sources */,
2112+
9CE4C56B24EF34F200471257 /* Google_Protobuf_NullValue+Extensions.swift in Sources */,
21032113
ECBC5C4F1DF6ABC500F658E8 /* ZigZag.swift in Sources */,
21042114
);
21052115
runOnlyForDeploymentPostprocessing = 0;

‎Tests/LinuxMain.swift‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,6 +582,7 @@ extension Test_JSON {
582582
("testRepeatedInt32", testRepeatedInt32),
583583
("testRepeatedString", testRepeatedString),
584584
("testRepeatedNestedMessage", testRepeatedNestedMessage),
585+
("testRepeatedEnum", testRepeatedEnum),
585586
("testOneof", testOneof),
586587
("testEmptyMessage", testEmptyMessage)
587588
]
@@ -637,6 +638,11 @@ extension Test_JSON_Conformance {
637638
("testNullSupport_regularTypes", testNullSupport_regularTypes),
638639
("testNullSupport_wellKnownTypes", testNullSupport_wellKnownTypes),
639640
("testNullSupport_Value", testNullSupport_Value),
641+
("testNullSupport_optionalNullValue", testNullSupport_optionalNullValue),
642+
("testNullSupport_oneofNullValue", testNullSupport_oneofNullValue),
643+
("testNullSupport_oneofNullValue_alternate", testNullSupport_oneofNullValue_alternate),
644+
("testNullSupport_oneofNullValue_numeric", testNullSupport_oneofNullValue_numeric),
645+
("testNullSupport_repeatedNullValue", testNullSupport_repeatedNullValue),
640646
("testNullSupport_Repeated", testNullSupport_Repeated),
641647
("testNullSupport_RepeatedValue", testNullSupport_RepeatedValue),
642648
("testNullConformance", testNullConformance),

0 commit comments

Comments
 (0)