Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate lint check to new PSI API #130

Merged
merged 3 commits into from
Nov 23, 2016
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion Lint/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ configurations {
}

dependencies {
compile 'com.android.tools.lint:lint-api:25.1.3'
compile 'com.android.tools.lint:lint-api:25.2.0'
testCompile 'org.assertj:assertj-core:3.0.0'
testCompile 'com.android.tools.lint:lint:25.2.0'
testCompile 'com.android.tools.lint:lint-tests:25.2.0'
testCompile 'com.android.tools:testutils:25.2.0'

lintChecks files(jar)
}
Expand Down
65 changes: 16 additions & 49 deletions Lint/src/main/java/com/braintreepayments/lint/BetaDetector.java
Original file line number Diff line number Diff line change
@@ -1,46 +1,37 @@
package com.braintreepayments.lint;

import com.android.annotations.NonNull;
import com.android.annotations.Nullable;
import com.android.tools.lint.detector.api.Category;
import com.android.tools.lint.detector.api.Detector;
import com.android.tools.lint.detector.api.Detector.JavaScanner;
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.android.tools.lint.detector.api.TextFormat;
import com.intellij.psi.JavaElementVisitor;
import com.intellij.psi.PsiMethod;
import com.intellij.psi.PsiMethodCallExpression;

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

import lombok.ast.AstVisitor;
import lombok.ast.MethodInvocation;
import lombok.ast.Node;

public class BetaDetector extends Detector implements JavaScanner {

private static final List<String> METHODS = Collections.emptyList();

public class BetaDetector extends Detector implements Detector.JavaPsiScanner {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Newline after this.

// This would normally be private final, but has exposed here for testing purposes, since there
// seems to be no way to inject it via the Detector constructor using LintDetectorTest
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fine, I don't think it really needs a comment.

static List<String> METHODS = Collections.emptyList();
private static final Implementation IMPLEMENTATION = new Implementation(BetaDetector.class,
Scope.JAVA_FILE_SCOPE);

public static final Issue ISSUE = Issue.create(
static final Issue ISSUE = Issue.create(
"com.braintreepayments.beta",
"API is in beta",
"This API is currently in beta and subject to change. Braintree makes no guarantees about future compatibility or existence of this API.",
"This API is currently in beta and subject to change. Braintree makes no guarantees about " +
"future compatibility or existence of this API.",
Category.CORRECTNESS,
6,
Severity.ERROR,
IMPLEMENTATION);

public BetaDetector() {}

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

@Override
Expand All @@ -49,35 +40,11 @@ public List<String> getApplicableMethodNames() {
}

@Override
public void visitMethod(@NonNull JavaContext context, @Nullable AstVisitor visitor,
@NonNull MethodInvocation node) {
if (METHODS.contains(node.astName().astValue())) {
context.report(ISSUE, node, context.getLocation(node), ISSUE.getBriefDescription(TextFormat.TEXT));
public void visitMethod(JavaContext context, JavaElementVisitor visitor,
PsiMethodCallExpression node, PsiMethod method) {
if (METHODS.contains(method.getName())) {
context.report(ISSUE, node, context.getLocation(node), ISSUE.getBriefDescription(
TextFormat.TEXT));
}
}

@Override
public AstVisitor createJavaVisitor(@NonNull JavaContext context) {
return null;
}

@Override
public List<Class<? extends Node>> getApplicableNodeTypes() {
return null;
}

@Override
public boolean appliesToResourceRefs() {
return false;
}

@Override
public void visitResourceReference(@NonNull JavaContext context, @Nullable AstVisitor visitor,
@NonNull Node node, @NonNull String type, @NonNull String name, boolean isFramework) {
}

@Override
public List<String> applicableSuperClasses() {
return null;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package com.braintreepayments.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 java.util.Collections;
import java.util.List;

import static org.assertj.core.api.Assertions.assertThat;

public class BetaDetectorTest extends LintDetectorTest {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Newline here.

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

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

public void testBetaMethod() throws Exception {
BetaDetector.METHODS = Collections.singletonList("fooBar");
String result = lintProject(
java("src/com/example/Foo.java", "package com.example;\n" +
"public class Foo {\n" +
" public void someMethod() {\n" +
" Baz baz = new Baz();\n" +
" baz.fooBar();" +
" }\n" +
"}"),
java("src/com/example/Baz.java", "package com.example;\n" +
"public class Baz {\n" +
" public void fooBar() {\n" +
" }\n" +
"}"));

assertThat(result).isEqualTo(
"src/com/example/Foo.java:5: Error: API is in beta [com.braintreepayments.beta]\n" +
" baz.fooBar(); }\n" +
" ~~~~~~~~~~~~\n" +
"1 errors, 0 warnings\n");
}

public void testNormalMethod() throws Exception {
BetaDetector.METHODS = Collections.singletonList("fooBar");
String result = lintProject(
java("src/com/example/Foo.java", "package com.example;\n" +
"public class Foo {\n" +
" public void someMethod() {\n" +
" Baz baz = new Baz();\n" +
" baz.fooBarBaz();" +
" }\n" +
"}"),
java("src/com/example/Baz.java", "package com.example;\n" +
"public class Baz {\n" +
" public void fooBarBaz() {\n" +
" }\n" +
"}"));

assertThat(result).isEqualTo("No warnings.");
}
}
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ buildscript {
jcenter()
}
dependencies {
classpath 'com.android.tools.build:gradle:2.1.3'
classpath 'com.android.tools.build:gradle:2.2.2'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This upgrade is not possible at the moment, Robolectric 3.0 does not work with gradle build plugin 2.2.2 and upgrading Robolectric to 3.1.4 causes a conflict with PowerMock.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm weird, we have Robolectric 3.0 working fine with the plugin v2.2.2, anyways this is unrelated so I'll just revert it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The difference is probably an Android application vs library. The location of the merged manifest changed in 2.2.2 and Robolectric has the path hard coded to the old path in 3.0 for libraries.


classpath 'io.codearte.gradle.nexus:gradle-nexus-staging-plugin:0.5.3'

Expand Down