outfoxx / swiftpoet

Kotlin and Java API for generating .swift source files.
Apache License 2.0
277 stars 26 forks source link

Including sub types within external extensions do not emit imports #88

Closed dnkoutso closed 6 months ago

dnkoutso commented 8 months ago

Given test:

@Test
  @DisplayName("Generates correct imports for extension types ")
  fun testParentExtensionBug() {
    val parentElement = typeName(".Parent")
    val obsElement = typeName("RxSwift.Observable.Element")

    val extension =
      ExtensionSpec.builder(parentElement) // <----- using parentElement vs obsElement.enclosingTypeName()!! fails.
        .addFunction(
          FunctionSpec.builder("test")
            .returns(obsElement)
            .build()
        )
        .build()

    val testFile = FileSpec.builder("Test", "Test")
      .addExtension(extension)
      .build()

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

    assertThat(
      out.toString(),
      equalTo(
        """
            import RxSwift // <------ this is missing but should be there.

            extension Parent {

              func test() -> RxSwift.Observable.Element { // <---- this looks right if its fully qualified imo?
              }

            }

        """.trimIndent()
      )
    )
  }

Branch: https://github.com/outfoxx/swiftpoet/compare/extension_import_bug?expand=1

Seems related to https://github.com/outfoxx/swiftpoet/pull/55

It seems when using obsElement as the extension type imports are emitted correctly which is why the test that was added in #55 passes correctly.

When removing:

if (typeSpec is ExternalTypeSpec) {
  return stackTypeName(i, simpleName)
}

Things start working, perhaps this is meant to check an additional condition? I am still getting familiarized with this code @kdubb

cc @lickel

dnkoutso commented 8 months ago

@kdubb any ideas would be greatly appreciated, or hints...

dnkoutso commented 8 months ago

Seems somehow that resolve returns back a type in the failing test.

Notice resolved came back as Parent.Element

Screenshot 2023-11-06 at 2 20 11 PM

In the passing test case this does not resolve to anything which then calls importableType(typeName) and adds the import to the suggested import types...

kdubb commented 8 months ago

@dnkoutso I'll have a look at this tonight

dnkoutso commented 8 months ago

Thanks @kdubb !

dnkoutso commented 8 months ago

@kdubb sorry for the additional ping, was hoping for any guidance on this. Currently it can cause an error in Wire output with a missing import.

Thanks again and apologies

dnkoutso commented 7 months ago

Would love an update here as this is currently blocking us.