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

Commit 165a0f6a authored by Thiébaud Weksteen's avatar Thiébaud Weksteen
Browse files

Remove support for @EnforcePermission on classes

The Java annotation only applies to methods.

Test: java -cp AndroidGlobalLintCheckerTest.jar \
      org.junit.runner.JUnitCore \
      com.google.android.lint.aidl.EnforcePermissionDetectorTest
Change-Id: Ibc30de5e88996728f4be4f1473014d0d12c5c248
parent 0dda935b
Loading
Loading
Loading
Loading
+2 −50
Original line number Diff line number Diff line
@@ -36,7 +36,6 @@ import com.intellij.psi.PsiClass
import com.intellij.psi.PsiElement
import com.intellij.psi.PsiMethod
import org.jetbrains.uast.UAnnotation
import org.jetbrains.uast.UClass
import org.jetbrains.uast.UElement
import org.jetbrains.uast.UMethod

@@ -158,50 +157,13 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner {
        }
    }

    private fun compareClasses(
        context: JavaContext,
        element: UElement,
        newClass: PsiClass,
        extendedClass: PsiClass,
        checkEquivalence: Boolean = true
    ) {
        val newAnnotation = newClass.getAnnotation(ANNOTATION_ENFORCE_PERMISSION)
        val extendedAnnotation = extendedClass.getAnnotation(ANNOTATION_ENFORCE_PERMISSION)

        val location = context.getLocation(element)
        val newClassName = newClass.qualifiedName
        val extendedClassName = extendedClass.qualifiedName
        if (newAnnotation == null) {
            val msg = "The class $newClassName extends the class $extendedClassName which " +
                "is annotated with @EnforcePermission. The same annotation must be used " +
                "on $newClassName."
            context.report(ISSUE_MISSING_ENFORCE_PERMISSION, element, location, msg)
        } else if (extendedAnnotation == null) {
            val msg = "The class $newClassName extends the class $extendedClassName which " +
                "is not annotated with @EnforcePermission. The same annotation must be used " +
                "on $extendedClassName. Did you forget to annotate the AIDL definition?"
            context.report(ISSUE_MISSING_ENFORCE_PERMISSION, element, location, msg)
        } else if (checkEquivalence && !areAnnotationsEquivalent(
            context, newAnnotation, extendedAnnotation)) {
            val msg = "The class $newClassName is annotated with ${newAnnotation.text} " +
                "which differs from the parent class $extendedClassName: " +
                "${extendedAnnotation.text}. The same annotation must be used for " +
                "both classes."
            context.report(ISSUE_MISMATCHING_ENFORCE_PERMISSION, element, location, msg)
        }
    }

    override fun visitAnnotationUsage(
        context: JavaContext,
        element: UElement,
        annotationInfo: AnnotationInfo,
        usageInfo: AnnotationUsageInfo
    ) {
        if (usageInfo.type == AnnotationUsageType.EXTENDS) {
            val newClass = element.sourcePsi?.parent?.parent as PsiClass
            val extendedClass: PsiClass = usageInfo.referenced as PsiClass
            compareClasses(context, element, newClass, extendedClass)
        } else if (usageInfo.type == AnnotationUsageType.METHOD_OVERRIDE &&
        if (usageInfo.type == AnnotationUsageType.METHOD_OVERRIDE &&
            annotationInfo.origin == AnnotationOrigin.METHOD) {
            val overridingMethod = element.sourcePsi as PsiMethod
            val overriddenMethod = usageInfo.referenced as PsiMethod
@@ -216,17 +178,7 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner {
                    return
                }
                val method = node.uastParent as? UMethod
                val klass = node.uastParent as? UClass
                if (klass != null) {
                    val newClass = klass as PsiClass
                    val extendedClass = newClass.getSuperClass()
                    if (extendedClass != null && extendedClass.qualifiedName != JAVA_OBJECT) {
                        // The equivalence check can be skipped, if both classes are
                        // annotated, it will be verified by visitAnnotationUsage.
                        compareClasses(context, klass, newClass,
                            extendedClass, checkEquivalence = false)
                    }
                } else if (method != null) {
                if (method != null) {
                    val overridingMethod = method as PsiMethod
                    val parents = overridingMethod.findSuperMethods()
                    for (overriddenMethod in parents) {
+1 −93
Original line number Diff line number Diff line
@@ -33,21 +33,6 @@ class EnforcePermissionDetectorTest : LintDetectorTest() {

    override fun lint(): TestLintTask = super.lint().allowMissingSdk(true)

    fun testDoesNotDetectIssuesCorrectAnnotationOnClass() {
        lint().files(java(
            """
            package test.pkg;
            @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
            public class TestClass1 extends IFoo.Stub {
                public void testMethod() {}
            }
            """).indented(),
                *stubs
        )
        .run()
        .expectClean()
    }

    fun testDoesNotDetectIssuesCorrectAnnotationOnMethod() {
        lint().files(java(
            """
@@ -133,28 +118,6 @@ class EnforcePermissionDetectorTest : LintDetectorTest() {
        .expectClean()
    }

    fun testDetectIssuesMismatchingAnnotationOnClass() {
        lint().files(java(
            """
            package test.pkg;
            @android.annotation.EnforcePermission(android.Manifest.permission.INTERNET)
            public class TestClass3 extends IFoo.Stub {
                public void testMethod() {}
            }
            """).indented(),
                *stubs
        )
        .run()
        .expect("""src/test/pkg/TestClass3.java:3: Error: The class test.pkg.TestClass3 is \
annotated with @android.annotation.EnforcePermission(android.Manifest.permission.INTERNET) \
which differs from the parent class IFoo.Stub: \
@android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE). The \
same annotation must be used for both classes. [MismatchingEnforcePermissionAnnotation]
public class TestClass3 extends IFoo.Stub {
                                ~~~~~~~~~
1 errors, 0 warnings""".addLineContinuation())
    }

    fun testDetectIssuesMismatchingAnnotationOnMethod() {
        lint().files(java(
            """
@@ -295,25 +258,6 @@ annotation must be used for both methods. [MismatchingEnforcePermissionAnnotatio
                """.addLineContinuation())
    }

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

    fun testDetectIssuesMissingAnnotationOnMethod() {
        lint().files(java(
            """
@@ -354,27 +298,6 @@ annotation must be used on Stub.testMethod. Did you forget to annotate the AIDL
1 errors, 0 warnings""".addLineContinuation())
    }

    fun testDetectIssuesExtraAnnotationInterface() {
        lint().files(java(
            """
            package test.pkg;
            @android.annotation.EnforcePermission(android.Manifest.permission.INTERNET)
            public class TestClass8 extends IBar.Stub {
                public void testMethod() {}
            }
            """).indented(),
                *stubs
        )
        .run()
        .expect("""src/test/pkg/TestClass8.java:2: Error: The class test.pkg.TestClass8 \
extends the class IBar.Stub which is not annotated with @EnforcePermission. The same annotation \
must be used on IBar.Stub. Did you forget to annotate the AIDL definition? \
[MissingEnforcePermissionAnnotation]
@android.annotation.EnforcePermission(android.Manifest.permission.INTERNET)
^
1 errors, 0 warnings""".addLineContinuation())
    }

    fun testDetectIssuesMissingAnnotationOnMethodWhenClassIsCalledDefault() {
        lint().files(java(
            """
@@ -399,21 +322,6 @@ must be used on IBar.Stub. Did you forget to annotate the AIDL definition? \

    /* Stubs */

    // A service with permission annotation on the class.
    private val interfaceIFooStub: TestFile = java(
        """
        @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
        public interface IFoo {
         @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
         public static abstract class Stub extends android.os.Binder implements IFoo {
           @Override
           public void testMethod() {}
         }
         public void testMethod();
        }
        """
    ).indented()

    // A service with permission annotation on the method.
    private val interfaceIFooMethodStub: TestFile = java(
        """
@@ -480,7 +388,7 @@ must be used on IBar.Stub. Did you forget to annotate the AIDL definition? \
        """
    ).indented()

    private val stubs = arrayOf(interfaceIFooStub, interfaceIFooMethodStub, interfaceIBarStub,
    private val stubs = arrayOf(interfaceIFooMethodStub, interfaceIBarStub,
            manifestPermissionStub, enforcePermissionAnnotationStub)

    // Substitutes "backslash + new line" with an empty string to imitate line continuation