apple/swift-protobuf

Use of 'self' in enum case adds trailing underscore instead of using backticks

Opened this issue · 6 comments

Before opening a new issue, please double check the past issues (both open and
closed ones) to see if your problem has been discussed before and already
contains an answer/solution. Likewise, check the FAQ in the Documentation folder
for some common things -
https://github.com/apple/swift-protobuf/blob/main/Documentation/FAQ.md

Please be sure to include:

  • what OS you are developing on (Linux or macOS, including the version)

Cannot disclose

  • for macOS, what version of Xcode you are using (xcodebuild -version),
    for Linux, what version of Swift (swift --version)

Cannot disclose

  • what version of Swift is your code set to compile with (i.e. from project
    settings, etc.)

Cannot disclose

  • what branch/tag of SwiftProtobuf you are using (1.0.0, etc.)

1.19.0

  • if you are getting compile errors, please be sure include all errors/warnings,
    sometimes the error before the one you are stuck on is important.

No compile errors

  • lastly, if it all possible, provide a snippet of .proto and/or source
    that shows the problem.

Proto

enum Foo {
    self = 0;
}

Swift

public enum Foo: Enum {
  public typealias RawValue = Int
  case self_ // = 0
  case UNRECOGNIZED(Int)

  public init() {
    self = .self_
  }
}

Is there a reason that self when used in enums creates a trailing underscore instead of using backticks? I've never seen this kind of naming convention in the swift language and I believe it's more commonplace to use backticks when there are collisions in keywords (e.g `default` as another example.)

tbkka commented

Backticks only help to avoid collisions with keywords, they don't help other kinds of collisions. For example, if someone names a field hashValue or description, we need to avoid collisions with the standard properties with those names. We decided it was better to have a uniform and consistent way to avoid naming collisions, so we instead append trailing underscores, which works for all cases, not just keywords.

You can see more details about the possible collisions and how we handle them here:
https://github.com/apple/swift-protobuf/blob/main/Sources/SwiftProtobufPluginLibrary/NamingUtils.swift

If you want to experiment with alternate approaches, please take a close look at how we test this point:
The Makefile generates a large collection of Swift words by dissecting the SwiftProtobuf library source code, and then builds proto files using these words as field names, message names, and enum cases. If the resulting Swift code can compile without error, then we've successfully avoided any conflicts.

We decided it was better to have a uniform and consistent way to avoid naming collisions, so we instead append trailing underscores, which works for all cases, not just keywords.

I'd love to know why the decision was made to do this universally, rather than use backticks for conflicting keywords. Is it because of the amount of effort or maintainability cost needed to split the two? I'd be happy to take a look if folks don't feel strongly about the decision.

I think part of this might go back to the pre Swift 3 days, when exactly what could/couldn't be handled via backticks wasn't as well defined.

main is currently open for breaking changes, so if one were to revisit this, now would be a possible time; but I'll defer to the Apple folks on if they want to take this in for the 2.0 release.

tbkka commented

I'm willing to consider such a change for 2.0. I have slight misgivings about breaking existing code more than necessary, but this should affect very few people. And as Thomas pointed out, now that the Swift syntax rules have settled down, maintenance cost is less of a concern.

tbkka commented

Note: Our testing for this uses the Swift Protobuf source code itself as a source of "words" that we need to handle. Any Swift keywords that aren't used in Swift Protobuf (e.g., concurrency-related stuff) may need additional testing.

Cycling back to this, @tbkka's comment about keywords seems still somewhat relevant. Using a playground:

import Foundation

public enum Foo : RawRepresentable {
  public typealias RawValue = Int
  case `self` // = 0
  case UNRECOGNIZED(Int)

  public init() {
    self = .self
  }

  public init?(rawValue: Int) {
    switch rawValue {
    case 0: self = .self
    default: self = .UNRECOGNIZED(rawValue)
    }
  }

  public var rawValue: Int {
    switch self {
    case .self: return 0
    case .UNRECOGNIZED(let i): return i
    }
  }
}

let a = Foo.self
let b = Foo.`self`
let c: Foo = .self
let d = Foo()
let e = Foo(rawValue: 0)
let f = Foo(rawValue: 1)

So, yes, we could skip the underscores, but notice this snippet:

let a = Foo.self
let b = Foo.`self`
let c: Foo = .self

a is a type reference, but b and c are the values. Exactly how one spells things in the sources decides if backticks are needed or not. So not using the underscore in this case seems like it might be setting folks up to make mistakes.