Skip to content

Commit

Permalink
Switched lint checks to the new PSI based api (closes #184)
Browse files Browse the repository at this point in the history
  • Loading branch information
EricKuck committed Dec 15, 2016
1 parent 553bae0 commit 01df673
Show file tree
Hide file tree
Showing 13 changed files with 294 additions and 160 deletions.
4 changes: 2 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ language: android
android:
components:
- tools
- build-tools-23.0.2
- android-23
- build-tools-23.0.3
- android-25
- extra-android-m2repository

script:
Expand Down
6 changes: 1 addition & 5 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,8 @@ buildscript {
allprojects {
repositories {
jcenter()
mavenLocal()
mavenCentral()
}
}

task wrapper(type: Wrapper) {
gradleVersion = '2.10'
}

apply from: rootProject.file('dependencies.gradle')
6 changes: 6 additions & 0 deletions conductor-lint/build.gradle
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
apply plugin: 'java'

targetCompatibility = JavaVersion.VERSION_1_7
sourceCompatibility = JavaVersion.VERSION_1_7

configurations {
lintChecks
}
Expand All @@ -8,6 +11,9 @@ dependencies {
compile rootProject.ext.lintapi
compile rootProject.ext.lintchecks

testCompile rootProject.ext.lint
testCompile rootProject.ext.lintTests

lintChecks files(jar)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,29 +1,20 @@
package com.bluelinelabs.conductor.lint;

import com.android.annotations.NonNull;
import com.android.tools.lint.client.api.JavaParser.ResolvedClass;
import com.android.tools.lint.client.api.JavaEvaluator;
import com.android.tools.lint.detector.api.Category;
import com.android.tools.lint.detector.api.Detector;
import com.android.tools.lint.detector.api.Implementation;
import com.android.tools.lint.detector.api.Issue;
import com.android.tools.lint.detector.api.JavaContext;
import com.android.tools.lint.detector.api.Scope;
import com.android.tools.lint.detector.api.Severity;
import com.android.tools.lint.detector.api.Speed;
import com.intellij.psi.PsiClass;
import com.intellij.psi.PsiMethod;

import java.lang.reflect.Modifier;
import java.util.Collections;
import java.util.List;

import lombok.ast.ClassDeclaration;
import lombok.ast.ConstructorDeclaration;
import lombok.ast.Node;
import lombok.ast.NormalTypeBody;
import lombok.ast.StrictListAccessor;
import lombok.ast.TypeMember;
import lombok.ast.VariableDefinition;

public final class ControllerChangeHandlerIssueDetector extends Detector implements Detector.JavaScanner, Detector.ClassScanner {
public final class ControllerChangeHandlerIssueDetector extends Detector implements Detector.JavaPsiScanner {

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

public ControllerChangeHandlerIssueDetector() { }

@NonNull
@Override
public Speed getSpeed() {
return Speed.FAST;
}

@Override
public List<String> applicableSuperClasses() {
return Collections.singletonList("com.bluelinelabs.conductor.ControllerChangeHandler");
}

@Override
public void checkClass(@NonNull JavaContext context, ClassDeclaration node,
@NonNull Node declarationOrAnonymous, @NonNull ResolvedClass cls) {

if (node == null) {
public void checkClass(JavaContext context, PsiClass declaration) {
final JavaEvaluator evaluator = context.getEvaluator();
if (evaluator.isAbstract(declaration)) {
return;
}

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

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

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

boolean hasConstructor = false;
boolean hasDefaultConstructor = false;
NormalTypeBody body = node.astBody();
if (body != null) {
for (TypeMember member : body.astMembers()) {
if (member instanceof ConstructorDeclaration) {
hasConstructor = true;
ConstructorDeclaration constructor = (ConstructorDeclaration)member;

if (constructor.astModifiers().isPublic()) {
StrictListAccessor<VariableDefinition, ConstructorDeclaration> params = constructor.astParameters();
if (params.isEmpty()) {
hasDefaultConstructor = true;
break;
}
}
PsiMethod[] constructors = declaration.getConstructors();
for (PsiMethod constructor : constructors) {
if (evaluator.isPublic(constructor)) {
if (constructor.getParameterList().getParametersCount() == 0) {
hasDefaultConstructor = true;
break;
}
}
}

if (hasConstructor && !hasDefaultConstructor) {
if (constructors.length > 0 && !hasDefaultConstructor) {
String message = String.format(
"This ControllerChangeHandler needs to have a public default constructor (`%1$s`)",
cls.getName());
context.report(ISSUE, node, context.getLocation(node.astName()), message);
"This ControllerChangeHandler needs to have a public default constructor (`%1$s`)", declaration.getQualifiedName());
context.report(ISSUE, declaration, context.getLocation(declaration), message);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,30 +1,22 @@
package com.bluelinelabs.conductor.lint;

import com.android.SdkConstants;
import com.android.annotations.NonNull;
import com.android.tools.lint.client.api.JavaParser.ResolvedClass;
import com.android.tools.lint.client.api.JavaEvaluator;
import com.android.tools.lint.detector.api.Category;
import com.android.tools.lint.detector.api.Detector;
import com.android.tools.lint.detector.api.Implementation;
import com.android.tools.lint.detector.api.Issue;
import com.android.tools.lint.detector.api.JavaContext;
import com.android.tools.lint.detector.api.Scope;
import com.android.tools.lint.detector.api.Severity;
import com.android.tools.lint.detector.api.Speed;
import com.intellij.psi.PsiClass;
import com.intellij.psi.PsiMethod;
import com.intellij.psi.PsiParameter;

import java.lang.reflect.Modifier;
import java.util.Collections;
import java.util.List;

import lombok.ast.ClassDeclaration;
import lombok.ast.ConstructorDeclaration;
import lombok.ast.Node;
import lombok.ast.NormalTypeBody;
import lombok.ast.StrictListAccessor;
import lombok.ast.TypeMember;
import lombok.ast.VariableDefinition;

public final class ControllerIssueDetector extends Detector implements Detector.JavaScanner, Detector.ClassScanner {
public final class ControllerIssueDetector extends Detector implements Detector.JavaPsiScanner {

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

public ControllerIssueDetector() { }

@NonNull
@Override
public Speed getSpeed() {
return Speed.FAST;
}

@Override
public List<String> applicableSuperClasses() {
return Collections.singletonList("com.bluelinelabs.conductor.Controller");
}

@Override
public void checkClass(@NonNull JavaContext context, ClassDeclaration node,
@NonNull Node declarationOrAnonymous, @NonNull ResolvedClass cls) {

if (node == null) {
public void checkClass(JavaContext context, PsiClass declaration) {
final JavaEvaluator evaluator = context.getEvaluator();
if (evaluator.isAbstract(declaration)) {
return;
}

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

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

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

boolean hasConstructor = false;
boolean hasDefaultConstructor = false;
boolean hasBundleConstructor = false;
NormalTypeBody body = node.astBody();
if (body != null) {
for (TypeMember member : body.astMembers()) {
if (member instanceof ConstructorDeclaration) {
hasConstructor = true;
ConstructorDeclaration constructor = (ConstructorDeclaration)member;

if (constructor.astModifiers().isPublic()) {
StrictListAccessor<VariableDefinition, ConstructorDeclaration> params = constructor.astParameters();
if (params.isEmpty()) {
hasDefaultConstructor = true;
break;
} else if (params.size() == 1 &&
(params.first().astTypeReference().getTypeName().equals(SdkConstants.CLASS_BUNDLE)) ||
params.first().astTypeReference().getTypeName().equals("Bundle")) {
hasBundleConstructor = true;
break;
}
}
PsiMethod[] constructors = declaration.getConstructors();
for (PsiMethod constructor : constructors) {
if (evaluator.isPublic(constructor)) {
PsiParameter[] parameters = constructor.getParameterList().getParameters();

if (parameters.length == 0) {
hasDefaultConstructor = true;
break;
} else if (parameters.length == 1 &&
parameters[0].getType().equalsToText(SdkConstants.CLASS_BUNDLE) ||
parameters[0].getType().equalsToText("Bundle")) {
hasBundleConstructor = true;
break;
}
}
}

if (hasConstructor && !hasDefaultConstructor && !hasBundleConstructor) {
if (constructors.length > 0 && !hasDefaultConstructor && !hasBundleConstructor) {
String message = String.format(
"This Controller needs to have either a public default constructor or a" +
" public single-argument constructor that takes a Bundle. (`%1$s`)",
cls.getName());
context.report(ISSUE, node, context.getLocation(node.astName()), message);
declaration.getQualifiedName());
context.report(ISSUE, declaration, context.getLocation(declaration), message);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
package com.bluelinelabs.conductor.lint;

import com.android.tools.lint.checks.infrastructure.LintDetectorTest;
import com.android.tools.lint.detector.api.Detector;
import com.android.tools.lint.detector.api.Issue;

import org.intellij.lang.annotations.Language;

import java.util.Collections;
import java.util.List;

import static com.google.common.truth.Truth.assertThat;

public class ControllerChangeHandlerDetectorTest extends LintDetectorTest {

private static final String NO_WARNINGS = "No warnings.";
private static final String CONSTRUCTOR =
"src/test/SampleHandler.java:2: Error: This ControllerChangeHandler needs to have a public default constructor (test.SampleHandler) [ValidControllerChangeHandler]\n"
+ "public class SampleHandler extends com.bluelinelabs.conductor.ControllerChangeHandler {\n"
+ "^\n"
+ "1 errors, 0 warnings\n";
private static final String PRIVATE_CLASS_ERROR =
"src/test/SampleHandler.java:2: Error: This ControllerChangeHandler class should be public (test.SampleHandler) [ValidControllerChangeHandler]\n"
+ "private class SampleHandler extends com.bluelinelabs.conductor.ControllerChangeHandler {\n"
+ "^\n"
+ "1 errors, 0 warnings\n";

public void testWithNoConstructor() throws Exception {
@Language("JAVA") String source = ""
+ "package test;\n"
+ "public class SampleHandler extends com.bluelinelabs.conductor.ControllerChangeHandler {\n"
+ "}";
assertThat(lintProject(java(source))).isEqualTo(NO_WARNINGS);
}

public void testWithEmptyConstructor() throws Exception {
@Language("JAVA") String source = ""
+ "package test;\n"
+ "public class SampleHandler extends com.bluelinelabs.conductor.ControllerChangeHandler {\n"
+ " public SampleHandler() { }\n"
+ "}";
assertThat(lintProject(java(source))).isEqualTo(NO_WARNINGS);
}

public void testWithInvalidConstructor() throws Exception {
@Language("JAVA") String source = ""
+ "package test;\n"
+ "public class SampleHandler extends com.bluelinelabs.conductor.ControllerChangeHandler {\n"
+ " public SampleHandler(int number) { }\n"
+ "}";
assertThat(lintProject(java(source))).isEqualTo(CONSTRUCTOR);
}

public void testWithEmptyAndInvalidConstructor() throws Exception {
@Language("JAVA") String source = ""
+ "package test;\n"
+ "public class SampleHandler extends com.bluelinelabs.conductor.ControllerChangeHandler {\n"
+ " public SampleHandler() { }\n"
+ " public SampleHandler(int number) { }\n"
+ "}";
assertThat(lintProject(java(source))).isEqualTo(NO_WARNINGS);
}

public void testWithPrivateConstructor() throws Exception {
@Language("JAVA") String source = ""
+ "package test;\n"
+ "public class SampleHandler extends com.bluelinelabs.conductor.ControllerChangeHandler {\n"
+ " private SampleHandler() { }\n"
+ "}";
assertThat(lintProject(java(source))).isEqualTo(CONSTRUCTOR);
}

public void testWithPrivateClass() throws Exception {
@Language("JAVA") String source = ""
+ "package test;\n"
+ "private class SampleHandler extends com.bluelinelabs.conductor.ControllerChangeHandler {\n"
+ " public SampleHandler() { }\n"
+ "}";
assertThat(lintProject(java(source))).isEqualTo(PRIVATE_CLASS_ERROR);
}

@Override
protected Detector getDetector() {
return new ControllerChangeHandlerIssueDetector();
}

@Override
protected List<Issue> getIssues() {
return Collections.singletonList(ControllerChangeHandlerIssueDetector.ISSUE);
}

@Override
protected boolean allowCompilationErrors() {
return true;
}
}
Loading

0 comments on commit 01df673

Please sign in to comment.