Fix KT-18706 in CodeWriter.generateImports

[mitasov-ra]

• [x] docs/changelog.md has been updated if applicable.
• [x] CLA signed.

Because of KT-18706 bug all aliases escaped with backticks are not resolved by the Kotlin compiler.

So I've added a new function to Utils - String.escapeAsAlias, which converts aliases to Java identifiers, so that backticks are no longer needed.

I tried to make a more "global" fix by adding escapeAsAlias to the Import constructor, but since Import is a data class the "easy way" is not possible. Import should be converted to class to do this.

So I've decided just to fix the generateImports function, so that aliases, generated by kotlinpoet itself, would work. This small fix helps a lot with code generation tools.

[JakeWharton]

Can you write an integration test? In general we prefer to test against the public API and generated code so that we can ensure we cover the cases that consumers see. The unit tests are sufficient for coverage of the new function's behavior, but we need to see it wired up in practice at least once.

[mitasov-ra]

@JakeWharton Sure, no problem. Can you please navigate me a bit, where are exactly other integration tests in this project?

[JakeWharton]

If your change doesn't affect imports you can write tests for the TypeAliasSpec in here: https://github.com/square/kotlinpoet/blob/d2090c3bb2f9935cbbef3d2bcc1b092b15558bc1/kotlinpoet/src/commonTest/kotlin/com/squareup/kotlinpoet/TypeAliasSpecTest.kt#L34

If it does affect imports, add a test to FileSpecTest: https://github.com/square/kotlinpoet/blob/d2090c3bb2f9935cbbef3d2bcc1b092b15558bc1/kotlinpoet/src/commonTest/kotlin/com/squareup/kotlinpoet/FileSpecTest.kt#L912-L937

[mitasov-ra]

@JakeWharton I think you've confused import aliases with typealias

This PR only affects import aliases, and only the code that generates them in cases where there's more than one type with the same simple name is used in CodeBlock

I've just added another test specifically for that case, but I've put it in separate file. Is that ok?

[JakeWharton]

Ah, all the more reason it needed an integration test. There's no indication what actual output the change it affecting without close inspection.

I would expect this test to change: https://github.com/square/kotlinpoet/blob/d2090c3bb2f9935cbbef3d2bcc1b092b15558bc1/kotlinpoet/src/commonTest/kotlin/com/squareup/kotlinpoet/FileSpecTest.kt#L425-L447. Since import emissions behavior is owned by FileSpec I would prefer any tests for it to go under FileSpecTest, especially since we already have import alias test cases there and below in that link.

[mitasov-ra]

I would expect this test to change

At first I tried to fix this case too, but as I said earlier, it became too complicated without rewriting Import from data class to class.

So I decided to fix just the internal code inside CodeWriter (this make sense since this is the only Import.alias related code, which cannot be fixed on consumer side)

I would prefer any tests for it to go under FileSpecTest

Done.

[Egorand]

@mitasov-ra one more thing: could you please update the "Unreleased" section of the changelog? Thanks!

[mitasov-ra]

@Egorand All done! Please rerun checks

[Egorand]

All looks good to me, thanks!

@JakeWharton please merge if you're happy with the changes.

[Egorand]

@mitasov-ra looks like there are conflicts blocking the merge (sorry for the delay in merging your PR), and it doesn't seem like I can resolve them myself - can you please rebase? I'm in the process of releasing 1.18.0, and will release 1.18.1 with your change once the PR is merged. Thanks!

[mitasov-ra]

@Egorand Done

[Egorand]

Merged, thank you!