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

The get / set index access operator overloads are incorrectly escaped #1869

Open
daniel-rusu opened this issue Mar 20, 2024 · 2 comments
Open
Labels

Comments

@daniel-rusu
Copy link

daniel-rusu commented Mar 20, 2024

Describe the bug
Using KotlinPoet 1.16.0 to generate the index-access operator overloads for get / set escapes the function name producing:

public operator fun `get`(index: Int): Int = values[index]

instead of:

public operator fun get(index: Int): Int = values[index]

To Reproduce
This code generates a half-baked collection to reproduce the issue:

val file = FileSpec.builder("", "MyCollectionExample")
        .addType(
            TypeSpec.classBuilder(ClassName("", "MyCollectionExample"))
                .primaryConstructor(
                    FunSpec.constructorBuilder()
                        .addParameter("values", typeNameOf<IntArray>())
                        .build()
                ).addProperty(
                    PropertySpec.builder("values", typeNameOf<IntArray>(), KModifier.PRIVATE)
                        .initializer("values")
                        .build()
                )
                .addFunction(
                    FunSpec.builder("get")
                        .addModifiers(KModifier.OPERATOR)
                        .returns(Int::class)
                        .addParameter("index", Int::class)
                        .addStatement("return values[index]")
                        .build()
                )
                .build()
        )
        .build()

    file.writeTo(System.out)

produces:

public class MyCollectionExample(
  private val values: IntArray,
) {
  public operator fun `get`(index: Int): Int = values[index]
}

Notice the function name.

Expected behavior
I would expect the function name not to be escaped when overloading the index access operator.

Additional context
This was probably introduced by:
#994

Skimming through the code, it's interesting that MemberName.emit tries to account for this scenario by only escaping the name when the operator is null. I wonder if we can remove the get / set soft-keywords from Util.KEYWORDS or if that would introduce any other side-effects.

@daniel-rusu
Copy link
Author

Looking at the KOperator enum, it looks like it doesn't have any representation for the index access operator forcing it to be null in MemberName thereby bypassing the check in emit and incorrectly escaping the member name

@Egorand
Copy link
Collaborator

Egorand commented Mar 21, 2024

Thanks! We can probably check if the function name fully matches one of the names in KOperator and avoid escaping it - if you're interested in contributing a fix we'd appreciate it. I think it's fairly low priority, as the presence of escaping backticks has no effect on the function's binary signature, it can still be called either via its name, or via the [] syntax. So the way we generate it is visually annoying, but practically correct.

Looking at the KOperator enum, it looks like it doesn't have any representation for the index access operator forcing it to be null in MemberName thereby bypassing the check in emit and incorrectly escaping the member name

I believe this is because for all the operators included in KOperator, the arguments are placed on either side of the operator, so it's easy to replace the function name with the operator symbol, whereas get and set wrap their args, making the replacement logic more complex: [index]. Not sure what the right fix for this would be and what the syntax would look like, and also not clear how important it is to support these operators.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants