Skip to content

Commit 56bff9a

Browse files
jamesonwilliamsjamesonwilliams
authored
Refine highlighting and error reporting in Lint rules (#693)
1 parent 5a1746b commit 56bff9a

5 files changed

Lines changed: 71 additions & 50 deletions

File tree

‎conductor-lint/src/main/java/com/bluelinelabs/conductor/lint/ControllerChangeHandlerIssueDetector.java‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import java.util.Collections;
1919
import java.util.List;
2020

21-
@SuppressWarnings("UnstableApiUsage")
2221
public final class ControllerChangeHandlerIssueDetector extends Detector implements Detector.UastScanner {
2322

2423
static final Issue ISSUE =
@@ -32,7 +31,7 @@ public final class ControllerChangeHandlerIssueDetector extends Detector impleme
3231

3332
@Override
3433
public List<Class<? extends UElement>> getApplicableUastTypes() {
35-
return Collections.<Class<? extends UElement>>singletonList(UClass.class);
34+
return Collections.singletonList(UClass.class);
3635
}
3736

3837
@Override
@@ -47,42 +46,41 @@ public void visitClass(@NotNull UClass node) {
4746
return;
4847
}
4948

50-
final boolean hasSuperType = evaluator.extendsClass(node.getPsi(), CLASS_NAME, true);
49+
final boolean hasSuperType = evaluator.extendsClass(node.getJavaPsi(), CLASS_NAME, true);
5150
if (!hasSuperType) {
5251
return;
5352
}
5453

5554
if (!evaluator.isPublic(node)) {
5655
String message = String.format("This ControllerChangeHandler class should be public (%1$s)", node.getQualifiedName());
57-
context.report(ISSUE, node, context.getLocation((UElement) node), message);
56+
context.report(ISSUE, node, context.getLocation(Identify.byName(node)), message);
5857
return;
5958
}
6059

6160
if (node.getContainingClass() != null && !evaluator.isStatic(node)) {
6261
String message = String.format("This ControllerChangeHandler inner class should be static (%1$s)", node.getQualifiedName());
63-
context.report(ISSUE, node, context.getLocation((UElement) node), message);
62+
context.report(ISSUE, node, context.getLocation(Identify.byName(node)), message);
6463
return;
6564
}
6665

67-
boolean hasConstructor = false;
66+
UMethod constructor = null;
6867
boolean hasDefaultConstructor = false;
6968
for (UMethod method : node.getMethods()) {
7069
if (method.isConstructor()) {
71-
hasConstructor = true;
72-
if (evaluator.isPublic(method) && method.getUastParameters().size() == 0) {
70+
constructor = method;
71+
if (evaluator.isPublic(method) && method.getUastParameters().isEmpty()) {
7372
hasDefaultConstructor = true;
7473
break;
7574
}
7675
}
7776
}
7877

79-
if (hasConstructor && !hasDefaultConstructor) {
78+
if (constructor != null && !hasDefaultConstructor) {
8079
String message = String.format(
8180
"This ControllerChangeHandler needs to have a public default constructor (`%1$s`)", node.getQualifiedName());
82-
context.report(ISSUE, node, context.getLocation((UElement) node), message);
81+
context.report(ISSUE, node, context.getLocation(Identify.byName(constructor)), message);
8382
}
8483
}
8584
};
8685
}
87-
8886
}

‎conductor-lint/src/main/java/com/bluelinelabs/conductor/lint/ControllerIssueDetector.java‎

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import com.android.tools.lint.detector.api.JavaContext;
1111
import com.android.tools.lint.detector.api.Scope;
1212
import com.android.tools.lint.detector.api.Severity;
13+
import com.intellij.psi.PsiType;
1314

1415
import org.jetbrains.annotations.NotNull;
1516
import org.jetbrains.uast.UClass;
@@ -20,7 +21,6 @@
2021
import java.util.Collections;
2122
import java.util.List;
2223

