search

ZacSweers / MoshiX

Extensions for Moshi including IR plugins, moshi-sealed, and more.
Apache License 2.0
515 stars 37 forks source link

KSAnnotation.toAnnotationSpec() doesn't handle enum properties correctly #103

Closed JvmName closed 3 years ago

JvmName commented 3 years ago

I have a class: 

@JsonClass(generateAdapter = true)
data class BbaStatement(
    @[JsonLocalDate(zone = MST) Json(name = "startDateMst")] val startDate: Instant,
)

The annotation itself is pretty boring: 

@[JsonQualifier Retention(AnnotationRetention.RUNTIME)]
annotation class JsonLocalDate(val zone: JsonTimeZone);

enum class JsonTimeZone { HST, AST, PST, MST, CST, EST }

The KSP-generated JsonAdapter has a field defined like so: 

@field:JsonLocalDate(zone = JsonTimeZone.MST::class)
private val instantAtJsonLocalDateAdapter: JsonAdapter<Instant> =
  moshi.adapter(Instant::class.java, Types.getFieldJsonQualifierAnnotations(javaClass,
  "instantAtJsonLocalDateAdapter"), "startDate")

The annotation that tags the field is incorrect. When generated with KAPT, I see @field:JsonLocalDate(zone = JsonTimeZone.MST), without the ::class suffix on the enum.

Here's the runtime exception stacktrace I see that alerted me to the problem: 

Caused by: java.lang.annotation.AnnotationTypeMismatchException: Incorrectly typed data found for annotation element public abstract hatch.common.annotations.JsonTimeZone hatch.common.annotations.JsonLocalDate.zone() (Found data of type java.lang.Class)
    at libcore.reflect.AnnotationMember.validateValue(AnnotationMember.java:351)
    at libcore.reflect.AnnotationFactory.invoke(AnnotationFactory.java:299)
    at java.lang.reflect.Proxy.invoke(Proxy.java:1006)
    at $Proxy14.zone(Unknown Source)
    at com.squareup.moshi.Moshi.adapter(Moshi.java:145)hatch.core.network.adapters.LocalDateJsonAdapterFactory.create(LocalDateJsonAdapterFactory.kt:18)
    at hatch.model.dashboard.BbaStatementJsonAdapter.<init>(BbaStatementJsonAdapter.kt:32)

This seems to be an issue only for annotations with an enum value -- the correct adapter is generated if I use zone: String instead.

ZacSweers commented 3 years ago

Thanks! Looks like it's a bug in how we convert KSAnnotation to AnnotationSpec here https://github.com/ZacSweers/MoshiX/blob/main/moshi-ksp/moshi-ksp/src/main/kotlin/dev/zacsweers/moshix/ksp/KspUtil.kt#L113-L144. Will take a look. PRs welcome too if you or anyone reading this want to take a swing at it!

pavlospt commented 3 years ago

I have taken a stub at it here https://github.com/pavlospt/MoshiX/commit/a09ce32f7f061aa7d8467e068197d0e4897ff592 but I am not sure If I am approaching it the correct way since the added test fails with the following error:

e: [ksp] org.jetbrains.kotlin.resolve.lazy.NoDescriptorForDeclarationException: Descriptor wasn't found for declaration CLASS
ZacSweers commented 3 years ago

Can you paste the full trace?

ZacSweers commented 3 years ago

Alternatively, you should be able to run that test in debug and breakpoint the code you added in the debugger to poke around

pavlospt commented 3 years ago

Can you paste the full trace?

Full stacktrace

