Skip to content

Commit 0c1a6b4

Browse files
hotchemihotchemi
authored
Fix CallNeedsPermissionDetector to scan only annotated classes (#536)
* Update BUILD_TOOLS_VERSION. * Add test for the case. * Fix to scan only annotated classes. #502
1 parent b7ebda0 commit 0c1a6b4

File tree

4 files changed

+111
-15
lines changed

4 files changed

+111
-15
lines changed

‎gradle.properties

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ COMMONS_IO_VERSION = 2.6
3333

3434
# Android configuration
3535
COMPILE_SDK_VERSION = android-28
36-
BUILD_TOOLS_VERSION = 28.0.0-rc2
36+
BUILD_TOOLS_VERSION = 28.0.3
3737
TARGET_SDK_VERSION = 28
3838
MIN_SDK_VERSION = 14
3939

‎lint/src/main/java/permissions/dispatcher/CallNeedsPermissionDetector.java

+23-12
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import com.android.tools.lint.detector.api.Scope;
1010
import com.android.tools.lint.detector.api.Severity;
1111

12+
import org.jetbrains.annotations.NotNull;
1213
import org.jetbrains.uast.UAnnotation;
1314
import org.jetbrains.uast.UCallExpression;
1415
import org.jetbrains.uast.UClass;
@@ -33,17 +34,18 @@ public final class CallNeedsPermissionDetector extends Detector implements Detec
3334
Severity.ERROR,
3435
new Implementation(CallNeedsPermissionDetector.class, EnumSet.of(Scope.ALL_JAVA_FILES)));
3536

36-
static Set<String> annotatedMethods = new HashSet<String>();
37+
private static Set<String> annotatedMethods = new HashSet<String>();
3738

3839
@Override
3940
public List<Class<? extends UElement>> getApplicableUastTypes() {
4041
return Collections.<Class<? extends UElement>>singletonList(UClass.class);
4142
}
4243

