Migrate PermissionAnnotationDetector to global lint checks

PermissionAnnotationDetector will no longer be a local lint check which
could optionally be added to build targets, but instead a global lint
check ran on every build. New AIDL Interfaces part of the system_server
process will need to use @EnforcePermission annotations. The detector
does not enforce old AIDL Interfaces to use the annotations. These are
included in the exemptAidlInterfaces set generated by
ExemptAidlInterfacesGenerator.

The CL removes the reference to the PermissionAnnotationDetector from
the Accessibility Service build targets. Instead the already-annotated
AIDL Interfaces are removed from exemptAidlInterfaces, achieving the
same effect.

Bug: 363248121
Test: PermissionAnnotationDetectorTest
Flag: EXEMPT lint check
Change-Id: I2cad77cbc7883087dd95b9558d3543fcb321bbc8

11 files changed, 263 insertions, 167 deletions

diff --git a/services/accessibility/Android.bp b/services/accessibility/Android.bp
index 3d7ad0b94d1b..34b9a5db580f 100644
--- a/services/accessibility/Android.bp
+++ b/services/accessibility/Android.bp
@@ -19,11 +19,6 @@ java_library_static {
     defaults: [
         "platform_service_defaults",
     ],
-    lint: {
-        error_checks: ["MissingPermissionAnnotation"],
-        baseline_filename: "lint-baseline.xml",
-
-    },
     srcs: [
         ":services.accessibility-sources",
         ":statslog-accessibility-java-gen",
diff --git a/services/accessibility/java/com/android/server/accessibility/AccessibilityInputFilter.java b/services/accessibility/java/com/android/server/accessibility/AccessibilityInputFilter.java
index 1b2447e2c58a..617cca9d3075 100644
--- a/services/accessibility/java/com/android/server/accessibility/AccessibilityInputFilter.java
+++ b/services/accessibility/java/com/android/server/accessibility/AccessibilityInputFilter.java
@@ -67,7 +67,6 @@ import java.util.StringJoiner;
  *
  * NOTE: This class has to be created and poked only from the main thread.
  */
-@SuppressWarnings("MissingPermissionAnnotation")
 class AccessibilityInputFilter extends InputFilter implements EventStreamTransformation {
 
     private static final String TAG = AccessibilityInputFilter.class.getSimpleName();
diff --git a/services/accessibility/java/com/android/server/accessibility/FingerprintGestureDispatcher.java b/services/accessibility/java/com/android/server/accessibility/FingerprintGestureDispatcher.java
index e10e87c51d59..c9ec16edc54e 100644
--- a/services/accessibility/java/com/android/server/accessibility/FingerprintGestureDispatcher.java
+++ b/services/accessibility/java/com/android/server/accessibility/FingerprintGestureDispatcher.java
@@ -33,7 +33,6 @@ import java.util.List;
 /**
  * Encapsulate fingerprint gesture logic
  */
-@SuppressWarnings("MissingPermissionAnnotation")
 public class FingerprintGestureDispatcher extends IFingerprintClientActiveCallback.Stub
         implements Handler.Callback{
     private static final int MSG_REGISTER = 1;
diff --git a/tools/lint/common/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt b/tools/lint/common/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt
index f5af99ec39ac..b79563f740ee 100644
--- a/tools/lint/common/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt
+++ b/tools/lint/common/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt
@@ -103,3 +103,26 @@ fun getHelperMethodFix(
 
     return fix.build()
 }
+
+/**
+ * PermissionAnnotationDetector uses this method to determine whether a specific file should be
+ * checked for unannotated methods. Only files located in directories whose paths begin with one
+ * of these prefixes will be considered.
+ */
+fun isSystemServicePath(context: JavaContext): Boolean {
+    val systemServicePathPrefixes = setOf(
+        "frameworks/base/services",
+        "frameworks/base/apex",
+        "frameworks/opt/wear",
+        "packages/modules"
+    )
+
+    val filePath = context.file.path
+
+    // We perform `filePath.contains` instead of `filePath.startsWith` since getting the
+    // relative path of a source file is non-trivial. That is because `context.file.path`
+    // returns the path to where soong builds the file (i.e. /out/soong/...). Moreover, the
+    // logic to extract the relative path would need to consider several /out/soong/...
+    // locations patterns.
+    return systemServicePathPrefixes.any { filePath.contains(it) }
+}
diff --git a/tools/lint/framework/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt b/tools/lint/framework/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt
index 5c6469706e18..af753e5963a3 100644
--- a/tools/lint/framework/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt
+++ b/tools/lint/framework/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt
@@ -20,7 +20,6 @@ import com.android.tools.lint.client.api.IssueRegistry
 import com.android.tools.lint.client.api.Vendor
 import com.android.tools.lint.detector.api.CURRENT_API
 import com.google.android.lint.parcel.SaferParcelChecker
-import com.google.android.lint.aidl.PermissionAnnotationDetector
 import com.google.auto.service.AutoService
 
 @AutoService(IssueRegistry::class)
@@ -38,7 +37,6 @@ class AndroidFrameworkIssueRegistry : IssueRegistry() {
         SaferParcelChecker.ISSUE_UNSAFE_API_USAGE,
         // TODO: Currently crashes due to OOM issue
         // PackageVisibilityDetector.ISSUE_PACKAGE_NAME_NO_PACKAGE_VISIBILITY_FILTERS,
-        PermissionAnnotationDetector.ISSUE_MISSING_PERMISSION_ANNOTATION,
         PermissionMethodDetector.ISSUE_PERMISSION_METHOD_USAGE,
         PermissionMethodDetector.ISSUE_CAN_BE_PERMISSION_METHOD,
         FeatureAutomotiveDetector.ISSUE,
diff --git a/tools/lint/framework/checks/src/test/java/com/google/android/lint/PermissionAnnotationDetectorTest.kt b/tools/lint/framework/checks/src/test/java/com/google/android/lint/PermissionAnnotationDetectorTest.kt
deleted file mode 100644
index bce848a2e3a7..000000000000
--- a/tools/lint/framework/checks/src/test/java/com/google/android/lint/PermissionAnnotationDetectorTest.kt
+++ /dev/null
@@ -1,134 +0,0 @@
-/*
- * Copyright (C) 2023 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.checks.infrastructure.LintDetectorTest
-import com.android.tools.lint.checks.infrastructure.TestFile
-import com.android.tools.lint.checks.infrastructure.TestLintTask
-import com.android.tools.lint.detector.api.Detector
-import com.android.tools.lint.detector.api.Issue
-
-@Suppress("UnstableApiUsage")
-class PermissionAnnotationDetectorTest : LintDetectorTest() {
-    override fun getDetector(): Detector = PermissionAnnotationDetector()
-
-    override fun getIssues(): List<Issue> = listOf(
-        PermissionAnnotationDetector.ISSUE_MISSING_PERMISSION_ANNOTATION,
-    )
-
-    override fun lint(): TestLintTask = super.lint().allowMissingSdk(true)
-
-    /** No issue scenario */
-
-    fun testDoesNotDetectIssuesInCorrectScenario() {
-        lint().files(
-            java(
-            """
-            public class Foo extends IFoo.Stub {
-                @Override
-                @android.annotation.EnforcePermission("android.Manifest.permission.READ_CONTACTS")
-                public void testMethod() { }
-            }
-            """
-            ).indented(),
-            *stubs
-        )
-            .run()
-            .expectClean()
-    }
-
-    fun testMissingAnnotation() {
-        lint().files(
-            java(
-            """
-            public class Bar extends IBar.Stub {
-                public void testMethod() { }
-            }
-            """
-            ).indented(),
-            *stubs
-        )
-            .run()
-            .expect(
-                """
-                src/Bar.java:2: Error: The method testMethod is not permission-annotated. [MissingPermissionAnnotation]
-                    public void testMethod() { }
-                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-                1 errors, 0 warnings
-                """
-            )
-    }
-
-    fun testNoIssueWhenExtendingWithAnotherSubclass() {
-        lint().files(
-            java(
-            """
-            public class Foo extends IFoo.Stub {
-                @Override
-                @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
-                public void testMethod() { }
-                // not an AIDL method, just another method
-                public void someRandomMethod() { }
-            }
-            """).indented(),
-            java(
-            """
-            public class Baz extends Bar {
-              @Override
-              public void someRandomMethod() { }
-            }
-            """).indented(),
-            *stubs
-        )
-            .run()
-            .expectClean()
-    }
-
-    /* Stubs */
-
-    // A service with permission annotation on the method.
-    private val interfaceIFoo: TestFile = java(
-        """
-        public interface IFoo extends android.os.IInterface {
-         public static abstract class Stub extends android.os.Binder implements IFoo {
-          }
-          @Override
-          @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
-          public void testMethod();
-          @Override
-          @android.annotation.RequiresNoPermission
-          public void testMethodNoPermission();
-          @Override
-          @android.annotation.PermissionManuallyEnforced
-          public void testMethodManual();
-        }
-        """
-    ).indented()
-
-    // A service with no permission annotation.
-    private val interfaceIBar: TestFile = java(
-        """
-        public interface IBar extends android.os.IInterface {
-         public static abstract class Stub extends android.os.Binder implements IBar {
-          }
-          public void testMethod();
-        }
-        """
-    ).indented()
-
-    private val stubs = arrayOf(interfaceIFoo, interfaceIBar)
-}
diff --git a/tools/lint/global/checks/src/main/java/com/google/android/lint/AndroidGlobalIssueRegistry.kt b/tools/lint/global/checks/src/main/java/com/google/android/lint/AndroidGlobalIssueRegistry.kt
index 28eab8f62e74..290e7be9f6c4 100644
--- a/tools/lint/global/checks/src/main/java/com/google/android/lint/AndroidGlobalIssueRegistry.kt
+++ b/tools/lint/global/checks/src/main/java/com/google/android/lint/AndroidGlobalIssueRegistry.kt
@@ -20,6 +20,7 @@ import com.android.tools.lint.client.api.IssueRegistry
 import com.android.tools.lint.client.api.Vendor
 import com.android.tools.lint.detector.api.CURRENT_API
 import com.google.android.lint.aidl.EnforcePermissionDetector
+import com.google.android.lint.aidl.PermissionAnnotationDetector
 import com.google.android.lint.aidl.SimpleManualPermissionEnforcementDetector
 import com.google.auto.service.AutoService
 
@@ -31,6 +32,7 @@ class AndroidGlobalIssueRegistry : IssueRegistry() {
             EnforcePermissionDetector.ISSUE_MISMATCHING_ENFORCE_PERMISSION,
             EnforcePermissionDetector.ISSUE_ENFORCE_PERMISSION_HELPER,
             EnforcePermissionDetector.ISSUE_MISUSING_ENFORCE_PERMISSION,
+            PermissionAnnotationDetector.ISSUE_MISSING_PERMISSION_ANNOTATION,
             SimpleManualPermissionEnforcementDetector.ISSUE_SIMPLE_MANUAL_PERMISSION_ENFORCEMENT,
     )
 
diff --git a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/ExemptAidlInterfaces.kt b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/ExemptAidlInterfaces.kt
index 8777712b0f04..675a59e6ae3e 100644
--- a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/ExemptAidlInterfaces.kt
+++ b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/ExemptAidlInterfaces.kt
@@ -20,12 +20,8 @@ package com.google.android.lint.aidl
  * The exemptAidlInterfaces set was generated by running ExemptAidlInterfacesGenerator on the
  * entire source tree. To reproduce the results, run generate-exempt-aidl-interfaces.sh
  * located in tools/lint/utils.
- *
- * TODO: b/363248121 - Use the exemptAidlInterfaces set inside PermissionAnnotationDetector when it
- * gets migrated to a global lint check.
  */
 val exemptAidlInterfaces = setOf(
-    "android.accessibilityservice.IAccessibilityServiceConnection",
     "android.accessibilityservice.IBrailleDisplayConnection",
     "android.accounts.IAccountAuthenticatorResponse",
     "android.accounts.IAccountManager",
@@ -663,10 +659,6 @@ val exemptAidlInterfaces = setOf(
     "android.uwb.IUwbOemExtensionCallback",
     "android.uwb.IUwbRangingCallbacks",
     "android.uwb.IUwbVendorUciCallback",
-    "android.view.accessibility.IAccessibilityInteractionConnectionCallback",
-    "android.view.accessibility.IAccessibilityManager",
-    "android.view.accessibility.IMagnificationConnectionCallback",
-    "android.view.accessibility.IRemoteMagnificationAnimationCallback",
     "android.view.autofill.IAutoFillManager",
     "android.view.autofill.IAutofillWindowPresenter",
     "android.view.contentcapture.IContentCaptureManager",
diff --git a/tools/lint/framework/checks/src/main/java/com/google/android/lint/PermissionAnnotationDetector.kt b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/PermissionAnnotationDetector.kt
index 6b50cfd9e5ab..d44c271e734c 100644
--- a/tools/lint/framework/checks/src/main/java/com/google/android/lint/PermissionAnnotationDetector.kt
+++ b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/PermissionAnnotationDetector.kt
@@ -43,8 +43,14 @@ class PermissionAnnotationDetector : AidlImplementationDetector() {
       interfaceName: String,
       body: UBlockExpression
     ) {
+        if (!isSystemServicePath(context)) return
+
         if (context.evaluator.isAbstract(node)) return
 
+        val fullyQualifiedInterfaceName =
+            getContainingAidlInterfaceQualified(context, node) ?: return
+        if (exemptAidlInterfaces.contains(fullyQualifiedInterfaceName)) return
+
         if (AIDL_PERMISSION_ANNOTATIONS.any { node.hasAnnotation(it) }) return
 
         context.report(
@@ -80,8 +86,7 @@ class PermissionAnnotationDetector : AidlImplementationDetector() {
             implementation = Implementation(
                 PermissionAnnotationDetector::class.java,
                 Scope.JAVA_FILE_SCOPE
-            ),
-            enabledByDefault = false
+            )
         )
     }
 }
diff --git a/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/PermissionAnnotationDetectorTest.kt b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/PermissionAnnotationDetectorTest.kt
new file mode 100644
index 000000000000..92d0829911bf
--- /dev/null
+++ b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/PermissionAnnotationDetectorTest.kt
@@ -0,0 +1,230 @@
+/*
+ * Copyright (C) 2023 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.checks.infrastructure.LintDetectorTest
+import com.android.tools.lint.checks.infrastructure.TestFile
+import com.android.tools.lint.checks.infrastructure.TestLintTask
+import com.android.tools.lint.detector.api.Detector
+import com.android.tools.lint.detector.api.Issue
+
+@Suppress("UnstableApiUsage")
+class PermissionAnnotationDetectorTest : LintDetectorTest() {
+    override fun getDetector(): Detector =
+        PermissionAnnotationDetector()
+
+    override fun getIssues(): List<Issue> = listOf(
+        PermissionAnnotationDetector.ISSUE_MISSING_PERMISSION_ANNOTATION,
+    )
+
+    override fun lint(): TestLintTask = super.lint().allowMissingSdk(true)
+
+    /** No issue scenario */
+
+    fun testDoesNotDetectIssuesInCorrectScenario() {
+        lint()
+            .files(
+                java(
+                    createVisitedPath("Foo.java"),
+                    """
+                        package com.android.server;
+                        public class Foo extends IFoo.Stub {
+                            @Override
+                            @android.annotation.EnforcePermission("android.Manifest.permission.READ_CONTACTS")
+                            public void testMethod() { }
+                        }
+                    """
+                )
+                    .indented(),
+                *stubs
+            )
+            .run()
+            .expectClean()
+    }
+
+    fun testMissingAnnotation() {
+        lint()
+            .files(
+                java(
+                    createVisitedPath("Bar.java"),
+                    """
+                        package com.android.server;
+                        public class Bar extends IBar.Stub {
+                            public void testMethod() { }
+                        }
+                    """
+                )
+                    .indented(),
+                *stubs
+            )
+            .run()
+            .expect(
+                """
+                src/frameworks/base/services/java/com/android/server/Bar.java:3: Error: The method testMethod is not permission-annotated. [MissingPermissionAnnotation]
+                    public void testMethod() { }
+                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+                1 errors, 0 warnings
+                """
+            )
+    }
+
+    fun testMissingAnnotationInIgnoredDirectory() {
+        lint()
+            .files(
+                java(
+                    ignoredPath,
+                    """
+                        package com.android.server;
+                        public class Bar extends IBar.Stub {
+                            public void testMethod() { }
+                        }
+                    """
+                )
+                    .indented(),
+                *stubs
+            )
+            .run()
+            .expectClean()
+    }
+
+    // If this test fails, consider the following steps:
+    //   1. Pick the first entry (interface) from `exemptAidlInterfaces`.
+    //   2. Change `interfaceIExempted` to use that interface.
+    //   3. Change this test's class to extend the interface's Stub.
+    fun testMissingAnnotationAidlInterfaceExempted() {
+        lint()
+            .files(
+                java(
+                    createVisitedPath("Bar.java"),
+                    """
+                        package com.android.server;
+                        public class Bar extends android.accessibilityservice.IBrailleDisplayConnection.Stub {
+                            public void testMethod() { }
+                        }
+                    """
+                )
+                    .indented(),
+                *stubs
+            )
+            .run()
+            .expectClean()
+    }
+
+    fun testMissingAnnotationAidlInterfaceAbstractMethod() {
+        lint()
+            .files(
+                java(
+                    createVisitedPath("Bar.java"),
+                    """
+                        package com.android.server;
+                        public abstract class Bar extends IBar.Stub {
+                            public abstract void testMethod();
+                        }
+                    """
+                )
+                    .indented(),
+                *stubs
+            )
+            .run()
+            .expectClean()
+    }
+
+    fun testNoIssueWhenExtendingWithAnotherSubclass() {
+        lint()
+            .files(
+                java(
+                    createVisitedPath("Foo.java"),
+                    """
+                        package com.android.server;
+                        public class Foo extends IFoo.Stub {
+                            @Override
+                            @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
+                            public void testMethod() { }
+                            // not an AIDL method, just another method
+                            public void someRandomMethod() { }
+                        }
+                    """
+                )
+                    .indented(),
+                java(
+                    createVisitedPath("Baz.java"),
+                    """
+                        package com.android.server;
+                        public class Baz extends Bar {
+                          @Override
+                          public void someRandomMethod() { }
+                        }
+                    """
+                )
+                    .indented(),
+                *stubs
+            )
+            .run()
+            .expectClean()
+    }
+
+    /* Stubs */
+
+    // A service with permission annotation on the method.
+    private val interfaceIFoo: TestFile = java(
+        """
+        public interface IFoo extends android.os.IInterface {
+         public static abstract class Stub extends android.os.Binder implements IFoo {
+          }
+          @Override
+          @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
+          public void testMethod();
+          @Override
+          @android.annotation.RequiresNoPermission
+          public void testMethodNoPermission();
+          @Override
+          @android.annotation.PermissionManuallyEnforced
+          public void testMethodManual();
+        }
+        """
+    ).indented()
+
+    // A service with no permission annotation.
+    private val interfaceIBar: TestFile = java(
+        """
+        public interface IBar extends android.os.IInterface {
+         public static abstract class Stub extends android.os.Binder implements IBar {
+          }
+          public void testMethod();
+        }
+        """
+    ).indented()
+
+    // A service whose AIDL Interface is exempted.
+    private val interfaceIExempted: TestFile = java(
+        """
+        package android.accessibilityservice;
+        public interface IBrailleDisplayConnection extends android.os.IInterface {
+         public static abstract class Stub extends android.os.Binder implements IBrailleDisplayConnection {
+          }
+          public void testMethod();
+        }
+        """
+    ).indented()
+
+    private val stubs = arrayOf(interfaceIFoo, interfaceIBar, interfaceIExempted)
+
+    private fun createVisitedPath(filename: String) =
+        "src/frameworks/base/services/java/com/android/server/$filename"
+
+    private val ignoredPath = "src/test/pkg/TestClass.java"
+}
diff --git a/tools/lint/utils/checks/src/main/java/com/google/android/lint/aidl/ExemptAidlInterfacesGenerator.kt b/tools/lint/utils/checks/src/main/java/com/google/android/lint/aidl/ExemptAidlInterfacesGenerator.kt
index 6ad223c87a29..57c2e5aa9767 100644
--- a/tools/lint/utils/checks/src/main/java/com/google/android/lint/aidl/ExemptAidlInterfacesGenerator.kt
+++ b/tools/lint/utils/checks/src/main/java/com/google/android/lint/aidl/ExemptAidlInterfacesGenerator.kt
@@ -33,12 +33,6 @@ import org.jetbrains.uast.UMethod
  */
 class ExemptAidlInterfacesGenerator : AidlImplementationDetector() {
     private val targetExemptAidlInterfaceNames = mutableSetOf<String>()
-    private val systemServicePathPrefixes = setOf(
-        "frameworks/base/services",
-        "frameworks/base/apex",
-        "frameworks/opt/wear",
-        "packages/modules"
-    )
 
     // We could've improved performance by visiting classes rather than methods, however, this lint
     // check won't be run regularly, hence we've decided not to add extra overrides to
@@ -49,14 +43,7 @@ class ExemptAidlInterfacesGenerator : AidlImplementationDetector() {
         interfaceName: String,
         body: UBlockExpression
     ) {
-        val filePath = context.file.path
-
-        // We perform `filePath.contains` instead of `filePath.startsWith` since getting the
-        // relative path of a source file is non-trivial. That is because `context.file.path`
-        // returns the path to where soong builds the file (i.e. /out/soong/...). Moreover, the
-        // logic to extract the relative path would need to consider several /out/soong/...
-        // locations patterns.
-        if (systemServicePathPrefixes.none { filePath.contains(it) }) return
+        if (!isSystemServicePath(context)) return
 
         val fullyQualifiedInterfaceName =
             getContainingAidlInterfaceQualified(context, node) ?: return