23-
@SuppressWarnings("UnstableApiUsage")
2424
public final class ControllerIssueDetector extends Detector implements Detector.UastScanner {
2525

2626
static final Issue ISSUE =
@@ -48,52 +48,53 @@ public void visitClass(@NotNull UClass node) {
4848
return;
4949
}
5050

51-
final boolean hasSuperType = evaluator.extendsClass(node.getPsi(), CLASS_NAME, true);
51+
final boolean hasSuperType = evaluator.extendsClass(node.getJavaPsi(), CLASS_NAME, true);
5252
if (!hasSuperType) {
5353
return;
5454
}
5555

5656
if (!evaluator.isPublic(node)) {
5757
String message = String.format("This Controller class should be public (%1$s)", node.getQualifiedName());
58-
context.report(ISSUE, node, context.getLocation((UElement) node), message);
58+
context.report(ISSUE, node, context.getLocation(Identify.byName(node)), message);
5959
return;
6060
}
6161

6262
if (node.getContainingClass() != null && !evaluator.isStatic(node)) {
6363
String message = String.format("This Controller inner class should be static (%1$s)", node.getQualifiedName());
64-
context.report(ISSUE, node, context.getLocation((UElement) node), message);
64+
context.report(ISSUE, node, context.getLocation(Identify.byName(node)), message);
6565
return;
6666
}
6767

68-
boolean hasConstructor = false;
68+
UMethod constructor = null;
6969
boolean hasDefaultConstructor = false;
7070
boolean hasBundleConstructor = false;
7171
for (UMethod method : node.getMethods()) {
7272
if (method.isConstructor()) {
73-
hasConstructor = true;
73+
constructor = method;
7474
if (evaluator.isPublic(method)) {
7575
List<UParameter> parameters = method.getUastParameters();
76-
if (parameters.size() == 0) {
76+
if (parameters.isEmpty()) {
7777
hasDefaultConstructor = true;
7878
break;
79-
} else if (parameters.size() == 1 &&
80-
(parameters.get(0).getType().equalsToText(SdkConstants.CLASS_BUNDLE)) ||
81-
parameters.get(0).getType().equalsToText("Bundle")) {
82-
hasBundleConstructor = true;
79+
} else if (parameters.size() == 1) {
80+
PsiType type = parameters.get(0).getType();
81+
if (type.equalsToText(SdkConstants.CLASS_BUNDLE) || type.equalsToText("Bundle")) {
82+
hasBundleConstructor = true;
83+
break;
84+
}
8385
}
8486
}
8587
}
8688
}
8789

88-
if (hasConstructor && !hasDefaultConstructor && !hasBundleConstructor) {
90+
if (constructor != null && !hasDefaultConstructor && !hasBundleConstructor) {
8991
String message = String.format(
9092
"This Controller needs to have either a public default constructor or a" +
9193
" public single-argument constructor that takes a Bundle. (`%1$s`)",
9294
node.getQualifiedName());
93-
context.report(ISSUE, node, context.getLocation((UElement) node), message);
95+
context.report(ISSUE, node, context.getLocation(Identify.byName(constructor)), message);
9496
}
9597
}
9698
};
9799
}
98-
99100
}

‎conductor-lint/src/main/java/com/bluelinelabs/conductor/lint/Identify.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package com.bluelinelabs.conductor.lint;
2+
3+
import com.intellij.psi.PsiElement;
4+
import com.intellij.psi.PsiNameIdentifierOwner;
5+
6+
final class Identify {
7+
private Identify() {}
8+
9+
static PsiElement byName(PsiNameIdentifierOwner psiNameIdentifierOwner) {
10+
if (psiNameIdentifierOwner.getNameIdentifier() != null) {
11+
return psiNameIdentifierOwner.getNameIdentifier();
12+
}
13+
return psiNameIdentifierOwner;
14+
}
15+
}

‎conductor-lint/src/test/java/com/bluelinelabs/conductor/lint/ControllerChangeHandlerDetectorTest.java‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,8 @@
88
import org.intellij.lang.annotations.Language;
99
import org.junit.Test;
1010

