google / ksp

Kotlin Symbol Processing API
https://github.com/google/ksp
Apache License 2.0
2.89k stars 274 forks source link

KSP does not read actualized typealias declaration #1676

Closed ZacSweers closed 2 weeks ago

ZacSweers commented 10 months ago

This is an odd case I ran into with a multiplatform project. In short, you can consider a case like this with dagger annotations:

// In commonMain
@Target(CLASS) expect annotation class AssistedFactory()

// In jvmMain
actual typealias AssistedFactory = dagger.assisted.AssistedFactory

The surprising behavior I've found is that when KSP represents this as an annotation on a declaration, it does not see the actualized typealias (and thus its target) and instead just reports the common type.

I'm not sure if this is intended to work or not, but I was surprised by it because this was while processing in the kspKotlinJvm task.

For a more involved example and repro, you can see this PR and disable the lenient mode I had to add to the processor to support this: https://github.com/slackhq/circuit/pull/1103

Whole-Hog-Bakery commented 8 months ago

ksp does not recognise typealias for android room dao

e.g.

given

typealias BaseDao<T> = AbstractDao<T, Long>

@Dao
abstract class AbstractDao<TEntity : IEntity<TKey?>, TKey : Any> {

    @Insert
    abstract fun save(obj: TEntity): Long

    @Insert
    abstract fun save(vararg obj: TEntity)

    @Insert
    abstract suspend fun saveAsync(vararg obj: TEntity)

    @Update
    abstract fun update(obj: TEntity)

    @Delete
    abstract fun delete(obj: TEntity)

ksp will not generate any of the functions if i extend BaseDao<> however by using AbstractDao directly ksp works fine

ZacSweers commented 8 months ago

It seems this may be a limit in kotlinc. @madsager was seeing some similar stuff at the compiler level with the parcelize plugin in K2 as well

ting-yuan commented 4 months ago

In the https://github.com/slackhq/circuit/pull/1103, this check failed:

          it.annotationType.resolve().declaration.qualifiedName?.asString() == qualifiedName

because it.annotationType.resolve().declaration points to a KSTypeAlias rather than KSClassDeclaration. Processors need to explicitly resolve type aliases to the underlying declaration, like here.

I'm sorry that this may appear to be inconvenient but it is actually a trade-off between convenience and the ability to visit the type aliases.

ZacSweers commented 4 months ago

Gotcha, thanks!

ZacSweers commented 4 months ago

That doesn't appear to work for expect/actual aliased annotations

See: https://github.com/slackhq/circuit/pull/1521

Namely, it's resolving as a KSClassDeclaration and not a KSTypeAlias

image
ting-yuan commented 2 weeks ago

Sorry my previous comment doesn't apply to expect / actual. These are 2 issues: expect/actual and typealias.

For typealias, processors either need to unwrap it explicitly, or they can use getSymbolsWithAnnotation after the fix is merged.

For expect / actual, I guess what you need is KSDeclaration.findActuals. Unfortunately, it is (imho) the Kotlin / K2 design that finding actuals from a expect is discouraged. Because of that, we can't find a reliable way to implement KSDeclaration.findActuals and have to make it part of incompatible change in KSP2.

Going forward, I guess the alternatives @ZacSweers mentioned in https://github.com/google/dagger/issues/4356#issue-2411769548 are the ways to go.

ting-yuan commented 2 weeks ago

@ZacSweers I'm closing this as WAI for now. Please feel free to reopen for more discussion or if I misunderstand anything.