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

Allow to opt out of escaping for soft & modifier keywords #1229

Closed
efemoney opened this issue Mar 25, 2022 · 5 comments
Closed

Allow to opt out of escaping for soft & modifier keywords #1229

efemoney opened this issue Mar 25, 2022 · 5 comments

Comments

@efemoney
Copy link

efemoney commented Mar 25, 2022

Soft keywords (eg. get) only act as keywords in very specific cases.

There should be a way for codegen authors to explicitly opt out of escaping soft keywords in specific cases.

One example is generating ktor client code where generated code reads better without the backticks

client.get { ... }
// VS
client.`get` { ... }
@efemoney efemoney changed the title Allow to opt out of escaping for soft keywords Allow to opt out of escaping for soft & modifier keywords Mar 25, 2022
@Egorand
Copy link
Collaborator

Egorand commented Mar 28, 2022

Our general philosophy for this library is to prefer sane defaults and smaller API surface over customizability. Exposing and maintaining extra API to customize the look and feel of generated code is not something we want to do. I'm closing this issue, however, if there's a stronger reason for having the ability to enable/disable backticks - please let us know.

@Egorand Egorand closed this as completed Mar 28, 2022
@Omico
Copy link
Contributor

Omico commented Jun 9, 2023

@Egorand Hello, I ran into an annoying problem with this. It makes Ktlint throw an error because it contains an internal with backticks in my package name. Maybe this is a good reason to let us work on this issue. Currently, I have to do secondary processing of code generated by kotlinpoet.

package me.omico.consensus.spotless2.`internal`
Step 'ktlint' found problem in 'src\main\kotlin\me\omico\consensus\spotless2\internal\ConsensusSpotless2ExtensionImpl.kt':
Error on line: 1, column: 9
rule: standard:package-name
Package name contains a disallowed character
java.lang.AssertionError: Error on line: 1, column: 9
rule: standard:package-name
Package name contains a disallowed character
private fun tidyUpGeneratedSourceFile(file: Path): Unit =
    file.readText()
        // Remove backticks for internal keyword
        .replace(".`internal`", ".internal")
        .let(file::writeText)

@Egorand
Copy link
Collaborator

Egorand commented Jun 9, 2023

@Omico this looks like a bug in Ktlint.

That said, I do think we're being overly cautious by escaping all keywords. Escaping of all keywords was introduced by #994, and it has a link to an issue we had with open, which acts as a keyword in some contexts and can be used as an identifier in other contexts - we weren't sure how to handle this properly. Let me know if you have thoughts!

@Omico
Copy link
Contributor

Omico commented Jun 9, 2023

@Omico this looks like a bug in Ktlint.

That said, I do think we're being overly cautious by escaping all keywords. Escaping of all keywords was introduced by #994, and it has a link to an issue we had with open, which acts as a keyword in some contexts and can be used as an identifier in other contexts - we weren't sure how to handle this properly. Let me know if you have thoughts!

Found something interesting, maybe we can follow this https://github.com/JetBrains/intellij-community/blob/master/plugins/kotlin/code-insight/inspections-shared/src/org/jetbrains/kotlin/idea/codeInsight/inspections/shared/RemoveRedundantBackticksInspection.kt

This solves part of the problem but not the problem I mentioned above with internal keyword inside the package name.

@Omico
Copy link
Contributor

Omico commented Jun 9, 2023

Hi, I just created this for testing https://github.com/Omico/kotlin-keywords-test. Seem open won't be an error now. I remember Kotlin got an update in 1.5.x or 1.6.x that removed the backticks for more keywords, but I cannot find the specific version now.

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