11-
@SuppressWarnings("UnstableApiUsage")
1211
public class ControllerChangeHandlerDetectorTest {
1312

14-
private static final String CONSTRUCTOR =
15-
"src/test/SampleHandler.java:2: Error: This ControllerChangeHandler needs to have a public default constructor (test.SampleHandler) [ValidControllerChangeHandler]\n"
16-
+ "public class SampleHandler extends com.bluelinelabs.conductor.ControllerChangeHandler {\n"
17-
+ "^\n"
18-
+ "1 errors, 0 warnings\n";
19-
2013
private final TestFile controllerChangeHandlerStub = java(
2114
"package com.bluelinelabs.conductor;\n"
2215
+ "abstract class ControllerChangeHandler {}"
@@ -63,7 +56,12 @@ public void testWithInvalidConstructor() {
6356
.files(controllerChangeHandlerStub, java(source))
6457
.issues(ControllerIssueDetector.ISSUE, ControllerChangeHandlerIssueDetector.ISSUE)
6558
.run()
66-
.expect(CONSTRUCTOR);
59+
.expect(""
60+
+ "src/test/SampleHandler.java:3: Error: This ControllerChangeHandler needs to have a public default constructor (test.SampleHandler) [ValidControllerChangeHandler]\n"
61+
+ " public SampleHandler(int number) { }\n"
62+
+ " ~~~~~~~~~~~~~\n"
63+
+ "1 errors, 0 warnings\n"
64+
);
6765
}
6866

6967
@Test
@@ -94,7 +92,12 @@ public void testWithPrivateConstructor() {
9492
.files(controllerChangeHandlerStub, java(source))
9593
.issues(ControllerIssueDetector.ISSUE, ControllerChangeHandlerIssueDetector.ISSUE)
9694
.run()
97-
.expect(CONSTRUCTOR);
95+
.expect(""
96+
+ "src/test/SampleHandler.java:3: Error: This ControllerChangeHandler needs to have a public default constructor (test.SampleHandler) [ValidControllerChangeHandler]\n"
97+
+ " private SampleHandler() { }\n"
98+
+ " ~~~~~~~~~~~~~\n"
99+
+ "1 errors, 0 warnings\n"
100+
);
98101
}
99102

100103
@Test
@@ -111,7 +114,7 @@ public void testWithPrivateClass() {
111114
.run()
112115
.expect("src/test/SampleHandler.java:2: Error: This ControllerChangeHandler class should be public (test.SampleHandler) [ValidControllerChangeHandler]\n"
113116
+ "private class SampleHandler extends com.bluelinelabs.conductor.ControllerChangeHandler {\n"
114-
+ "^\n"
117+
+ " ~~~~~~~~~~~~~\n"
115118
+ "1 errors, 0 warnings\n");
116119
}
117120

@@ -131,7 +134,7 @@ public void testWithPrivateClassOfBaseClass() {
131134
.run()
132135
.expect("src/test/SampleHandler.java:2: Error: This ControllerChangeHandler class should be public (test.SampleHandler) [ValidControllerChangeHandler]\n" +
133136
"private class SampleHandler extends test.BaseChangeHandler {}\n" +
134-
"~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" +
137+
" ~~~~~~~~~~~~~\n" +
135138
"1 errors, 0 warnings");
136139
}
137140
}

‎conductor-lint/src/test/java/com/bluelinelabs/conductor/lint/ControllerDetectorTest.java‎

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,12 @@
88
import org.intellij.lang.annotations.Language;
99
import org.junit.Test;
1010

11-
@SuppressWarnings("UnstableApiUsage")
1211
public class ControllerDetectorTest {
1312

14-
private static final String CONSTRUCTOR_ERROR =
15-
"src/test/SampleController.java:2: Error: This Controller needs to have either a public default constructor or a public single-argument constructor that takes a Bundle. (test.SampleController) [ValidController]\n"
16-
+ "public class SampleController extends com.bluelinelabs.conductor.Controller {\n"
17-
+ "^\n"
18-
+ "1 errors, 0 warnings\n";
1913
private static final String CLASS_ERROR =
2014
"src/test/SampleController.java:2: Error: This Controller class should be public (test.SampleController) [ValidController]\n"
2115
+ "private class SampleController extends com.bluelinelabs.conductor.Controller {\n"
22-
+ "^\n"
16+
+ " ~~~~~~~~~~~~~~~~\n"
2317
+ "1 errors, 0 warnings\n";
2418

2519
private final TestFile controllerStub = java(
@@ -69,7 +63,12 @@ public void testWithInvalidConstructor() {
6963
.files(controllerStub, java(source))
7064
.issues(ControllerIssueDetector.ISSUE, ControllerChangeHandlerIssueDetector.ISSUE)
7165
.run()
72-
.expect(CONSTRUCTOR_ERROR);
66+
.expect(""
67+
+ "src/test/SampleController.java:3: Error: This Controller needs to have either a public default constructor or a public single-argument constructor that takes a Bundle. (test.SampleController) [ValidController]\n"
68+
+ " public SampleController(int number) { }\n"
69+
+ " ~~~~~~~~~~~~~~~~\n"
70+
+ "1 errors, 0 warnings\n"
71+
);
7372
}
7473

7574
@Test
@@ -106,11 +105,11 @@ public void testWithBaseClassAndPrivateConstructor() {
106105
.files(controllerStub, java(baseClass), java(source))
107106
.issues(ControllerIssueDetector.ISSUE, ControllerChangeHandlerIssueDetector.ISSUE)
108107
.run()
109-
.expect(
110-
"src/test/SampleController.java:2: Error: This Controller needs to have either a public default constructor or a public single-argument constructor that takes a Bundle. (test.SampleController) [ValidController]\n" +
111-
"public class SampleController extends BaseController {\n" +
112-
"^\n" +
113-
"1 errors, 0 warnings"
108+
.expect(""
109+
+ "src/test/SampleController.java:3: Error: This Controller needs to have either a public default constructor or a public single-argument constructor that takes a Bundle. (test.SampleController) [ValidController]\n"
110+
+ " private SampleController() { }\n"
111+
+ " ~~~~~~~~~~~~~~~~\n"
112+
+ "1 errors, 0 warnings"
114113
);
115114
}
116115

@@ -126,7 +125,12 @@ public void testWithPrivateConstructor() {
126125
.files(controllerStub, java(source))
127126
.issues(ControllerIssueDetector.ISSUE, ControllerChangeHandlerIssueDetector.ISSUE)
128127
.run()
129-
.expect(CONSTRUCTOR_ERROR);
128+
.expect(""
129+
+ "src/test/SampleController.java:3: Error: This Controller needs to have either a public default constructor or a public single-argument constructor that takes a Bundle. (test.SampleController) [ValidController]\n"
130+
+ " private SampleController() { }\n"
131+
+ " ~~~~~~~~~~~~~~~~\n"
132+
+ "1 errors, 0 warnings\n"
133+
);
130134
}
131135

132136
@Test

0 commit comments

Comments
 (0)