Report location of code causing KSP exceptions when generating assisted factories #73

jmartinesp · 2024-10-06T18:58:36Z

When having a code like:

class SomeClass @AssistedInject constructor(
    private val presenter: Presenter<Foo>,
) { ... }

If Foo hasn't been properly imported (i.e. in the middle of a refactor), KotlinPoet code generation will fail with a not really helpful error like:

[ksp] java.lang.IllegalArgumentException: Error type '<ERROR TYPE>' is not resolvable in the current round of processing.
	at com.squareup.kotlinpoet.ksp.KsTypesKt.requireNotErrorType(KsTypes.kt:40)
	at com.squareup.kotlinpoet.ksp.KsTypesKt.toTypeName(KsTypes.kt:71)
	at com.squareup.kotlinpoet.ksp.KsTypesKt.toTypeName(KsTypes.kt:199)
	at com.squareup.kotlinpoet.ksp.KsTypesKt.toTypeName(KsTypes.kt:175)
	at com.squareup.kotlinpoet.ksp.KsTypesKt.toTypeName(KsTypes.kt:74)
	at com.squareup.kotlinpoet.ksp.KsTypesKt.toTypeName(KsTypes.kt:65)
	at com.squareup.anvil.compiler.codegen.ksp.KspUtilKt.contextualToTypeName(KspUtil.kt:368)
	at com.squareup.anvil.compiler.codegen.dagger.DaggerGenerationUtilsKt.toConstructorParameter(DaggerGenerationUtils.kt:141)
	at com.squareup.anvil.compiler.codegen.dagger.DaggerGenerationUtilsKt.mapToConstructorParametersKsp(DaggerGenerationUtils.kt:125)
	at com.squareup.anvil.compiler.codegen.dagger.AssistedInjectCodeGen$KspGenerator.generateFactoryClass(AssistedInjectCodeGen.kt:91)
...

In the latest version, this has improved and it now displays at least:

[ksp] java.lang.IllegalArgumentException: Error type '<ERROR TYPE: Foo>' is not resolvable in the current round of processing.

But it still won't tell you where the code generation is failing, so you'd have to check every single usage of this class to try to understand where the error is.

I added some code that, when trying to get the underlying type for KotlinPoet fails, will wrap the original exception adding some context about the name and location of the parameter that caused the code generation issue, like this:

e: [ksp] java.lang.IllegalArgumentException: Failed to resolve type for parameter 'typingNotificationPresenter' at /some/long/path/TimelinePresenter.kt:74
        at com.squareup.anvil.compiler.codegen.dagger.DaggerGenerationUtilsKt.loggingLocationOnError(DaggerGenerationUtils.kt:627)
        at com.squareup.anvil.compiler.codegen.dagger.DaggerGenerationUtilsKt.toConstructorParameter(DaggerGenerationUtils.kt:145)
        at com.squareup.anvil.compiler.codegen.dagger.DaggerGenerationUtilsKt.mapToConstructorParametersKsp(DaggerGenerationUtils.kt:129)
        at com.squareup.anvil.compiler.codegen.dagger.AssistedInjectCodeGen$KspGenerator.generateFactoryClass(AssistedInjectCodeGen.kt:91)
        at com.squareup.anvil.compiler.codegen.dagger.AssistedInjectCodeGen$KspGenerator.processChecked(AssistedInjectCodeGen.kt:75)
        at com.squareup.anvil.compiler.codegen.ksp.AnvilSymbolProcessor.runInternal(AnvilSymbolProcessing.kt:60)
        at com.squareup.anvil.compiler.codegen.ksp.AnvilSymbolProcessor.process(AnvilSymbolProcessing.kt:50)
        at com.google.devtools.ksp.AbstractKotlinSymbolProcessingExtension$doAnalysis$8$1.invoke(KotlinSymbolProcessingExtension.kt:310)
        at com.google.devtools.ksp.AbstractKotlinSymbolProcessingExtension$doAnalysis$8$1.invoke(KotlinSymbolProcessingExtension.kt:308)
        at com.google.devtools.ksp.AbstractKotlinSymbolProcessingExtension.handleException(KotlinSymbolProcessingExtension.kt:414)
        at com.google.devtools.ksp.AbstractKotlinSymbolProcessingExtension.doAnalysis(KotlinSymbolProcessingExtension.kt:308)
        at org.jetbrains.kotlin.cli.jvm.compiler.TopDownAnalyzerFacadeForJVM.analyzeFilesWithJavaIntegration(TopDownAnalyzerFacadeForJVM.kt:112)
        at org.jetbrains.kotlin.cli.jvm.compiler.TopDownAnalyzerFacadeForJVM.analyzeFilesWithJavaIntegration$default(TopDownAnalyzerFacadeForJVM.kt:75)
        at org.jetbrains.kotlin.cli.jvm.compiler.KotlinToJVMBytecodeCompiler.analyze$lambda$12(KotlinToJVMBytecodeCompiler.kt:373)
        at org.jetbrains.kotlin.cli.common.messages.AnalyzerWithCompilerReport.analyzeAndReport(AnalyzerWithCompilerReport.kt:112)
        at org.jetbrains.kotlin.cli.jvm.compiler.KotlinToJVMBytecodeCompiler.analyze(KotlinToJVMBytecodeCompiler.kt:364)
        at org.jetbrains.kotlin.cli.jvm.compiler.KotlinToJVMBytecodeCompiler.runFrontendAndGenerateIrUsingClassicFrontend(KotlinToJVMBytecodeCompiler.kt:195)
        at org.jetbrains.kotlin.cli.jvm.compiler.KotlinToJVMBytecodeCompiler.compileModules$cli(KotlinToJVMBytecodeCompiler.kt:106)
        at org.jetbrains.kotlin.cli.jvm.K2JVMCompiler.doExecute(K2JVMCompiler.kt:170)
        at org.jetbrains.kotlin.cli.jvm.K2JVMCompiler.doExecute(K2JVMCompiler.kt:43)
        at org.jetbrains.kotlin.cli.common.CLICompiler.execImpl(CLICompiler.kt:103)
        at org.jetbrains.kotlin.cli.common.CLICompiler.execImpl(CLICompiler.kt:49)
        at org.jetbrains.kotlin.cli.common.CLITool.exec(CLITool.kt:101)
        at org.jetbrains.kotlin.daemon.CompileServiceImpl.compile(CompileServiceImpl.kt:1555)
        at jdk.internal.reflect.GeneratedMethodAccessor12.invoke(Unknown Source)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:568)
        at java.rmi/sun.rmi.server.UnicastServerRef.dispatch(UnicastServerRef.java:360)
        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(AccessController.java:712)
        at java.rmi/sun.rmi.transport.Transport.serviceCall(Transport.java:196)
        at java.rmi/sun.rmi.transport.tcp.TCPTransport.handleMessages(TCPTransport.java:587)
        at java.rmi/sun.rmi.transport.tcp.TCPTransport$ConnectionHandler.run0(TCPTransport.java:828)
        at java.rmi/sun.rmi.transport.tcp.TCPTransport$ConnectionHandler.lambda$run$0(TCPTransport.java:705)
        at java.base/java.security.AccessController.doPrivileged(AccessController.java:399)
        at java.rmi/sun.rmi.transport.tcp.TCPTransport$ConnectionHandler.run(TCPTransport.java:704)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
        at java.base/java.lang.Thread.run(Thread.java:840)
