https://kotlinlang.org logo
#ksp
Title
# ksp
u

Uli Bubenheimer

02/25/2023, 8:41 PM
What approach do folks here use to insure against processor crashes due to malformed annotations? ksp runs before kotlinCompile, so annotations may be bad. This can trigger crash in processor without any indication to user about what the malformed element is, and compiler won't get to run to show the offending element, either. Many of the examples below cause various exceptions in ksp while trying to access the annotation and its value.
Copy code
@MyIntThing(1) // correct
@MyIntThing(1,2) // no
@MyIntThing("nope")
@MyIntThing(1) @MyIntThing(2) //no, it's not repeatable
@MyIntThing(1L) // no, really
y

yigit

02/25/2023, 8:43 PM
What is the exact exception? Ideally, KSP itself shouldn't be crashing. As for the annotation processor, the approach i like is to report error through error reporting APIs and keep processing as much as possible. That also ensures you can report as many errors as possible in 1 compilation
For instance in room, if it cannot produce some code due to a user error, it still tries to produce compiling code to avoid any additional confusing error messages from the compiler itself
We suffered from it in data binding where it stopped generation when an error is detected which resulted in thousands of errors from the compiler due to missing classes
u

Uli Bubenheimer

02/25/2023, 8:53 PM
Thanks for the input. Sorry, I meant that the examples can trigger exceptions in the processor, including when calling standard ksp utilities. For example, exceptions when trying to read the annotation property:
Copy code
val myIntThingAnnotation = getAnnotationsByType(instanceLimitKClass).first()
myIntThingAnnotation.value // ClassCastException upon property access with '@MyIntThing("nope")'
// NoSuchElementException upon property access with '@MyIntThing` (no arguments)
validate() does not help here
It's like super brittle, really.
I mean, as a processor developer I have to anticipate all the possible ways that someone could misuse the annotation to not crash
y

yigit

02/25/2023, 8:55 PM
So that is the defensive programming part, you need to check properties of the annotation before reading the value. Though I'm not sure on this particular case TBH.
Yep, without exaggeration, at least 30% of annotation processor code is error detection.
Likely 90% of room compiler tests are for error detection (we test happy paths with runtime tests)
Because if you don't have good error detection, developer experience drop significantly (see: data binding, dagger 🤷‍♂️)
u

Uli Bubenheimer

02/25/2023, 8:58 PM
So I just came up with this snippet to be more robust. I'm not good with sequences, is there something better?
Copy code
resolver.getSymbolsWithAnnotation(annotationQualifiedName)
            .flatMap {
                try {
                    sequenceOf(it).process()
                } catch (t: Throwable) { // print unspecified errors with code reference
                    logger.error(t.toString(), it)
                    throw t
                }
            }
            .forEach {}
(My entire processing pipeline is a long sequence, so I need to catch random errors in there somehow and print the offending user code element)
I would have thought that ksp would have some API for standard annotation correctness verification. Like validate(), but that does not help here
It seems like everyone would have to reinvent the wheel here to be robust
y

yigit

02/25/2023, 9:03 PM
Validate is very useful for missing classes etc so it is more about deferring processing than detecting syntax errors. As for your sequences, it depends if user code accesses generated code. If that's the case, you really want to be generating that code to prevent future false negative errors (even when the code won't work since there is already a compilation error)
u

Uli Bubenheimer

02/25/2023, 9:11 PM
Yes, makes sense. My concern is more elementary than that, though. When processor crashes, user does not know what the bad element was. Annotation validation seems standard functionality. Seems odd that ksp does not have some standard utils for it. Or the ksp playground examples could demonstrate annotation validation gotchas, and show a robustness fallback snippet like what I tried to come up with. I just figure there must be a lot of crashy ksp processors out there for these reasons.
My snippet does not even catch anything
y

yigit

02/25/2023, 10:38 PM
Maybe there is room for some higher level verification utility but we didn't feel the need in room. In general, your processor shouldn't throw , just report errors gracefully. In room, we have some concept of
Context
which has utilities for error reporting. Each time we visit an element, we pass it a new context for that element and Context's error reporting defaults to it's element. Which makes it a bit easier to send errors with referenced elements. https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:room/room-compiler/src/main/kotlin/androidx/room/processor/Context.kt;l=36?q=Context&sq=&ss=androidx%2Fplatform%2Fframeworks%2Fsupport:room%2Froom-compiler%2F I like that pattern quite a lot
You can also see an example of graceful failure here: https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:room/room-compiler/src/main/kotlin/androidx/room/processor/PojoProcessor.kt;l=119 Where it returns an empty pojo if generic validation fails. Or here, even when something is wrong, it still returns whatever it can instead of throwing to let processing continue: https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:room/room-compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt;l=155
u

Uli Bubenheimer

02/25/2023, 10:55 PM
Thank you for all the pointers, that sounds quite useful. I tried to anticipate all possible problems, and intended that snippet just for what I missed, but I guess it does make more sense to simply not throw at all no matter what and just report the problem to the user, log stacktrace at a different level for troubleshooting. It's a pretty simple processor, and user code will not directly access generated code (just via ServiceLoader), so for now I'll stick with a simple catch-all. Guessing I'll write some more complex processors in the future where these pieces will come in handy. Thanks again.
And I do remember the Dagger & databinding error bonanzas, back in the day 😱. My stackoverflow post suggests that I managed to cap errors at no more than 500 usually: https://stackoverflow.com/a/47355113/436417
y

yigit

02/25/2023, 11:51 PM
Yea that was very unfortunate. Even worse, Javac would ignore processor generated code if there is a compilation error in user code, then report thousands more errors because it ignored the generated code. That was a big design constraint in room, don't require user to access generated code😁
11 Views