Skip to content

Commit

Permalink
[GH] [Room][Compiler Processing] Add additional type element information
Browse files Browse the repository at this point in the history
## Proposed Changes
Adds an interface function to get element kdoc. Implements this for javac, and uses null for KSP until google/ksp#392 is fixed

Also surfaces more XTypeElement information, such as whether the type is a data class, a fun interface, a companion object, etc.

I implemented everything exposed in kotlin class metadata that seemed reasonable in order to future proof it a bit.

I was not able to implement fun interface checking for KSP since it is not yet surfaced in KSP - google/ksp#393

## Testing

Test: Added cases to XTypeElementTest.modifiers

Note, I wasn't able to test the `expect` modifier. First I had to add support for custom compiler arguments so I could enable multiplatform via `"-Xmulti-platform"` but to get the test sources to compile I had to include a `actual` implementation as well, and since both the expected and actual implementations were in the same sources it seems the processor only picks up the actual. Omitting "actual" implementation doesn't work as compilation fails with "no actual implementation found in module", and I'm not sure how to have the compiler processing testing work with multiple module sources. The implementation of the "expect" modifier is very simple so I'm not sure how important the test is.

## Issues Fixed

Fixes: https://issuetracker.google.com/issues/185672887

This is an imported pull request from androidx#164.

Resolves #164
Github-Pr-Head-Sha: 0bcd0af
GitOrigin-RevId: 04f408a
Change-Id: Ifcf6b7aac73643866034e4bb2e95df81ee997b72
  • Loading branch information
