602:Improve method identification logic (#627)

* Add a unit test.

* Temporal commit.

* Implemented temporal logic.

* Clean up and add unit test.

* Tweak a bit.

* Remove parameter logic.

* Add test case for Java.

Author: hotchemi

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) {

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()
+    }
 }

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()
+    }
 }