Caused by: java.lang.IllegalArgumentException: Error type '<ERROR TYPE: TypingNotificationState>' is not resolvable in the current round of processing.
        at com.squareup.kotlinpoet.ksp.KsTypesKt.requireNotErrorType(KsTypes.kt:40)
        at com.squareup.kotlinpoet.ksp.KsTypesKt.toTypeName(KsTypes.kt:71)
        at com.squareup.kotlinpoet.ksp.KsTypesKt.toTypeName(KsTypes.kt:202)
        at com.squareup.kotlinpoet.ksp.KsTypesKt.toTypeName(KsTypes.kt:177)
        at com.squareup.kotlinpoet.ksp.KsTypesKt.toTypeName(KsTypes.kt:74)
        at com.squareup.kotlinpoet.ksp.KsTypesKt.toTypeName(KsTypes.kt:65)
        at com.squareup.anvil.compiler.codegen.ksp.KspUtilKt.contextualToTypeName(KspUtil.kt:368)
        at com.squareup.anvil.compiler.codegen.dagger.DaggerGenerationUtilsKt$toConstructorParameter$paramTypeName$1.invoke(DaggerGenerationUtils.kt:145)
        at com.squareup.anvil.compiler.codegen.dagger.DaggerGenerationUtilsKt$toConstructorParameter$paramTypeName$1.invoke(DaggerGenerationUtils.kt:145)
        at com.squareup.anvil.compiler.codegen.dagger.DaggerGenerationUtilsKt.loggingLocationOnError(DaggerGenerationUtils.kt:621)
        ... 39 more

e: Error occurred in KSP, check log for detail

I'm not really sure about how this was handled, so feel free to suggest or replace the implementation for something better, or add it in other places where this issue may happen: I only know of this one, but there could be more.

…assisted factories with a parameter of unknown type. This will wrap the original KotlinPoet exception adding some context about the name and location of the parameter that caused the code generation issue.

ZacSweers

contextualTypeName is supposed to cover this case. Can we just improve that function to allow adding extra context if needed? Without a test case it's a little difficult to imagine the case this is solving

jmartinesp · 2024-10-07T06:46:55Z

I'm trying to come up with some test for it, but it's difficult because I can't find a way to get access to the expected exception with the location values, JvmCompilationResult.messages contains a very different one:

e: [ksp] InjectProcessingStep was unable to process 'AssistedService(int,java.util.List<error.NonExistentClass>)' because 'error.NonExistentClass' could not be resolved.

EDIT: ok, it seems like this happens because it was being run with a configuration that never called the code I modified.

…the KSP output

