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.

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

KSP Processing order doesn't process room class first before typealias #881

Closed
cp-yfukuda opened this issue Mar 2, 2022 · 13 comments · Fixed by #928
Closed

KSP Processing order doesn't process room class first before typealias #881

cp-yfukuda opened this issue Mar 2, 2022 · 13 comments · Fixed by #928
Assignees
Milestone

Comments

@cp-yfukuda
Copy link

cp-yfukuda commented Mar 2, 2022

With room being KSP dependent, if I have this code,

pacakge com.mypackage

@Entity(tableName = "my_data_table")
data class MyData(
}

And in other file in the same package, But different fille

pacakge com.mypackage

import com.mypackage.DataProvider

data class Identifier<T> private constructor(private val uuid: UUID) {
}

...

typealias DataProviderIdentifier = Identifier<MyData>

This compiration with ksp with room gives me

`
[ksp] java.lang.IllegalStateException: Unexpected type name for KSType: com.mypackage.Identifier<com.mypackage.DataProvider>

`

This makes me think that alias processing happenes before ksp is processing room annotation.

@cp-yfukuda cp-yfukuda changed the title KSP Processing order KSP Processing order doesn't process room class first before typealias Mar 2, 2022
@ting-yuan
Copy link
Collaborator

Can you share the stacktrace and/or a sample project that we can reproduce it?

@cp-yfukuda
Copy link
Author

Hi Ting-Yuan

I made a simple version of the app, with typealias using Room data, but
it works.
https://drive.google.com/file/d/1Dbp217h-I9_M25BYNng5slfR9V2Y7SI3/view?usp=sharing
But here is actual stacktrace.

