Skip to content

Commit 88e535a

Browse files
cpovirknick-someone
authored andcommitted
Make enclosingClass return null for module symbols.
This solves a problem with UnusedNestedClass on module-info.java files. Fixes #1240 Somewhat relevant to google/guava-beta-checker#8 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=273590160
1 parent 4a16e80 commit 88e535a

File tree

3 files changed

+49
-1
lines changed

3 files changed

+49
-1
lines changed

Diff for: check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -957,7 +957,8 @@ public Tree visitParameterizedType(ParameterizedTypeTree tree, Void unused) {
957957

958958
/** Return the enclosing {@code ClassSymbol} of the given symbol, or {@code null}. */
959959
public static ClassSymbol enclosingClass(Symbol sym) {
960-
return sym.owner.enclClass();
960+
// sym.owner is null in the case of module symbols.
961+
return sym.owner == null ? null : sym.owner.enclClass();
961962
}
962963

963964
/** Return the enclosing {@code PackageSymbol} of the given symbol, or {@code null}. */

Diff for: core/src/test/java/com/google/errorprone/bugpatterns/UnusedNestedClassTest.java

+18
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,12 @@
1616

1717
package com.google.errorprone.bugpatterns;
1818

19+
import static java.util.Arrays.asList;
20+
1921
import com.google.errorprone.BugCheckerRefactoringTestHelper;
2022
import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
2123
import com.google.errorprone.CompilationTestHelper;
24+
import com.google.errorprone.util.RuntimeVersion;
2225
import org.junit.Test;
2326
import org.junit.runner.RunWith;
2427
import org.junit.runners.JUnit4;
@@ -118,4 +121,19 @@ public void refactoring() {
118121
"}")
119122
.doTest(TestMode.TEXT_MATCH);
120123
}
124+
125+
@Test
126+
public void moduleInfo() {
127+
if (!RuntimeVersion.isAtLeast9()) {
128+
return;
129+
}
130+
compilationHelper
131+
.setArgs(asList("-source", "9", "-target", "9"))
132+
.addSourceLines(
133+
"module-info.java", //
134+
"module foo {",
135+
" requires java.base;",
136+
"}")
137+
.doTest();
138+
}
121139
}

Diff for: test_helpers/src/main/java/com/google/errorprone/ErrorProneInMemoryFileManager.java

+29
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.google.errorprone;
1818

1919
import static java.nio.charset.StandardCharsets.UTF_8;
20+
import static javax.tools.StandardLocation.SOURCE_PATH;
2021

2122
import com.google.common.base.Joiner;
2223
import com.google.common.base.Optional;
@@ -89,6 +90,34 @@ public JavaFileObject forResource(Class<?> clazz, String fileName) {
8990
return Iterables.getOnlyElement(getJavaFileObjects(path));
9091
}
9192

93+
@Override
94+
public boolean hasLocation(Location location) {
95+
/*
96+
* Short-circuit the check that module-info.java is found under the sourcepath.
97+
*
98+
* Here's why this short-circuits it:
99+
* http://hg.openjdk.java.net/jdk/jdk11/file/1ddf9a99e4ad/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java#l523
100+
*
101+
* Here's why we want to do so:
102+
*
103+
* To determine whether a module-info.java is found under the sourcepath, javac looks for the
104+
* sourcepath Path objects on the default filesystem (i.e., using Paths.get(...)):
105+
* http://hg.openjdk.java.net/jdk/jdk11/file/1ddf9a99e4ad/src/jdk.compiler/share/classes/com/sun/tools/javac/file/JavacFileManager.java#l112
106+
*
107+
* With some reflection on javac internals, we can override it to use our fileSystem.get(...).
108+
* However, there's still a problem, as javac converts the Path objects to File objects to do
109+
* its work:
110+
* http://hg.openjdk.java.net/jdk/jdk11/file/1ddf9a99e4ad/src/jdk.compiler/share/classes/com/sun/tools/javac/file/JavacFileManager.java#l940
111+
*
112+
* This doesn't work for custom file systems like jimfs. So javac will never find anything under
113+
* our sourcepath (unless we start writing files to disk, which we could, like in our
114+
* integration tests).
115+
*
116+
* Thus, we short-circuit the check entirely.
117+
*/
118+
return location != SOURCE_PATH && super.hasLocation(location);
119+
}
120+
92121
// TODO(cushon): the testdata/ fallback is a hack, fix affected tests and remove it
93122
private InputStream findResource(Class<?> clazz, String name) {
94123
InputStream is = clazz.getResourceAsStream(name);

0 commit comments

Comments
 (0)