Check if CVF fix method already exists before adding it
Test: Added test with existing fix method
Bug: 243402089
Change-Id: Ib8c094b3915cf4c140d0029a2660e4c5c2ebd32d
diff --git a/lint-checks/integration-tests/src/main/java/androidx/AutofixUnsafeReferenceWithExistingFix.java b/lint-checks/integration-tests/src/main/java/androidx/AutofixUnsafeReferenceWithExistingFix.java
new file mode 100644
index 0000000..efa821b
--- /dev/null
+++ b/lint-checks/integration-tests/src/main/java/androidx/AutofixUnsafeReferenceWithExistingFix.java
@@ -0,0 +1,59 @@
+/*
+ * Copyright 2022 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;
+
+import android.content.res.ColorStateList;
+import android.graphics.drawable.Drawable;
+import android.view.View;
+
+import androidx.annotation.DoNotInline;
+import androidx.annotation.RequiresApi;
+
+/**
+ * Test class containing unsafe method references.
+ */
+@SuppressWarnings("unused")
+public class AutofixUnsafeReferenceWithExistingFix {
+
+ /**
+ * Unsafe reference to a new API with an already existing fix method in Api21Impl.
+ */
+ @RequiresApi(21)
+ void unsafeReferenceFixMethodExists(View view) {
+ view.setBackgroundTintList(new ColorStateList(null, null));
+ }
+
+ /**
+ * Unsafe reference to a new API without an existing fix method, but requiring API 21.
+ */
+ @RequiresApi(21)
+ void unsafeReferenceFixClassExists(Drawable drawable) {
+ drawable.getOutline(null);
+ }
+
+ @RequiresApi(21)
+ static class Api21Impl {
+ private Api21Impl() {
+ // This class is not instantiable.
+ }
+
+ @DoNotInline
+ static void setBackgroundTintList(View view, ColorStateList tint) {
+ view.setBackgroundTintList(tint);
+ }
+ }
+}
diff --git a/lint-checks/src/main/java/androidx/build/lint/ClassVerificationFailureDetector.kt b/lint-checks/src/main/java/androidx/build/lint/ClassVerificationFailureDetector.kt
index e46c90b..4bc5ab0 100644
--- a/lint-checks/src/main/java/androidx/build/lint/ClassVerificationFailureDetector.kt
+++ b/lint-checks/src/main/java/androidx/build/lint/ClassVerificationFailureDetector.kt
@@ -509,15 +509,18 @@
// The host class should never be null if we're looking at Java code.
val callContainingClass = call.getContainingUClass() ?: return null
- val (wrapperMethodName, methodForInsertion) = generateWrapperMethod(
- method,
- // Find what type the result of this call is used as
- getExpectedTypeByParent(callPsi)
- ) ?: return null
+ val (wrapperMethodName, wrapperMethodParams, methodForInsertion) =
+ generateWrapperMethod(
+ method,
+ // Find what type the result of this call is used as
+ getExpectedTypeByParent(callPsi)
+ ) ?: return null
val (wrapperClassName, insertionPoint, insertionSource) = generateInsertionSource(
api,
callContainingClass,
+ wrapperMethodName,
+ wrapperMethodParams,
methodForInsertion
)
@@ -529,22 +532,29 @@
wrapperMethodName
)
- return fix().name("Extract to static inner class")
- .composite(
- fix()
- .replace()
- .range(insertionPoint)
- .beginning()
- .with(insertionSource)
- .shortenNames()
- .build(),
- fix()
- .replace()
- .range(context.getLocation(call))
- .with(replacementCall)
- .shortenNames()
- .build(),
+ val fix = fix()
+ .name("Extract to static inner class")
+ .composite()
+ .add(fix()
+ .replace()
+ .range(context.getLocation(call))
+ .with(replacementCall)
+ .shortenNames()
+ .build()
)
+
+ if (insertionPoint != null) {
+ fix.add(fix()
+ .replace()
+ .range(insertionPoint)
+ .beginning()
+ .with(insertionSource)
+ .shortenNames()
+ .build()
+ )
+ }
+
+ return fix.build()
}
/**
@@ -587,6 +597,8 @@
* Generates source code for a wrapper method and class (where applicable) and calculates
* the insertion point. If the wrapper class already exists, returns source code for the
* method body only with an insertion point at the end of the existing wrapper class body.
+ * If the wrapper class and method both already exists, just returns the name of the
+ * wrapper class.
*
* Source code follows the general format:
*
@@ -600,19 +612,26 @@
*
* @param api API level at which the platform method can be safely called
* @param callContainingClass Class containing the call to the platform method
+ * @param wrapperMethodName The name of the wrapper method, used to check if the wrapper
+ * method already exists
+ * @param wrapperMethodParams List of the types of the wrapper method's parameters, used to
+ * check if the wrapper method already exists
* @param wrapperMethodBody Source code for the wrapper method
* @return Triple containing (1) the name of the static wrapper class, (2) the insertion
- * point for the generated source code, and (3) generated source code for a static wrapper
- * method, including a static wrapper class if necessary
+ * point for the generated source code (or null if the wrapper method already exists), and
+ * (3) generated source code for a static wrapper method, including a static wrapper class
+ * if necessary (or null if the wrapper method already exists)
*/
private fun generateInsertionSource(
api: Int,
callContainingClass: UClass,
+ wrapperMethodName: String,
+ wrapperMethodParams: List<PsiType>,
wrapperMethodBody: String,
- ): Triple<String, Location, String> {
+ ): Triple<String, Location?, String?> {
val wrapperClassName = "Api${api}Impl"
- val implInsertionPoint: Location
- val implForInsertion: String
+ val implInsertionPoint: Location?
+ val implForInsertion: String?
val existingWrapperClass = callContainingClass.innerClasses.find { innerClass ->
innerClass.name == wrapperClassName
@@ -631,8 +650,18 @@
""".trimIndent()
} else {
- implInsertionPoint = context.getLocation(existingWrapperClass.lastChild)
- implForInsertion = wrapperMethodBody.trimIndent()
+ val existingWrapperMethod = existingWrapperClass.methods.find { method ->
+ method.name == wrapperMethodName &&
+ wrapperMethodParams == getParameterTypes(method)
+ }
+ if (existingWrapperMethod == null) {
+ implInsertionPoint = context.getLocation(existingWrapperClass.lastChild)
+ // Add a newline to force the `}`s for the class and method onto different lines
+ implForInsertion = wrapperMethodBody.trimIndent() + "\n"
+ } else {
+ implInsertionPoint = null
+ implForInsertion = null
+ }
}
return Triple(
@@ -727,7 +756,7 @@
private fun generateWrapperMethod(
method: PsiMethod,
expectedReturnType: PsiType?
- ): Pair<String, String>? {
+ ): Triple<String, List<PsiType>, String>? {
val evaluator = context.evaluator
val isStatic = evaluator.isStatic(method)
val isConstructor = method.isConstructor
@@ -753,6 +782,9 @@
}
val typedParamsStr = (listOfNotNull(hostParam) + typedParams).joinToString(", ")
+ val paramTypes = listOf(PsiTypesUtil.getClassType(containingClass)) +
+ getParameterTypes(method)
+
val namedParamsStr = method.parameters.joinToString(separator = ", ") { param ->
"${param.name}"
}
@@ -797,8 +829,9 @@
val returnStmtStr = if ("void" == returnTypeStr) "" else "return "
- return Pair(
+ return Triple(
wrapperMethodName,
+ paramTypes,
"""
@androidx.annotation.DoNotInline
static $typeParamsStr$returnTypeStr $wrapperMethodName($typedParamsStr) {
@@ -809,6 +842,12 @@
}
/**
+ * Returns a list of the method's parameter types.
+ */
+ private fun getParameterTypes(method: PsiMethod): List<PsiType> =
+ method.parameterList.parameters.map { it.type }
+
+ /**
* Check if the specified class is available at the min SDK.
*/
private fun classAvailableAtMinSdk(className: String): Boolean {
diff --git a/lint-checks/src/test/java/androidx/build/lint/ClassVerificationFailureDetectorTest.kt b/lint-checks/src/test/java/androidx/build/lint/ClassVerificationFailureDetectorTest.kt
index 945254e..efdf27c 100644
--- a/lint-checks/src/test/java/androidx/build/lint/ClassVerificationFailureDetectorTest.kt
+++ b/lint-checks/src/test/java/androidx/build/lint/ClassVerificationFailureDetectorTest.kt
@@ -18,6 +18,7 @@
package androidx.build.lint
+import androidx.build.lint.Stubs.Companion.DoNotInline
import androidx.build.lint.Stubs.Companion.RequiresApi
import androidx.build.lint.Stubs.Companion.IntRange
import org.junit.Ignore
@@ -347,6 +348,48 @@
check(*input).expectFixDiffs(expectedFix)
}
+ @Test
+ fun `Auto-fix unsafe reference in Java source when the fix code already exists`() {
+ val input = arrayOf(
+ javaSample("androidx.AutofixUnsafeReferenceWithExistingFix"),
+ RequiresApi,
+ DoNotInline
+ )
+
+ /* ktlint-disable max-line-length */
+ val expected = """
+src/androidx/AutofixUnsafeReferenceWithExistingFix.java:37: Error: This call references a method added in API level 21; however, the containing class androidx.AutofixUnsafeReferenceWithExistingFix is reachable from earlier API levels and will fail run-time class verification. [ClassVerificationFailure]
+ view.setBackgroundTintList(new ColorStateList(null, null));
+ ~~~~~~~~~~~~~~~~~~~~~
+src/androidx/AutofixUnsafeReferenceWithExistingFix.java:45: Error: This call references a method added in API level 21; however, the containing class androidx.AutofixUnsafeReferenceWithExistingFix is reachable from earlier API levels and will fail run-time class verification. [ClassVerificationFailure]
+ drawable.getOutline(null);
+ ~~~~~~~~~~
+2 errors, 0 warnings
+ """
+
+ val expectedFix = """
+Fix for src/androidx/AutofixUnsafeReferenceWithExistingFix.java line 37: Extract to static inner class:
+@@ -37 +37
+- view.setBackgroundTintList(new ColorStateList(null, null));
++ Api21Impl.setBackgroundTintList(view, new ColorStateList(null, null));
+Fix for src/androidx/AutofixUnsafeReferenceWithExistingFix.java line 45: Extract to static inner class:
+@@ -45 +45
+- drawable.getOutline(null);
++ Api21Impl.getOutline(drawable, null);
+@@ -58 +58
+- }
++ @annotation.DoNotInline
++ static void getOutline(Drawable drawable, android.graphics.Outline outline) {
++ drawable.getOutline(outline);
+@@ -60 +62
++ }
++ }
+ """
+ /* ktlint-enable max-line-length */
+
+ check(*input).expect(expected).expectFixDiffs(expectedFix)
+ }
+
@Ignore("Ignored until the fix for b/241573146 is in the current version of studio")
@Test
fun `Detection and auto-fix for qualified expressions (issue 205026874)`() {
diff --git a/lint-checks/src/test/java/androidx/build/lint/Stubs.kt b/lint-checks/src/test/java/androidx/build/lint/Stubs.kt
index b48f7d9..3feb926 100644
--- a/lint-checks/src/test/java/androidx/build/lint/Stubs.kt
+++ b/lint-checks/src/test/java/androidx/build/lint/Stubs.kt
@@ -261,6 +261,23 @@
annotation class Ignore
"""
)
+
+ val DoNotInline = TestFiles.java(
+ """
+package androidx.annotation;
+
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.RetentionPolicy.CLASS;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+@Retention(CLASS)
+@Target({METHOD})
+public @interface DoNotInline {
+}
+ """
+ )
/* ktlint-enable max-line-length */
}
}
