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

[JSON Serialization]Support Default Value #2

Merged
merged 6 commits into from
Sep 27, 2019
Merged

Conversation

seanliu1
Copy link
Owner

@seanliu1 seanliu1 commented May 9, 2019

apple#861

See discussion in the ticket, this PR adds a new method in Visitor Protocol.

  func traverse<V: SwiftProtobuf.Visitor>(visitor: inout V) throws {
    let shouldIncludeDefault = visitor.shouldIncludeDefault()
    if !self.text.isEmpty || shouldIncludeDefault {
      try visitor.visitSingularStringField(value: self.text, fieldNumber: 1)
    }
    if self.action != .searchCancelEventActionInvalid || shouldIncludeDefault {
      try visitor.visitSingularEnumField(value: self.action, fieldNumber: 2)
    }
    if self.resultCount != 0  || shouldIncludeDefault {
      try visitor.visitSingularUInt32Field(value: self.resultCount, fieldNumber: 3)
    }
    try unknownFields.traverse(visitor: &visitor)
  }

@@ -198,7 +198,7 @@ class MessageFieldGenerator: FieldGeneratorBase, FieldGenerator {

let conditional: String
if isRepeated { // Also covers maps
conditional = "!\(varName).isEmpty"
conditional = "!\(varName).isEmpty || visitor.shouldIncludeDefault()"
} else if hasFieldPresence {
conditional = "let v = \(storedProperty)"
Copy link
Owner Author

@seanliu1 seanliu1 May 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not so sure whether we need to handle this. Looks like in proto3, only submessage uses it. Most of fields in proto2 using it. If we also want to get nil in JsonFormat, we can add another method call visitNull(fieldNumber: Int) throws into Visitor Protocol. So on the codegen side, it will become


if let v = _storage._message {
    try visitor.visitSingularMessageField(value: v, fieldNumber: 1)
} else if visitor.shouldIncludeDefault {
   try visitor.visitingNull(fieldNumber: 1)
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't looked at the other languages, do they really generate an empty list in json, that seems odd?

Copy link
Owner Author

@seanliu1 seanliu1 May 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

verified on python, it will return empty array

@seanliu1 seanliu1 changed the title add new func in visitor protocol [JSON Serialization]Support Default Value May 9, 2019
@seanliu1 seanliu1 merged commit 86d16d0 into master Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants