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

Casing of "Id" changed to "ID" #817

Closed
seanadkinson opened this issue Dec 12, 2018 · 4 comments
Closed

Casing of "Id" changed to "ID" #817

seanadkinson opened this issue Dec 12, 2018 · 4 comments

Comments

@seanadkinson
Copy link

Hi there,

Using protoc-gen-swift 1.2.0 on Mac, I'm compiling the following Example.proto:

syntax = "proto3";

message Example {
    string requestedOfUserId = 1;
}

And I get the following output:

// DO NOT EDIT.
//
// Generated by the Swift generator plugin for the protocol buffer compiler.
// Source: Example.proto
//
// For information on using the generated types, please see the documenation:
//   https://github.com/apple/swift-protobuf/

import Foundation
import SwiftProtobuf

// If the compiler emits an error on this type, it is because this file
// was generated by a version of the `protoc` Swift plug-in that is
// incompatible with the version of SwiftProtobuf to which you are linking.
// Please ensure that your are building against the same version of the API
// that was used to generate this file.
fileprivate struct _GeneratedWithProtocGenSwiftVersion: SwiftProtobuf.ProtobufAPIVersionCheck {
  struct _2: SwiftProtobuf.ProtobufAPIVersion_2 {}
  typealias Version = _2
}

public struct Example {
  // SwiftProtobuf.Message conformance is added in an extension below. See the
  // `Message` and `Message+*Additions` files in the SwiftProtobuf library for
  // methods supported on all messages.

  public var requestedOfUserID: String = String()

  public var unknownFields = SwiftProtobuf.UnknownStorage()

  public init() {}
}

// MARK: - Code below here is support for the SwiftProtobuf runtime.

extension Example: SwiftProtobuf.Message, SwiftProtobuf._MessageImplementationBase, SwiftProtobuf._ProtoNameProviding {
  public static let protoMessageName: String = "Example"
  public static let _protobuf_nameMap: SwiftProtobuf._NameMap = [
    1: .same(proto: "requestedOfUserId"),
  ]

  public mutating func decodeMessage<D: SwiftProtobuf.Decoder>(decoder: inout D) throws {
    while let fieldNumber = try decoder.nextFieldNumber() {
      switch fieldNumber {
      case 1: try decoder.decodeSingularStringField(value: &self.requestedOfUserID)
      default: break
      }
    }
  }

  public func traverse<V: SwiftProtobuf.Visitor>(visitor: inout V) throws {
    if !self.requestedOfUserID.isEmpty {
      try visitor.visitSingularStringField(value: self.requestedOfUserID, fieldNumber: 1)
    }
    try unknownFields.traverse(visitor: &visitor)
  }

  public static func ==(lhs: Example, rhs: Example) -> Bool {
    if lhs.requestedOfUserID != rhs.requestedOfUserID {return false}
    if lhs.unknownFields != rhs.unknownFields {return false}
    return true
  }
}

Note that Example.requestedOfUserID has a capital "D" at the end.

Is this expected behavior? I'd prefer to keep my specified capitalization, and couldn't find an option to do that.

Thanks!

@thomasvl
Copy link
Collaborator

Protocol buffers provides a style guide (for better or worse). The Swift support (like some other languages) tries to convert the enum and field names to follow the language conventions. So in generating it attempts to find word breaks (with underscores or via CamelCase) and then transform them. Some "words" (URL, HTTP, HTTPS, ID) we attempt to follow the Apple naming guidelines and always make them all uppercase, again so the names will match what developers are used to with Foundation/etc.

There isn't an option to control this.

@JonasVautherin
Copy link

I understand that you may want to follow Apple naming guidelines, but I wanted to give my opinion on this one 😇.

From what I could see:

  • swift-protobuf has 4 exceptions here (HTTP, HTTPS, ID, URL) that require special handling and corresponding unit tests.
  • Other initialisms (TCP, UDP, FTP, IP, ...) probably don't have the same treatment (at least not from what I saw in the code), because that's not practical to maintain.
  • This is confusing to some developers ("why do they have exceptions?"), and on the other side, I doubt anybody would misunderstand "Http" just because of the capitalization.

A style convention that is not practical, not consistent, adds code and unit tests (and therefore more opportunities for bugs) just because of an arbitrary style decision at some point is not a good convention to me.

Note that I could deal with it in my code (and unit tests), so I really just wanted to share my opinion here. swift-protobuf is working super well for me otherwise, thanks for the good job!

@tbkka
Copy link
Contributor

tbkka commented Mar 21, 2019

You may be right. Unfortunately, given that a lot of people are now using this, changing it seems impractical (it would potentially break a lot of code).

Maybe if/when we do a version 2, we'll consider changing this behavior at that time.

@tbkka tbkka closed this as completed Mar 21, 2019
@c0diq
Copy link

c0diq commented May 2, 2019

I have encountered the issue. The problem is that the name map when using the .standard enum value, will use lowerCamelCase instead and expose a different string than the property. In the example, it would be requestedOfUserId. This create a problem because we have a custom protobuf implementation that looks at this string to determine a default value to use for JSON serialization (which is not available yet #861). We use Sourcery with a stencil to generate this default values that parses the swift output of the protobuf code generation which is exposing ID instead of Id. The result is that there's a mismatch. I guess we could modify our stencil to convert ID to Id instead ...

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

No branches or pull requests

5 participants