Skip to content

Commit

Permalink
Have ErrorProneRuntimeClasspath ignore non-public types
Browse files Browse the repository at this point in the history
  • Loading branch information
Stephan202 committed Jan 14, 2024
1 parent 7c2991c commit 9c34275
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import com.sun.tools.javac.code.Symbol.CompletionFailure;
import com.sun.tools.javac.code.Symbol.ModuleSymbol;
import com.sun.tools.javac.code.Symtab;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.util.Name;

/**
Expand Down Expand Up @@ -70,12 +71,12 @@ public boolean isIntroductionAllowed(VisitorState state) {
/**
* Tells whether the given fully qualified type is available on the current class path.
*
* @param className The type of interest.
* @param typeName The type of interest.
* @param state The context under consideration.
* @return {@code true} iff it is okay to assume or create a dependency on this type.
*/
public static boolean canIntroduceUsage(String className, VisitorState state) {
return shouldIgnoreClasspath(state) || isKnownClass(className, state);
public static boolean canIntroduceUsage(String typeName, VisitorState state) {
return shouldIgnoreClasspath(state) || isKnownClass(typeName, state);
}

/**
Expand All @@ -84,22 +85,26 @@ public static boolean canIntroduceUsage(String className, VisitorState state) {
* <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 isKnownClass(String typeName, VisitorState state) {
return isPublicClassInSymbolTable(typeName, state) || canLoadPublicClass(typeName, state);
}

private static boolean canLoadClass(String className, VisitorState state) {
private static boolean isPublicClassInSymbolTable(String typeName, VisitorState state) {
Type type = state.getTypeFromString(typeName);
return type != null && type.tsym.isPublic();
}

private static boolean canLoadPublicClass(String typeName, VisitorState state) {
ClassFinder classFinder = ClassFinder.instance(state.context);
Symtab symtab = state.getSymtab();
// XXX: Drop support for targeting Java 8 once the oldest supported JDK drops such support.
ModuleSymbol module =
Source.instance(state.context).compareTo(Source.JDK9) < 0
? symtab.noModule
: symtab.unnamedModule;
Name binaryName = state.binaryNameFromClassname(className);
Name binaryName = state.binaryNameFromClassname(typeName);
try {
classFinder.loadClass(module, binaryName);
return true;
return classFinder.loadClass(module, binaryName).isPublic();
} catch (
@SuppressWarnings("java:S1166" /* Not exceptional. */)
CompletionFailure e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ void identification() {
" m(\"com.google.errorprone.NonExistent\");",
" m(\"com.google.common.NonExistent.toString\");",
" m(\"java.lang.NonExistent\");",
" m(\"com.google.common.collect.ImmutableEnumSet\");",
" // BUG: Diagnostic matches: USE_CLASS_REFERENCE",
" m(\"com.google.errorprone.BugPattern\");",
" // BUG: Diagnostic matches: USE_CLASS_REFERENCE",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import static java.util.stream.Collectors.joining;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.errorprone.BugPattern;
import com.google.errorprone.CompilationTestHelper;
import com.google.errorprone.VisitorState;
Expand All @@ -20,7 +21,7 @@
final class ThirdPartyLibraryTest {
@Test
void isIntroductionAllowed() {
CompilationTestHelper.newInstance(TestChecker.class, getClass())
CompilationTestHelper.newInstance(IsIntroductionAllowedTestChecker.class, getClass())
.addSourceLines(
"A.java",
"// BUG: Diagnostic contains: ASSERTJ: true, GUAVA: true, REACTOR: true",
Expand All @@ -29,8 +30,8 @@ void isIntroductionAllowed() {
}

@Test
void isIntroductionAllowedWitnessClassesInSymtab() {
CompilationTestHelper.newInstance(TestChecker.class, getClass())
void isIntroductionAllowedWitnessClassesInSymbolTable() {
CompilationTestHelper.newInstance(IsIntroductionAllowedTestChecker.class, getClass())
.addSourceLines(
"A.java",
"import com.google.common.collect.ImmutableList;",
Expand All @@ -50,7 +51,7 @@ void isIntroductionAllowedWitnessClassesInSymtab() {

@Test
void isIntroductionAllowedWitnessClassesPartiallyOnClassPath() {
CompilationTestHelper.newInstance(TestChecker.class, getClass())
CompilationTestHelper.newInstance(IsIntroductionAllowedTestChecker.class, getClass())
.withClasspath(ImmutableList.class, Flux.class)
.addSourceLines(
"A.java",
Expand All @@ -61,7 +62,7 @@ void isIntroductionAllowedWitnessClassesPartiallyOnClassPath() {

@Test
void isIntroductionAllowedWitnessClassesNotOnClassPath() {
CompilationTestHelper.newInstance(TestChecker.class, getClass())
CompilationTestHelper.newInstance(IsIntroductionAllowedTestChecker.class, getClass())
.withClasspath()
.addSourceLines(
"A.java",
Expand All @@ -74,7 +75,7 @@ void isIntroductionAllowedWitnessClassesNotOnClassPath() {
@ParameterizedTest
@ValueSource(booleans = {true, false})
void isIntroductionAllowedIgnoreClasspathCompat(boolean ignoreClassPath) {
CompilationTestHelper.newInstance(TestChecker.class, getClass())
CompilationTestHelper.newInstance(IsIntroductionAllowedTestChecker.class, getClass())
.setArgs("-XepOpt:ErrorProneSupport:IgnoreClasspathCompat=" + ignoreClassPath)
.withClasspath(ImmutableList.class, Flux.class)
.addSourceLines(
Expand All @@ -86,23 +87,64 @@ void isIntroductionAllowedIgnoreClasspathCompat(boolean ignoreClassPath) {
.doTest();
}

@Test
void canIntroduceUsage() {
CompilationTestHelper.newInstance(CanIntroduceUsageTestChecker.class, getClass())
.addSourceLines(
"A.java",
"// BUG: Diagnostic contains: GUAVA_PUBLIC: true, GUAVA_PRIVATE: false, ERROR_PRONE_PUBLIC_NESTED:",
"// true",
"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 {
public static final class IsIntroductionAllowedTestChecker 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 -> lib.name() + ": " + lib.isIntroductionAllowed(state))
.collect(joining(", ")))
.build();
}
}

/**
* Flags classes with a diagnostics message that indicates, for selected types, the result of
* {@link ThirdPartyLibrary#canIntroduceUsage(String, VisitorState)}.
*/
@BugPattern(severity = ERROR, summary = "Interacts with `ThirdPartyLibrary` for testing purposes")
public static final class CanIntroduceUsageTestChecker extends BugChecker
implements ClassTreeMatcher {
private static final long serialVersionUID = 1L;
private static final ImmutableMap<String, String> TYPES =
ImmutableMap.of(
"GUAVA_PUBLIC",
ImmutableList.class.getCanonicalName(),
"GUAVA_PRIVATE",
"com.google.common.collect.ImmutableEnumSet",
"ERROR_PRONE_PUBLIC_NESTED",
"com.google.errorprone.BugCheckerRefactoringTestHelper.ExpectOutput");

@Override
public Description matchClass(ClassTree tree, VisitorState state) {
return buildDescription(tree)
.setMessage(
TYPES.entrySet().stream()
.map(
lib ->
String.join(
": ", lib.name(), String.valueOf(lib.isIntroductionAllowed(state))))
e ->
e.getKey()
+ ": "
+ ThirdPartyLibrary.canIntroduceUsage(e.getValue(), state))
.collect(joining(", ")))
.build();
}
Expand Down

0 comments on commit 9c34275

Please sign in to comment.