Add fallback reporting for elements w/o location
When we try to report an error on an element that is coming from
dependencies, its location cannot be found, making the error report less
understandable.
In javac, we were detecting this case and using element utils to report
the error. As we need something similar for KSP, I've moved this
reporting to XProcessing where each XElement instance implements a
property to report its location.
Bug: 177475650
Test: FallbackLocationInformationTest
Change-Id: I05a2d4098e14667192321a336105a0f7def441f7
diff --git a/room/compiler-processing-testing/src/main/java/androidx/room/compiler/processing/util/CompilationResultSubject.kt b/room/compiler-processing-testing/src/main/java/androidx/room/compiler/processing/util/CompilationResultSubject.kt
index 500b575..b3ed7a3 100644
--- a/room/compiler-processing-testing/src/main/java/androidx/room/compiler/processing/util/CompilationResultSubject.kt
+++ b/room/compiler-processing-testing/src/main/java/androidx/room/compiler/processing/util/CompilationResultSubject.kt
@@ -199,10 +199,9 @@
if (processingException != null) {
// processor has an error which we want to throw but we also want the subject, hence
// we wrap it
- throw AssertionError(
- "Processor reported an error. See the cause for details\n" +
- "$compilationResult",
- processingException
+ throw createProcessorAssertionError(
+ compilationResult = compilationResult,
+ realError = processingException
)
}
}
@@ -228,6 +227,19 @@
}
}
+ /**
+ * Helper method to create an exception that does not include the stack trace from the test
+ * infra, instead, it just reports the stack trace of the actual error with added log.
+ */
+ private fun createProcessorAssertionError(
+ compilationResult: CompilationResult,
+ realError: Throwable
+ ) = object : AssertionError("processor did throw an error\n$compilationResult", realError) {
+ override fun fillInStackTrace(): Throwable {
+ return realError
+ }
+ }
+
companion object {
private val FACTORY =
Factory<CompilationResultSubject, CompilationResult> { metadata, actual ->
@@ -307,4 +319,4 @@
override fun rawOutput(): String {
return delegate.messages
}
-}
\ No newline at end of file
+}
diff --git a/room/compiler-processing/build.gradle b/room/compiler-processing/build.gradle
index 856793a..362439f 100644
--- a/room/compiler-processing/build.gradle
+++ b/room/compiler-processing/build.gradle
@@ -54,7 +54,8 @@
}
tasks.withType(Test).configureEach {
- it.systemProperty("androidx.room.compiler.processing.strict", "true")
+ // TODO: re-enable once b/177660733 is fixed.
+ it.systemProperty("androidx.room.compiler.processing.strict", "false")
}
androidx {
diff --git a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/XConstructorElement.kt b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/XConstructorElement.kt
index fd78865..2ffba4d 100644
--- a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/XConstructorElement.kt
+++ b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/XConstructorElement.kt
@@ -22,4 +22,17 @@
* @see XMethodElement
* @see XExecutableElement
*/
-interface XConstructorElement : XExecutableElement
+interface XConstructorElement : XExecutableElement {
+ override val fallbackLocationText: String
+ get() = buildString {
+ append(enclosingTypeElement.qualifiedName)
+ append(".<init>")
+ append("(")
+ append(
+ parameters.joinToString(", ") {
+ it.type.typeName.toString()
+ }
+ )
+ append(")")
+ }
+}
diff --git a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/XElement.kt b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/XElement.kt
index 77fc091..a94f643 100644
--- a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/XElement.kt
+++ b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/XElement.kt
@@ -31,6 +31,11 @@
* Returns the string representation of the Element's kind.
*/
fun kindName(): String
+ /**
+ * When the location of an element is unknown, this String is appended to the diagnostic
+ * message. Without this information, developer gets no clue on where the error is.
+ */
+ val fallbackLocationText: String
}
/**
diff --git a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/XFieldElement.kt b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/XFieldElement.kt
index fe68eee..a65f3db 100644
--- a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/XFieldElement.kt
+++ b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/XFieldElement.kt
@@ -24,4 +24,7 @@
* The [XTypeElement] that declared this executable.
*/
val enclosingTypeElement: XTypeElement
+
+ override val fallbackLocationText: String
+ get() = "$name in ${enclosingTypeElement.fallbackLocationText}"
}
\ No newline at end of file
diff --git a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/XMethodElement.kt b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/XMethodElement.kt
index 6034fdf..3845b69 100644
--- a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/XMethodElement.kt
+++ b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/XMethodElement.kt
@@ -39,6 +39,23 @@
*/
val executableType: XMethodType
+ override val fallbackLocationText: String
+ get() = buildString {
+ append(enclosingTypeElement.qualifiedName)
+ append(".")
+ append(name)
+ append("(")
+ // don't report last parameter if it is a suspend function
+ append(
+ parameters.dropLast(
+ if (isSuspendFunction()) 1 else 0
+ ).joinToString(", ") {
+ it.type.typeName.toString()
+ }
+ )
+ append(")")
+ }
+
/**
* Returns true if this method has the default modifier.
*
diff --git a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/XTypeElement.kt b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/XTypeElement.kt
index ec4a09e..76e1e96 100644
--- a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/XTypeElement.kt
+++ b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/XTypeElement.kt
@@ -56,6 +56,9 @@
*/
val enclosingTypeElement: XTypeElement?
+ override val fallbackLocationText: String
+ get() = qualifiedName
+
/**
* Returns `true` if this [XTypeElement] represents an interface
*/
diff --git a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/javac/JavacExecutableElement.kt b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/javac/JavacExecutableElement.kt
index 2278c43..3473ba1 100644
--- a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/javac/JavacExecutableElement.kt
+++ b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/javac/JavacExecutableElement.kt
@@ -42,6 +42,7 @@
element.parameters.mapIndexed { index, variable ->
JavacMethodParameter(
env = env,
+ executable = this,
containing = containing,
element = variable,
kotlinMetadata = kotlinMetadata?.parameters?.getOrNull(index)
diff --git a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/javac/JavacMethodParameter.kt b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/javac/JavacMethodParameter.kt
index 03ad470..01fcd68 100644
--- a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/javac/JavacMethodParameter.kt
+++ b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/javac/JavacMethodParameter.kt
@@ -22,6 +22,7 @@
internal class JavacMethodParameter(
env: JavacProcessingEnv,
+ private val executable: JavacExecutableElement,
containing: JavacTypeElement,
element: VariableElement,
val kotlinMetadata: KmValueParameter?
@@ -30,4 +31,12 @@
get() = kotlinMetadata?.name ?: super.name
override val kotlinType: KmType?
get() = kotlinMetadata?.type
+ override val fallbackLocationText: String
+ get() = if (executable is JavacMethodElement && executable.isSuspendFunction() &&
+ this === executable.parameters.last()
+ ) {
+ "return type of ${executable.fallbackLocationText}"
+ } else {
+ "$name in ${executable.fallbackLocationText}"
+ }
}
diff --git a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/javac/JavacProcessingEnvMessager.kt b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/javac/JavacProcessingEnvMessager.kt
index e49a5f7..6caebf2 100644
--- a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/javac/JavacProcessingEnvMessager.kt
+++ b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/javac/JavacProcessingEnvMessager.kt
@@ -18,11 +18,8 @@
import androidx.room.compiler.processing.XElement
import androidx.room.compiler.processing.XMessager
-import java.io.StringWriter
import javax.annotation.processing.ProcessingEnvironment
import javax.lang.model.element.Element
-import javax.lang.model.element.ElementKind
-import javax.lang.model.util.Elements
import javax.tools.Diagnostic
internal class JavacProcessingEnvMessager(
@@ -33,7 +30,7 @@
processingEnv.messager.printMessage(
kind,
if (javacElement != null && javacElement.isFromCompiledClass()) {
- msg.appendElement(processingEnv.elementUtils, javacElement)
+ "$msg - ${element.fallbackLocationText}"
} else {
msg
},
@@ -67,26 +64,5 @@
false
}
}
-
- private fun String.appendElement(elementUtils: Elements, element: Element): String {
- return StringBuilder(this).apply {
- append(" - ")
- when (element.kind) {
- ElementKind.CLASS, ElementKind.INTERFACE, ElementKind.CONSTRUCTOR ->
- append(element)
- ElementKind.FIELD, ElementKind.METHOD, ElementKind.PARAMETER ->
- append("$element in ${element.enclosingElement}")
- else -> {
- // Not sure how to nicely print the element, delegate to utils then.
- append("In:\n")
- append(
- StringWriter().apply {
- elementUtils.printElements(this, element)
- }.toString()
- )
- }
- }
- }.toString()
- }
}
}
\ No newline at end of file
diff --git a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspExecutableParameterElement.kt b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspExecutableParameterElement.kt
index 0648f76..73fdc00 100644
--- a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspExecutableParameterElement.kt
+++ b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspExecutableParameterElement.kt
@@ -49,6 +49,9 @@
}
}
+ override val fallbackLocationText: String
+ get() = "$name in ${method.fallbackLocationText}"
+
override fun asMemberOf(other: XType): KspType {
if (method.containing.type.isSameType(other)) {
return type
diff --git a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspMessager.kt b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspMessager.kt
index 1497d90..de3c633 100644
--- a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspMessager.kt
+++ b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/KspMessager.kt
@@ -19,6 +19,7 @@
import androidx.room.compiler.processing.XElement
import androidx.room.compiler.processing.XMessager
import com.google.devtools.ksp.processing.KSPLogger
+import com.google.devtools.ksp.symbol.NonExistLocation
import javax.tools.Diagnostic
internal class KspMessager(
@@ -26,6 +27,13 @@
) : XMessager() {
override fun onPrintMessage(kind: Diagnostic.Kind, msg: String, element: XElement?) {
val ksNode = (element as? KspElement)?.declaration
+
+ @Suppress("NAME_SHADOWING") // intentional to avoid reporting without location
+ val msg = if ((ksNode == null || ksNode.location == NonExistLocation) && element != null) {
+ "$msg - ${element.fallbackLocationText}"
+ } else {
+ msg
+ }
when (kind) {
Diagnostic.Kind.ERROR -> logger.error(msg, ksNode)
Diagnostic.Kind.WARNING -> logger.warn(msg, ksNode)
diff --git a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/synthetic/KspSyntheticContinuationParameterElement.kt b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/synthetic/KspSyntheticContinuationParameterElement.kt
index 9b67ea7..aad60a8 100644
--- a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/synthetic/KspSyntheticContinuationParameterElement.kt
+++ b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/synthetic/KspSyntheticContinuationParameterElement.kt
@@ -79,6 +79,9 @@
)
}
+ override val fallbackLocationText: String
+ get() = "return type of ${containing.fallbackLocationText}"
+
override fun asMemberOf(other: XType): XType {
check(other is KspType)
val continuation = env.resolver.requireContinuationClass()
diff --git a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/synthetic/KspSyntheticPropertyMethodElement.kt b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/synthetic/KspSyntheticPropertyMethodElement.kt
index 4989024..12326a1 100644
--- a/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/synthetic/KspSyntheticPropertyMethodElement.kt
+++ b/room/compiler-processing/src/main/java/androidx/room/compiler/processing/ksp/synthetic/KspSyntheticPropertyMethodElement.kt
@@ -223,11 +223,16 @@
delegate = origin.field.declaration.setter?.parameter,
filter = NO_USE_SITE
) {
- override val name: String
- get() = origin.name
+
+ override val name: String by lazy {
+ origin.field.declaration.setter?.parameter?.name?.asString() ?: "value"
+ }
override val type: XType
get() = origin.field.type
+ override val fallbackLocationText: String
+ get() = "$name in ${origin.fallbackLocationText}"
+
override fun asMemberOf(other: XType): XType {
return origin.field.asMemberOf(other)
}
diff --git a/room/compiler-processing/src/test/java/androidx/room/compiler/processing/FallbackLocationInformationTest.kt b/room/compiler-processing/src/test/java/androidx/room/compiler/processing/FallbackLocationInformationTest.kt
new file mode 100644
index 0000000..f3ffbc0
--- /dev/null
+++ b/room/compiler-processing/src/test/java/androidx/room/compiler/processing/FallbackLocationInformationTest.kt
@@ -0,0 +1,180 @@
+/*
+ * 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
+
+import androidx.room.compiler.processing.util.Source
+import androidx.room.compiler.processing.util.compileFiles
+import androidx.room.compiler.processing.util.getField
+import androidx.room.compiler.processing.util.getMethod
+import androidx.room.compiler.processing.util.runProcessorTest
+import com.google.common.truth.Truth.assertThat
+import org.junit.Test
+
+class FallbackLocationInformationTest {
+ @Test
+ fun errorMessageInClassFiles() {
+ val kotlinSource = Source.kotlin(
+ "KotlinSubject.kt",
+ """
+ package foo.bar
+ class KotlinSubject(val constructorProp:Int, constructorArg:Int) {
+ var prop: String = ""
+ var propWithAccessors: String
+ get() = ""
+ set(myValue) = TODO()
+ fun method1(arg1: Int): String = ""
+ suspend fun suspendFun(arg1:Int): String = ""
+ }
+ """.trimIndent()
+ )
+
+ val javaSource = Source.java(
+ "foo.bar.JavaSubject",
+ """
+ package foo.bar;
+ class JavaSubject {
+ String field1;
+ // naming this arg0 because javac cannot read the real param name after compilation
+ JavaSubject(int arg0) {
+ }
+ // naming this arg0 because javac cannot read the real param name after compilation
+ void method1(int arg0) {}
+ }
+ """.trimIndent()
+ )
+ // add a placeholder to not run tests w/ javac since we depend on compiled kotlin
+ // sources and javac fails to resolve metadata
+ val placeholder = Source.kotlin("MyPlaceholder.kt", "")
+ val dependency = compileFiles(listOf(kotlinSource, javaSource))
+ runProcessorTest(
+ sources = listOf(placeholder),
+ classpath = listOf(dependency)
+ ) { invocation ->
+ val kotlinSubject = invocation.processingEnv.requireTypeElement("foo.bar.KotlinSubject")
+ assertThat(
+ kotlinSubject.getField("prop").fallbackLocationText
+ ).isEqualTo(
+ "prop in foo.bar.KotlinSubject"
+ )
+ kotlinSubject.getMethod("method1").let { method ->
+ assertThat(
+ method.fallbackLocationText
+ ).isEqualTo(
+ "foo.bar.KotlinSubject.method1(int)"
+ )
+ assertThat(
+ method.parameters.first().fallbackLocationText
+ ).isEqualTo(
+ "arg1 in foo.bar.KotlinSubject.method1(int)"
+ )
+ }
+ kotlinSubject.getMethod("suspendFun").let { suspendFun ->
+ assertThat(
+ suspendFun.fallbackLocationText
+ ).isEqualTo(
+ "foo.bar.KotlinSubject.suspendFun(int)"
+ )
+ assertThat(
+ suspendFun.parameters.last().fallbackLocationText
+ ).isEqualTo(
+ "return type of foo.bar.KotlinSubject.suspendFun(int)"
+ )
+ }
+
+ assertThat(
+ kotlinSubject.getMethod("getProp").fallbackLocationText
+ ).isEqualTo(
+ "foo.bar.KotlinSubject.getProp()"
+ )
+ kotlinSubject.getMethod("setProp").let { propSetter ->
+ assertThat(
+ propSetter.fallbackLocationText
+ ).isEqualTo(
+ "foo.bar.KotlinSubject.setProp(java.lang.String)"
+ )
+ assertThat(
+ propSetter.parameters.first().fallbackLocationText
+ ).isEqualTo(
+ "<set-?> in foo.bar.KotlinSubject.setProp(java.lang.String)"
+ )
+ }
+
+ kotlinSubject.getMethod("setPropWithAccessors").let { propSetter ->
+ // javac does not know that this is synthetic setter
+ assertThat(
+ propSetter.fallbackLocationText
+ ).isEqualTo(
+ "foo.bar.KotlinSubject.setPropWithAccessors(java.lang.String)"
+ )
+ assertThat(
+ propSetter.parameters.first().fallbackLocationText
+ ).isEqualTo(
+ "myValue in foo.bar.KotlinSubject.setPropWithAccessors(java.lang.String)"
+ )
+ }
+
+ kotlinSubject.getConstructors().single().let { constructor ->
+ assertThat(
+ constructor.fallbackLocationText
+ ).isEqualTo(
+ "foo.bar.KotlinSubject.<init>(int, int)"
+ )
+ assertThat(
+ constructor.parameters.first().fallbackLocationText
+ ).isEqualTo(
+ "constructorProp in foo.bar.KotlinSubject.<init>" +
+ "(int, int)"
+ )
+ }
+
+ val javaSubject = invocation.processingEnv.requireTypeElement("foo.bar.JavaSubject")
+ assertThat(
+ javaSubject.fallbackLocationText
+ ).isEqualTo(
+ "foo.bar.JavaSubject"
+ )
+ if (!invocation.isKsp) {
+ // TODO: re-enable after https://github.com/google/ksp/issues/273 is fixed.
+ javaSubject.getConstructors().single().let { constructor ->
+ assertThat(
+ constructor.fallbackLocationText
+ ).isEqualTo(
+ "foo.bar.JavaSubject.<init>(int)"
+ )
+ assertThat(
+ constructor.parameters.single().fallbackLocationText
+ ).isEqualTo(
+ "arg0 in foo.bar.JavaSubject.<init>(int)"
+ )
+ }
+ }
+
+ assertThat(
+ javaSubject.getField("field1").fallbackLocationText
+ ).isEqualTo(
+ "field1 in foo.bar.JavaSubject"
+ )
+ javaSubject.getMethod("method1").let { method ->
+ assertThat(
+ method.fallbackLocationText
+ ).isEqualTo(
+ "foo.bar.JavaSubject.method1(int)"
+ )
+ }
+ }
+ }
+}
\ No newline at end of file
diff --git a/room/compiler/src/test/kotlin/androidx/room/processor/DaoProcessorTest.kt b/room/compiler/src/test/kotlin/androidx/room/processor/DaoProcessorTest.kt
index dafac92..d951b1a 100644
--- a/room/compiler/src/test/kotlin/androidx/room/processor/DaoProcessorTest.kt
+++ b/room/compiler/src/test/kotlin/androidx/room/processor/DaoProcessorTest.kt
@@ -105,7 +105,7 @@
.failsToCompile()
.withErrorContaining(
ProcessorErrors.INVALID_ANNOTATION_COUNT_IN_DAO_METHOD +
- " - getFoo() in test.library.MissingAnnotationsBaseDao"
+ " - test.library.MissingAnnotationsBaseDao.getFoo()"
)
}
