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

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

Open
cprovatas opened this issue May 23, 2022 · 6 comments

Comments

@cprovatas
Copy link

cprovatas commented May 23, 2022

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
Copy link
Contributor

tbkka commented May 23, 2022

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.

@cprovatas
Copy link
Author

cprovatas commented May 23, 2022

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.

@thomasvl
Copy link
Collaborator

thomasvl commented Jun 7, 2022

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
Copy link
Contributor

tbkka commented Jun 7, 2022

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
Copy link
Contributor

tbkka commented Jun 7, 2022

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.

@thomasvl
Copy link
Collaborator

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.

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

3 participants