Permissionsdispatcher: Remove requirement for "kotlin = true"

[mannodermaus]

As discussed before, we might not have to force users to indicate the language of their annotated @RuntimePermissions class with kotlin = true. Taking a hint from how the JUnit 5 team solves this issue, we might also be able to detect a Kotlin class by looking at its annotations during the annotation processor phase.

I'll play around with this a little and see if I can produce some results. Would be great to remove this additional step for our Kotlin-based users!

[hotchemi]

Thx! obviously a way to go!

[mannodermaus]

It seems like the annotation processor doesn't receive the "proper" class when processing the Round Environment with kapt. Unfortunately, we can't detect kotlin.Metadata:

roundEnv.getElementsAnnotatedWith(RuntimePermissions::class.java)
    .sortedBy { it.simpleName.toString() }
    .forEach {
        val rpe = RuntimePermissionsElement(it as TypeElement)
        messager.printMessage(Diagnostic.Kind.WARNING, "RuntimePermissions Element: $it")
        messager.printMessage(Diagnostic.Kind.WARNING, "* Annotation Mirrors: ${it.annotationMirrors} (${it.annotationMirrors.size})")

        val metadataCls : Class<Annotation> = Class.forName("kotlin.Metadata") as Class<Annotation>
        messager.printMessage(Diagnostic.Kind.WARNING, "* Metadata Class: $metadataCls")
        messager.printMessage(Diagnostic.Kind.WARNING, "* Metadata On Cls: ${it.getAnnotation(metadataCls)}")
    }

Output:

w: warning: RuntimePermissions Element: permissions.dispatcher.samplekotlin.MainActivity
w: warning: * Annotation Mirrors: @permissions.dispatcher.RuntimePermissions(kotlin=true) (1)
w: warning: * Metadata Class: interface kotlin.Metadata
w: warning: * Metadata On Cls: null

When decompiling the Kotlin bytecode using the IDE, the following Java is yielded:

@Metadata(
   mv = {1, 1, 6},
   bv = {1, 0, 1},
   k = 1,
   d1 = {"\u0000B\n\u0002\u0018..."},
   d2 = {"Lpermissions/dispatcher/samplekotlin/MainActivity;", ...}
)
@RuntimePermissions(
   kotlin = true
)
public final class MainActivity extends AppCompatActivity {
    // ...
}

But this doesn't seem to be the class used during annotation processing. Or maybe kapt excludes the internal Kotlin annotations from the annotated Elements...

Do you guys suppose we should raise an issue & ask about this for annotation processing specifically?

[hotchemi]

@aurae Sorry for the late! Thank you for your investigation and I suppose it's worth to file an issue to improve Kotlin dealing from APT!
I found similar forum thread but there's no response yet... 🤔
https://discuss.kotlinlang.org/t/kotlin-data-classes-inspection-from-annotation-processor/2163

[hotchemi]

seems there's no way to detect annotated class is Kotlin or not within ATP... 😢
@shiraji any idea?

[mannodermaus]

To recap from the meeting: If the distinction between Java and Kotlin can't be made on a per-class basis, it should be moved "a level higher", and converted into a question of "kapt vs. javac apt". Because we are able to detect kapt during the annotation processor's runtime, we can go the Kotlin route for all annotated classes in this case, modelling the interoperability with Java accordingly (e.g. using @file:JvmName). The last dealbreaker with this approach still remains to be the lack of a Kotlin version to Google's compile-testing library, with which we can validate the correct behaviour during unit testing. We should be reaching out to JetBrains & ask for the plans for a library like that.

[hotchemi]

Thx for recapping!
Actually that's so so clever way and I'd like to go with it! So next step is I guess:

• Fix generated file template to allow Java to call its methods easily
• Create PR

As for unit testing, I've not investigated deeply but I wonder we can do file-base testing with google/compiler-testing like that at least they test manifest file generation in google/auto 📝

[jmatsu]

After upgrading Kotlin to 1.1.3, I confirmed a processor could see kotlin.Metadata by the same way https://github.com/permissions-dispatcher/PermissionsDispatcher/issues/350#issuecomment-319587938 described.
Though it seems that you already got a conclusion of this discussion.... this is just for information!

[hotchemi]

@jmatsu Wow!!!

[mannodermaus]

Really? I'm pretty sure I tested it on 1.1.4-2 and it wouldn't show up. Thanks for the info!

Edit: Actually yes, you are right - using a more recent version, the @Metadata is resolved correctly:

w: warning: RuntimePermissions Element: permissions.dispatcher.samplekotlin.MainActivity
w: warning: * Annotation Mirrors: @permissions.dispatcher.RuntimePermissions(kotlin=true),@kotlin.Metadata(mv={1, 1, 7}, ...) (2)
w: warning: * Metadata Class: interface kotlin.Metadata
w: warning: * Metadata On Cls: @kotlin.Metadata(xi=0, ...)

@hotchemi @shiraji There's a comment in our versions file relating to this issue. I'm unfamiliar with the problem there, but it might have been resolved in 1.1.4-2? Can we verify that it's save to update now? In that case, I might actually prefer the initial way of doing things, now that @jmatsu has confirmed that we can actually detect on a per-class basis.

[mannodermaus]

I've started work on the migration to @kotlin.Metadata here.

[hotchemi]

@aurae as for lint issue actually I'm not sure it was addressed already or not(seems no update on youtrack?). But please give it a go!

[shiraji]

331 is the issue

[david-becher]

Hey - just a quick question: any note on when 3.0.0 will be released? :)

[hotchemi]

@dbecher-ito we're trying to release on 8/31 or 9/1 for now! https://github.com/permissions-dispatcher/PermissionsDispatcher/milestone/22

[mannodermaus]

This is considered as part of #358