4344
@Override
44-
public UElementHandler createUastHandler(final JavaContext context) {
45+
public UElementHandler createUastHandler(@NotNull final JavaContext context) {
4546
return new UElementHandler() {
46-
@Override public void visitClass(UClass node) {
47+
@Override
48+
public void visitClass(@NotNull UClass node) {
4749
node.accept(new AnnotationChecker(context));
4850
}
4951
};
@@ -53,13 +55,24 @@ private static class AnnotationChecker extends AbstractUastVisitor {
5355

5456
private final JavaContext context;
5557

58+
private boolean hasRuntimePermissionAnnotation;
59+
5660
private AnnotationChecker(JavaContext context) {
5761
this.context = context;
5862
}
5963

6064
@Override
61-
public boolean visitCallExpression(UCallExpression node) {
62-
if (isGeneratedFiles(context)) {
65+
public boolean visitAnnotation(@NotNull UAnnotation node) {
66+
if (!"permissions.dispatcher.RuntimePermissions".equals(node.getQualifiedName())) {
67+
return true;
68+
}
69+
hasRuntimePermissionAnnotation = true;
70+
return true;
71+
}
72+
73+
@Override
74+
public boolean visitCallExpression(@NotNull UCallExpression node) {
75+
if (isGeneratedFiles(context) || !hasRuntimePermissionAnnotation) {
6376
return true;
6477
}
6578
if (node.getReceiver() == null && annotatedMethods.contains(node.getMethodName())) {
@@ -69,7 +82,7 @@ public boolean visitCallExpression(UCallExpression node) {
6982
}
7083

7184
@Override
72-
public boolean visitMethod(UMethod node) {
85+
public boolean visitMethod(@NotNull UMethod node) {
7386
if (isGeneratedFiles(context)) {
7487
return super.visitMethod(node);
7588
}
@@ -87,13 +100,11 @@ private static boolean isGeneratedFiles(JavaContext context) {
87100
return false;
88101
}
89102
List<UClass> classes = sourceFile.getClasses();
90-
if (!classes.isEmpty()) {
91-
String qualifiedName = classes.get(0).getName();
92-
if (qualifiedName != null && qualifiedName.contains("PermissionsDispatcher")) {
93-
return true;
94-
}
103+
if (classes.isEmpty()) {
104+
return false;
95105
}
96-
return false;
106+
String qualifiedName = classes.get(0).getName();
107+
return qualifiedName != null && qualifiedName.contains("PermissionsDispatcher");
97108
}
98109
}
99110
}

‎lint/src/test/java/permissions/dispatcher/CallNeedsPermissionDetectorKtTest.kt

+44-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import com.android.tools.lint.checks.infrastructure.TestFiles.java
77
import com.android.tools.lint.checks.infrastructure.TestFiles.kt
88
import com.android.tools.lint.checks.infrastructure.TestLintTask.lint
99
import permissions.dispatcher.Utils.onNeedsPermission
10+
import permissions.dispatcher.Utils.runtimePermission
1011

1112
class CallNeedsPermissionDetectorKtTest {
1213

@@ -17,7 +18,9 @@ class CallNeedsPermissionDetectorKtTest {
1718
package com.example
1819
1920
import permissions.dispatcher.NeedsPermission
21+
import permissions.dispatcher.RuntimePermissions
2022
23+
@RuntimePermissions
2124
class Foo {
2225
@NeedsPermission("Test")
2326
fun fooBar() {
@@ -30,14 +33,15 @@ class CallNeedsPermissionDetectorKtTest {
3033
""".trimMargin()
3134

3235
val expectedText = """
33-
|src/com/example/Foo.kt:11: Error: Trying to access permission-protected method directly [CallNeedsPermission]
36+
|src/com/example/Foo.kt:13: Error: Trying to access permission-protected method directly [CallNeedsPermission]
3437
| fooBar()
3538
| ~~~~~~~~
3639
|1 errors, 0 warnings
3740
""".trimMargin()
3841

3942
lint()
4043
.files(
44+
java(runtimePermission),
4145
java(onNeedsPermission),
4246
kt(foo))
4347
.issues(CallNeedsPermissionDetector.ISSUE)
@@ -65,7 +69,9 @@ class CallNeedsPermissionDetectorKtTest {
6569
package com.example
6670
6771
import permissions.dispatcher.NeedsPermission
72+
import permissions.dispatcher.RuntimePermissions
6873
74+
@RuntimePermissions
6975
class Baz {
7076
@NeedsPermission("Test")
7177
fun fooBar() {
@@ -78,11 +84,48 @@ class CallNeedsPermissionDetectorKtTest {
7884

7985
lint()
8086
.files(
87+
java(runtimePermission),
8188
java(onNeedsPermission),
8289
kt(foo),
8390
kt(baz))
8491
.issues(CallNeedsPermissionDetector.ISSUE)
8592
.run()
8693
.expectClean()
8794
}
95+
96+
@Test
97+
@Throws(Exception::class)
98+
fun issues502() {
99+
@Language("kotlin") val foo = """
100+
package com.example
101+
102+
import permissions.dispatcher.NeedsPermission
103+
import permissions.dispatcher.RuntimePermissions
104+
105+
@RuntimePermissions
106+
class Foo: AppCompatActivity {
107+
@NeedsPermission(Manifest.permission.READ_SMS)
108+
fun requestOTP() {
109+
PhoneVerificationInputFragment().requestOTP()
110+
}
111+
}
112+
113+
class FooFragment: Fragment {
114+
fun resendOTP() {
115+
requestOTP()
116+
}
117+
private fun requestOTP() {
118+
}
119+
}
120+
""".trimMargin()
121+
122+
lint()
123+
.files(
124+
java(runtimePermission),
125+
java(onNeedsPermission),
126+
kt(foo))
127+
.issues(CallNeedsPermissionDetector.ISSUE)
128+
.run()
129+
.expectClean()
130+
}
88131
}

‎lint/src/test/java/permissions/dispatcher/CallNeedsPermissionDetectorTest.kt

+43-1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import org.junit.Test
66
import com.android.tools.lint.checks.infrastructure.TestFiles.java
77
import com.android.tools.lint.checks.infrastructure.TestLintTask.lint
88
import permissions.dispatcher.Utils.onNeedsPermission
9+
import permissions.dispatcher.Utils.runtimePermission
910

1011
class CallNeedsPermissionDetectorTest {
1112

@@ -16,7 +17,9 @@ class CallNeedsPermissionDetectorTest {
1617
package com.example;
1718
1819
import permissions.dispatcher.NeedsPermission;
20+
import permissions.dispatcher.RuntimePermissions;
1921
22+
@RuntimePermissions
2023
public class Foo {
2124
@NeedsPermission("Test")
2225
void fooBar() {
@@ -29,14 +32,15 @@ class CallNeedsPermissionDetectorTest {
2932
""".trimMargin()
3033

3134
val expectedText = """
32-
|src/com/example/Foo.java:11: Error: Trying to access permission-protected method directly [CallNeedsPermission]
35+
|src/com/example/Foo.java:13: Error: Trying to access permission-protected method directly [CallNeedsPermission]
3336
| fooBar();
3437
| ~~~~~~~~
3538
|1 errors, 0 warnings
3639
""".trimMargin()
3740

3841
lint()
3942
.files(
43+
java(runtimePermission),
4044
java(onNeedsPermission),
4145
java(foo))
4246
.issues(CallNeedsPermissionDetector.ISSUE)
@@ -64,7 +68,9 @@ class CallNeedsPermissionDetectorTest {
6468
package com.example;
6569
6670
import permissions.dispatcher.NeedsPermission;
71+
import permissions.dispatcher.RuntimePermissions;
6772
73+
@RuntimePermissions
6874
public class Baz {
6975
@NeedsPermission("Test")
7076
void fooBar() {
@@ -84,4 +90,40 @@ class CallNeedsPermissionDetectorTest {
8490
.run()
8591
.expectClean()
8692
}
93+
94+
@Test
95+
@Throws(Exception::class)
96+
fun issues502() {
97+
@Language("JAVA") val foo = """
98+
package com.example;
99+
100+
import permissions.dispatcher.NeedsPermission;
101+
import permissions.dispatcher.RuntimePermissions;
102+
103+
@RuntimePermissions
104+
public class Foo extends AppCompatActivity {
105+
@NeedsPermission({Manifest.permission.READ_SMS})
106+
void requestOTP() {
107+
new PhoneVerificationInputFragment().requestOTP();
108+
}
109+
}
110+
111+
class FooFragment extends Fragment {
112+
public void resendOTP() {
113+
requestOTP();
114+
}
115+
private void requestOTP() {
116+
}
117+
}
118+
""".trimMargin()
119+
120+
lint()
121+
.files(
122+
java(runtimePermission),
123+
java(onNeedsPermission),
124+
java(foo))
125+
.issues(CallNeedsPermissionDetector.ISSUE)
126+
.run()
127+
.expectClean()
128+
}
87129
}

0 commit comments

Comments
 (0)