Skip to content

Commit 01df673

Browse files
committed
Switched lint checks to the new PSI based api (closes bluelinelabs#184)
1 parent 553bae0 commit 01df673

File tree

13 files changed

+294
-160
lines changed

13 files changed

+294
-160
lines changed

.travis.yml

+2-2
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ language: android
33
android:
44
components:
55
- tools
6-
- build-tools-23.0.2
7-
- android-23
6+
- build-tools-23.0.3
7+
- android-25
88
- extra-android-m2repository
99

1010
script:

build.gradle

+1-5
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,8 @@ buildscript {
1010
allprojects {
1111
repositories {
1212
jcenter()
13-
mavenLocal()
13+
mavenCentral()
1414
}
1515
}
1616

17-
task wrapper(type: Wrapper) {
18-
gradleVersion = '2.10'
19-
}
20-
2117
apply from: rootProject.file('dependencies.gradle')

conductor-lint/build.gradle

+6
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
apply plugin: 'java'
22

3+
targetCompatibility = JavaVersion.VERSION_1_7
4+
sourceCompatibility = JavaVersion.VERSION_1_7
5+
36
configurations {
47
lintChecks
58
}
@@ -8,6 +11,9 @@ dependencies {
811
compile rootProject.ext.lintapi
912
compile rootProject.ext.lintchecks
1013

14+
testCompile rootProject.ext.lint
15+
testCompile rootProject.ext.lintTests
16+
1117
lintChecks files(jar)
1218
}
1319

Original file line numberDiff line numberDiff line change
@@ -1,29 +1,20 @@
11
package com.bluelinelabs.conductor.lint;
22

3-
import com.android.annotations.NonNull;
4-
import com.android.tools.lint.client.api.JavaParser.ResolvedClass;
3+
import com.android.tools.lint.client.api.JavaEvaluator;
54
import com.android.tools.lint.detector.api.Category;
65
import com.android.tools.lint.detector.api.Detector;
76
import com.android.tools.lint.detector.api.Implementation;
87
import com.android.tools.lint.detector.api.Issue;
98
import com.android.tools.lint.detector.api.JavaContext;
109
import com.android.tools.lint.detector.api.Scope;
1110
import com.android.tools.lint.detector.api.Severity;
12-
import com.android.tools.lint.detector.api.Speed;
11+
import com.intellij.psi.PsiClass;
12+
import com.intellij.psi.PsiMethod;
1313

14-
import java.lang.reflect.Modifier;
1514
import java.util.Collections;
1615
import java.util.List;
1716

18-
import lombok.ast.ClassDeclaration;
19-
import lombok.ast.ConstructorDeclaration;
20-
import lombok.ast.Node;
21-
import lombok.ast.NormalTypeBody;
22-
import lombok.ast.StrictListAccessor;
23-
import lombok.ast.TypeMember;
24-
import lombok.ast.VariableDefinition;
25-
26-
public final class ControllerChangeHandlerIssueDetector extends Detector implements Detector.JavaScanner, Detector.ClassScanner {
17+
public final class ControllerChangeHandlerIssueDetector extends Detector implements Detector.JavaPsiScanner {
2718

2819
public static final Issue ISSUE =
2920
Issue.create("ValidControllerChangeHandler", "ControllerChangeHandler not instantiatable",
@@ -34,67 +25,45 @@ public final class ControllerChangeHandlerIssueDetector extends Detector impleme
3425

3526
public ControllerChangeHandlerIssueDetector() { }
3627

37-
@NonNull
38-
@Override
39-
public Speed getSpeed() {
40-
return Speed.FAST;
41-
}
42-
4328
@Override
4429
public List<String> applicableSuperClasses() {
4530
return Collections.singletonList("com.bluelinelabs.conductor.ControllerChangeHandler");
4631
}
4732

4833
@Override
49-
public void checkClass(@NonNull JavaContext context, ClassDeclaration node,
50-
@NonNull Node declarationOrAnonymous, @NonNull ResolvedClass cls) {
51-
52-
if (node == null) {
34+
public void checkClass(JavaContext context, PsiClass declaration) {
35+
final JavaEvaluator evaluator = context.getEvaluator();
36+
if (evaluator.isAbstract(declaration)) {
5337
return;
5438
}
5539

56-
final int flags = node.astModifiers().getEffectiveModifierFlags();
57-
if ((flags & Modifier.ABSTRACT) != 0) {
40+
if (!evaluator.isPublic(declaration)) {
41+
String message = String.format("This ControllerChangeHandler class should be public (%1$s)", declaration.getQualifiedName());
42+
context.report(ISSUE, declaration, context.getLocation(declaration), message);
5843
return;
5944
}
6045

61-
if ((flags & Modifier.PUBLIC) == 0) {
62-
String message = String.format("This ControllerChangeHandler class should be public (%1$s)", cls.getName());
63-
context.report(ISSUE, node, context.getLocation(node.astName()), message);
46+
if (declaration.getContainingClass() != null && !evaluator.isStatic(declaration)) {
47+
String message = String.format("This ControllerChangeHandler inner class should be static (%1$s)", declaration.getQualifiedName());
48+
context.report(ISSUE, declaration, context.getLocation(declaration), message);
6449
return;
6550
}
6651

67-
if (cls.getContainingClass() != null && (flags & Modifier.STATIC) == 0) {
68-
String message = String.format("This ControllerChangeHandler inner class should be static (%1$s)", cls.getName());
69-
context.report(ISSUE, node, context.getLocation(node.astName()), message);
70-
return;
71-
}
72-
73-
boolean hasConstructor = false;
7452
boolean hasDefaultConstructor = false;
75-
NormalTypeBody body = node.astBody();
76-
if (body != null) {
77-
for (TypeMember member : body.astMembers()) {
78-
if (member instanceof ConstructorDeclaration) {
79-
hasConstructor = true;
80-
ConstructorDeclaration constructor = (ConstructorDeclaration)member;
81-
82-
if (constructor.astModifiers().isPublic()) {
83-
StrictListAccessor<VariableDefinition, ConstructorDeclaration> params = constructor.astParameters();
84-
if (params.isEmpty()) {
85-
hasDefaultConstructor = true;
86-
break;
87-
}
88-
}
53+
PsiMethod[] constructors = declaration.getConstructors();
54+
for (PsiMethod constructor : constructors) {
55+
if (evaluator.isPublic(constructor)) {
56+
if (constructor.getParameterList().getParametersCount() == 0) {
57+
hasDefaultConstructor = true;
58+
break;
8959
}
9060
}
9161
}
9262

93-
if (hasConstructor && !hasDefaultConstructor) {
63+
if (constructors.length > 0 && !hasDefaultConstructor) {
9464
String message = String.format(
95-
"This ControllerChangeHandler needs to have a public default constructor (`%1$s`)",
96-
cls.getName());
97-
context.report(ISSUE, node, context.getLocation(node.astName()), message);
65+
"This ControllerChangeHandler needs to have a public default constructor (`%1$s`)", declaration.getQualifiedName());
66+
context.report(ISSUE, declaration, context.getLocation(declaration), message);
9867
}
9968
}
10069
}
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,22 @@
11
package com.bluelinelabs.conductor.lint;
22

33
import com.android.SdkConstants;
4-
import com.android.annotations.NonNull;
5-
import com.android.tools.lint.client.api.JavaParser.ResolvedClass;
4+
import com.android.tools.lint.client.api.JavaEvaluator;
65
import com.android.tools.lint.detector.api.Category;
76
import com.android.tools.lint.detector.api.Detector;
87
import com.android.tools.lint.detector.api.Implementation;
98
import com.android.tools.lint.detector.api.Issue;
109
import com.android.tools.lint.detector.api.JavaContext;
1110
import com.android.tools.lint.detector.api.Scope;
1211
import com.android.tools.lint.detector.api.Severity;
13-
import com.android.tools.lint.detector.api.Speed;
12+
import com.intellij.psi.PsiClass;
13+
import com.intellij.psi.PsiMethod;
14+
import com.intellij.psi.PsiParameter;
1415

15-
import java.lang.reflect.Modifier;
1616
import java.util.Collections;
1717
import java.util.List;
1818

19-
import lombok.ast.ClassDeclaration;
20-
import lombok.ast.ConstructorDeclaration;
21-
import lombok.ast.Node;
22-
import lombok.ast.NormalTypeBody;
23-
import lombok.ast.StrictListAccessor;
24-
import lombok.ast.TypeMember;
25-
import lombok.ast.VariableDefinition;
26-
27-
public final class ControllerIssueDetector extends Detector implements Detector.JavaScanner, Detector.ClassScanner {
19+
public final class ControllerIssueDetector extends Detector implements Detector.JavaPsiScanner {
2820

2921
public static final Issue ISSUE =
3022
Issue.create("ValidController", "Controller not instantiatable",
@@ -35,74 +27,56 @@ public final class ControllerIssueDetector extends Detector implements Detector.
3527

3628
public ControllerIssueDetector() { }
3729

38-
@NonNull
39-
@Override
40-
public Speed getSpeed() {
41-
return Speed.FAST;
42-
}
43-
4430
@Override
4531
public List<String> applicableSuperClasses() {
4632
return Collections.singletonList("com.bluelinelabs.conductor.Controller");
4733
}
4834

4935
@Override
50-
public void checkClass(@NonNull JavaContext context, ClassDeclaration node,
51-
@NonNull Node declarationOrAnonymous, @NonNull ResolvedClass cls) {
52-
53-
if (node == null) {
36+
public void checkClass(JavaContext context, PsiClass declaration) {
37+
final JavaEvaluator evaluator = context.getEvaluator();
38+
if (evaluator.isAbstract(declaration)) {
5439
return;
5540
}
5641

57-
final int flags = node.astModifiers().getEffectiveModifierFlags();
58-
if ((flags & Modifier.ABSTRACT) != 0) {
42+
if (!evaluator.isPublic(declaration)) {
43+
String message = String.format("This Controller class should be public (%1$s)", declaration.getQualifiedName());
44+
context.report(ISSUE, declaration, context.getLocation(declaration), message);
5945
return;
6046
}
6147

62-
if ((flags & Modifier.PUBLIC) == 0) {
63-
String message = String.format("This Controller class should be public (%1$s)", cls.getName());
64-
context.report(ISSUE, node, context.getLocation(node.astName()), message);
48+
if (declaration.getContainingClass() != null && !evaluator.isStatic(declaration)) {
49+
String message = String.format("This Controller inner class should be static (%1$s)", declaration.getQualifiedName());
50+
context.report(ISSUE, declaration, context.getLocation(declaration), message);
6551
return;
6652
}
6753

68-
if (cls.getContainingClass() != null && (flags & Modifier.STATIC) == 0) {
69-
String message = String.format("This Controller inner class should be static (%1$s)", cls.getName());
70-
context.report(ISSUE, node, context.getLocation(node.astName()), message);
71-
return;
72-
}
7354

74-
boolean hasConstructor = false;
7555
boolean hasDefaultConstructor = false;
7656
boolean hasBundleConstructor = false;
77-
NormalTypeBody body = node.astBody();
78-
if (body != null) {
79-
for (TypeMember member : body.astMembers()) {
80-
if (member instanceof ConstructorDeclaration) {
81-
hasConstructor = true;
82-
ConstructorDeclaration constructor = (ConstructorDeclaration)member;
83-
84-
if (constructor.astModifiers().isPublic()) {
85-
StrictListAccessor<VariableDefinition, ConstructorDeclaration> params = constructor.astParameters();
86-
if (params.isEmpty()) {
87-
hasDefaultConstructor = true;
88-
break;
89-
} else if (params.size() == 1 &&
90-
(params.first().astTypeReference().getTypeName().equals(SdkConstants.CLASS_BUNDLE)) ||
91-
params.first().astTypeReference().getTypeName().equals("Bundle")) {
92-
hasBundleConstructor = true;
93-
break;
94-
}
95-
}
57+
PsiMethod[] constructors = declaration.getConstructors();
58+
for (PsiMethod constructor : constructors) {
59+
if (evaluator.isPublic(constructor)) {
60+
PsiParameter[] parameters = constructor.getParameterList().getParameters();
61+
62+
if (parameters.length == 0) {
63+
hasDefaultConstructor = true;
64+
break;
65+
} else if (parameters.length == 1 &&
66+
parameters[0].getType().equalsToText(SdkConstants.CLASS_BUNDLE) ||
67+
parameters[0].getType().equalsToText("Bundle")) {
68+
hasBundleConstructor = true;
69+
break;
9670
}
9771
}
9872
}
9973

100-
if (hasConstructor && !hasDefaultConstructor && !hasBundleConstructor) {
74+
if (constructors.length > 0 && !hasDefaultConstructor && !hasBundleConstructor) {
10175
String message = String.format(
10276
"This Controller needs to have either a public default constructor or a" +
10377
" public single-argument constructor that takes a Bundle. (`%1$s`)",
104-
cls.getName());
105-
context.report(ISSUE, node, context.getLocation(node.astName()), message);
78+
declaration.getQualifiedName());
79+
context.report(ISSUE, declaration, context.getLocation(declaration), message);
10680
}
10781
}
10882
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
package com.bluelinelabs.conductor.lint;
2+
3+
import com.android.tools.lint.checks.infrastructure.LintDetectorTest;
4+
import com.android.tools.lint.detector.api.Detector;
5+
import com.android.tools.lint.detector.api.Issue;
6+
7+
import org.intellij.lang.annotations.Language;
8+
9+
import java.util.Collections;
10+
import java.util.List;
11+
12+
import static com.google.common.truth.Truth.assertThat;
13+
14+
public class ControllerChangeHandlerDetectorTest extends LintDetectorTest {
15+
16+
private static final String NO_WARNINGS = "No warnings.";
17+
private static final String CONSTRUCTOR =
18+
"src/test/SampleHandler.java:2: Error: This ControllerChangeHandler needs to have a public default constructor (test.SampleHandler) [ValidControllerChangeHandler]\n"
19+
+ "public class SampleHandler extends com.bluelinelabs.conductor.ControllerChangeHandler {\n"
20+
+ "^\n"
21+
+ "1 errors, 0 warnings\n";
22+
private static final String PRIVATE_CLASS_ERROR =
23+
"src/test/SampleHandler.java:2: Error: This ControllerChangeHandler class should be public (test.SampleHandler) [ValidControllerChangeHandler]\n"
24+
+ "private class SampleHandler extends com.bluelinelabs.conductor.ControllerChangeHandler {\n"
25+
+ "^\n"
26+
+ "1 errors, 0 warnings\n";
27+
28+
public void testWithNoConstructor() throws Exception {
29+
@Language("JAVA") String source = ""
30+
+ "package test;\n"
31+
+ "public class SampleHandler extends com.bluelinelabs.conductor.ControllerChangeHandler {\n"
32+
+ "}";
33+
assertThat(lintProject(java(source))).isEqualTo(NO_WARNINGS);
34+
}
35+
36+
public void testWithEmptyConstructor() throws Exception {
37+
@Language("JAVA") String source = ""
38+
+ "package test;\n"
39+
+ "public class SampleHandler extends com.bluelinelabs.conductor.ControllerChangeHandler {\n"
40+
+ " public SampleHandler() { }\n"
41+
+ "}";
42+
assertThat(lintProject(java(source))).isEqualTo(NO_WARNINGS);
43+
}
44+
45+
public void testWithInvalidConstructor() throws Exception {
46+
@Language("JAVA") String source = ""
47+
+ "package test;\n"
48+
+ "public class SampleHandler extends com.bluelinelabs.conductor.ControllerChangeHandler {\n"
49+
+ " public SampleHandler(int number) { }\n"
50+
+ "}";
51+
assertThat(lintProject(java(source))).isEqualTo(CONSTRUCTOR);
52+
}
53+
54+
public void testWithEmptyAndInvalidConstructor() throws Exception {
55+
@Language("JAVA") String source = ""
56+
+ "package test;\n"
57+
+ "public class SampleHandler extends com.bluelinelabs.conductor.ControllerChangeHandler {\n"
58+
+ " public SampleHandler() { }\n"
59+
+ " public SampleHandler(int number) { }\n"
60+
+ "}";
61+
assertThat(lintProject(java(source))).isEqualTo(NO_WARNINGS);
62+
}
63+
64+
public void testWithPrivateConstructor() throws Exception {
65+
@Language("JAVA") String source = ""
66+
+ "package test;\n"
67+
+ "public class SampleHandler extends com.bluelinelabs.conductor.ControllerChangeHandler {\n"
68+
+ " private SampleHandler() { }\n"
69+
+ "}";
70+
assertThat(lintProject(java(source))).isEqualTo(CONSTRUCTOR);
71+
}
72+
73+
public void testWithPrivateClass() throws Exception {
74+
@Language("JAVA") String source = ""
75+
+ "package test;\n"
76+
+ "private class SampleHandler extends com.bluelinelabs.conductor.ControllerChangeHandler {\n"
77+
+ " public SampleHandler() { }\n"
78+
+ "}";
79+
assertThat(lintProject(java(source))).isEqualTo(PRIVATE_CLASS_ERROR);
80+
}
81+
82+
@Override
83+
protected Detector getDetector() {
84+
return new ControllerChangeHandlerIssueDetector();
85+
}
86+
87+
@Override
88+
protected List<Issue> getIssues() {
89+
return Collections.singletonList(ControllerChangeHandlerIssueDetector.ISSUE);
90+
}
91+
92+
@Override
93+
protected boolean allowCompilationErrors() {
94+
return true;
95+
}
96+
}

0 commit comments

Comments
 (0)