Fix KT-18706 in CodeWriter.generateImports

[mitasov-ra]

• docs/changelog.md has been updated if applicable.
• 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:

kotlinpoet/kotlinpoet/src/commonTest/kotlin/com/squareup/kotlinpoet/TypeAliasSpecTest.kt

Line 34 in d2090c3

|public typealias Word = kotlin.String

If it does affect imports, add a test to FileSpecTest:

kotlinpoet/kotlinpoet/src/commonTest/kotlin/com/squareup/kotlinpoet/FileSpecTest.kt

Lines 912 to 937 in d2090c3

	@Test fun simpleTypeAliases() {
	val source = FileSpec.builder("com.squareup.tacos", "Taco")
	.addTypeAlias(TypeAliasSpec.builder("Int8", Byte::class).build())
	.addTypeAlias(
	TypeAliasSpec.builder(
	"FileTable",
	Map::class.parameterizedBy(String::class, Int::class),
	).build(),
	)
	.build()
	assertThat(source.toString()).isEqualTo(
	"""
	|package com.squareup.tacos
	|
	|import kotlin.Byte
	|import kotlin.Int
	|import kotlin.String
	|import kotlin.collections.Map
	|
	|public typealias Int8 = Byte
	|
	|public typealias FileTable = Map<String, Int>
	|
	""".trimMargin(),
	)
	}

[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:

kotlinpoet/kotlinpoet/src/commonTest/kotlin/com/squareup/kotlinpoet/FileSpecTest.kt

Lines 425 to 447 in d2090c3

	@Test fun escapeSpacesInAliasedImports() {
	val tacoFactory = ClassName("com.squareup.taco factory", "TacoFactory")
	val file = FileSpec.builder("com.example", "TacoFactoryDemo")
	.addAliasedImport(tacoFactory, "La Taqueria")
	.addFunction(
	FunSpec.builder("main")
	.addStatement("println(%T.produceTacos())", tacoFactory)
	.build(),
	)
	.build()
	assertThat(file.toString()).isEqualTo(
	"""
	|package com.example
	|
	|import com.squareup.`taco factory`.TacoFactory as `La Taqueria`
	|
	|public fun main() {
	| println(`La Taqueria`.produceTacos())
	|}
	|
	""".trimMargin(),
	)
	}

. 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]

Nice!

[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

[JakeWharton]

Can we change this to be a bit more user friendly so someone reading this would understand what the impact on their code is.

[mitasov-ra]

I've updated changelog, if new description is still seems off to you, please provide your version

[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!