diff --git a/lint/src/main/java/permissions/dispatcher/CallNeedsPermissionDetector.java b/lint/src/main/java/permissions/dispatcher/CallNeedsPermissionDetector.java index 3f3f6f69..1a3ec37c 100644 --- a/lint/src/main/java/permissions/dispatcher/CallNeedsPermissionDetector.java +++ b/lint/src/main/java/permissions/dispatcher/CallNeedsPermissionDetector.java @@ -10,6 +10,7 @@ import com.android.tools.lint.detector.api.Severity; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.jetbrains.uast.UAnnotation; import org.jetbrains.uast.UCallExpression; import org.jetbrains.uast.UClass; @@ -26,6 +27,8 @@ public final class CallNeedsPermissionDetector extends Detector implements Detector.UastScanner { + private static final String COLON = ":"; + static final Issue ISSUE = Issue.create("CallNeedsPermission", "Call the corresponding \"WithPermissionCheck\" method of the generated PermissionsDispatcher class instead", "Directly invoking a method annotated with @NeedsPermission may lead to misleading behaviour on devices running Marshmallow and up. Therefore, it is advised to use the generated PermissionsDispatcher class instead, which provides a \"WithPermissionCheck\" method that safely handles runtime permissions.", @@ -75,12 +78,34 @@ public boolean visitCallExpression(@NotNull UCallExpression node) { if (isGeneratedFiles(context) || !hasRuntimePermissionAnnotation) { return true; } - if (node.getReceiver() == null && annotatedMethods.contains(node.getMethodName())) { + if (node.getReceiver() == null && annotatedMethods.contains(methodIdentifier(node))) { context.report(ISSUE, node, context.getLocation(node), "Trying to access permission-protected method directly"); } return true; } + /** + * Generate method identifier from method information. + * + * @param node UCallExpression + * @return className + methodName + parametersType + */ + @Nullable + private static String methodIdentifier(@NotNull UCallExpression node) { + UElement element = node.getUastParent(); + while (element != null) { + if (element instanceof UClass) { + break; + } + element = element.getUastParent(); + } + UClass uClass = (UClass) element; + if (uClass == null || node.getMethodName() == null) { + return null; + } + return uClass.getName() + COLON + node.getMethodName(); + } + @Override public boolean visitMethod(@NotNull UMethod node) { if (isGeneratedFiles(context)) { @@ -90,10 +115,30 @@ public boolean visitMethod(@NotNull UMethod node) { if (annotation == null) { return super.visitMethod(node); } - annotatedMethods.add(node.getName()); + String methodIdentifier = methodIdentifier(node); + if (methodIdentifier == null) { + return super.visitMethod(node); + } + annotatedMethods.add(methodIdentifier); return super.visitMethod(node); } + /** + * Generate method identifier from method information. + * + * @param node UMethod + * @return className + methodName + parametersType + */ + @Nullable + private static String methodIdentifier(@NotNull UMethod node) { + UElement parent = node.getUastParent(); + if (!(parent instanceof UClass)) { + return null; + } + UClass uClass = (UClass) parent; + return uClass.getName() + COLON + node.getName(); + } + private static boolean isGeneratedFiles(JavaContext context) { UFile sourceFile = context.getUastFile(); if (sourceFile == null) { diff --git a/lint/src/test/java/permissions/dispatcher/CallNeedsPermissionDetectorKtTest.kt b/lint/src/test/java/permissions/dispatcher/CallNeedsPermissionDetectorKtTest.kt index 9eae3fc5..2c7046bf 100644 --- a/lint/src/test/java/permissions/dispatcher/CallNeedsPermissionDetectorKtTest.kt +++ b/lint/src/test/java/permissions/dispatcher/CallNeedsPermissionDetectorKtTest.kt @@ -1,18 +1,16 @@ package permissions.dispatcher -import org.intellij.lang.annotations.Language -import org.junit.Test - import com.android.tools.lint.checks.infrastructure.TestFiles.java import com.android.tools.lint.checks.infrastructure.TestFiles.kt import com.android.tools.lint.checks.infrastructure.TestLintTask.lint +import org.intellij.lang.annotations.Language +import org.junit.Test import permissions.dispatcher.Utils.onNeedsPermission import permissions.dispatcher.Utils.runtimePermission class CallNeedsPermissionDetectorKtTest { @Test - @Throws(Exception::class) fun callNeedsPermissionMethod() { @Language("kotlin") val foo = """ package com.example @@ -52,7 +50,6 @@ class CallNeedsPermissionDetectorKtTest { } @Test - @Throws(Exception::class) fun callNeedsPermissionMethodNoError() { @Language("kotlin") val foo = """ package com.example @@ -94,7 +91,6 @@ class CallNeedsPermissionDetectorKtTest { } @Test - @Throws(Exception::class) fun issues502() { @Language("kotlin") val foo = """ package com.example @@ -128,4 +124,42 @@ class CallNeedsPermissionDetectorKtTest { .run() .expectClean() } + + @Test + fun `same name methods in different class(issue602)`() { + @Language("kotlin") val foo = """ + package com.example + + import permissions.dispatcher.NeedsPermission + import permissions.dispatcher.RuntimePermissions + + @RuntimePermissions + class FirstActivity : AppCompatActivity() { + @NeedsPermission(Manifest.permission.CAMERA) + fun someFun() { + } + } + + @RuntimePermissions + class SecondActivity : AppCompatActivity() { + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + someFun() + } + + fun someFun() { + } + + @NeedsPermission(Manifest.permission.CAMERA) + fun otherFun() { + } + } + """.trimMargin() + + lint() + .files(java(runtimePermission), java(onNeedsPermission), kt(foo)) + .issues(CallNeedsPermissionDetector.ISSUE) + .run() + .expectClean() + } } diff --git a/lint/src/test/java/permissions/dispatcher/CallNeedsPermissionDetectorTest.kt b/lint/src/test/java/permissions/dispatcher/CallNeedsPermissionDetectorTest.kt index 383932e1..fc14e5b9 100644 --- a/lint/src/test/java/permissions/dispatcher/CallNeedsPermissionDetectorTest.kt +++ b/lint/src/test/java/permissions/dispatcher/CallNeedsPermissionDetectorTest.kt @@ -1,10 +1,9 @@ package permissions.dispatcher -import org.intellij.lang.annotations.Language -import org.junit.Test - import com.android.tools.lint.checks.infrastructure.TestFiles.java import com.android.tools.lint.checks.infrastructure.TestLintTask.lint +import org.intellij.lang.annotations.Language +import org.junit.Test import permissions.dispatcher.Utils.onNeedsPermission import permissions.dispatcher.Utils.runtimePermission @@ -126,4 +125,43 @@ class CallNeedsPermissionDetectorTest { .run() .expectClean() } + + @Test + fun `same name methods in different class(issue602)`() { + @Language("java") val foo = """ + package com.example; + + import permissions.dispatcher.NeedsPermission; + import permissions.dispatcher.RuntimePermissions; + + @RuntimePermissions + public class FirstActivity extends AppCompatActivity { + @NeedsPermission({Manifest.permission.READ_SMS}) + void someFun() { + } + } + + @RuntimePermissions + public class SecondActivity extends AppCompatActivity { + @Override + protected void onCreate(@Nullable Bundle savedInstanceState) { + super.onCreate(savedInstanceState); + someFun(); + } + + void someFun() { + } + + @NeedsPermission({Manifest.permission.READ_SMS}) + void otherFun() { + } + } + """.trimMargin() + + lint() + .files(java(runtimePermission), java(onNeedsPermission), java(foo)) + .issues(CallNeedsPermissionDetector.ISSUE) + .run() + .expectClean() + } }