-
Notifications
You must be signed in to change notification settings - Fork 39
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
By default, prevent
BugCheckers from introducing new dependencies (#…
…308)
- Loading branch information
1 parent
022a3d2
commit 8fa3ff3
Showing
7 changed files
with
264 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
105 changes: 105 additions & 0 deletions
105
...rone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/ThirdPartyLibrary.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| package tech.picnic.errorprone.bugpatterns.util; | ||
|
|
||
| import com.google.errorprone.VisitorState; | ||
| import com.google.errorprone.bugpatterns.BugChecker; | ||
| import com.google.errorprone.suppliers.Supplier; | ||
| import com.sun.tools.javac.code.ClassFinder; | ||
| import com.sun.tools.javac.code.Symbol.CompletionFailure; | ||
| import com.sun.tools.javac.util.Name; | ||
|
|
||
| /** | ||
| * Utility class that helps decide whether it is appropriate to introduce references to (well-known) | ||
| * third-party libraries. | ||
| * | ||
| * <p>This class should be used by {@link BugChecker}s that may otherwise suggest the introduction | ||
| * of code that depends on possibly-not-present third-party libraries. | ||
| */ | ||
| // XXX: Consider giving users more fine-grained control. This would be beneficial in cases where a | ||
| // dependency is on the classpath, but new usages are undesirable. | ||
| public enum ThirdPartyLibrary { | ||
| /** | ||
| * AssertJ. | ||
| * | ||
| * @see <a href="https://assertj.github.io/doc">AssertJ documentation</a> | ||
| */ | ||
| ASSERTJ("org.assertj.core.api.Assertions"), | ||
| /** | ||
| * Google's Guava. | ||
| * | ||
| * @see <a href="https://github.com/google/guava">Guava on GitHub</a> | ||
| */ | ||
| GUAVA("com.google.common.collect.ImmutableList"), | ||
| /** | ||
| * New Relic's Java agent API. | ||
| * | ||
| * @see <a href="https://github.com/newrelic/newrelic-java-agent/tree/main/newrelic-api">New Relic | ||
| * Java agent API on GitHub</a> | ||
| */ | ||
| NEW_RELIC_AGENT_API("com.newrelic.api.agent.Agent"), | ||
| /** | ||
| * VMWare's Project Reactor. | ||
| * | ||
| * @see <a href="https://projectreactor.io">Home page</a> | ||
| */ | ||
| REACTOR("reactor.core.publisher.Flux"); | ||
|
|
||
| private static final String IGNORE_CLASSPATH_COMPAT_FLAG = | ||
| "ErrorProneSupport:IgnoreClasspathCompat"; | ||
|
|
||
| @SuppressWarnings("ImmutableEnumChecker" /* Supplier is deterministic. */) | ||
| private final Supplier<Boolean> canUse; | ||
|
|
||
| /** | ||
| * Instantiates a {@link ThirdPartyLibrary} enum value. | ||
| * | ||
| * @param witnessFqcn The fully-qualified class name of a type that is expected to be on the | ||
| * classpath iff the associated third-party library is on the classpath. | ||
| */ | ||
| ThirdPartyLibrary(String witnessFqcn) { | ||
| this.canUse = VisitorState.memoize(state -> canIntroduceUsage(witnessFqcn, state)); | ||
| } | ||
|
|
||
| /** | ||
| * Tells whether it is okay to introduce a dependency on this well-known third party library in | ||
| * the given context. | ||
| * | ||
| * @param state The context under consideration. | ||
| * @return {@code true} iff it is okay to assume or create a dependency on this library. | ||
| */ | ||
| public boolean isIntroductionAllowed(VisitorState state) { | ||
| return canUse.get(state); | ||
| } | ||
|
|
||
| private static boolean canIntroduceUsage(String className, VisitorState state) { | ||
| return shouldIgnoreClasspath(state) || isKnownClass(className, state); | ||
| } | ||
|
|
||
| /** | ||
| * Attempts to determine whether a class with the given FQCN is on the classpath. | ||
| * | ||
| * <p>The {@link VisitorState}'s symbol table is consulted first. If the type has not yet been | ||
| * loaded, then an attempt is made to do so. | ||
| */ | ||
| private static boolean isKnownClass(String className, VisitorState state) { | ||
| return state.getTypeFromString(className) != null || canLoadClass(className, state); | ||
| } | ||
|
|
||
| private static boolean canLoadClass(String className, VisitorState state) { | ||
| ClassFinder classFinder = ClassFinder.instance(state.context); | ||
| Name binaryName = state.binaryNameFromClassname(className); | ||
| try { | ||
| classFinder.loadClass(state.getSymtab().unnamedModule, binaryName); | ||
| return true; | ||
| } catch (CompletionFailure e) { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| private static boolean shouldIgnoreClasspath(VisitorState state) { | ||
| return state | ||
| .errorProneOptions() | ||
| .getFlags() | ||
| .getBoolean(IGNORE_CLASSPATH_COMPAT_FLAG) | ||
| .orElse(Boolean.FALSE); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
115 changes: 115 additions & 0 deletions
115
...-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/ThirdPartyLibraryTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| package tech.picnic.errorprone.bugpatterns.util; | ||
|
|
||
| import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; | ||
| import static java.util.stream.Collectors.joining; | ||
|
|
||
| import com.google.common.collect.ImmutableList; | ||
| import com.google.errorprone.BugPattern; | ||
| import com.google.errorprone.CompilationTestHelper; | ||
| import com.google.errorprone.VisitorState; | ||
| import com.google.errorprone.bugpatterns.BugChecker; | ||
| import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher; | ||
| import com.google.errorprone.matchers.Description; | ||
| import com.sun.source.tree.ClassTree; | ||
| import java.util.Arrays; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.params.ParameterizedTest; | ||
| import org.junit.jupiter.params.provider.ValueSource; | ||
| import reactor.core.publisher.Flux; | ||
|
|
||
| final class ThirdPartyLibraryTest { | ||
| private final CompilationTestHelper compilationTestHelper = | ||
| CompilationTestHelper.newInstance(TestChecker.class, getClass()); | ||
|
|
||
| @Test | ||
| void isIntroductionAllowed() { | ||
| compilationTestHelper | ||
| .addSourceLines( | ||
| "A.java", | ||
| "// BUG: Diagnostic contains: ASSERTJ: true, GUAVA: true, NEW_RELIC_AGENT_API: true, REACTOR: true", | ||
| "class A {}") | ||
| .doTest(); | ||
| } | ||
|
|
||
| @Test | ||
| void isIntroductionAllowedWitnessClassesInSymtab() { | ||
| compilationTestHelper | ||
| .addSourceLines( | ||
| "A.java", | ||
| "import com.google.common.collect.ImmutableList;", | ||
| "import com.newrelic.api.agent.Agent;", | ||
| "import org.assertj.core.api.Assertions;", | ||
| "import reactor.core.publisher.Flux;", | ||
| "", | ||
| "// BUG: Diagnostic contains: ASSERTJ: true, GUAVA: true, NEW_RELIC_AGENT_API: true, REACTOR: true", | ||
| "class A {", | ||
| " void m(Class<?> clazz) {", | ||
| " m(Assertions.class);", | ||
| " m(ImmutableList.class);", | ||
| " m(Agent.class);", | ||
| " m(Flux.class);", | ||
| " }", | ||
| "}") | ||
| .doTest(); | ||
| } | ||
|
|
||
| @Test | ||
| void isIntroductionAllowedWitnessClassesPartiallyOnClassPath() { | ||
| compilationTestHelper | ||
| .withClasspath(ImmutableList.class, Flux.class) | ||
| .addSourceLines( | ||
| "A.java", | ||
| "// BUG: Diagnostic contains: ASSERTJ: false, GUAVA: true, NEW_RELIC_AGENT_API: false, REACTOR: true", | ||
| "class A {}") | ||
| .doTest(); | ||
| } | ||
|
|
||
| @Test | ||
| void isIntroductionAllowedWitnessClassesNotOnClassPath() { | ||
| compilationTestHelper | ||
| .withClasspath() | ||
| .addSourceLines( | ||
| "A.java", | ||
| "// BUG: Diagnostic contains: ASSERTJ: false, GUAVA: false, NEW_RELIC_AGENT_API: false, REACTOR:", | ||
| "// false", | ||
| "class A {}") | ||
| .doTest(); | ||
| } | ||
|
|
||
| @ParameterizedTest | ||
| @ValueSource(booleans = {true, false}) | ||
| void isIntroductionAllowedIgnoreClasspathCompat(boolean ignoreClassPath) { | ||
| compilationTestHelper | ||
| .setArgs("-XepOpt:ErrorProneSupport:IgnoreClasspathCompat=" + ignoreClassPath) | ||
| .withClasspath(ImmutableList.class, Flux.class) | ||
| .addSourceLines( | ||
| "A.java", | ||
| String.format( | ||
| "// BUG: Diagnostic contains: ASSERTJ: %s, GUAVA: true, NEW_RELIC_AGENT_API: %s, REACTOR: true", | ||
| ignoreClassPath, ignoreClassPath), | ||
| "class A {}") | ||
| .doTest(); | ||
| } | ||
|
|
||
| /** | ||
| * Flags classes with a diagnostics message that indicates, for each {@link ThirdPartyLibrary} | ||
| * element, whether they can be used. | ||
| */ | ||
| @BugPattern(severity = ERROR, summary = "Interacts with `ThirdPartyLibrary` for testing purposes") | ||
| public static final class TestChecker extends BugChecker implements ClassTreeMatcher { | ||
| private static final long serialVersionUID = 1L; | ||
|
|
||
| @Override | ||
| public Description matchClass(ClassTree tree, VisitorState state) { | ||
| return buildDescription(tree) | ||
| .setMessage( | ||
| Arrays.stream(ThirdPartyLibrary.values()) | ||
| .map( | ||
| lib -> | ||
| String.join( | ||
| ": ", lib.name(), String.valueOf(lib.isIntroductionAllowed(state)))) | ||
| .collect(joining(", "))) | ||
| .build(); | ||
| } | ||
| } | ||
| } |