From 003fbc74f4a7a508ffb0746a4dfe4f9e52100413 Mon Sep 17 00:00:00 2001 From: mattgilbride Date: Thu, 29 Dec 2022 19:16:22 +0000 Subject: [PATCH] SimpleManualPermissionEnforcementDetector: suggest calling helper method and turn on The previous fix only suggested removing the manual permission enforcement. Suggest replacing with the required helper method call. Also turn this detector on, but reporting all incidents at WARNING level. This will surface the call sites that need migration in the errorprone build without actually causing failure. Once all call sites that would cause an ERROR level incident have been migrated, we can turn that functionality back on (b/265014041). Bug: 261976627 Test: SimpleManualPermissionEnforcementDetectorTest Change-Id: I9e2e6d1981d9a8dc59b9c8078d7ce80b1d0b10ab --- .../com/google/android/lint/aidl/Constants.kt | 2 + .../android/lint/aidl/EnforcePermissionFix.kt | 39 ++- .../aidl/EnforcePermissionHelperDetector.kt | 15 +- .../lint/aidl/EnforcePermissionUtils.kt | 25 ++ ...mpleManualPermissionEnforcementDetector.kt | 12 +- ...ManualPermissionEnforcementDetectorTest.kt | 317 +++++++++--------- 6 files changed, 226 insertions(+), 184 deletions(-) diff --git a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/Constants.kt b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/Constants.kt index 72de00f885ba2..dcfbe953f955b 100644 --- a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/Constants.kt +++ b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/Constants.kt @@ -29,6 +29,8 @@ val AIDL_PERMISSION_ANNOTATIONS = listOf( const val BINDER_CLASS = "android.os.Binder" const val IINTERFACE_INTERFACE = "android.os.IInterface" +const val AIDL_PERMISSION_HELPER_SUFFIX = "_enforcePermission" + /** * If a non java (e.g. c++) backend is enabled, the @EnforcePermission * annotation cannot be used. At time of writing, the mechanism 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 485765ba90909..39d6eb22ef20b 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 @@ -36,6 +36,7 @@ import org.jetbrains.uast.UCallExpression import org.jetbrains.uast.UExpression import org.jetbrains.uast.UExpressionList import org.jetbrains.uast.UIfExpression +import org.jetbrains.uast.UMethod import org.jetbrains.uast.UThrowExpression import org.jetbrains.uast.UastBinaryOperator import org.jetbrains.uast.evaluateString @@ -46,29 +47,37 @@ import org.jetbrains.uast.visitor.AbstractUastVisitor * Helper class that facilitates the creation of lint auto fixes */ data class EnforcePermissionFix( - val locations: List, + val manualCheckLocations: List, val permissionNames: List, val errorLevel: Boolean, val anyOf: Boolean, ) { - fun toLintFix(annotationLocation: Location): LintFix { - val removeFixes = this.locations.map { - LintFix.create() - .replace() - .reformat(true) - .range(it) - .with("") - .autoFix() - .build() + fun toLintFix(context: JavaContext, node: UMethod): LintFix { + val methodLocation = context.getLocation(node) + val replaceOrRemoveFixes = manualCheckLocations.mapIndexed { index, manualCheckLocation -> + if (index == 0) { + // Replace the first manual check with a call to the helper method + getHelperMethodFix(node, manualCheckLocation, false) + } else { + // Remove all subsequent manual checks + LintFix.create() + .replace() + .reformat(true) + .range(manualCheckLocation) + .with("") + .autoFix() + .build() + } } + // Annotate the method with @EnforcePermission(...) val annotateFix = LintFix.create() - .annotate(this.annotation) - .range(annotationLocation) + .annotate(annotation) + .range(methodLocation) .autoFix() .build() - return LintFix.create().composite(annotateFix, *removeFixes.toTypedArray()) + return LintFix.create().composite(annotateFix, *replaceOrRemoveFixes.toTypedArray()) } private val annotation: String @@ -90,6 +99,7 @@ data class EnforcePermissionFix( companion object { /** * Conditionally constructs EnforcePermissionFix from a UCallExpression + * * @return EnforcePermissionFix if the called method is annotated with @PermissionMethod, else null */ fun fromCallExpression( @@ -111,6 +121,7 @@ data class EnforcePermissionFix( /** * Conditionally constructs EnforcePermissionFix from a UCallExpression + * * @return EnforcePermissionFix IF AND ONLY IF: * * The condition of the if statement compares the return value of a * PermissionMethod to one of the PackageManager.PermissionResult values @@ -180,7 +191,7 @@ data class EnforcePermissionFix( throw AnyOfAllOfException() } return EnforcePermissionFix( - individuals.flatMap(EnforcePermissionFix::locations), + individuals.flatMap(EnforcePermissionFix::manualCheckLocations), individuals.flatMap(EnforcePermissionFix::permissionNames), errorLevel = individuals.all(EnforcePermissionFix::errorLevel), anyOf = anyOfs.isNotEmpty() diff --git a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionHelperDetector.kt b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionHelperDetector.kt index b65c0fc6a7db7..df13af516514c 100644 --- a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionHelperDetector.kt +++ b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionHelperDetector.kt @@ -55,7 +55,7 @@ class EnforcePermissionHelperDetector : Detector(), SourceCodeScanner { return } - val targetExpression = "${node.name}$HELPER_SUFFIX()" + val targetExpression = getHelperMethodCallSourceString(node) val message = "Method must start with $targetExpression or super.${node.name}(), if applicable" @@ -85,22 +85,11 @@ class EnforcePermissionHelperDetector : Detector(), SourceCodeScanner { val locationTarget = getLocationTarget(firstExpression) val expressionLocation = context.getLocation(locationTarget) - val indent = " ".repeat(expressionLocation.start?.column ?: 0) - - val fix = fix() - .replace() - .range(expressionLocation) - .beginning() - .with("$targetExpression;\n\n$indent") - .reformat(true) - .autoFix() - .build() - context.report( ISSUE_ENFORCE_PERMISSION_HELPER, context.getLocation(node), message, - fix + getHelperMethodFix(node, expressionLocation), ) } } diff --git a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt index 2239ea169632d..d41fee3fc0dc8 100644 --- a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt +++ b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt @@ -17,6 +17,8 @@ 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.intellij.psi.PsiClass import com.intellij.psi.PsiReferenceList import org.jetbrains.uast.UMethod @@ -69,3 +71,26 @@ private fun hasSingleAncestor(references: PsiReferenceList?, qualifiedName: Stri references != null && references.referenceElements.size == 1 && references.referenceElements[0].qualifiedName == qualifiedName + +fun getHelperMethodCallSourceString(node: UMethod) = "${node.name}$AIDL_PERMISSION_HELPER_SUFFIX()" + +fun getHelperMethodFix( + node: UMethod, + manualCheckLocation: Location, + prepend: Boolean = true +): LintFix { + val helperMethodSource = getHelperMethodCallSourceString(node) + val indent = " ".repeat(manualCheckLocation.start?.column ?: 0) + val newText = "$helperMethodSource;${if (prepend) "\n\n$indent" else ""}" + + val fix = LintFix.create() + .replace() + .range(manualCheckLocation) + .with(newText) + .reformat(true) + .autoFix() + + if (prepend) fix.beginning() + + return fix.build() +} 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 11a283af30495..44afb1a7e60b2 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 @@ -46,7 +46,7 @@ class SimpleManualPermissionEnforcementDetector : AidlImplementationDetector() { body: UBlockExpression ) { val enforcePermissionFix = accumulateSimplePermissionCheckFixes(body, context) ?: return - val lintFix = enforcePermissionFix.toLintFix(context.getLocation(node)) + val lintFix = enforcePermissionFix.toLintFix(context, node) val message = "$interfaceName permission check ${ if (enforcePermissionFix.errorLevel) "should" else "can" @@ -54,14 +54,15 @@ class SimpleManualPermissionEnforcementDetector : AidlImplementationDetector() { val incident = Incident( ISSUE_SIMPLE_MANUAL_PERMISSION_ENFORCEMENT, - enforcePermissionFix.locations.last(), + enforcePermissionFix.manualCheckLocations.last(), message, lintFix ) - if (enforcePermissionFix.errorLevel) { - incident.overrideSeverity(Severity.ERROR) - } + // TODO(b/265014041): turn on errors once all code that would cause one is fixed + // if (enforcePermissionFix.errorLevel) { + // incident.overrideSeverity(Severity.ERROR) + // } context.report(incident) } @@ -142,7 +143,6 @@ class SimpleManualPermissionEnforcementDetector : AidlImplementationDetector() { SimpleManualPermissionEnforcementDetector::class.java, Scope.JAVA_FILE_SCOPE ), - enabledByDefault = false, // TODO: enable once b/241171714 is resolved ) } } 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 2ac550bd468aa..c4dfa04c8d71a 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 @@ -51,10 +51,10 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { .run() .expect( """ - src/Foo.java:7: Error: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement] + src/Foo.java:7: Warning: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement] mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo"); ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - 1 errors, 0 warnings + 0 errors, 1 warnings """ ) .expectFixDiffs( @@ -64,6 +64,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { + @android.annotation.EnforcePermission("android.permission.READ_CONTACTS") @@ -7 +8 - mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo"); + + test_enforcePermission(); """ ) } @@ -101,6 +102,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { + @android.annotation.EnforcePermission("android.permission.READ_CONTACTS") @@ -7 +8 - mContext.enforceCallingPermission("android.permission.READ_CONTACTS", "foo"); + + test_enforcePermission(); """ ) } @@ -138,6 +140,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { + @android.annotation.EnforcePermission("android.permission.READ_CONTACTS") @@ -7 +8 - mContext.checkCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo"); + + test_enforcePermission(); """ ) } @@ -165,10 +168,10 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { .run() .expect( """ - src/Foo.java:8: Error: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement] + src/Foo.java:8: Warning: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement] mContext.enforceCallingOrSelfPermission( ^ - 1 errors, 0 warnings + 0 errors, 1 warnings """ ) .expectFixDiffs( @@ -179,6 +182,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { @@ -8 +9 - mContext.enforceCallingOrSelfPermission( - "android.permission.READ_CONTACTS", "foo"); + + test_enforcePermission(); """ ) } @@ -205,19 +209,20 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { .run() .expect( """ - src/Foo.java:8: Error: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement] + src/Foo.java:8: Warning: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement] mContext.enforceCallingOrSelfPermission(android.Manifest.permission.READ_CONTACTS, "foo"); ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - 1 errors, 0 warnings + 0 errors, 1 warnings """ ) .expectFixDiffs( """ - Fix for src/Foo.java line 7: Annotate with @EnforcePermission: + Fix for src/Foo.java line 8: Annotate with @EnforcePermission: @@ -6 +6 + @android.annotation.EnforcePermission("android.permission.READ_CONTACTS") @@ -8 +9 - mContext.enforceCallingOrSelfPermission(android.Manifest.permission.READ_CONTACTS, "foo"); + + test_enforcePermission(); """ ) } @@ -247,10 +252,10 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { .run() .expect( """ - src/Foo.java:10: Error: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement] + src/Foo.java:10: Warning: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement] mContext.enforceCallingOrSelfPermission( ^ - 1 errors, 0 warnings + 0 errors, 1 warnings """ ) .expectFixDiffs( @@ -263,98 +268,101 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { - "android.permission.READ_CONTACTS", "foo"); - mContext.enforceCallingOrSelfPermission( - "android.permission.WRITE_CONTACTS", "foo"); + + test_enforcePermission(); """ ) } fun testAllOf_mixedOrSelf_warning() { 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.enforceCallingPermission( - "android.permission.WRITE_CONTACTS", "foo"); - } - }; - } - """ - ).indented(), - *stubs + 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.enforceCallingPermission( + "android.permission.WRITE_CONTACTS", "foo"); + } + }; + } + """ + ).indented(), + *stubs ) - .run() - .expect( - """ - src/Foo.java:10: Warning: ITest permission check can be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement] - mContext.enforceCallingPermission( - ^ - 0 errors, 1 warnings - """ - ) - .expectFixDiffs( - """ - Fix for src/Foo.java line 10: Annotate with @EnforcePermission: - @@ -6 +6 - + @android.annotation.EnforcePermission(allOf={"android.permission.READ_CONTACTS", "android.permission.WRITE_CONTACTS"}) - @@ -8 +9 - - mContext.enforceCallingOrSelfPermission( - - "android.permission.READ_CONTACTS", "foo"); - - mContext.enforceCallingPermission( - - "android.permission.WRITE_CONTACTS", "foo"); - """ - ) + .run() + .expect( + """ + src/Foo.java:10: Warning: ITest permission check can be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement] + mContext.enforceCallingPermission( + ^ + 0 errors, 1 warnings + """ + ) + .expectFixDiffs( + """ + Fix for src/Foo.java line 10: Annotate with @EnforcePermission: + @@ -6 +6 + + @android.annotation.EnforcePermission(allOf={"android.permission.READ_CONTACTS", "android.permission.WRITE_CONTACTS"}) + @@ -8 +9 + - mContext.enforceCallingOrSelfPermission( + - "android.permission.READ_CONTACTS", "foo"); + - mContext.enforceCallingPermission( + - "android.permission.WRITE_CONTACTS", "foo"); + + test_enforcePermission(); + """ + ) } fun testAllOf_mixedEnforces_warning() { 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.checkCallingOrSelfPermission( - "android.permission.WRITE_CONTACTS", "foo"); - } - }; - } - """ - ).indented(), - *stubs + 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.checkCallingOrSelfPermission( + "android.permission.WRITE_CONTACTS", "foo"); + } + }; + } + """ + ).indented(), + *stubs ) - .run() - .expect( - """ - src/Foo.java:10: Warning: ITest permission check can be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement] - mContext.checkCallingOrSelfPermission( - ^ - 0 errors, 1 warnings - """ - ) - .expectFixDiffs( - """ - Fix for src/Foo.java line 10: Annotate with @EnforcePermission: - @@ -6 +6 - + @android.annotation.EnforcePermission(allOf={"android.permission.READ_CONTACTS", "android.permission.WRITE_CONTACTS"}) - @@ -8 +9 - - mContext.enforceCallingOrSelfPermission( - - "android.permission.READ_CONTACTS", "foo"); - - mContext.checkCallingOrSelfPermission( - - "android.permission.WRITE_CONTACTS", "foo"); - """ - ) + .run() + .expect( + """ + src/Foo.java:10: Warning: ITest permission check can be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement] + mContext.checkCallingOrSelfPermission( + ^ + 0 errors, 1 warnings + """ + ) + .expectFixDiffs( + """ + Fix for src/Foo.java line 10: Annotate with @EnforcePermission: + @@ -6 +6 + + @android.annotation.EnforcePermission(allOf={"android.permission.READ_CONTACTS", "android.permission.WRITE_CONTACTS"}) + @@ -8 +9 + - mContext.enforceCallingOrSelfPermission( + - "android.permission.READ_CONTACTS", "foo"); + - mContext.checkCallingOrSelfPermission( + - "android.permission.WRITE_CONTACTS", "foo"); + + test_enforcePermission(); + """ + ) } fun testPrecedingExpressions() { @@ -406,10 +414,10 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { .run() .expect( """ - src/Foo.java:14: Error: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement] + src/Foo.java:14: Warning: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement] helper(); ~~~~~~~~~ - 1 errors, 0 warnings + 0 errors, 1 warnings """ ) .expectFixDiffs( @@ -419,6 +427,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { + @android.annotation.EnforcePermission("android.permission.READ_CONTACTS") @@ -14 +15 - helper(); + + test_enforcePermission(); """ ) } @@ -463,6 +472,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { + @android.annotation.EnforcePermission("android.permission.READ_CONTACTS") @@ -14 +15 - helper(); + + test_enforcePermission(); """ ) } @@ -496,10 +506,10 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { .run() .expect( """ - src/Foo.java:16: Error: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement] + src/Foo.java:16: Warning: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement] mContext.enforceCallingOrSelfPermission("FOO", "foo"); ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - 1 errors, 0 warnings + 0 errors, 1 warnings """ ) .expectFixDiffs( @@ -510,6 +520,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { @@ -15 +16 - helper(); - mContext.enforceCallingOrSelfPermission("FOO", "foo"); + + test_enforcePermission(); """ ) } @@ -547,10 +558,10 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { .run() .expect( """ - src/Foo.java:19: Error: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement] + src/Foo.java:19: Warning: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement] helperHelper(); ~~~~~~~~~~~~~~~ - 1 errors, 0 warnings + 0 errors, 1 warnings """ ) .expectFixDiffs( @@ -560,6 +571,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { + @android.annotation.EnforcePermission(allOf={"android.permission.WRITE_CONTACTS", "android.permission.READ_CONTACTS"}) @@ -19 +20 - helperHelper(); + + test_enforcePermission(); """ ) } @@ -587,10 +599,10 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { .run() .expect( """ - src/Foo.java:7: Error: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement] + src/Foo.java:7: Warning: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement] if (mContext.checkCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo") ^ - 1 errors, 0 warnings + 0 errors, 1 warnings """ ) .expectFixDiffs( @@ -603,76 +615,78 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { - != PackageManager.PERMISSION_GRANTED) { - throw new SecurityException("yikes!"); - } + + test_enforcePermission(); """ ) } fun testIfExpression_orSelfFalse_warning() { 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 { - if (mContext.checkCallingPermission("android.permission.READ_CONTACTS", "foo") - != PackageManager.PERMISSION_GRANTED) { - throw new SecurityException("yikes!"); - } + 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 { + if (mContext.checkCallingPermission("android.permission.READ_CONTACTS", "foo") + != PackageManager.PERMISSION_GRANTED) { + throw new SecurityException("yikes!"); } } - """ - ).indented(), - *stubs + } + """ + ).indented(), + *stubs ) - .run() - .expect( - """ - src/Foo.java:7: Warning: ITest permission check can be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement] - if (mContext.checkCallingPermission("android.permission.READ_CONTACTS", "foo") - ^ - 0 errors, 1 warnings - """ - ) - .expectFixDiffs( - """ - Fix for src/Foo.java line 7: Annotate with @EnforcePermission: - @@ -5 +5 - + @android.annotation.EnforcePermission("android.permission.READ_CONTACTS") - @@ -7 +8 - - if (mContext.checkCallingPermission("android.permission.READ_CONTACTS", "foo") - - != PackageManager.PERMISSION_GRANTED) { - - throw new SecurityException("yikes!"); - - } - """ - ) + .run() + .expect( + """ + src/Foo.java:7: Warning: ITest permission check can be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement] + if (mContext.checkCallingPermission("android.permission.READ_CONTACTS", "foo") + ^ + 0 errors, 1 warnings + """ + ) + .expectFixDiffs( + """ + Fix for src/Foo.java line 7: Annotate with @EnforcePermission: + @@ -5 +5 + + @android.annotation.EnforcePermission("android.permission.READ_CONTACTS") + @@ -7 +8 + - if (mContext.checkCallingPermission("android.permission.READ_CONTACTS", "foo") + - != PackageManager.PERMISSION_GRANTED) { + - throw new SecurityException("yikes!"); + - } + + test_enforcePermission(); + """ + ) } fun testIfExpression_otherSideEffect_ignored() { 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 { - if (mContext.checkCallingPermission("android.permission.READ_CONTACTS", "foo") - != PackageManager.PERMISSION_GRANTED) { - doSomethingElse(); - throw new SecurityException("yikes!"); - } + 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 { + if (mContext.checkCallingPermission("android.permission.READ_CONTACTS", "foo") + != PackageManager.PERMISSION_GRANTED) { + doSomethingElse(); + throw new SecurityException("yikes!"); } } - """ - ).indented(), - *stubs + } + """ + ).indented(), + *stubs ) - .run() - .expectClean() + .run() + .expectClean() } fun testAnyOf_hardCodedAndVarArgs() { @@ -718,6 +732,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { + @android.annotation.EnforcePermission(anyOf={"BAZ", "BUZZ", "FOO", "BAR"}) @@ -17 +18 - helperHelper(); + + test_enforcePermission(); """ ) }