compiler/src/main/java/com/squareup/anvil/compiler/codegen/ksp/KspUtil.kt

- Resolve argument types only once. - Add extra info for property and function declarations too.

ZacSweers · 2024-10-08T19:46:05Z

Please stop re-requesting review, I am reviewing. I am also on an airplane and will get to it when I get to it :).

jmartinesp · 2024-10-08T19:48:57Z

Ah sorry, the flow in my job is to make any needed changes and then requesting a new review to notify the reviewer it's ready, so I just followed it out of habit 😅 . It doesn't mean I want an immediate request, I just wanted to flag this was ready for you to look at it again when you get the time, my bad.

ZacSweers

Thanks!

…the warning

ZacSweers · 2024-10-09T21:06:31Z

compiler/src/main/java/com/squareup/anvil/compiler/codegen/ksp/KspUtil.kt

What was the warning? toString() calls on KSP declarations aren’t always super reliable so I’d rather use something more intentional

It was:

Conditional branch result of type KSNode is implicitly cast to Any?
Conditional branch result of type KSType? is implicitly cast to Any?

The first branch of the if returned a KSNode and the 2nd one a KSType?. I had assumed the KSType would also implement KSNode, but it turns out it's a completely different type and it's not related. So I guess the options are:

Since we only want to print its info, use the toString() strategy, as done in the changes.

Explicitly cast to Any?.

Change the implementation to something else altogether.

Took a crack at it in 6828daa

Thanks! It looks way cleaner.

CHANGELOG.md

Report location of source code causing KSP exception when generating …

0d946a7

…assisted factories with a parameter of unknown type. This will wrap the original KotlinPoet exception adding some context about the name and location of the parameter that caused the code generation issue.

ZacSweers requested changes Oct 7, 2024

View reviewed changes

jmartinesp added 2 commits October 7, 2024 10:05

Move the code adding location context to contextualToTypeName

cf99ab0

Add a test that shows the type of errors this is fixing and verifies …

2b2ddc3

…the KSP output

jmartinesp requested a review from ZacSweers October 7, 2024 08:07

ZacSweers reviewed Oct 8, 2024

View reviewed changes

compiler/src/main/java/com/squareup/anvil/compiler/codegen/ksp/KspUtil.kt Outdated Show resolved Hide resolved

Simplify the checks and make checkErrorType messages more descriptive

24ba1e5

jmartinesp requested a review from ZacSweers October 8, 2024 13:46

ZacSweers reviewed Oct 8, 2024

View reviewed changes

compiler/src/main/java/com/squareup/anvil/compiler/codegen/ksp/KspUtil.kt Outdated Show resolved Hide resolved

compiler/src/main/java/com/squareup/anvil/compiler/codegen/ksp/KspUtil.kt Outdated Show resolved Hide resolved

Address review comments:

9ed633e

- Resolve argument types only once. - Add extra info for property and function declarations too.

jmartinesp requested a review from ZacSweers October 8, 2024 19:44

ZacSweers approved these changes Oct 9, 2024

View reviewed changes

jmartinesp added 2 commits October 9, 2024 21:46

Fix Ktlint

41d40ae

Instead of getting an errorType: Any? get its description to avoid …

b334727

…the warning

ZacSweers reviewed Oct 9, 2024

View reviewed changes

Finish implementing

6828daa

jmartinesp commented Oct 11, 2024

View reviewed changes

CHANGELOG.md Show resolved Hide resolved

Update CHANGELOG.md

d9f56d3

ZacSweers merged commit dd089fa into ZacSweers:main Oct 11, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Report location of code causing KSP exceptions when generating assisted factories #73

Report location of code causing KSP exceptions when generating assisted factories #73

jmartinesp commented Oct 6, 2024

ZacSweers left a comment

jmartinesp commented Oct 7, 2024 •

edited

Loading

ZacSweers commented Oct 8, 2024

jmartinesp commented Oct 8, 2024

ZacSweers left a comment

ZacSweers Oct 9, 2024

jmartinesp Oct 9, 2024 •

edited

Loading

ZacSweers Oct 10, 2024

jmartinesp Oct 11, 2024

Report location of code causing KSP exceptions when generating assisted factories #73

Report location of code causing KSP exceptions when generating assisted factories #73

Conversation

jmartinesp commented Oct 6, 2024

ZacSweers left a comment

Choose a reason for hiding this comment

jmartinesp commented Oct 7, 2024 • edited Loading

ZacSweers commented Oct 8, 2024

jmartinesp commented Oct 8, 2024

ZacSweers left a comment

Choose a reason for hiding this comment

ZacSweers Oct 9, 2024

Choose a reason for hiding this comment

jmartinesp Oct 9, 2024 • edited Loading

Choose a reason for hiding this comment

ZacSweers Oct 10, 2024

Choose a reason for hiding this comment

jmartinesp Oct 11, 2024

Choose a reason for hiding this comment

jmartinesp commented Oct 7, 2024 •

edited

Loading

jmartinesp Oct 9, 2024 •

edited

Loading