Centralize method / field collection
This CL replaces MethodCollector with a generic DeclarationCollector
that can be re-used for collecting declarations from a type element
and its parents. This new class is also used to collect fields instead
of the custom implementations in each XTypeElement (javac, ksp).
As part of it, I also fixed the JavacTypeElement implementation not to
include enum constant in declared fields list.
In a followup CL, i'll change those methods to return sequence instead
of list in XTypeElement to avoid collecting too many methods.
Bug: n/a
Test: existing tests
Change-Id: Ia805c862a0a841edc8f50a84d1ad7d17399fb660
diff --git a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/DeclarationCollector.kt b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/DeclarationCollector.kt
new file mode 100644
index 0000000..792a88d
--- /dev/null
+++ b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/DeclarationCollector.kt
@@ -0,0 +1,166 @@
+/*
+ * Copyright 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package androidx.room.compiler.processing
+
+/**
+ * Helper method to collect declarations from a type element and its parents.
+ *
+ * To be able to benefit from multi level caching (e.g. each type element caching its important
+ * fields / declarations), parents are not visited recursively. Instead, callers are expected to
+ * call the right methods when traversing immediate parents / interfaces.
+ */
+private fun <T> collectDeclarations(
+ /**
+ * The root element
+ */
+ target: XTypeElement,
+ /**
+ * Elements that will be added without verification. These are usually inherited from the
+ * root element.
+ */
+ initialList: List<T>,
+ /**
+ * Returns a list of possible elements that can be included in the final list.
+ * The receiver is either a super class or interface.
+ */
+ getCandidateDeclarations: XTypeElement.() -> Iterable<T>,
+ /**
+ * Returns a partitan key for each element to optimize override checks etc.
+ */
+ getPartitionKey: T.() -> String,
+ /**
+ * Returns true if this item should be added to the list, false otherwise.
+ */
+ isAcceptable: (candidate: T, existing: List<T>) -> Boolean,
+ /**
+ * Copies the element to the root element
+ */
+ copyToTarget: (T) -> T,
+ /**
+ * If true, parent class will be visited.
+ */
+ visitParent: Boolean,
+ /**
+ * If true, parent interfaces will be visited.
+ */
+ visitInterfaces: Boolean
+): Sequence<T> {
+ val parents = sequence {
+ if (visitParent) {
+ target.superType?.typeElement?.let {
+ yield(it)
+ }
+ }
+ if (visitInterfaces) {
+ yieldAll(target.getSuperInterfaceElements())
+ }
+ }
+ return sequence {
+ // group members by name for faster override checks
+ val selectionByName = mutableMapOf<String, MutableList<T>>()
+ suspend fun SequenceScope<T>.addToSelection(item: T) {
+ val key = getPartitionKey(item)
+ selectionByName.getOrPut(key) {
+ mutableListOf()
+ }.add(item)
+ yield(item)
+ }
+
+ suspend fun SequenceScope<T>.maybeAddToSelection(candidate: T) {
+ val partitionKey = candidate.getPartitionKey()
+ val existing = selectionByName[partitionKey] ?: emptyList()
+ if (isAcceptable(candidate, existing)) {
+ addToSelection(copyToTarget(candidate))
+ }
+ }
+
+ // yield everything in the root list
+ initialList.forEach {
+ addToSelection(it)
+ }
+ // now start yielding it from parents
+ parents.flatMap { it.getCandidateDeclarations() }.forEach {
+ maybeAddToSelection(copyToTarget(it))
+ }
+ }
+}
+
+private fun XTypeElement.canAccessSuperMethod(other: XMethodElement): Boolean {
+ if (other.isPublic() || other.isProtected()) {
+ return true
+ }
+ if (other.isPrivate()) {
+ return false
+ }
+ // check package
+ return packageName == other.enclosingElement.className.packageName()
+}
+
+/**
+ * see [XTypeElement.getAllFieldsIncludingPrivateSupers]
+ */
+internal fun collectFieldsIncludingPrivateSupers(
+ xTypeElement: XTypeElement
+): Sequence<XFieldElement> {
+ return collectDeclarations(
+ target = xTypeElement,
+ visitParent = true,
+ visitInterfaces = false,
+ getPartitionKey = XFieldElement::name,
+ initialList = xTypeElement.getDeclaredFields(),
+ copyToTarget = {
+ it.copyTo(xTypeElement)
+ },
+ isAcceptable = { _, existing ->
+ existing.isEmpty()
+ },
+ getCandidateDeclarations = XTypeElement::getAllFieldsIncludingPrivateSupers
+ )
+}
+
+/**
+ * see [XTypeElement.getAllMethods]
+ */
+internal fun collectAllMethods(
+ xTypeElement: XTypeElement
+): Sequence<XMethodElement> {
+ return collectDeclarations(
+ target = xTypeElement,
+ visitParent = true,
+ visitInterfaces = true,
+ getPartitionKey = XMethodElement::name,
+ initialList = xTypeElement.getDeclaredMethods(),
+ copyToTarget = {
+ it.copyTo(xTypeElement)
+ },
+ isAcceptable = { candidate, existing ->
+ when {
+ // my method, accept all
+ candidate.enclosingElement == xTypeElement -> true
+ // cannot access, reject
+ !xTypeElement.canAccessSuperMethod(candidate) -> false
+ // static in an interface
+ candidate.isStatic() &&
+ (candidate.enclosingElement as? XTypeElement)?.isInterface() == true ->
+ false
+ // accept if not overridden
+ else -> existing.none { it.overrides(candidate, xTypeElement) }
+ }
+ },
+ getCandidateDeclarations = XTypeElement::getAllMethods,
+ )
+}
\ No newline at end of file
diff --git a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/MethodCollector.kt b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/MethodCollector.kt
deleted file mode 100644
index 7cac32a..0000000
--- a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/MethodCollector.kt
+++ /dev/null
@@ -1,89 +0,0 @@
-/*
- * Copyright 2020 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package androidx.room.compiler.processing
-
-/**
- * Helper class to collect all methods of an [XTypeElement] to implement
- * [XTypeElement.getAllMethods].
- */
-private class MethodCollector(
- val target: XTypeElement
-) {
- // group methods by name for fast overrides check
- private val selectionByName = mutableMapOf<String, MutableList<XMethodElement>>()
-
- // we keep a duplicate list to preserve declaration order, makes the generated code match
- // user code
- private val selection = mutableListOf<XMethodElement>()
-
- fun collect() {
- val selection = target.getDeclaredMethods().forEach(::addToSelection)
-
- target.superType
- ?.typeElement
- ?.getAllMethods()
- ?.forEach(::addIfNotOverridden)
- target.getSuperInterfaceElements().forEach {
- it.getAllMethods().forEach {
- if (!it.isStatic()) {
- addIfNotOverridden(it)
- }
- }
- }
- return selection
- }
-
- fun getResult(): List<XMethodElement> {
- return selection
- }
-
- private fun addIfNotOverridden(candidate: XMethodElement) {
- if (!target.canAccessSuperMethod(candidate)) {
- return
- }
- val overridden = selectionByName[candidate.name]?.any { existing ->
- existing.overrides(candidate, target)
- } ?: false
- if (!overridden) {
- addToSelection(candidate.copyTo(target))
- }
- }
-
- private fun addToSelection(method: XMethodElement) {
- selectionByName.getOrPut(method.name) {
- mutableListOf()
- }.add(method)
- selection.add(method)
- }
-
- private fun XTypeElement.canAccessSuperMethod(other: XMethodElement): Boolean {
- if (other.isPublic() || other.isProtected()) {
- return true
- }
- if (other.isPrivate()) {
- return false
- }
- // check package
- return packageName == other.enclosingElement.className.packageName()
- }
-}
-
-internal fun XTypeElement.collectAllMethods(): List<XMethodElement> {
- val collector = MethodCollector(this)
- collector.collect()
- return collector.getResult()
-}
\ No newline at end of file
diff --git a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/XFieldElement.kt b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/XFieldElement.kt
index 6d9540f..f25053d 100644
--- a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/XFieldElement.kt
+++ b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/XFieldElement.kt
@@ -36,4 +36,6 @@
override val fallbackLocationText: String
get() = "$name in ${enclosingElement.fallbackLocationText}"
+
+ fun copyTo(newContainer: XTypeElement): XFieldElement
}
diff --git a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/XTypeElement.kt b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/XTypeElement.kt
index 90fe22c..9773a05 100644
--- a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/XTypeElement.kt
+++ b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/XTypeElement.kt
@@ -140,7 +140,7 @@
* does not include static methods in parent interfaces
*/
fun getAllMethods(): List<XMethodElement> {
- return collectAllMethods()
+ return collectAllMethods(this).toList()
}
/**
diff --git a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/javac/ElementExt.kt b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/javac/ElementExt.kt
index d9bee51..14e4b06 100644
--- a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/javac/ElementExt.kt
+++ b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/javac/ElementExt.kt
@@ -18,14 +18,7 @@
import androidx.room.compiler.processing.XNullability
import com.google.auto.common.MoreElements
-import com.google.auto.common.MoreTypes
import javax.lang.model.element.Element
-import javax.lang.model.element.ElementKind
-import javax.lang.model.element.TypeElement
-import javax.lang.model.element.VariableElement
-import javax.lang.model.type.TypeKind
-import javax.lang.model.util.ElementFilter
-import javax.lang.model.util.Elements
private val NONNULL_ANNOTATIONS = arrayOf(
androidx.annotation.NonNull::class.java,
@@ -37,36 +30,6 @@
org.jetbrains.annotations.Nullable::class.java
)
-/**
- * Returns all fields including private fields (including private fields in super). Removes
- * duplicate fields if class has a field with the same name as the parent.
- * Note that enum constants are not included in the list even thought they are fields in java.
- * To access enum constants, use [JavacTypeElement.JavacEnumTypeElement].
- */
-internal fun TypeElement.getAllFieldsIncludingPrivateSupers(
- elementUtils: Elements
-): Set<VariableElement> {
- val selection = ElementFilter
- .fieldsIn(elementUtils.getAllMembers(this))
- .filterIsInstance<VariableElement>()
- .filterNot { it.kind == ElementKind.ENUM_CONSTANT }
- .toMutableSet()
- val selectionNames = selection.mapTo(mutableSetOf()) {
- it.simpleName
- }
- if (superclass.kind != TypeKind.NONE) {
- val superFields = MoreTypes.asTypeElement(superclass)
- .getAllFieldsIncludingPrivateSupers(elementUtils)
- // accept super fields only if the name does not conflict
- superFields.forEach { superField ->
- if (selectionNames.add(superField.simpleName)) {
- selection.add(superField)
- }
- }
- }
- return selection
-}
-
@Suppress("UnstableApiUsage")
private fun Element.hasAnyOf(annotations: Array<Class<out Annotation>>) = annotations.any {
MoreElements.isAnnotationPresent(this, it)
diff --git a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/javac/JavacFieldElement.kt b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/javac/JavacFieldElement.kt
index dca2f56..3f0b3ba 100644
--- a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/javac/JavacFieldElement.kt
+++ b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/javac/JavacFieldElement.kt
@@ -41,4 +41,15 @@
override val enclosingElement: XTypeElement by lazy {
element.requireEnclosingType(env)
}
+
+ override fun copyTo(newContainer: XTypeElement): JavacFieldElement {
+ check(newContainer is JavacTypeElement) {
+ "Unexpected container (${newContainer::class}), expected JavacTypeElement"
+ }
+ return JavacFieldElement(
+ env = env,
+ containing = newContainer,
+ element = element
+ )
+ }
}
diff --git a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/javac/JavacTypeElement.kt b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/javac/JavacTypeElement.kt
index 85189bb..36d311f 100644
--- a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/javac/JavacTypeElement.kt
+++ b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/javac/JavacTypeElement.kt
@@ -22,6 +22,7 @@
import androidx.room.compiler.processing.XHasModifiers
import androidx.room.compiler.processing.XMethodElement
import androidx.room.compiler.processing.XTypeElement
+import androidx.room.compiler.processing.collectFieldsIncludingPrivateSupers
import androidx.room.compiler.processing.javac.kotlin.KotlinMetadataElement
import com.google.auto.common.MoreElements
import com.google.auto.common.MoreTypes
@@ -59,13 +60,15 @@
}
private val _declaredFields by lazy {
- ElementFilter.fieldsIn(element.enclosedElements).map {
- JavacFieldElement(
- env = env,
- element = it,
- containing = this
- )
- }
+ ElementFilter.fieldsIn(element.enclosedElements)
+ .filterNot { it.kind == ElementKind.ENUM_CONSTANT }
+ .map {
+ JavacFieldElement(
+ env = env,
+ element = it,
+ containing = this
+ )
+ }
}
override fun getDeclaredFields(): List<XFieldElement> {
@@ -73,15 +76,7 @@
}
private val _allFieldsIncludingPrivateSupers by lazy {
- element.getAllFieldsIncludingPrivateSupers(
- env.elementUtils
- ).map {
- JavacFieldElement(
- env = env,
- element = it,
- containing = this
- )
- }
+ collectFieldsIncludingPrivateSupers(this).toList()
}
override fun getAllFieldsIncludingPrivateSupers(): List<XFieldElement> {
diff --git a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspFieldElement.kt b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspFieldElement.kt
index 11a870b..689698e 100644
--- a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspFieldElement.kt
+++ b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspFieldElement.kt
@@ -20,6 +20,7 @@
import androidx.room.compiler.processing.XFieldElement
import androidx.room.compiler.processing.XHasModifiers
import androidx.room.compiler.processing.XType
+import androidx.room.compiler.processing.XTypeElement
import androidx.room.compiler.processing.ksp.KspAnnotated.UseSiteFilter.Companion.NO_USE_SITE_OR_FIELD
import com.google.devtools.ksp.symbol.KSPropertyDeclaration
@@ -63,11 +64,16 @@
)
}
- fun copyTo(newContaining: KspTypeElement) = KspFieldElement(
- env = env,
- declaration = declaration,
- containing = newContaining
- )
+ override fun copyTo(newContainer: XTypeElement): KspFieldElement {
+ check(newContainer is KspTypeElement) {
+ "Unexpected container (${newContainer::class}), expected KspTypeElement"
+ }
+ return KspFieldElement(
+ env = env,
+ declaration = declaration,
+ containing = newContainer
+ )
+ }
companion object {
fun create(
diff --git a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspTypeElement.kt b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspTypeElement.kt
index e58d3cc..1138d76 100644
--- a/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspTypeElement.kt
+++ b/room/room-compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspTypeElement.kt
@@ -25,10 +25,10 @@
import androidx.room.compiler.processing.XMethodElement
import androidx.room.compiler.processing.XType
import androidx.room.compiler.processing.XTypeElement
+import androidx.room.compiler.processing.collectFieldsIncludingPrivateSupers
import androidx.room.compiler.processing.ksp.KspAnnotated.UseSiteFilter.Companion.NO_USE_SITE
import androidx.room.compiler.processing.ksp.synthetic.KspSyntheticPropertyMethodElement
import androidx.room.compiler.processing.tryBox
-import com.google.devtools.ksp.getAllSuperTypes
import com.google.devtools.ksp.getConstructors
import com.google.devtools.ksp.getDeclaredFunctions
import com.google.devtools.ksp.getDeclaredProperties
@@ -37,7 +37,6 @@
import com.google.devtools.ksp.isPrivate
import com.google.devtools.ksp.symbol.ClassKind
import com.google.devtools.ksp.symbol.KSClassDeclaration
-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
@@ -134,32 +133,7 @@
}
private val _declaredFieldsIncludingSupers by lazy {
- // Read all properties from all supers and select the ones that are not overridden.
- val myPropertyFields = getDeclaredFields()
- val selectedNames = myPropertyFields.mapTo(mutableSetOf()) {
- it.name
- }
- val selection = mutableListOf<KSPropertyDeclaration>()
- declaration.getAllSuperTypes().map {
- it.declaration
- }.filterIsInstance(KSClassDeclaration::class.java)
- .filter {
- it.classKind != ClassKind.INTERFACE
- }
- .flatMap {
- it.getDeclaredProperties().asSequence()
- }.forEach {
- if (selectedNames.add(it.simpleName.asString())) {
- selection.add(it)
- }
- }
- myPropertyFields + selection.map {
- KspFieldElement(
- env = env,
- declaration = it,
- containing = this
- )
- }
+ collectFieldsIncludingPrivateSupers(this).toList()
}
private val syntheticGetterSetterMethods: List<XMethodElement> by lazy {
diff --git a/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XTypeElementTest.kt b/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XTypeElementTest.kt
index 8ea3fae..bd4fdbd 100644
--- a/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XTypeElementTest.kt
+++ b/room/room-compiler-processing/src/test/java/androidx/room/compiler/processing/XTypeElementTest.kt
@@ -974,6 +974,9 @@
contains("x")
containsNoneOf("VAL1", "VAL2")
}
+ assertWithMessage("$qName does not report enum constants in declared fields")
+ .that(typeElement.getDeclaredFields().map { it.name })
+ .containsExactly("x")
}
}
}
