Skip to content

Commit

Permalink
602:Improve method identification logic (#627)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
hotchemi committed Jun 18, 2019
1 parent 2509d04 commit b4cc599
Show file tree
Hide file tree
Showing 3 changed files with 128 additions and 11 deletions.
Expand Up @@ -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;
Expand All @@ -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.",
Expand Down Expand Up @@ -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)) {
Expand All @@ -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) {
Expand Down
@@ -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
Expand Down Expand Up @@ -52,7 +50,6 @@ class CallNeedsPermissionDetectorKtTest {
}

@Test
@Throws(Exception::class)
fun callNeedsPermissionMethodNoError() {
@Language("kotlin") val foo = """
package com.example
Expand Down Expand Up @@ -94,7 +91,6 @@ class CallNeedsPermissionDetectorKtTest {
}

@Test
@Throws(Exception::class)
fun issues502() {
@Language("kotlin") val foo = """
package com.example
Expand Down Expand Up @@ -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()
}
}
@@ -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

Expand Down Expand Up @@ -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()
}
}

0 comments on commit b4cc599

Please sign in to comment.