Skip to content

Commit

Permalink
Make enclosingClass return null for module symbols.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
cpovirk authored and nick-someone committed Oct 9, 2019
1 parent 4a16e80 commit 88e535a
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -957,7 +957,8 @@ public Tree visitParameterizedType(ParameterizedTypeTree tree, Void unused) {

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

/** Return the enclosing {@code PackageSymbol} of the given symbol, or {@code null}. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,12 @@

package com.google.errorprone.bugpatterns;

import static java.util.Arrays.asList;

import com.google.errorprone.BugCheckerRefactoringTestHelper;
import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
import com.google.errorprone.CompilationTestHelper;
import com.google.errorprone.util.RuntimeVersion;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
Expand Down Expand Up @@ -118,4 +121,19 @@ public void refactoring() {
"}")
.doTest(TestMode.TEXT_MATCH);
}

@Test
public void moduleInfo() {
if (!RuntimeVersion.isAtLeast9()) {
return;
}
compilationHelper
.setArgs(asList("-source", "9", "-target", "9"))
.addSourceLines(
"module-info.java", //
"module foo {",
" requires java.base;",
"}")
.doTest();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.google.errorprone;

import static java.nio.charset.StandardCharsets.UTF_8;
import static javax.tools.StandardLocation.SOURCE_PATH;

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

@Override
public boolean hasLocation(Location location) {
/*
* Short-circuit the check that module-info.java is found under the sourcepath.
*
* Here's why this short-circuits it:
* http://hg.openjdk.java.net/jdk/jdk11/file/1ddf9a99e4ad/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java#l523
*
* Here's why we want to do so:
*
* To determine whether a module-info.java is found under the sourcepath, javac looks for the
* sourcepath Path objects on the default filesystem (i.e., using Paths.get(...)):
* http://hg.openjdk.java.net/jdk/jdk11/file/1ddf9a99e4ad/src/jdk.compiler/share/classes/com/sun/tools/javac/file/JavacFileManager.java#l112
*
* With some reflection on javac internals, we can override it to use our fileSystem.get(...).
* However, there's still a problem, as javac converts the Path objects to File objects to do
* its work:
* http://hg.openjdk.java.net/jdk/jdk11/file/1ddf9a99e4ad/src/jdk.compiler/share/classes/com/sun/tools/javac/file/JavacFileManager.java#l940
*
* This doesn't work for custom file systems like jimfs. So javac will never find anything under
* our sourcepath (unless we start writing files to disk, which we could, like in our
* integration tests).
*
* Thus, we short-circuit the check entirely.
*/
return location != SOURCE_PATH && super.hasLocation(location);
}

// TODO(cushon): the testdata/ fallback is a hack, fix affected tests and remove it
private InputStream findResource(Class<?> clazz, String name) {
InputStream is = clazz.getResourceAsStream(name);
Expand Down

0 comments on commit 88e535a

Please sign in to comment.