yahoo / squidb

SquiDB is a SQLite database library for Android and iOS
https://github.com/yahoo/squidb/wiki
Apache License 2.0
1.31k stars 132 forks source link

Annotation processor fails with unexpected input (Kotlin) #165

Closed arvola closed 8 years ago

arvola commented 8 years ago

The ModelSpecProcessor will attempt to process all annotations it receives, even if they're part of the list of supported annotations returned by getSupportedAnnotationTypes().

From what I could tell, the JSR-269 specification for annotation processing doesn't explicitly say that a processor will only receive annotations it supports, although it is implied.

The specific use case is with Kotlin and its own processing tool (kapt), there are some workarounds that appear to result in the processor being handed annotations outside of its own supported ones. By skipping these other annotations in ModelSpecProcessor, SquiDB works with Kotlin as well based on my testing.

I plan on filing an issue with Kotlin as well, since that doesn't seem like desired behavior, but it's pretty simple to work around as well. I can create a pull request if you feel like this change is desirable.

sbosley commented 8 years ago

Thanks for opening the issue! I think you're right that the spec is a little ambiguous, although I had assumed that the Processor interface docs specified the behavior where it says "A processor will be asked to process a subset of the annotation types it supports, possibly an empty set." Given that I think I agree with you that what kapt is doing doesn't seem like desired behavior. Kotlin's not on our radar at all, but if the workaround is straightforward and not super invasive then I think we'd probably be happy to accept a PR for it.

sbosley commented 8 years ago

Could the workaround be as simple as this?

for (TypeElement annotationType : annotations) {
    if (getSupportedAnnotationTypes().contains(annotationType.getQualifiedName().toString()) {
        for (Element element : env.getElementsAnnotatedWith(annotationType)) {
    ....
}
arvola commented 8 years ago

It is indeed, although in the version I made the supported types collection is a member so it's only created once. I'll get a pull request together.