Extensions don't handle imports

[PaulWoitaschek]

The import for extension types is not added using swiftpoet 1.3.1.

FileSpec.builder("file")
  .addExtension(
    ExtensionSpec.builder(DeclaredTypeName.typeName("RxSwift.Observable"))
      .build()
  )
  .build()

The generated code is:

extension RxSwift.Observable {
}

But it should be:

import RxSwift

extension RxSwift.Observable {
}

[kdubb]

You have the extension statement still qualifying Observable as RsSwift.Observable. Are you not expecting it to be unqualified?

E.g.

import RsSwift

extension Observable {
}

[PaulWoitaschek]

I am not a swift developer and am lacking some level of understand it different areas. Generated code does not have to be beautiful, it has to be correct. I do not care about unnecessary full qualifications.

If I understand the language correctly, not fully qualifying the extension would give a local type named Observable precedence here which would be a bug

[PaulWoitaschek]

Actually this non explicit imports has caused me a lot of issues. I basically can't use DeclaredTypeName and always have to us TypeVariableName instead because else I end up in a lot of situations where the generated code is amgiuous.

For example I have an import on a package that contains an UUID and an import on Foundation. As swiftpoet tries to be clever and omits the Foundation prefix, the swift code does not compile because it can't know which UUID I meant. Therefore I think generated code should always add imports to anything, especially on a language like swift where all these things are resolved dependant on the current project classes and setup.

[kdubb]

Generated code does not have to be beautiful, it has to be correct.

False. One of the goals of this project, and all of the "Poet" libraries (e.g. JavaPoet, KotlinPoet, etc.) is to create correctly formatted and concise code. Code that is as easy to read as handwritten code.

I appreciate that it is not working for you and some solution needs to be created for the issue you're having but we're not gonna throw the baby out with the bathwater and reduce code beauty to solve this issue.

Also, we generate a lot of code with this library in our team and it's now used in Square's Wire library, so it's doing its job quite well for many users.

[kdubb]

@PaulWoitaschek I've fixed this in the latest PR and should be available in 1.4.0-SNAPSHOT.

To extend an externally defined type (e.g. like RxSwift.Observable) you use the new ExtensionTypeSpec.build(TypeName); notice that it takes a type name.

[kdubb]

Also, I've added PR #56 which you can test to see if it solves your problem with needing types that are always fully qualified and don't generate imports.

[PaulWoitaschek]

That's great and will solve my issues!

I still think it is very important to solve this generically for exactly everything.

Also, we generate a lot of code with this library in our team and it's now used in Square's Wire library, so it's doing its job quite well for many users.

Just that it works for many users does not mean that it's correct in every context.

and all of the "Poet" libraries (e.g. JavaPoet, KotlinPoet, etc.) is to create correctly formatted and concise code. Code that is as easy to read as handwritten code.

The other poet Libraries have their very primary focus on correct code. If we take a look at the queries that Sqldelight generates for Kotlin Poet:

package com.yazio.shared.database

import com.squareup.sqldelight.Query
import com.squareup.sqldelight.Transacter
import kotlin.Any
import kotlin.ByteArray
import kotlin.Long
import kotlin.Unit
import kotlin.collections.Collection

public interface CachedEventQueries : Transacter {
  public fun <T : Any> selectAll(mapper: (
    id: Long,
    proto: ByteArray,
    insertedAt: Long
  ) -> T): Query<T>

  public fun selectAll(): Query<CachedEvent>

  public fun <T : Any> lastInsertion(mapper: (max: Long?) -> T): Query<T>

  public fun lastInsertion(): Query<LastInsertion>

  public fun insert(proto: ByteArray, insertedAt: Long): Unit

  public fun deleteById(ids: Collection<Long>): Unit
}

You have explicit imports for all types, even the primitives - even though it is not strictly required.

Also take a look at this issue on wildcard imports:

https://github.com/square/kotlinpoet/issues/349

Wildcard imports are the fastest way to breaking KotlinPoets automatic imports from working correctly.

And exactly that what's happening when kotlin poet is trying to omit imports.

[kdubb]

@PaulWoitaschek Point well taken. Assuredly we are working against Swift's limited import syntax; there just is no syntax for explicit imports and essentially everything is a wildcard.

Swift knows this and there has been a lot of discussion about adding them in the Swift forums. Rest assured that if/when they are added I'll take the stance of KotlinPoet.

Until then let me know if #56 works for you, I believe it should. Just generate all types with alwaysQualify set to true and you should have basically no imports.

[kdubb]

Also, type names with alwaysQualify are "infectious", so generating nested, child, parent type names from them will also set the alwaysQualify of the new type name to true as well.

[PaulWoitaschek]

I currently can't test it in production due to pre-new-year-workload, but from the implementation and the test case it looks like exactly what I need. Thanks for implementing this! 👍