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.

Sign up for GitHub

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

Jump to bottom

Correctly disambiguate conflicting type names between the module itself and external modules. #111

Merged
merged 1 commit into from
Jan 14, 2024
Merged

Correctly disambiguate conflicting type names between the module itself and external modules. #111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 10 additions & 20 deletions src/main/java/io/outfoxx/swiftpoet/CodeWriter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ private val NO_MODULE = String()
internal class CodeWriter(
out: Appendable,
private val indent: String = DEFAULT_INDENT,
internal val importedTypes: Map<String, List<DeclaredTypeName>> = emptyMap(),
private val importedTypes: Map<String, List<DeclaredTypeName>> = emptyMap(),
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to be internal

private val sameModuleReferencedTypes: Set<String> = emptySet(),
private val importedModules: Set<String> = emptySet()
) : Closeable {

Expand All @@ -42,6 +43,7 @@ internal class CodeWriter(
private val typeSpecStack = mutableListOf<AnyTypeSpec>()
private val importableTypes = mutableMapOf<String, List<DeclaredTypeName>>()
private val referencedTypes = mutableMapOf<String, DeclaredTypeName>()
private val noModuleReferencedTypes = mutableSetOf<String>()
private var trailingNewline = false

/**
Expand Down Expand Up @@ -272,6 +274,7 @@ internal class CodeWriter(

private fun referenceTypeName(typeName: DeclaredTypeName) {
if (typeName.moduleName.isEmpty()) {
noModuleReferencedTypes.add(typeName.simpleName)
return
}
referencedTypes[typeName.canonicalName] = typeName
Expand Down Expand Up @@ -376,7 +379,9 @@ internal class CodeWriter(
*/
private fun resolveImport(typeName: DeclaredTypeName): String {
val topLevelTypeName = typeName.topLevelTypeName()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a val topLevelSimpleName = topLevelTypeName.simpleName since you need it a few times?

return if (importedTypes[typeName.topLevelTypeName().simpleName]?.count() == 1 && importedTypes.values.flatMap { it }.any { it == topLevelTypeName }) {
return if (topLevelTypeName.moduleName.isNotEmpty() && sameModuleReferencedTypes.contains(topLevelTypeName.simpleName)) {
typeName.canonicalName
} else if (importedTypes[topLevelTypeName.simpleName]?.count() == 1 && importedTypes.values.flatten().any { it == topLevelTypeName }) {
typeName.simpleNames.joinToString(".")
} else {
typeName.canonicalName
Expand Down Expand Up @@ -447,26 +452,11 @@ internal class CodeWriter(
/**
* Returns the non-colliding importable types and module names for all referenced types.
*/
private fun generateImports(): Pair<Map<String, List<DeclaredTypeName>>, Set<String>> {
fun generateImports(): Pair<Map<String, List<DeclaredTypeName>>, Set<String>> {
return importableTypes to referencedTypes.values.map { it.moduleName }.toSet()
}

companion object {
Copy link
Collaborator Author

@dnkoutso dnkoutso Jan 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kdubb I moved away from this style as it is a bit hard to carry additional information we need between the first pass and the second pass.

It was not being super helpful anyway and I did it initially to mirror KotlinPoet, but we are not KotlinPoet. I also thought by doing this we were avoiding writing the file twice but that is not true either because we use NullAppendable.


/**
* Collect imports by executing [emitStep], and returns the non-colliding imported types
* and referenced modules.
*/
fun collectImports(
indent: String,
emitStep: (importsCollector: CodeWriter) -> Unit,
): Pair<Map<String, List<DeclaredTypeName>>, Set<String>> =
CodeWriter(NullAppendable, indent)
.use { importsCollector ->

emitStep(importsCollector)

importsCollector.generateImports()
}
fun generateSameModuleReferencedTypes(): Set<String> {
return noModuleReferencedTypes
}
}
33 changes: 22 additions & 11 deletions src/main/java/io/outfoxx/swiftpoet/FileSpec.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,22 @@ class FileSpec private constructor(
@Throws(IOException::class)
fun writeTo(out: Appendable) {

val (importedTypes, referencedModules) =
CodeWriter.collectImports(
indent = indent,
emitStep = { importsCollector -> emit(importsCollector) },
)

val codeWriter = CodeWriter(out, indent = indent, importedTypes = importedTypes)
emit(codeWriter, referencedModules = referencedModules)
val importsCollector = CodeWriter(NullAppendable, indent)
importsCollector.use {
emit(importsCollector)
}
val (importedTypes, referencedModules) = importsCollector.generateImports()
val sameModuleReferencedTypes = importsCollector.generateSameModuleReferencedTypes()

val codeWriter = CodeWriter(
out,
indent = indent,
importedTypes = importedTypes,
sameModuleReferencedTypes = sameModuleReferencedTypes
)
codeWriter.use {
emit(codeWriter, referencedModules = referencedModules)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is now wrapped in a .use so close gets called automatically.

}
}

/** Writes this to `directory` as UTF-8 using the standard directory structure. */
Expand Down Expand Up @@ -180,13 +188,16 @@ class FileSpec private constructor(

private val NON_IMPORTED_MODULES = setOf("Swift")

@JvmStatic fun get(moduleName: String, typeSpec: AnyTypeSpec): FileSpec {
@JvmStatic
fun get(moduleName: String, typeSpec: AnyTypeSpec): FileSpec {
return builder(moduleName, typeSpec.name).addType(typeSpec).build()
}

@JvmStatic fun builder(moduleName: String, fileName: String) = Builder(moduleName, fileName)
@JvmStatic
fun builder(moduleName: String, fileName: String) = Builder(moduleName, fileName)

@JvmStatic fun builder(fileName: String) = Builder("", fileName)
@JvmStatic
fun builder(fileName: String) = Builder("", fileName)
}
}

Expand Down
108 changes: 108 additions & 0 deletions src/test/java/io/outfoxx/swiftpoet/test/FileSpecTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import io.outfoxx.swiftpoet.STRING
import io.outfoxx.swiftpoet.TupleTypeName
import io.outfoxx.swiftpoet.TypeSpec
import io.outfoxx.swiftpoet.TypeVariableName
import io.outfoxx.swiftpoet.parameterizedBy
import io.outfoxx.swiftpoet.tag
import org.hamcrest.CoreMatchers.equalTo
import org.hamcrest.MatcherAssert.assertThat
Expand Down Expand Up @@ -282,6 +283,19 @@ class FileSpecTests {
@DisplayName("Generates all required imports with conflicts (alwaysQualify)")
fun testGeneratesAllRequiredImportsWithConflictsUsingAlwaysQualify() {
val type = TypeSpec.structBuilder("SomeType")
.addProperty(
PropertySpec.varBuilder(
"yet_another_module_order",
typeName("Swift.Array")
.parameterizedBy(typeName("yet_another_module.SortOrder"))
).build()
)
.addProperty(
PropertySpec.varBuilder(
"order",
typeName(".SortOrder")
).build()
)
.addProperty(
PropertySpec.varBuilder(
"foundation_order",
Expand Down Expand Up @@ -316,9 +330,12 @@ class FileSpecTests {
import Foundation
import some_module
import some_other_module
import yet_another_module

struct SomeType {

var yet_another_module_order: [yet_another_module.SortOrder]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test fails without my change. It outputs var yet_another_module_order: [SortOrder] which conflicts with the same module SortOrder that is added here.

var order: SortOrder
var foundation_order: Foundation.SortOrder
var some_module_order: some_module.SortOrder
var some_other_module_order: some_other_module.SortOrder
Expand All @@ -330,6 +347,88 @@ class FileSpecTests {
)
}

@Test
@DisplayName("Generates all required imports without conflicts")
fun testGeneratesAllRequiredImportsWithoutConflicts() {
val type = TypeSpec.structBuilder("SomeType")
.addProperty(
PropertySpec.varBuilder(
"yet_another_module_order",
typeName("Swift.Array")
.parameterizedBy(typeName("yet_another_module.SortOrder"))
).build()
)
.build()

val testFile = FileSpec.builder("Test", "Test")
.addType(type)
.build()

val out = StringWriter()
testFile.writeTo(out)

assertThat(
out.toString(),
equalTo(
"""
import yet_another_module

struct SomeType {

var yet_another_module_order: [SortOrder]

}

""".trimIndent()
)
)
}

@Test
@DisplayName("Generates all required imports with same module conflict")
fun testGeneratesAllRequiredImportsWithSameModuleConflict() {
val type =
TypeSpec.structBuilder("SomeType")
.addProperty(
PropertySpec.varBuilder(
"order",
typeName(".SortOrder")
).build()
)
.addProperty(
PropertySpec.varBuilder(
"yet_another_module_order",
typeName("Swift.Array")
.parameterizedBy(typeName("yet_another_module.SortOrder"))
).build()
)
.build()

val testFile = FileSpec.builder("Test", "Test")
.addType(type)
.build()

val out = StringWriter()
testFile.writeTo(out)

assertThat(
out.toString(),
equalTo(
"""
import yet_another_module

struct SomeType {

var order: SortOrder
var yet_another_module_order: [yet_another_module.SortOrder]

}

""".trimIndent()
)
)
}

@Test
@DisplayName("Generates all required imports with conflicts")
fun testGeneratesAllRequiredImportsWithConflicts() {
Expand All @@ -347,6 +446,13 @@ class FileSpecTests {
typeName("some_other_module.SortOrder")
).build()
)
.addProperty(
PropertySpec.varBuilder(
"yet_another_module_order",
typeName("Swift.Array")
.parameterizedBy(typeName("yet_another_module.SortOrder"))
).build()
)
.build()

val testFile = FileSpec.builder("Test", "Test")
Expand All @@ -362,11 +468,13 @@ class FileSpecTests {
"""
import Foundation
import some_other_module
import yet_another_module

struct SomeType {

var foundation_order: Foundation.SortOrder
var order: some_other_module.SortOrder
var yet_another_module_order: [yet_another_module.SortOrder]

}

Expand Down