elihart authored and olonho committed May 25, 2021
1 parent de27788 commit 9ec072e
Show file tree
Hide file tree
Showing 13 changed files with 229 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ interface XElement : XAnnotated {
* message. Without this information, developer gets no clue on where the error is.
*/
val fallbackLocationText: String

/**
* The documentation comment of the element, or null if there is none.
*/
val docComment: String?
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ abstract class XMessager {
onPrintMessage(kind, msg, element)
}

abstract fun onPrintMessage(kind: Diagnostic.Kind, msg: String, element: XElement? = null)
protected abstract fun onPrintMessage(
kind: Diagnostic.Kind,
msg: String,
element: XElement? = null
)

fun addMessageWatcher(watcher: XMessager) {
watchers.add(watcher)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,48 @@ interface XTypeElement : XHasModifiers, XElement, XMemberContainer {
fun isInterface(): Boolean

/**
* Returns `true` if this [XTypeElement] is declared as a Kotlin `object`
* Returns `true` if this [XTypeElement] represents a Kotlin functional interface,
* i.e. marked with the keyword `fun`
*/
fun isFunctionalInterface(): Boolean

/**
* Returns `true` if this [XTypeElement] represents an ordinary class. ie not an enum, object,
* annotation, interface, or other type of specialty class.
*/
fun isClass(): Boolean

/**
* Returns `true` if this [XTypeElement] represents a Kotlin data class
*/
fun isDataClass(): Boolean

/**
* Returns `true` if this [XTypeElement] represents a Kotlin value class
*/
fun isValueClass(): Boolean

/**
* Returns `true` if this [XTypeElement] represents a class with the Kotlin `expect` keyword
*/
fun isExpect(): Boolean

/**
* Returns `true` if this [XTypeElement] represents a Kotlin annotation class or a Java
* annotation type.
*/
fun isAnnotationClass(): Boolean

/**
* Returns `true` if this [XTypeElement] is a non-companion `object` in Kotlin
*/
fun isKotlinObject(): Boolean

/**
* Returns `true` if this [XTypeElement] is declared as a Kotlin `companion object`
*/
fun isCompanionObject(): Boolean

/**
* All fields, including private supers.
* Room only ever reads fields this way.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,8 @@ internal abstract class JavacElement(
MoreElements.getPackage(it.annotationType.asElement()).toString() == pkg
}
}

override val docComment: String? by lazy {
env.elementUtils.getDocComment(element)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import androidx.room.compiler.processing.XFieldElement
import androidx.room.compiler.processing.XHasModifiers
import androidx.room.compiler.processing.XMethodElement
import androidx.room.compiler.processing.XTypeElement
import androidx.room.compiler.processing.javac.JavacTypeElement.JavacEnumTypeElement
import androidx.room.compiler.processing.javac.kotlin.KotlinMetadataElement
import com.google.auto.common.MoreElements
import com.google.auto.common.MoreTypes
Expand Down Expand Up @@ -58,8 +57,6 @@ internal sealed class JavacTypeElement(
element.enclosingType(env)
}

override fun isInterface() = element.kind == ElementKind.INTERFACE

private val _allFieldsIncludingPrivateSupers by lazy {
element.getAllFieldsIncludingPrivateSupers(
env.elementUtils
Expand All @@ -77,6 +74,24 @@ internal sealed class JavacTypeElement(
}

override fun isKotlinObject() = kotlinMetadata?.isObject() == true
override fun isCompanionObject() = kotlinMetadata?.isCompanionObject() == true
override fun isDataClass() = kotlinMetadata?.isDataClass() == true
override fun isValueClass() = kotlinMetadata?.isValueClass() == true
override fun isFunctionalInterface() = kotlinMetadata?.isFunctionalInterface() == true
override fun isExpect() = kotlinMetadata?.isExpect() == true

override fun isAnnotationClass(): Boolean {
return kotlinMetadata?.isAnnotationClass()
?: (element.kind == ElementKind.ANNOTATION_TYPE)
}

override fun isClass(): Boolean {
return kotlinMetadata?.isClass() ?: (element.kind == ElementKind.CLASS)
}

override fun isInterface(): Boolean {
return kotlinMetadata?.isInterface() ?: (element.kind == ElementKind.INTERFACE)
}

override fun findPrimaryConstructor(): JavacConstructorElement? {
val primarySignature = kotlinMetadata?.findPrimaryConstructorSignature() ?: return null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import kotlinx.metadata.jvm.KotlinClassMetadata
internal interface KmExecutable {
val parameters: List<KmValueParameter>
}

/**
* Represents the kotlin metadata of a function
*/
Expand Down Expand Up @@ -68,6 +69,7 @@ internal data class KmProperty(
) {
val typeParameters
get() = type.typeArguments

fun isNullable() = Flag.Type.IS_NULLABLE(type.flags)
}

Expand Down Expand Up @@ -188,24 +190,41 @@ private class ConstructorReader(val result: MutableList<KmConstructor>) : KmClas
}
}

internal fun KotlinClassMetadata.Class.isObject(): Boolean = ObjectReader().let {
this@isObject.accept(it)
it.isObject
internal class KotlinMetadataClassFlags(val classMetadata: KotlinClassMetadata.Class) {

private val flags: Flags by lazy {
var theFlags: Flags = 0
classMetadata.accept(object : KmClassVisitor() {
override fun visit(flags: Flags, name: ClassName) {
theFlags = flags
super.visit(flags, name)
}
})
return@lazy theFlags
}

fun isObject(): Boolean = Flag.Class.IS_OBJECT(flags)

fun isCompanionObject(): Boolean = Flag.Class.IS_COMPANION_OBJECT(flags)

fun isAnnotationClass(): Boolean = Flag.Class.IS_ANNOTATION_CLASS(flags)

fun isInterface(): Boolean = Flag.Class.IS_INTERFACE(flags)

fun isClass(): Boolean = Flag.Class.IS_CLASS(flags)

fun isDataClass(): Boolean = Flag.Class.IS_DATA(flags)

fun isValueClass(): Boolean = Flag.Class.IS_INLINE(flags)

fun isFunctionalInterface(): Boolean = Flag.Class.IS_FUN(flags)

fun isExpect(): Boolean = Flag.Class.IS_EXPECT(flags)
}

internal fun KotlinClassMetadata.Class.readProperties(): List<KmProperty> =
mutableListOf<KmProperty>().apply { accept(PropertyReader(this)) }

/**
* Reads whether the given class is a kotlin object
*/
private class ObjectReader : KmClassVisitor() {
var isObject: Boolean = false
override fun visit(flags: Flags, name: ClassName) {
isObject = Flag.Class.IS_OBJECT(flags)
}
}

/**
* Reads the properties of a class declaration
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ internal class KotlinMetadataElement(
private val functionList: List<KmFunction> by lazy { classMetadata.readFunctions() }
private val constructorList: List<KmConstructor> by lazy { classMetadata.readConstructors() }
private val propertyList: List<KmProperty> by lazy { classMetadata.readProperties() }
private val classFlags: KotlinMetadataClassFlags by lazy {
KotlinMetadataClassFlags(classMetadata)
}

private val ExecutableElement.descriptor: String
get() = descriptor()
Expand All @@ -53,7 +56,15 @@ internal class KotlinMetadataElement(
it.isPrimary()
}?.descriptor

fun isObject(): Boolean = classMetadata.isObject()
fun isObject(): Boolean = classFlags.isObject()
fun isCompanionObject(): Boolean = classFlags.isCompanionObject()
fun isAnnotationClass(): Boolean = classFlags.isAnnotationClass()
fun isClass(): Boolean = classFlags.isClass()
fun isInterface(): Boolean = classFlags.isInterface()
fun isDataClass(): Boolean = classFlags.isDataClass()
fun isValueClass(): Boolean = classFlags.isValueClass()
fun isFunctionalInterface(): Boolean = classFlags.isFunctionalInterface()
fun isExpect(): Boolean = classFlags.isExpect()

fun getFunctionMetadata(method: ExecutableElement): KmFunction? {
check(method.kind == ElementKind.METHOD) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,10 @@ internal abstract class KspElement(
KSFileAsOriginatingElement(it)
}
}

override val docComment: String? by lazy {
// TODO: Not yet implemented in KSP.
// https://github.com/google/ksp/issues/392
null
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ internal class KspFileMemberContainer(

override val fallbackLocationText: String = ksFile.filePath

override val docComment: String? by lazy {
// TODO: Not yet implemented in KSP.
// https://github.com/google/ksp/issues/392
null
}

companion object {
private fun KSFile.findClassName(): String {
return annotations.firstOrNull {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import com.google.devtools.ksp.symbol.KSPropertyDeclaration
import com.google.devtools.ksp.symbol.Modifier
import com.google.devtools.ksp.symbol.Origin
import com.squareup.javapoet.ClassName
import javax.tools.Diagnostic

internal sealed class KspTypeElement(
env: KspProcessingEnv,
Expand Down Expand Up @@ -253,6 +254,40 @@ internal sealed class KspTypeElement(
return declaration.classKind == ClassKind.OBJECT
}

override fun isCompanionObject(): Boolean {
return declaration.isCompanionObject
}

override fun isAnnotationClass(): Boolean {
return declaration.classKind == ClassKind.ANNOTATION_CLASS
}

override fun isClass(): Boolean {
return declaration.classKind == ClassKind.CLASS
}

override fun isDataClass(): Boolean {
return Modifier.DATA in declaration.modifiers
}

override fun isValueClass(): Boolean {
return Modifier.INLINE in declaration.modifiers
}

override fun isFunctionalInterface(): Boolean {
// TODO: Update this once KSP supports it
// https://github.com/google/ksp/issues/393
env.messager.printMessage(
Diagnostic.Kind.WARNING,
"XProcessing does not yet support checking for functional interfaces in KSP."
)
return false
}

override fun isExpect(): Boolean {
return Modifier.EXPECT in declaration.modifiers
}

override fun isFinal(): Boolean {
// workaround for https://github.com/android/kotlin/issues/128
return !isInterface() && !declaration.isOpen()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ internal class KspSyntheticContinuationParameterElement(
override val fallbackLocationText: String
get() = "return type of ${containing.fallbackLocationText}"

// Not applicable
override val docComment: String? get() = null

override fun asMemberOf(other: XType): XType {
check(other is KspType)
val continuation = env.resolver.requireContinuationClass()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,12 @@ internal sealed class KspSyntheticPropertyMethodElement(
)
}

override val docComment: String? by lazy {
// Not yet implemented in KSP.
// https://github.com/google/ksp/issues/392
null
}

final override fun asMemberOf(other: XType): XMethodType {
return KspSyntheticPropertyMethodType.create(
element = this,
Expand Down Expand Up @@ -226,6 +232,12 @@ internal sealed class KspSyntheticPropertyMethodElement(
return origin.field.asMemberOf(other)
}

override val docComment: String? by lazy {
// Not yet implemented in KSP.
// https://github.com/google/ksp/issues/392
null
}

override fun kindName(): String {
return "method parameter"
}
Expand Down
Loading

0 comments on commit 9ec072e

Please sign in to comment.