Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f5959281 authored by Matt Gilbride's avatar Matt Gilbride Committed by Android (Google) Code Review
Browse files

Merge "Fix EnforcePermission lint false positive on generated interfaces"

parents 49f594ca 3fc484ce
Loading
Loading
Loading
Loading
+1 −1
Original line number Diff line number Diff line
@@ -43,7 +43,7 @@ java_test_host {
    // compiled for java 15. The soong build doesn't support
    // java 15 yet, so we can't compile against "lint". Disable
    // the test until java 15 is supported.
    enabled: false,
    // enabled: false,
    srcs: ["checks/src/test/java/**/*.kt"],
    static_libs: [
        "AndroidGlobalLintChecker",
+40 −0
Original line number Diff line number Diff line
@@ -114,6 +114,13 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner {
        val overridingName = "${overridingClass.name}.${overridingMethod.name}"
        val overriddenName = "${overriddenClass.name}.${overriddenMethod.name}"
        if (overridingAnnotation == null) {
            if (shouldIgnoreGeneratedMethod(
                            context,
                            overriddenClass = overriddenClass,
                            overridingClass = overridingClass)
            ) {
                return
            }
            val msg = "The method $overridingName overrides the method $overriddenName which " +
                "is annotated with @EnforcePermission. The same annotation must be used " +
                "on $overridingName"
@@ -215,6 +222,39 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner {
        }
    }

    /**
     * since this lint runs globally, it will also run against generated
     * test code e.g.
     * system/tools/aidl/tests/golden_output/aidl-test-interface-permission-java-source/gen/android/aidl/tests/permission/IProtected.java
     * system/tools/aidl/tests/golden_output/aidl-test-interface-permission-java-source/gen/android/aidl/tests/permission/IProtectedInterface.java
     * we do not want to report errors against generated `Stub` and `Proxy` classes in those files
     */
    private fun shouldIgnoreGeneratedMethod(
            context: JavaContext,
            overriddenClass: PsiClass,
            overridingClass: PsiClass,

    ): Boolean {
        if (isInterfaceAndExtendsIInterface(overriddenClass) &&
                context.evaluator.isStatic(overridingClass)) {
            if (overridingClass.name == "Default") return true
            if (overridingClass.name == "Proxy") {
                val shouldBeStub = overridingClass.parent as? PsiClass ?: return false
                return shouldBeStub.name == "Stub" &&
                        context.evaluator.isAbstract(shouldBeStub) &&
                        context.evaluator.isStatic(shouldBeStub) &&
                        shouldBeStub.extendsList?.referenceElements
                        ?.any { it.qualifiedName == BINDER_CLASS } == true
            }
        }
        return false
    }

    private fun isInterfaceAndExtendsIInterface(overriddenClass: PsiClass): Boolean =
            overriddenClass.isInterface &&
                    overriddenClass.extendsList?.referenceElements
                    ?.any { it.qualifiedName == IINTERFACE_INTERFACE } == true

    companion object {
        val EXPLANATION = """
            The @EnforcePermission annotation is used to indicate that the underlying binder code
+2 −1
Original line number Diff line number Diff line
@@ -28,8 +28,8 @@ import com.android.tools.lint.detector.api.SourceCodeScanner
import com.intellij.psi.PsiElement
import org.jetbrains.uast.UBlockExpression
import org.jetbrains.uast.UDeclarationsExpression
import org.jetbrains.uast.UExpression
import org.jetbrains.uast.UElement
import org.jetbrains.uast.UExpression
import org.jetbrains.uast.UMethod

class EnforcePermissionHelperDetector : Detector(), SourceCodeScanner {
@@ -40,6 +40,7 @@ class EnforcePermissionHelperDetector : Detector(), SourceCodeScanner {

    private inner class AidlStubHandler(val context: JavaContext) : UElementHandler() {
        override fun visitMethod(node: UMethod) {
            if (context.evaluator.isAbstract(node)) return
            if (!node.hasAnnotation(ANNOTATION_ENFORCE_PERMISSION)) return

            val targetExpression = "super.${node.name}$HELPER_SUFFIX()"
+557 −0

File added.

Preview size limit exceeded, changes collapsed.

+22 −0
Original line number Diff line number Diff line
@@ -189,6 +189,28 @@ must be used on IBar.Stub. Did you forget to annotate the AIDL definition? \
1 errors, 0 warnings""".addLineContinuation())
    }

    fun testDetectIssuesMissingAnnotationOnMethodWhenClassIsCalledDefault() {
        lint().files(java(
            """
            package test.pkg;
            public class Default extends IFooMethod.Stub {
                public void testMethod() {}
            }
            """).indented(),
            *stubs
        )
            .run()
            .expect(
                """
                src/test/pkg/Default.java:3: Error: The method Default.testMethod \
                overrides the method Stub.testMethod which is annotated with @EnforcePermission. The same annotation must be used on Default.testMethod [MissingEnforcePermissionAnnotation]
                    public void testMethod() {}
                                ~~~~~~~~~~
                1 errors, 0 warnings 
                """.addLineContinuation()
            )
    }

    /* Stubs */

    // A service with permission annotation on the class.
Loading