What approach do folks here use to insure against ...

# ksp

[Uli Bubenheimer]

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.

@MyIntThing(1) // correct
@MyIntThing(1,2) // no
@MyIntThing("nope")
@MyIntThing(1) @MyIntThing(2) //no, it's not repeatable
@MyIntThing(1L) // no, really

[yigit]

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

[yigit]

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

[yigit]

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

[Uli Bubenheimer]

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:

val myIntThingAnnotation = getAnnotationsByType(instanceLimitKClass).first()
myIntThingAnnotation.value // ClassCastException upon property access with '@MyIntThing("nope")'
// NoSuchElementException upon property access with '@MyIntThing` (no arguments)

[Uli Bubenheimer]

validate() does not help here

[Uli Bubenheimer]

It's like super brittle, really.

[Uli Bubenheimer]

I mean, as a processor developer I have to anticipate all the possible ways that someone could misuse the annotation to not crash

[yigit]

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.

[yigit]

Yep, without exaggeration, at least 30% of annotation processor code is error detection.

[yigit]

Likely 90% of room compiler tests are for error detection (we test happy paths with runtime tests)

[yigit]

Because if you don't have good error detection, developer experience drop significantly (see: data binding, dagger 🤷‍♂️)

[Uli Bubenheimer]

So I just came up with this snippet to be more robust. I'm not good with sequences, is there something better?

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 {}

[Uli Bubenheimer]

(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)

[Uli Bubenheimer]

I would have thought that ksp would have some API for standard annotation correctness verification. Like validate(), but that does not help here

[Uli Bubenheimer]

It seems like everyone would have to reinvent the wheel here to be robust

[yigit]

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)

[Uli Bubenheimer]

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.

[Uli Bubenheimer]

My snippet does not even catch anything

[yigit]

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

[yigit]

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

[Uli Bubenheimer]

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.

[Uli Bubenheimer]

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

[yigit]

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😁