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 413e19717d50b..c5cf0fb2f9631 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 @@ -40,7 +40,7 @@ class AndroidFrameworkIssueRegistry : IssueRegistry() { EnforcePermissionDetector.ISSUE_MISSING_ENFORCE_PERMISSION, EnforcePermissionDetector.ISSUE_MISMATCHING_ENFORCE_PERMISSION, EnforcePermissionHelperDetector.ISSUE_ENFORCE_PERMISSION_HELPER, - SimpleManualPermissionEnforcementDetector.ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION, + SimpleManualPermissionEnforcementDetector.ISSUE_SIMPLE_MANUAL_PERMISSION_ENFORCEMENT, SaferParcelChecker.ISSUE_UNSAFE_API_USAGE, PackageVisibilityDetector.ISSUE_PACKAGE_NAME_NO_PACKAGE_VISIBILITY_FILTERS, RegisterReceiverFlagDetector.ISSUE_RECEIVER_EXPORTED_FLAG, 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 b377d503f3189..a20266a9b1403 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 @@ -31,7 +31,7 @@ class AndroidGlobalIssueRegistry : IssueRegistry() { EnforcePermissionDetector.ISSUE_MISSING_ENFORCE_PERMISSION, EnforcePermissionDetector.ISSUE_MISMATCHING_ENFORCE_PERMISSION, EnforcePermissionHelperDetector.ISSUE_ENFORCE_PERMISSION_HELPER, - SimpleManualPermissionEnforcementDetector.ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION, + SimpleManualPermissionEnforcementDetector.ISSUE_SIMPLE_MANUAL_PERMISSION_ENFORCEMENT, ) override val api: Int diff --git a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionFix.kt b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionFix.kt index f1b634898ec9d..5c177d59e349a 100644 --- a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionFix.kt +++ b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionFix.kt @@ -17,6 +17,7 @@ package com.google.android.lint.aidl import com.android.tools.lint.detector.api.JavaContext +import com.android.tools.lint.detector.api.LintFix import com.android.tools.lint.detector.api.Location import com.android.tools.lint.detector.api.getUMethod import com.google.android.lint.hasPermissionNameAnnotation @@ -38,11 +39,34 @@ data class EnforcePermissionFix( val locations: List, val permissionNames: List ) { - val annotation: String + fun toLintFix(annotationLocation: Location): LintFix { + val removeFixes = this.locations.map { + LintFix.create() + .replace() + .reformat(true) + .range(it) + .with("") + .autoFix() + .build() + } + + val annotateFix = LintFix.create() + .annotate(this.annotation) + .range(annotationLocation) + .autoFix() + .build() + + return LintFix.create().composite(annotateFix, *removeFixes.toTypedArray()) + } + + private val annotation: String get() { val quotedPermissions = permissionNames.joinToString(", ") { """"$it"""" } + val annotationParameter = - if (permissionNames.size > 1) "allOf={$quotedPermissions}" else quotedPermissions + if (permissionNames.size > 1) "allOf={$quotedPermissions}" + else quotedPermissions + return "@$ANNOTATION_ENFORCE_PERMISSION($annotationParameter)" } diff --git a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/SimpleManualPermissionEnforcementDetector.kt b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/SimpleManualPermissionEnforcementDetector.kt index 4c0cbe7b3adf8..8ff813e4663e1 100644 --- a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/SimpleManualPermissionEnforcementDetector.kt +++ b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/SimpleManualPermissionEnforcementDetector.kt @@ -28,6 +28,7 @@ import org.jetbrains.uast.UElement import org.jetbrains.uast.UIfExpression import org.jetbrains.uast.UMethod import org.jetbrains.uast.UQualifiedReferenceExpression +import org.jetbrains.uast.skipParenthesizedExprDown /** * Looks for methods implementing generated AIDL interface stubs @@ -44,29 +45,16 @@ class SimpleManualPermissionEnforcementDetector : AidlImplementationDetector() { interfaceName: String, body: UBlockExpression ) { - val fix = accumulateSimplePermissionCheckFixes(body, context) ?: return - - val javaRemoveFixes = fix.locations.map { - fix() - .replace() - .reformat(true) - .range(it) - .with("") - .autoFix() - .build() - } - - val javaAnnotateFix = fix() - .annotate(fix.annotation) - .range(context.getLocation(node)) - .autoFix() - .build() + val enforcePermissionFix = accumulateSimplePermissionCheckFixes(body, context) ?: return + val lintFix = enforcePermissionFix.toLintFix(context.getLocation(node)) + val message = + "$interfaceName permission check can be converted to @EnforcePermission annotation" context.report( - ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION, - fix.locations.last(), - "$interfaceName permission check can be converted to @EnforcePermission annotation", - fix().composite(*javaRemoveFixes.toTypedArray(), javaAnnotateFix) + ISSUE_SIMPLE_MANUAL_PERMISSION_ENFORCEMENT, + enforcePermissionFix.locations.last(), + message, + lintFix ) } @@ -89,7 +77,8 @@ class SimpleManualPermissionEnforcementDetector : AidlImplementationDetector() { EnforcePermissionFix? { val singleFixes = mutableListOf() for (expression in methodBody.expressions) { - singleFixes.add(getPermissionCheckFix(expression, context) ?: break) + singleFixes.add(getPermissionCheckFix(expression.skipParenthesizedExprDown(), context) + ?: break) } return when (singleFixes.size) { 0 -> null @@ -133,7 +122,7 @@ class SimpleManualPermissionEnforcementDetector : AidlImplementationDetector() { """.trimIndent() @JvmField - val ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION = Issue.create( + val ISSUE_SIMPLE_MANUAL_PERMISSION_ENFORCEMENT = Issue.create( id = "SimpleManualPermissionEnforcement", briefDescription = "Manual permission check can be @EnforcePermission annotation", explanation = EXPLANATION, diff --git a/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/SimpleManualPermissionEnforcementDetectorTest.kt b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/SimpleManualPermissionEnforcementDetectorTest.kt index 150fc264506f4..50d5081c4b0df 100644 --- a/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/SimpleManualPermissionEnforcementDetectorTest.kt +++ b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/SimpleManualPermissionEnforcementDetectorTest.kt @@ -18,7 +18,6 @@ package com.google.android.lint.aidl import com.android.tools.lint.checks.infrastructure.LintDetectorTest 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 @@ -27,7 +26,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { override fun getDetector(): Detector = SimpleManualPermissionEnforcementDetector() override fun getIssues(): List = listOf( SimpleManualPermissionEnforcementDetector - .ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION + .ISSUE_SIMPLE_MANUAL_PERMISSION_ENFORCEMENT ) override fun lint(): TestLintTask = super.lint().allowMissingSdk() @@ -36,15 +35,15 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { 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.permission.READ_CONTACTS", "foo"); - } + 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.permission.READ_CONTACTS", "foo"); } + } """ ).indented(), *stubs @@ -73,18 +72,18 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { lint().files( java( """ - import android.content.Context; - import android.test.ITest; - public class Foo { - private Context mContext; - private ITest itest = new ITest.Stub() { - @Override - public void test() throws android.os.RemoteException { - mContext.enforceCallingOrSelfPermission( - "android.permission.READ_CONTACTS", "foo"); - } - }; - } + import android.content.Context; + import android.test.ITest; + public class Foo { + private Context mContext; + private ITest itest = new ITest.Stub() { + @Override + public void test() throws android.os.RemoteException { + mContext.enforceCallingOrSelfPermission( + "android.permission.READ_CONTACTS", "foo"); + } + }; + } """ ).indented(), *stubs @@ -114,16 +113,16 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { lint().files( java( """ - import android.content.Context; - import android.test.ITest; + 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"); - } + 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, @@ -153,20 +152,20 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { lint().files( java( """ - import android.content.Context; - import android.test.ITest; - public class Foo { - private Context mContext; - private ITest itest = new ITest.Stub() { - @Override - public void test() throws android.os.RemoteException { - mContext.enforceCallingOrSelfPermission( - "android.permission.READ_CONTACTS", "foo"); - mContext.enforceCallingOrSelfPermission( - "android.permission.WRITE_CONTACTS", "foo"); - } - }; - } + import android.content.Context; + import android.test.ITest; + public class Foo { + private Context mContext; + private ITest itest = new ITest.Stub() { + @Override + public void test() throws android.os.RemoteException { + mContext.enforceCallingOrSelfPermission( + "android.permission.READ_CONTACTS", "foo"); + mContext.enforceCallingOrSelfPermission( + "android.permission.WRITE_CONTACTS", "foo"); + } + }; + } """ ).indented(), *stubs @@ -198,16 +197,16 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { lint().files( java( """ - import android.os.Binder; - import android.test.ITest; - public class Foo extends ITest.Stub { - private mContext Context; - @Override - public void test() throws android.os.RemoteException { - long uid = Binder.getCallingUid(); - mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo"); - } + import android.os.Binder; + import android.test.ITest; + public class Foo extends ITest.Stub { + private mContext Context; + @Override + public void test() throws android.os.RemoteException { + long uid = Binder.getCallingUid(); + mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo"); } + } """ ).indented(), *stubs @@ -217,25 +216,25 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { } fun testPermissionHelper() { - lint().skipTestModes(TestMode.PARENTHESIZED).files( + lint().files( java( """ - import android.content.Context; - import android.test.ITest; + import android.content.Context; + import android.test.ITest; - public class Foo extends ITest.Stub { - private Context mContext; + 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(); - } + @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 @@ -261,7 +260,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { } fun testPermissionHelperAllOf() { - lint().skipTestModes(TestMode.PARENTHESIZED).files( + lint().files( java( """ import android.content.Context; @@ -309,7 +308,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { fun testPermissionHelperNested() { - lint().skipTestModes(TestMode.PARENTHESIZED).files( + lint().files( java( """ import android.content.Context;