e: [ksp] org.jetbrains.kotlin.resolve.lazy.NoDescriptorForDeclarationException: Descriptor wasn't found for declaration CLASS at org.jetbrains.kotlin.resolve.lazy.BasicAbsentDescriptorHandler.diagnoseDescriptorNotFound(AbsentDescriptorHandler.kt:18) at org.jetbrains.kotlin.resolve.lazy.BasicAbsentDescriptorHandler.diagnoseDescriptorNotFound(AbsentDescriptorHandler.kt:17) at org.jetbrains.kotlin.resolve.lazy.LazyDeclarationResolver.findClassDescriptor(LazyDeclarationResolver.kt:88) at org.jetbrains.kotlin.resolve.lazy.LazyDeclarationResolver.getClassDescriptor(LazyDeclarationResolver.kt:62) at org.jetbrains.kotlin.resolve.lazy.LazyDeclarationResolver.getMemberScopeDeclaredIn$frontend(LazyDeclarationResolver.kt:227) at org.jetbrains.kotlin.resolve.lazy.LazyDeclarationResolver$resolveToDescriptor$1.visitProperty(LazyDeclarationResolver.kt:181) at org.jetbrains.kotlin.resolve.lazy.LazyDeclarationResolver$resolveToDescriptor$1.visitProperty(LazyDeclarationResolver.kt:94) at org.jetbrains.kotlin.psi.KtProperty.accept(KtProperty.java:58) at org.jetbrains.kotlin.resolve.lazy.LazyDeclarationResolver.resolveToDescriptor(LazyDeclarationResolver.kt:94) at org.jetbrains.kotlin.resolve.lazy.LazyDeclarationResolver.resolveToDescriptor(LazyDeclarationResolver.kt:91) at org.jetbrains.kotlin.resolve.lazy.ResolveSession.resolveToDescriptor(ResolveSession.java:361) at com.google.devtools.ksp.processing.impl.ResolverImpl.resolveDeclaration(ResolverImpl.kt:321) at com.google.devtools.ksp.symbol.impl.kotlin.KSPropertyDeclarationImpl$propertyDescriptor$2.invoke(KSPropertyDeclarationImpl.kt:41) at com.google.devtools.ksp.symbol.impl.kotlin.KSPropertyDeclarationImpl$propertyDescriptor$2.invoke(KSPropertyDeclarationImpl.kt:34) at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:74) at com.google.devtools.ksp.symbol.impl.kotlin.KSPropertyDeclarationImpl.getPropertyDescriptor(KSPropertyDeclarationImpl.kt) at com.google.devtools.ksp.symbol.impl.kotlin.KSPropertyDeclarationImpl.access$getPropertyDescriptor$p(KSPropertyDeclarationImpl.kt:34) at com.google.devtools.ksp.symbol.impl.kotlin.KSPropertyDeclarationImpl$getter$2.invoke(KSPropertyDeclarationImpl.kt:63) at com.google.devtools.ksp.symbol.impl.kotlin.KSPropertyDeclarationImpl$getter$2.invoke(KSPropertyDeclarationImpl.kt:34) at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:74) at com.google.devtools.ksp.symbol.impl.kotlin.KSPropertyDeclarationImpl.getGetter(KSPropertyDeclarationImpl.kt) at com.google.devtools.ksp.processing.impl.ResolverImpl$getSymbolsWithAnnotation$visitor$1.visitPropertyDeclaration(ResolverImpl.kt:232) at com.google.devtools.ksp.processing.impl.ResolverImpl$getSymbolsWithAnnotation$visitor$1.visitPropertyDeclaration(ResolverImpl.kt:192) at com.google.devtools.ksp.symbol.impl.kotlin.KSPropertyDeclarationImpl.accept(KSPropertyDeclarationImpl.kt:100) at com.google.devtools.ksp.processing.impl.ResolverImpl$getSymbolsWithAnnotation$visitor$1.visitClassDeclaration(ResolverImpl.kt:208) at com.google.devtools.ksp.processing.impl.ResolverImpl$getSymbolsWithAnnotation$visitor$1.visitClassDeclaration(ResolverImpl.kt:192) at com.google.devtools.ksp.symbol.impl.kotlin.KSClassDeclarationImpl.accept(KSClassDeclarationImpl.kt:121) at com.google.devtools.ksp.processing.impl.ResolverImpl$getSymbolsWithAnnotation$visitor$1.visitFile(ResolverImpl.kt:202) at com.google.devtools.ksp.processing.impl.ResolverImpl$getSymbolsWithAnnotation$visitor$1.visitFile(ResolverImpl.kt:192) at com.google.devtools.ksp.symbol.impl.kotlin.KSFileImpl.accept(KSFileImpl.kt:59) at com.google.devtools.ksp.processing.impl.ResolverImpl.getSymbolsWithAnnotation(ResolverImpl.kt:243) at com.google.devtools.ksp.processing.Resolver$DefaultImpls.getSymbolsWithAnnotation$default(Resolver.kt:49) at dev.zacsweers.moshix.ksp.JsonClassSymbolProcessor.process(JsonClassSymbolProcessor.kt:100) at com.google.devtools.ksp.AbstractKotlinSymbolProcessingExtension$doAnalysis$$inlined$forEach$lambda$2.invoke(KotlinSymbolProcessingExtension.kt:151) at com.google.devtools.ksp.AbstractKotlinSymbolProcessingExtension$doAnalysis$$inlined$forEach$lambda$2.invoke(KotlinSymbolProcessingExtension.kt:71) at com.google.devtools.ksp.AbstractKotlinSymbolProcessingExtension.handleException(KotlinSymbolProcessingExtension.kt:228) at com.google.devtools.ksp.AbstractKotlinSymbolProcessingExtension.doAnalysis(KotlinSymbolProcessingExtension.kt:150) at org.jetbrains.kotlin.cli.jvm.compiler.TopDownAnalyzerFacadeForJVM.analyzeFilesWithJavaIntegration(TopDownAnalyzerFacadeForJVM.kt:116) at org.jetbrains.kotlin.cli.jvm.compiler.TopDownAnalyzerFacadeForJVM.analyzeFilesWithJavaIntegration$default(TopDownAnalyzerFacadeForJVM.kt:85) at org.jetbrains.kotlin.cli.jvm.compiler.KotlinToJVMBytecodeCompiler$analyze$1.invoke(KotlinToJVMBytecodeCompiler.kt:517) at org.jetbrains.kotlin.cli.jvm.compiler.KotlinToJVMBytecodeCompiler$analyze$1.invoke(KotlinToJVMBytecodeCompiler.kt:508) at org.jetbrains.kotlin.cli.common.messages.AnalyzerWithCompilerReport.analyzeAndReport(AnalyzerWithCompilerReport.kt:114) at org.jetbrains.kotlin.cli.jvm.compiler.KotlinToJVMBytecodeCompiler.analyze(KotlinToJVMBytecodeCompiler.kt:508) at org.jetbrains.kotlin.cli.jvm.compiler.KotlinToJVMBytecodeCompiler.repeatAnalysisIfNeeded(KotlinToJVMBytecodeCompiler.kt:478) at org.jetbrains.kotlin.cli.jvm.compiler.KotlinToJVMBytecodeCompiler.compileModules$cli(KotlinToJVMBytecodeCompiler.kt:188) at org.jetbrains.kotlin.cli.jvm.compiler.KotlinToJVMBytecodeCompiler.compileModules$cli$default(KotlinToJVMBytecodeCompiler.kt:154) at org.jetbrains.kotlin.cli.jvm.K2JVMCompiler.doExecute(K2JVMCompiler.kt:169) at org.jetbrains.kotlin.cli.jvm.K2JVMCompiler.doExecute(K2JVMCompiler.kt:52) at org.jetbrains.kotlin.cli.common.CLICompiler.execImpl(CLICompiler.kt:88) at org.jetbrains.kotlin.cli.common.CLICompiler.execImpl(CLICompiler.kt:44) at org.jetbrains.kotlin.cli.common.CLITool.exec(CLITool.kt:98) at com.tschuchort.compiletesting.AbstractKotlinCompilation.compileKotlin(AbstractKotlinCompilation.kt:180) at com.tschuchort.compiletesting.KotlinCompilation.compileJvmKotlin(KotlinCompilation.kt:465) at com.tschuchort.compiletesting.KotlinCompilation.compile(KotlinCompilation.kt:641) at dev.zacsweers.moshix.ksp.JsonClassSymbolProcessorTest.compile(JsonClassSymbolProcessorTest.kt:835) at dev.zacsweers.moshix.ksp.JsonClassSymbolProcessorTest.enumUsageInAnnotationValue(JsonClassSymbolProcessorTest.kt:551) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59) at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12) at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56) at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17) at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:54) at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306) at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100) at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366) at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103) at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63) at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331) at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79) at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329) at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66) at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293) at org.junit.runners.ParentRunner.run(ParentRunner.java:413) at org.junit.runners.Suite.runChild(Suite.java:128) at org.junit.runners.Suite.runChild(Suite.java:27) at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331) at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79) at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329) at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66) at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293) at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306) at org.junit.runners.ParentRunner.run(ParentRunner.java:413) at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.runTestClass(JUnitTestClassExecutor.java:110) at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:58) at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:38) at org.gradle.api.internal.tasks.testing.junit.AbstractJUnitTestClassProcessor.processTestClass(AbstractJUnitTestClassProcessor.java:62) at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.processTestClass(SuiteTestClassProcessor.java:51) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36) at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24) at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33) at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94) at com.sun.proxy.$Proxy2.processTestClass(Unknown Source) at org.gradle.api.internal.tasks.testing.worker.TestWorker.processTestClass(TestWorker.java:121) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36) at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24) at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:182) at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:164) at org.gradle.internal.remote.internal.hub.MessageHub$Handler.run(MessageHub.java:414) at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:64) at org.gradle.internal.concurrent.ManagedExecutorImpl$1.run(ManagedExecutorImpl.java:48) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) at org.gradle.internal.concurrent.ThreadFactoryImpl$ManagedThreadRunnable.run(ThreadFactoryImpl.java:56) at java.lang.Thread.run(Thread.java:748)

ZacSweers commented 3 years ago

This is fixed on main now and will be available in the next release. @pavlospt your fix was on the right track

pavlospt commented 3 years ago

@ZacSweers cool thank you :) looking forward to the release!