[ksp] java.lang.IllegalStateException: Unexpected type name for KSType: org.test.Identifier< com.mypackage.DataProvider>
	at androidx.room.compiler.processing.ksp.KSTypeExtKt.typeName(KSTypeExt.kt:214)
	at androidx.room.compiler.processing.ksp.KSTypeExtKt.typeName(KSTypeExt.kt:71)
	at androidx.room.compiler.processing.ksp.KSTypeExtKt.typeName$resolveTypeName(KSTypeExt.kt:153)
	at androidx.room.compiler.processing.ksp.KSTypeExtKt.typeName(KSTypeExt.kt:171)
	at androidx.room.compiler.processing.ksp.KSTypeExtKt.typeName(KSTypeExt.kt:193)
	at androidx.room.compiler.processing.ksp.KSTypeExtKt.typeName(KSTypeExt.kt:181)
	at androidx.room.compiler.processing.ksp.DefaultKspType.resolveTypeName(DefaultKspType.kt:32)
	at androidx.room.compiler.processing.ksp.KspType$typeName$2.invoke(KspType.kt:54)
	at androidx.room.compiler.processing.ksp.KspType$typeName$2.invoke(KspType.kt:51)
	at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:74)
	at androidx.room.compiler.processing.ksp.KspType.getTypeName(KspType.kt:51)
	at androidx.room.processor.FieldProcessor.process(FieldProcessor.kt:39)
	at androidx.room.processor.PojoProcessor.doProcess(PojoProcessor.kt:173)
	at androidx.room.processor.PojoProcessor.access$doProcess(PojoProcessor.kt:56)
	at androidx.room.processor.PojoProcessor$process$1.invoke(PojoProcessor.kt:112)
	at androidx.room.processor.PojoProcessor$process$1.invoke(PojoProcessor.kt:109)
	at androidx.room.processor.cache.Cache$Bucket.get(Cache.kt:53)
	at androidx.room.processor.PojoProcessor.process(PojoProcessor.kt:109)
	at androidx.room.processor.TableEntityProcessor.doProcess(TableEntityProcessor.kt:93)
	at androidx.room.processor.TableEntityProcessor.access$doProcess(TableEntityProcessor.kt:44)
	at androidx.room.processor.TableEntityProcessor$process$1.invoke(TableEntityProcessor.kt:53)
	at androidx.room.processor.TableEntityProcessor$process$1.invoke(TableEntityProcessor.kt:52)
	at androidx.room.processor.cache.Cache$Bucket.get(Cache.kt:53)
	at androidx.room.processor.TableEntityProcessor.process(TableEntityProcessor.kt:52)
	at androidx.room.processor.DatabaseProcessor.processEntities(DatabaseProcessor.kt:443)
	at androidx.room.processor.DatabaseProcessor.doProcess(DatabaseProcessor.kt:69)
	at androidx.room.processor.DatabaseProcessor.process(DatabaseProcessor.kt:60)
	at androidx.room.DatabaseProcessingStep$process$databases$1$1.invoke(DatabaseProcessingStep.kt:74)
	at androidx.room.DatabaseProcessingStep$process$databases$1$1.invoke(DatabaseProcessingStep.kt:70)
	at androidx.room.processor.Context.collectLogs(Context.kt:149)
	at androidx.room.DatabaseProcessingStep.process(DatabaseProcessingStep.kt:70)
	at androidx.room.DatabaseProcessingStep.process(DatabaseProcessingStep.kt:41)
	at androidx.room.compiler.processing.CommonProcessorDelegate.processRound(XBasicAnnotationProcessor.kt:114)
	at androidx.room.compiler.processing.ksp.KspBasicAnnotationProcessor.process(KspBasicAnnotationProcessor.kt:61)
	at com.google.devtools.ksp.AbstractKotlinSymbolProcessingExtension$doAnalysis$4$1.invoke(KotlinSymbolProcessingExtension.kt:223)
	at com.google.devtools.ksp.AbstractKotlinSymbolProcessingExtension$doAnalysis$4$1.invoke(KotlinSymbolProcessingExtension.kt:221)
	at com.google.devtools.ksp.AbstractKotlinSymbolProcessingExtension.handleException(KotlinSymbolProcessingExtension.kt:316)
	at com.google.devtools.ksp.AbstractKotlinSymbolProcessingExtension.doAnalysis(KotlinSymbolProcessingExtension.kt:221)
	at org.jetbrains.kotlin.cli.jvm.compiler.TopDownAnalyzerFacadeForJVM.analyzeFilesWithJavaIntegration(TopDownAnalyzerFacadeForJVM.kt:120)
	at org.jetbrains.kotlin.cli.jvm.compiler.TopDownAnalyzerFacadeForJVM.analyzeFilesWithJavaIntegration$default(TopDownAnalyzerFacadeForJVM.kt:96)
	at org.jetbrains.kotlin.cli.jvm.compiler.KotlinToJVMBytecodeCompiler$analyze$1.invoke(KotlinToJVMBytecodeCompiler.kt:262)
	at org.jetbrains.kotlin.cli.jvm.compiler.KotlinToJVMBytecodeCompiler$analyze$1.invoke(KotlinToJVMBytecodeCompiler.kt:53)
	at org.jetbrains.kotlin.cli.common.messages.AnalyzerWithCompilerReport.analyzeAndReport(AnalyzerWithCompilerReport.kt:113)
	at org.jetbrains.kotlin.cli.jvm.compiler.KotlinToJVMBytecodeCompiler.analyze(KotlinToJVMBytecodeCompiler.kt:253)
	at org.jetbrains.kotlin.cli.jvm.compiler.KotlinToJVMBytecodeCompiler.compileModules$cli(KotlinToJVMBytecodeCompiler.kt:100)
	at org.jetbrains.kotlin.cli.jvm.compiler.KotlinToJVMBytecodeCompiler.compileModules$cli$default(KotlinToJVMBytecodeCompiler.kt:58)
	at org.jetbrains.kotlin.cli.jvm.K2JVMCompiler.doExecute(K2JVMCompiler.kt:170)
	at org.jetbrains.kotlin.cli.jvm.K2JVMCompiler.doExecute(K2JVMCompiler.kt:52)
	at org.jetbrains.kotlin.cli.common.CLICompiler.execImpl(CLICompiler.kt:92)
	at org.jetbrains.kotlin.cli.common.CLICompiler.execImpl(CLICompiler.kt:44)
	at org.jetbrains.kotlin.cli.common.CLITool.exec(CLITool.kt:98)
	at org.jetbrains.kotlin.daemon.CompileServiceImpl.compile(CompileServiceImpl.kt:1618)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at java.rmi/sun.rmi.server.UnicastServerRef.dispatch(UnicastServerRef.java:359)
	at java.rmi/sun.rmi.transport.Transport$1.run(Transport.java:200)
	at java.rmi/sun.rmi.transport.Transport$1.run(Transport.java:197)
	at java.base/java.security.AccessController.doPrivileged(Native Method)
	at java.rmi/sun.rmi.transport.Transport.serviceCall(Transport.java:196)
	at java.rmi/sun.rmi.transport.tcp.TCPTransport.handleMessages(TCPTransport.java:562)
	at java.rmi/sun.rmi.transport.tcp.TCPTransport$ConnectionHandler.run0(TCPTransport.java:796)
	at java.rmi/sun.rmi.transport.tcp.TCPTransport$ConnectionHandler.lambda$run$0(TCPTransport.java:677)
	at java.base/java.security.AccessController.doPrivileged(Native Method)
	at java.rmi/sun.rmi.transport.tcp.TCPTransport$ConnectionHandler.run(TCPTransport.java:676)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:834)`

@cp-yfukuda
Copy link
Author

Hi Ting-Yuan

Could you please check newer build here

In Test.kt, Adding foreign key

@ColumnInfo(name = "related_ids") val relatedId: List<TestIdentifier>

Seems to produce same error.

@cp-yfukuda
Copy link
Author

Any advice? last link contains simple app that can reproduce the error. Please let me know if I can be any helpful.

@ting-yuan
Copy link
Collaborator

I don't know why Room only expects ClassName and ArrayTypeName, while TypeName can also be ParameterizedTypeName, TypeVariableName and WildcardTypeName. From the given exception, it looks like that the type passed is indeed a parameterized type. What if Identifier<Test> is used instead of TestIdentifier? Will the issue be gone or still there?

It looks like to me that type type alias is correctly "un-aliased". Can you file a bug with Room and see what they say?

@cp-yfukuda
Copy link
Author

Thanks for your insight. Will do. Could you please suggest me the correct room repo? I accidentally created to the wrong repo.

@yigit
Copy link
Collaborator

yigit commented Apr 2, 2022

So the reason this is happening is that, when we observe TestIdentifier, it returns a single TypeArgument (Test) but that kind of makes it look like TestIdentifier<Test> (instead of Identifier).
We can work around this in Room but this rather seems like a KSP bug where it shouldn't give any type arguments for the TypeAlias (until it is resolved to the original type).

@ting-yuan
Copy link
Collaborator

Thanks for the explanation. Will fix in KSP.

@ting-yuan ting-yuan self-assigned this Apr 2, 2022
@ting-yuan ting-yuan added this to the 1.0.5 milestone Apr 2, 2022
@yigit
Copy link
Collaborator

yigit commented Apr 2, 2022

Btw, playing a bit more with it, this might also have some interesting equality considerations.
a0d21fb

If I change the test there to use List<Long> instead of String, it starts failing the equality assertion.
I've not yet dig deeper yet to understand why. But if we support equality between type aliases, we would also need to make sure their hashcodes are the same, which might complicate the matters, unless the KotlinType.hashCode for aliases are guaranteed to be the same.

yigit added a commit to yigit/ksp that referenced this issue Apr 2, 2022
@yigit
Copy link
Collaborator

yigit commented Apr 2, 2022

repro:
https://github.com/google/ksp/compare/main...yigit:kstypealias-arguments?expand=1

I changed the test a little bit to print type arguments in the signature as well and also group by final signature for equality checks.

Good news, equality checks seems to be working, bad news; it is printing the extra type arguments.
Line 7-11 from the output:

ListOfInt<Int> = List<Int>
ListOfInt<Int> = List<Int>
ListOfInt_B<Int> = ListOfInt<Int> = List<Int>
ListOfInt_B<Int> = ListOfInt<Int> = List<Int>
ListOfInt_C<Int> = ListOfInt_B<Int> = ListOfInt<Int> = List<Int>

Where ListOfInt<Int> = List<Int> should've been ListOfInt = List<Int> because it is declared as:

typealias ListOfInt = List<Int>

ting-yuan pushed a commit that referenced this issue Apr 4, 2022
github-actions bot pushed a commit that referenced this issue Apr 4, 2022
(cherry picked from commit 99518cb)
@cp-yfukuda
Copy link
Author

I change the code to
id 'com.google.devtools.ksp' version "1.6.20-1.0.4"
But it still gives me same error.

@ting-yuan
Copy link
Collaborator

The fix will be available in 1.6.20-1.0.5 soon.

@cp-yfukuda
Copy link
Author

Got it. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants