Merge "Refactor ManualPermissionCheckDetector for @PermissionMethod"

5 files changed, 404 insertions, 126 deletions

diff --git a/tools/lint/checks/src/main/java/com/google/android/lint/PermissionMethodDetector.kt b/tools/lint/checks/src/main/java/com/google/android/lint/PermissionMethodDetector.kt
index 68a450d956a8..1b0f03564c3b 100644
--- a/tools/lint/checks/src/main/java/com/google/android/lint/PermissionMethodDetector.kt
+++ b/tools/lint/checks/src/main/java/com/google/android/lint/PermissionMethodDetector.kt
@@ -26,6 +26,7 @@ import com.android.tools.lint.detector.api.Scope
 import com.android.tools.lint.detector.api.Severity
 import com.android.tools.lint.detector.api.SourceCodeScanner
 import com.android.tools.lint.detector.api.getUMethod
+import com.google.android.lint.aidl.hasPermissionMethodAnnotation
 import com.intellij.psi.PsiType
 import org.jetbrains.uast.UAnnotation
 import org.jetbrains.uast.UBlockExpression
@@ -149,11 +150,6 @@ class PermissionMethodDetector : Detector(), SourceCodeScanner {
             enabledByDefault = false
         )
 
-        private fun hasPermissionMethodAnnotation(method: UMethod): Boolean = method.annotations
-            .any {
-                it.hasQualifiedName(ANNOTATION_PERMISSION_METHOD)
-            }
-
         private fun isPermissionMethodReturnType(method: UMethod): Boolean =
             listOf(PsiType.VOID, PsiType.INT, PsiType.BOOLEAN).contains(method.returnType)
 
diff --git a/tools/lint/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionFix.kt b/tools/lint/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionFix.kt
index 510611161ea8..d120e1d41c99 100644
--- a/tools/lint/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionFix.kt
+++ b/tools/lint/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionFix.kt
@@ -18,37 +18,54 @@ package com.google.android.lint.aidl
 
 import com.android.tools.lint.detector.api.JavaContext
 import com.android.tools.lint.detector.api.Location
-import com.intellij.psi.PsiVariable
+import com.android.tools.lint.detector.api.getUMethod
+import org.jetbrains.kotlin.psi.psiUtil.parameterIndex
 import org.jetbrains.uast.UCallExpression
-import org.jetbrains.uast.ULiteralExpression
-import org.jetbrains.uast.UQualifiedReferenceExpression
-import org.jetbrains.uast.USimpleNameReferenceExpression
-import org.jetbrains.uast.asRecursiveLogString
+import org.jetbrains.uast.evaluateString
+import org.jetbrains.uast.visitor.AbstractUastVisitor
 
 /**
- * Helper ADT class that facilitates the creation of lint auto fixes
+ * Helper class that facilitates the creation of lint auto fixes
  *
  * Handles "Single" permission checks that should be migrated to @EnforcePermission(...), as well as consecutive checks
  * that should be migrated to @EnforcePermission(allOf={...})
  *
  * TODO: handle anyOf style annotations
  */
-sealed class EnforcePermissionFix {
-    abstract fun locations(): List<Location>
-    abstract fun javaAnnotationParameter(): String
-
-    fun javaAnnotation(): String = "@$ANNOTATION_ENFORCE_PERMISSION(${javaAnnotationParameter()})"
+data class EnforcePermissionFix(
+    val locations: List<Location>,
+    val permissionNames: List<String>
+) {
+    val annotation: String
+        get() {
+            val quotedPermissions = permissionNames.joinToString(", ") { """"$it"""" }
+            val annotationParameter =
+                if (permissionNames.size > 1) "allOf={$quotedPermissions}" else quotedPermissions
+            return "@$ANNOTATION_ENFORCE_PERMISSION($annotationParameter)"
+        }
 
     companion object {
-        fun fromCallExpression(callExpression: UCallExpression, context: JavaContext): SingleFix =
-            SingleFix(
-                getPermissionCheckLocation(context, callExpression),
-                getPermissionCheckArgumentValue(callExpression)
-            )
+        /**
+         * conditionally constructs EnforcePermissionFix from a UCallExpression
+         * @return EnforcePermissionFix if the called method is annotated with @PermissionMethod, else null
+         */
+        fun fromCallExpression(
+            context: JavaContext,
+            callExpression: UCallExpression
+        ): EnforcePermissionFix? =
+            if (isPermissionMethodCall(callExpression)) {
+                EnforcePermissionFix(
+                    listOf(getPermissionCheckLocation(context, callExpression)),
+                    getPermissionCheckValues(callExpression)
+                )
+            } else null
 
-        fun maybeAddManifestPrefix(permissionName: String): String =
-            if (permissionName.contains(".")) permissionName
-            else "android.Manifest.permission.$permissionName"
+
+        fun compose(individuals: List<EnforcePermissionFix>): EnforcePermissionFix =
+            EnforcePermissionFix(
+                individuals.flatMap { it.locations },
+                individuals.flatMap { it.permissionNames }
+            )
 
         /**
          * Given a permission check, get its proper location
@@ -70,49 +87,51 @@ sealed class EnforcePermissionFix {
         }
 
         /**
-         * Given a permission check and an argument,
-         * pull out the permission value that is being used
+         * Given a @PermissionMethod, find arguments annotated with @PermissionName
+         * and pull out the permission value(s) being used.  Also evaluates nested calls
+         * to @PermissionMethod(s) in the given method's body.
          */
-        private fun getPermissionCheckArgumentValue(
-            callExpression: UCallExpression,
-            argumentPosition: Int = 0
-        ): String {
+        private fun getPermissionCheckValues(
+            callExpression: UCallExpression
+        ): List<String> {
+            if (!isPermissionMethodCall(callExpression)) return emptyList()
 
-            val identifier = when (
-                val argument = callExpression.valueArguments.getOrNull(argumentPosition)
-            ) {
-                is UQualifiedReferenceExpression -> when (val selector = argument.selector) {
-                    is USimpleNameReferenceExpression ->
-                        ((selector.resolve() as PsiVariable).computeConstantValue() as String)
+            val result = mutableSetOf<String>() // protect against duplicate permission values
+            val visitedCalls = mutableSetOf<UCallExpression>() // don't visit the same call twice
+            val bfsQueue = ArrayDeque(listOf(callExpression))
 
-                    else -> throw RuntimeException(
-                        "Couldn't resolve argument: ${selector.asRecursiveLogString()}"
-                    )
-                }
+            // Breadth First Search - evalutaing nested @PermissionMethod(s) in the available
+            // source code for @PermissionName(s).
+            while (bfsQueue.isNotEmpty()) {
+                val current = bfsQueue.removeFirst()
+                visitedCalls.add(current)
+                result.addAll(findPermissions(current))
 
-                is USimpleNameReferenceExpression -> (
-                        (argument.resolve() as PsiVariable).computeConstantValue() as String)
+                current.resolve()?.getUMethod()?.accept(object : AbstractUastVisitor() {
+                    override fun visitCallExpression(node: UCallExpression): Boolean {
+                        if (isPermissionMethodCall(node) && node !in visitedCalls) {
+                            bfsQueue.add(node)
+                        }
+                        return false
+                    }
+                })
+            }
 
-                is ULiteralExpression -> argument.value as String
+            return result.toList()
+        }
 
-                else -> throw RuntimeException(
-                    "Couldn't resolve argument: ${argument?.asRecursiveLogString()}"
-                )
-            }
+        private fun findPermissions(
+            callExpression: UCallExpression,
+        ): List<String> {
+            val indices = callExpression.resolve()?.getUMethod()
+                ?.uastParameters
+                ?.filter(::hasPermissionNameAnnotation)
+                ?.mapNotNull { it.sourcePsi?.parameterIndex() }
+                ?: emptyList()
 
-            return identifier.substringAfterLast(".")
+            return indices.mapNotNull {
+                callExpression.getArgumentForParameter(it)?.evaluateString()
+            }
         }
     }
 }
-
-data class SingleFix(val location: Location, val permissionName: String) : EnforcePermissionFix() {
-    override fun locations(): List<Location> = listOf(this.location)
-    override fun javaAnnotationParameter(): String = maybeAddManifestPrefix(this.permissionName)
-}
-data class AllOfFix(val checks: List<SingleFix>) : EnforcePermissionFix() {
-    override fun locations(): List<Location> = this.checks.map { it.location }
-    override fun javaAnnotationParameter(): String =
-        "allOf={${
-            this.checks.joinToString(", ") { maybeAddManifestPrefix(it.permissionName) }
-        }}"
-}
diff --git a/tools/lint/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt b/tools/lint/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt
new file mode 100644
index 000000000000..edbdd8d2adf3
--- /dev/null
+++ b/tools/lint/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt
@@ -0,0 +1,67 @@
+/*
+ * Copyright (C) 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 com.google.android.lint.aidl
+
+import com.android.tools.lint.detector.api.getUMethod
+import com.google.android.lint.ANNOTATION_PERMISSION_METHOD
+import com.google.android.lint.ANNOTATION_PERMISSION_NAME
+import com.google.android.lint.CLASS_STUB
+import com.intellij.psi.PsiAnonymousClass
+import org.jetbrains.uast.UCallExpression
+import org.jetbrains.uast.UMethod
+import org.jetbrains.uast.UParameter
+
+/**
+ * Given a UMethod, determine if this method is
+ * an entrypoint to an interface generated by AIDL,
+ * returning the interface name if so
+ */
+fun getContainingAidlInterface(node: UMethod): String? {
+    if (!isInClassCalledStub(node)) return null
+    for (superMethod in node.findSuperMethods()) {
+        for (extendsInterface in superMethod.containingClass?.extendsList?.referenceElements
+            ?: continue) {
+            if (extendsInterface.qualifiedName == IINTERFACE_INTERFACE) {
+                return superMethod.containingClass?.name
+            }
+        }
+    }
+    return null
+}
+
+private fun isInClassCalledStub(node: UMethod): Boolean {
+    (node.containingClass as? PsiAnonymousClass)?.let {
+        return it.baseClassReference.referenceName == CLASS_STUB
+    }
+    return node.containingClass?.extendsList?.referenceElements?.any {
+        it.referenceName == CLASS_STUB
+    } ?: false
+}
+
+fun isPermissionMethodCall(callExpression: UCallExpression): Boolean {
+    val method = callExpression.resolve()?.getUMethod() ?: return false
+    return hasPermissionMethodAnnotation(method)
+}
+
+fun hasPermissionMethodAnnotation(method: UMethod): Boolean = method.annotations
+    .any {
+        it.hasQualifiedName(ANNOTATION_PERMISSION_METHOD)
+    }
+
+fun hasPermissionNameAnnotation(parameter: UParameter) = parameter.annotations.any {
+    it.hasQualifiedName(ANNOTATION_PERMISSION_NAME)
+}
diff --git a/tools/lint/checks/src/main/java/com/google/android/lint/aidl/ManualPermissionCheckDetector.kt b/tools/lint/checks/src/main/java/com/google/android/lint/aidl/ManualPermissionCheckDetector.kt
index 2cea39423f1d..2c53f390128c 100644
--- a/tools/lint/checks/src/main/java/com/google/android/lint/aidl/ManualPermissionCheckDetector.kt
+++ b/tools/lint/checks/src/main/java/com/google/android/lint/aidl/ManualPermissionCheckDetector.kt
@@ -25,9 +25,6 @@ import com.android.tools.lint.detector.api.JavaContext
 import com.android.tools.lint.detector.api.Scope
 import com.android.tools.lint.detector.api.Severity
 import com.android.tools.lint.detector.api.SourceCodeScanner
-import com.google.android.lint.CLASS_STUB
-import com.google.android.lint.ENFORCE_PERMISSION_METHODS
-import com.intellij.psi.PsiAnonymousClass
 import org.jetbrains.uast.UBlockExpression
 import org.jetbrains.uast.UCallExpression
 import org.jetbrains.uast.UElement
@@ -56,7 +53,7 @@ class ManualPermissionCheckDetector : Detector(), SourceCodeScanner {
             val body = (node.uastBody as? UBlockExpression) ?: return
             val fix = accumulateSimplePermissionCheckFixes(body) ?: return
 
-            val javaRemoveFixes = fix.locations().map {
+            val javaRemoveFixes = fix.locations.map {
                 fix()
                     .replace()
                     .reformat(true)
@@ -67,7 +64,7 @@ class ManualPermissionCheckDetector : Detector(), SourceCodeScanner {
             }
 
             val javaAnnotateFix = fix()
-                .annotate(fix.javaAnnotation())
+                .annotate(fix.annotation)
                 .range(context.getLocation(node))
                 .autoFix()
                 .build()
@@ -77,7 +74,7 @@ class ManualPermissionCheckDetector : Detector(), SourceCodeScanner {
 
             context.report(
                 ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION,
-                fix.locations().last(),
+                fix.locations.last(),
                 message,
                 fix().composite(*javaRemoveFixes.toTypedArray(), javaAnnotateFix)
             )
@@ -97,14 +94,14 @@ class ManualPermissionCheckDetector : Detector(), SourceCodeScanner {
          */
         private fun accumulateSimplePermissionCheckFixes(methodBody: UBlockExpression):
                 EnforcePermissionFix? {
-            val singleFixes = mutableListOf<SingleFix>()
+            val singleFixes = mutableListOf<EnforcePermissionFix>()
             for (expression in methodBody.expressions) {
                 singleFixes.add(getPermissionCheckFix(expression) ?: break)
             }
             return when (singleFixes.size) {
                 0 -> null
                 1 -> singleFixes[0]
-                else -> AllOfFix(singleFixes)
+                else -> EnforcePermissionFix.compose(singleFixes)
             }
         }
 
@@ -113,7 +110,7 @@ class ManualPermissionCheckDetector : Detector(), SourceCodeScanner {
          * the helper for creating a lint auto fix to @EnforcePermission
          */
         private fun getPermissionCheckFix(startingExpression: UElement?):
-                SingleFix? {
+                EnforcePermissionFix? {
             return when (startingExpression) {
                 is UQualifiedReferenceExpression -> getPermissionCheckFix(
                     startingExpression.selector
@@ -121,11 +118,8 @@ class ManualPermissionCheckDetector : Detector(), SourceCodeScanner {
 
                 is UIfExpression -> getPermissionCheckFix(startingExpression.condition)
 
-                is UCallExpression -> {
-                    return if (isPermissionCheck(startingExpression))
-                        EnforcePermissionFix.fromCallExpression(startingExpression, context)
-                    else null
-                }
+                is UCallExpression -> return EnforcePermissionFix
+                            .fromCallExpression(context, startingExpression)
 
                 else -> null
             }
@@ -160,40 +154,5 @@ class ManualPermissionCheckDetector : Detector(), SourceCodeScanner {
             ),
             enabledByDefault = false, // TODO: enable once b/241171714 is resolved
         )
-
-        private fun isPermissionCheck(callExpression: UCallExpression): Boolean {
-            val method = callExpression.resolve() ?: return false
-            val className = method.containingClass?.qualifiedName
-            return ENFORCE_PERMISSION_METHODS.any {
-                it.clazz == className && it.name == method.name
-            }
-        }
-
-        /**
-         * given a UMethod, determine if this method is
-         * an entrypoint to an interface generated by AIDL,
-         * returning the interface name if so
-         */
-        fun getContainingAidlInterface(node: UMethod): String? {
-            if (!isInClassCalledStub(node)) return null
-            for (superMethod in node.findSuperMethods()) {
-                for (extendsInterface in superMethod.containingClass?.extendsList?.referenceElements
-                    ?: continue) {
-                    if (extendsInterface.qualifiedName == IINTERFACE_INTERFACE) {
-                        return superMethod.containingClass?.name
-                    }
-                }
-            }
-            return null
-        }
-
-        private fun isInClassCalledStub(node: UMethod): Boolean {
-            (node.containingClass as? PsiAnonymousClass)?.let {
-                return it.baseClassReference.referenceName == CLASS_STUB
-            }
-            return node.containingClass?.extendsList?.referenceElements?.any {
-                it.referenceName == CLASS_STUB
-            } ?: false
-        }
     }
 }
diff --git a/tools/lint/checks/src/test/java/com/google/android/lint/aidl/ManualPermissionCheckDetectorTest.kt b/tools/lint/checks/src/test/java/com/google/android/lint/aidl/ManualPermissionCheckDetectorTest.kt
index 1a1c6bc77785..a968f5e6cb1c 100644
--- a/tools/lint/checks/src/test/java/com/google/android/lint/aidl/ManualPermissionCheckDetectorTest.kt
+++ b/tools/lint/checks/src/test/java/com/google/android/lint/aidl/ManualPermissionCheckDetectorTest.kt
@@ -19,6 +19,7 @@ package com.google.android.lint.aidl
 import com.android.tools.lint.checks.infrastructure.LintDetectorTest
 import com.android.tools.lint.checks.infrastructure.TestFile
 import com.android.tools.lint.checks.infrastructure.TestLintTask
+import com.android.tools.lint.checks.infrastructure.TestMode
 import com.android.tools.lint.detector.api.Detector
 import com.android.tools.lint.detector.api.Issue
 
@@ -42,7 +43,7 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() {
                         private Context mContext;
                         @Override
                         public void test() throws android.os.RemoteException {
-                            mContext.enforceCallingOrSelfPermission("android.Manifest.permission.READ_CONTACTS", "foo");
+                            mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
                         }
                     }
                 """
@@ -53,8 +54,8 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() {
             .expect(
                 """
                 src/Foo.java:7: Warning: ITest permission check can be converted to @EnforcePermission annotation [UseEnforcePermissionAnnotation]
-                        mContext.enforceCallingOrSelfPermission("android.Manifest.permission.READ_CONTACTS", "foo");
-                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+                        mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
+                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                 0 errors, 1 warnings
                 """
             )
@@ -62,9 +63,9 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() {
                 """
                 Fix for src/Foo.java line 7: Annotate with @EnforcePermission:
                 @@ -5 +5
-                +     @android.annotation.EnforcePermission(android.Manifest.permission.READ_CONTACTS)
+                +     @android.annotation.EnforcePermission("android.permission.READ_CONTACTS")
                 @@ -7 +8
-                -         mContext.enforceCallingOrSelfPermission("android.Manifest.permission.READ_CONTACTS", "foo");
+                -         mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
                 """
             )
     }
@@ -81,7 +82,7 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() {
                             @Override
                             public void test() throws android.os.RemoteException {
                                 mContext.enforceCallingOrSelfPermission(
-                                    "android.Manifest.permission.READ_CONTACTS", "foo");
+                                    "android.permission.READ_CONTACTS", "foo");
                             }
                         };
                     }
@@ -102,10 +103,49 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() {
                 """
                 Fix for src/Foo.java line 8: Annotate with @EnforcePermission:
                 @@ -6 +6
-                +         @android.annotation.EnforcePermission(android.Manifest.permission.READ_CONTACTS)
+                +         @android.annotation.EnforcePermission("android.permission.READ_CONTACTS")
                 @@ -8 +9
                 -             mContext.enforceCallingOrSelfPermission(
-                -                 "android.Manifest.permission.READ_CONTACTS", "foo");
+                -                 "android.permission.READ_CONTACTS", "foo");
+                """
+            )
+    }
+
+    fun testConstantEvaluation() {
+        lint().files(
+            java(
+                """
+                    import android.content.Context;
+                    import android.test.ITest;
+
+                    public class Foo extends ITest.Stub {
+                        private Context mContext;
+                        @Override
+                        public void test() throws android.os.RemoteException {
+                            mContext.enforceCallingOrSelfPermission(android.Manifest.permission.READ_CONTACTS, "foo");
+                        }
+                    }
+                """
+            ).indented(),
+            *stubs,
+            manifestStub
+        )
+            .run()
+            .expect(
+                """
+                src/Foo.java:8: Warning: ITest permission check can be converted to @EnforcePermission annotation [UseEnforcePermissionAnnotation]
+                        mContext.enforceCallingOrSelfPermission(android.Manifest.permission.READ_CONTACTS, "foo");
+                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+                0 errors, 1 warnings
+                """
+            )
+            .expectFixDiffs(
+                """
+                Fix for src/Foo.java line 7: Annotate with @EnforcePermission:
+                @@ -6 +6
+                +     @android.annotation.EnforcePermission("android.permission.READ_CONTACTS")
+                @@ -8 +9
+                -         mContext.enforceCallingOrSelfPermission(android.Manifest.permission.READ_CONTACTS, "foo");
                 """
             )
     }
@@ -122,9 +162,9 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() {
                             @Override
                             public void test() throws android.os.RemoteException {
                                 mContext.enforceCallingOrSelfPermission(
-                                    "android.Manifest.permission.READ_CONTACTS", "foo");
+                                    "android.permission.READ_CONTACTS", "foo");
                                 mContext.enforceCallingOrSelfPermission(
-                                    "android.Manifest.permission.WRITE_CONTACTS", "foo");
+                                    "android.permission.WRITE_CONTACTS", "foo");
                             }
                         };
                     }
@@ -144,13 +184,13 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() {
             .expectFixDiffs(
                 """
                 Fix for src/Foo.java line 10: Annotate with @EnforcePermission:
-                @@ -6 +6                                                                                                                                                                                                       
-                +         @android.annotation.EnforcePermission(allOf={android.Manifest.permission.READ_CONTACTS, android.Manifest.permission.WRITE_CONTACTS})
+                @@ -6 +6
+                +         @android.annotation.EnforcePermission(allOf={"android.permission.READ_CONTACTS", "android.permission.WRITE_CONTACTS"})
                 @@ -8 +9
                 -             mContext.enforceCallingOrSelfPermission(
-                -                 "android.Manifest.permission.READ_CONTACTS", "foo");
+                -                 "android.permission.READ_CONTACTS", "foo");
                 -             mContext.enforceCallingOrSelfPermission(
-                -                 "android.Manifest.permission.WRITE_CONTACTS", "foo");
+                -                 "android.permission.WRITE_CONTACTS", "foo");
                 """
             )
     }
@@ -166,7 +206,7 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() {
                         @Override
                         public void test() throws android.os.RemoteException {
                             long uid = Binder.getCallingUid();
-                            mContext.enforceCallingOrSelfPermission("android.Manifest.permission.READ_CONTACTS", "foo");
+                            mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
                         }
                     }
                 """
@@ -177,6 +217,149 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() {
             .expectClean()
     }
 
+    fun testPermissionHelper() {
+        lint().skipTestModes(TestMode.PARENTHESIZED).files(
+            java(
+                """
+                    import android.content.Context;
+                    import android.test.ITest;
+
+                    public class Foo extends ITest.Stub {
+                        private Context mContext;
+
+                        @android.content.pm.PermissionMethod
+                        private void helper() {
+                            mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
+                        }
+
+                        @Override
+                        public void test() throws android.os.RemoteException {
+                            helper();
+                        }
+                    }
+                """
+            ).indented(),
+            *stubs
+        )
+            .run()
+            .expect(
+                """
+                src/Foo.java:14: Warning: ITest permission check can be converted to @EnforcePermission annotation [UseEnforcePermissionAnnotation]
+                        helper();
+                        ~~~~~~~~~
+                0 errors, 1 warnings
+                """
+            )
+            .expectFixDiffs(
+                """
+                Fix for src/Foo.java line 14: Annotate with @EnforcePermission:
+                @@ -12 +12
+                +     @android.annotation.EnforcePermission("android.permission.READ_CONTACTS")
+                @@ -14 +15
+                -         helper();
+                """
+            )
+    }
+
+    fun testPermissionHelperAllOf() {
+        lint().skipTestModes(TestMode.PARENTHESIZED).files(
+            java(
+                """
+                import android.content.Context;
+                import android.test.ITest;
+
+                public class Foo extends ITest.Stub {
+                    private Context mContext;
+
+                    @android.content.pm.PermissionMethod
+                    private void helper() {
+                        mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
+                        mContext.enforceCallingOrSelfPermission("android.permission.WRITE_CONTACTS", "foo");
+                    }
+
+                    @Override
+                    public void test() throws android.os.RemoteException {
+                        helper();
+                        mContext.enforceCallingOrSelfPermission("FOO", "foo");
+                    }
+                }
+                """
+            ).indented(),
+            *stubs
+        )
+            .run()
+            .expect(
+                """
+                src/Foo.java:16: Warning: ITest permission check can be converted to @EnforcePermission annotation [UseEnforcePermissionAnnotation]
+                        mContext.enforceCallingOrSelfPermission("FOO", "foo");
+                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+                0 errors, 1 warnings
+                """
+            )
+            .expectFixDiffs(
+                """
+                Fix for src/Foo.java line 16: Annotate with @EnforcePermission:
+                @@ -13 +13
+                +     @android.annotation.EnforcePermission(allOf={"android.permission.READ_CONTACTS", "android.permission.WRITE_CONTACTS", "FOO"})
+                @@ -15 +16
+                -         helper();
+                -         mContext.enforceCallingOrSelfPermission("FOO", "foo");
+                """
+            )
+    }
+
+
+    fun testPermissionHelperNested() {
+        lint().skipTestModes(TestMode.PARENTHESIZED).files(
+            java(
+                """
+                import android.content.Context;
+                import android.test.ITest;
+
+                public class Foo extends ITest.Stub {
+                    private Context mContext;
+
+                    @android.content.pm.PermissionMethod
+                    private void helperHelper() {
+                        helper("android.permission.WRITE_CONTACTS");
+                    }
+
+                    @android.content.pm.PermissionMethod
+                    private void helper(@android.content.pm.PermissionName String extraPermission) {
+                        mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
+                    }
+
+                    @Override
+                    public void test() throws android.os.RemoteException {
+                        helperHelper();
+                    }
+                }
+                """
+            ).indented(),
+            *stubs
+        )
+            .run()
+            .expect(
+                """
+                src/Foo.java:19: Warning: ITest permission check can be converted to @EnforcePermission annotation [UseEnforcePermissionAnnotation]
+                        helperHelper();
+                        ~~~~~~~~~~~~~~~
+                0 errors, 1 warnings
+                """
+            )
+            .expectFixDiffs(
+                """
+                Fix for src/Foo.java line 19: Annotate with @EnforcePermission:
+                @@ -17 +17
+                +     @android.annotation.EnforcePermission(allOf={"android.permission.WRITE_CONTACTS", "android.permission.READ_CONTACTS"})
+                @@ -19 +20
+                -         helperHelper();
+                """
+            )
+    }
+
+
+
     companion object {
         private val aidlStub: TestFile = java(
             """
@@ -192,7 +375,8 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() {
             """
                 package android.content;
                 public class Context {
-                    public void enforceCallingOrSelfPermission(String permission, String message) {}
+                    @android.content.pm.PermissionMethod
+                    public void enforceCallingOrSelfPermission(@android.content.pm.PermissionName String permission, String message) {}
                 }
             """
         ).indented()
@@ -206,6 +390,59 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() {
             """
         ).indented()
 
-        val stubs = arrayOf(aidlStub, contextStub, binderStub)
+        private val permissionMethodStub: TestFile = java(
+            """
+                package android.content.pm;
+
+                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 PermissionMethod {}
+            """
+        ).indented()
+
+        private val permissionNameStub: TestFile = java(
+            """
+                package android.content.pm;
+
+                import static java.lang.annotation.ElementType.FIELD;
+                import static java.lang.annotation.ElementType.LOCAL_VARIABLE;
+                import static java.lang.annotation.ElementType.METHOD;
+                import static java.lang.annotation.ElementType.PARAMETER;
+                import static java.lang.annotation.RetentionPolicy.CLASS;
+
+                import java.lang.annotation.Retention;
+                import java.lang.annotation.Target;
+
+                @Retention(CLASS)
+                @Target({PARAMETER, METHOD, LOCAL_VARIABLE, FIELD})
+                public @interface PermissionName {}
+            """
+        ).indented()
+
+        private val manifestStub: TestFile = java(
+            """
+                package android;
+
+                public final class Manifest {
+                    public static final class permission {
+                        public static final String READ_CONTACTS="android.permission.READ_CONTACTS";
+                    }
+                }
+            """.trimIndent()
+        )
+
+        val stubs = arrayOf(
+            aidlStub,
+            contextStub,
+            binderStub,
+            permissionMethodStub,
+            permissionNameStub
+        )
     }
 }