Skip to content

Commit

Permalink
In ApiDiffChecker, also recognize APIs that have a given annotation.
Browse files Browse the repository at this point in the history
RELNOTES: none

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=198956503
  • Loading branch information
cpovirk authored and cushon committed Jun 12, 2018
1 parent 2028588 commit 4a0765b
Show file tree
Hide file tree
Showing 7 changed files with 860 additions and 7 deletions.
Expand Up @@ -27,6 +27,7 @@
import com.google.errorprone.util.ASTHelpers;
import com.google.errorprone.util.Signatures;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.IdentifierTree;
import com.sun.source.tree.ImportTree;
import com.sun.source.tree.MemberSelectTree;
import com.sun.tools.javac.code.Symbol;
Expand All @@ -35,19 +36,28 @@
import com.sun.tools.javac.code.Symbol.VarSymbol;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.code.Types;
import java.lang.annotation.Annotation;
import java.util.Optional;

/** A base Error Prone check implementation to enforce compliance with a given API diff. */
public abstract class ApiDiffChecker extends BugChecker
implements IdentifierTreeMatcher, MemberSelectTreeMatcher {

private final ApiDiff apiDiff;
private final Optional<Class<? extends Annotation>> alsoForbidApisAnnotated;

protected ApiDiffChecker(ApiDiff apiDiff) {
this.apiDiff = apiDiff;
this.alsoForbidApisAnnotated = Optional.empty();
}

protected ApiDiffChecker(ApiDiff apiDiff, Class<? extends Annotation> alsoForbidApisAnnotated) {
this.apiDiff = apiDiff;
this.alsoForbidApisAnnotated = Optional.of(alsoForbidApisAnnotated);
}

@Override
public Description matchIdentifier(com.sun.source.tree.IdentifierTree tree, VisitorState state) {
public Description matchIdentifier(IdentifierTree tree, VisitorState state) {
return check(tree, state);
}

Expand All @@ -70,28 +80,47 @@ private Description check(ExpressionTree tree, VisitorState state) {
return Description.NO_MATCH;
}
Types types = state.getTypes();
// check for diff information associated with the class
if (apiDiff.isClassUnsupported(Signatures.classDescriptor(receiver.type, types))) {
// check for information associated with the class
if (apiDiff.isClassUnsupported(Signatures.classDescriptor(receiver.type, types))
|| classOrEnclosingClassIsForbiddenByAnnotation(receiver, state)) {
return buildDescription(tree)
.setMessage(String.format("%s is not available", receiver))
.build();
}
// check for fields and methods that are not present in the new API
// check for fields and methods that are not present in the old API
if (!(sym instanceof VarSymbol || sym instanceof MethodSymbol)) {
return Description.NO_MATCH;
}
ClassMemberKey memberKey =
ClassMemberKey.create(
sym.getSimpleName().toString(), Signatures.descriptor(sym.type, types));
ClassSymbol owner = sym.owner.enclClass();
if (apiDiff.isMemberUnsupported(Signatures.classDescriptor(owner.type, types), memberKey)) {
if (apiDiff.isMemberUnsupported(Signatures.classDescriptor(owner.type, types), memberKey)
|| hasAnnotationForbiddingUse(sym, state)) {
return buildDescription(tree)
.setMessage(String.format("%s#%s is not available in %s", owner, sym, receiver))
.build();
}
return Description.NO_MATCH;
}

private boolean classOrEnclosingClassIsForbiddenByAnnotation(Symbol clazz, VisitorState state) {
if (!alsoForbidApisAnnotated.isPresent()) {
return false;
}
for (; clazz instanceof ClassSymbol; clazz = clazz.owner) {
if (hasAnnotationForbiddingUse(clazz, state)) {
return true;
}
}
return false;
}

private boolean hasAnnotationForbiddingUse(Symbol sym, VisitorState state) {
return alsoForbidApisAnnotated.isPresent()
&& ASTHelpers.hasAnnotation(sym, alsoForbidApisAnnotated.get(), state);
}

/**
* Finds the class of the expression's receiver: the declaring class of a static member access, or
* the type that an instance member is accessed on.
Expand Down
Expand Up @@ -32,7 +32,10 @@
"Code that needs to be compatible with Java 7 cannot use types or members"
+ " that are only present in the JDK 8 class libraries",
category = JDK,
severity = ERROR)
severity = ERROR,
suppressionAnnotations = {
SuppressWarnings.class
})
public class Java7ApiChecker extends ApiDiffChecker {

static final ApiDiff API_DIFF = loadApiDiff();
Expand Down Expand Up @@ -70,6 +73,8 @@ private static ApiDiff loadApiDiff() {
}

public Java7ApiChecker() {
super(API_DIFF);
super(
API_DIFF
);
}
}
@@ -0,0 +1,150 @@
/*
* Copyright 2018 The Error Prone Authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.google.errorprone.bugpatterns.apidiff;


import com.google.errorprone.CompilationTestHelper;
import java.util.Collections;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

/** {@link AndroidJdkLibsChecker}Test. */

@RunWith(JUnit4.class)
public class AndroidJdkLibsCheckerTest extends Java7ApiCheckerTest {

private final CompilationTestHelper allowJava8Helper =
CompilationTestHelper.newInstance(AndroidJdkLibsChecker.class, getClass())
.setArgs(Collections.singletonList("-XepOpt:Android:Java8Libs"));

public AndroidJdkLibsCheckerTest() {
super(AndroidJdkLibsChecker.class);
}

@Test
public void repeatedAnnotationAllowed() {
compilationHelper
.addSourceLines(
"Test.java",
"import java.lang.annotation.Repeatable;",
"@Repeatable(Test.Container.class)",
"public @interface Test {",
" public @interface Container {",
" Test[] value();",
" }",
"}")
.doTest();
}

@Test
public void typeAnnotationAllowed() {
compilationHelper
.addSourceLines(
"Test.java",
"import java.lang.annotation.Target;",
"import java.lang.annotation.ElementType;",
"@Target({ElementType.TYPE_PARAMETER, ElementType.TYPE_USE})",
"public @interface Test {",
"}")
.doTest();
}

@Test
public void defaultMethod() {
compilationHelper
.addSourceLines(
"Test.java",
"import java.util.Map;",
"public class Test {",
" abstract static class A implements Map<Object, Object> {}",
" abstract static class B implements Map<Object, Object> {",
" @Override",
" public Object getOrDefault(Object key, Object defaultValue) {",
" return null;",
" }",
" }",
" void f(A a, B b) {",
" // BUG: Diagnostic contains: java.util.Map#getOrDefault(java.lang.Object,V)"
+ " is not available in Test.A",
" a.getOrDefault(null, null);",
" b.getOrDefault(null, null); // OK: overrides getOrDefault",
" }",
"}")
.doTest();
}

@Test
public void typeKind() {
compilationHelper
.addSourceLines(
"Test.java",
"public class Test {",
" // BUG: Diagnostic contains:",
" javax.lang.model.type.TypeKind tk;",
"}")
.doTest();
}

@Test
public void allowJava8Flag_packageWhitelist() {
allowJava8Helper
.addSourceLines(
"Test.java",
"import java.time.Duration;",
"import java.util.stream.Stream;",
"import com.google.common.base.Predicates;",
"import java.util.Arrays;",
"public class Test {",
" Duration d = Duration.ofSeconds(10);",
" public static void test(Stream s) {",
" s.forEach(i -> {});",
" }",
"}")
.doTest();
}

@Test
public void allowJava8Flag_memberWhitelist() {
allowJava8Helper
.addSourceLines(
"Test.java",
"import java.util.Arrays;",
"public class Test {",
" public static void main(String... args) {",
" Arrays.stream(args);",
" }",
"}")
.doTest();
}

@Test
public void allowJava8Flag_memberBlacklist() {
allowJava8Helper
.addSourceLines(
"Test.java",
"import java.util.stream.Stream;",
"public class Test {",
" public static void test(Stream s) {",
" // BUG: Diagnostic contains: parallel",
" s.parallel();",
" }",
"}")
.doTest();
}

}

0 comments on commit 4a0765b

Please sign in to comment.