swift: surround identifiers with backticks to avoid reserved word conflicts

[eugene-babichenko]

Sometimes identifiers from a UDL file may conflict with Swift reserved words. Swift allows to resolve such conflicts by surrounding an identifier with backticks (`). For example:

var private: Bool

is not a valid expression, but

var `private`: Bool

is.

This pull request adds backticks automatically when required. Sometimes they may be redundant, but this does not trigger a compilation error and is a purely stylistic issue, so we rely on swiftformat to remove them when they are unnecessary.

The list of keywords was taken from Swift documentation. I skipped keywords that I think are not valid UDL identifiers.

[mhammond]

This sounds like a reasonable idea, thanks! Is there a good reason to not just quote every name instead of just reserved words?

[eugene-babichenko]

Is there a good reason to not just quote every name instead of just reserved words?

Honestly, there's none. And doing it that way the formatter would remove them anyway if they are not needed. I can make a change to quote everything and rely on the formatter.

P.S. if you think of it a bit, such solution is more future-proof :)

[jhugman]

I don't love this patch: I think I'd rather error out instead of allow using reserved words.

However, if you were unfortunate enough to design an API for one language, (e.g. you use val in Swift), then decide to adopt Kotlin, where val is a reserved word, then that would mean some unscheduled API evolution for your Swift clients.

In that case, there'd be multiple paths:

• detect the error, and error out. sorry, :( you have to eat the cost of evolving the API in Swift.
• backtick the val in Kotlin, as in this patch
• backtick everything, and hope the formatter (if available) cleans up the identifiers that we didn't need backticks for.
• for languages that don't support backticks, we error out anyway?

Is there a good reason to not just quote every name instead of just reserved words?

If we are going to backtick, I think I'd rather not rely on the formatter to clean up the code; why not produce nice code to start with?

In that case, in order to make this PR landable it should have corresponding checks and reserved word lists for Kotlin and Python (do we still support Ruby?).

[mhammond]

if you were unfortunate enough to design an API for one language, (e.g. you use val in Swift), then decide to adopt Kotlin

I'm not quite sure I agree with that TBH. In a world where binding generators can be in different crates (eg, #1150), I could imagine a world where "personA" develops a nice rust package with Uniffi support, "personB" develops a separate binding generator for some obscure language, then "person C" comes alone and wants to use PersonA's library with the language binding by PersonB - there's nothing reasonable we can ask PersonC to do if PersonA happened to use an identifier that is a reserved word in PersonB's binding.

If we are going to backtick, I think I'd rather not rely on the formatter to clean up the code; why not produce nice code to start with?

I think that's a reflection on my suggestion that instead of hard-coding all reserved words for a language (which may be fragile across language versions), I suggested backticking everything, then the OP noted that the formatter would clean this up. IOW, have the language formatter be the only thing that needs to know the exact set of reserved words.

In that case, in order to make this PR landable it should have corresponding checks and reserved word lists for Kotlin

It already does though, right? [edit: oops, this PR has swift, not kotlin! I'm not sure why it's a requirement to "fix" all languages or fix none of them though? Can't we make this a non-problem for swift and leave the making it a non-problem for other bindings for others in the future?]

[bendk]

I think different bindings are going to want different solutions. For Swift, Kotlin, and other languages that support some quoting method, we should try to do that. For Python, Ruby and other languages that don't support quoting, we could transform the name (for -> arg_for or something like that). That's not great, but I think it's better than failing. Given that we need to put some thought into how each binding wants to handle this issue, I'm happy to accept a change that only targets Kotlin, then work on the rest later.

What's the worst downside to producing uglier bindings code? This is a question I run into a lot. My instinct is to make everything look nice, but then when I stop to think about it I don't have a strong justification. Pretty code is good when you're trying to read the generated code to debug a bindings issue. But for that use case we can make sure the formatter is run beforehand.

[mhammond]

Thanks Ben - that's my take on the formatter too - instinct is to make it pretty, but I can't justify that when I think more about it, especially now that good formatters exist - and these formatters tend to enforce a very explicit code-style, so trying to make sure we generate pretty code that conforms to some code style is really a bridge too far!

@eugene-babichenko do you intend updating this PR?

[eugene-babichenko]

do you intend updating this PR?

So after all the consensus is to generate uglier code, right?

[mhammond]

So after all the consensus is to generate uglier code, right?

Yep, thanks for your patience!

[thunderbiscuit]

We ran into problems with the reserved Swift keyword internal for bdk-swift. I think this would have prevented the issue, and I just wanted to give a thumbs up for the feature to be added in the future!

[notmandatory]

We ran into problems with the reserved Swift keyword internal for bdk-swift. I think this would have prevented the issue, and I just wanted to give a thumbs up for the feature to be added in the future!

For a little more context in our bdk-ffi project we're using uniffi-rs 0.19.3 which should already include #1237. But it looks like that PR doesn't work for Swift, and I'm not sure if it handles enum variants with a reserved word name, we had an variant named "internal" which is not allowed in Swift (but is OK in rust, python, kotlin).

[notmandatory]

I think this one can be closed now that #1350 is merged. Thanks @eugene-babichenko for your research here, it was a big help.