From 88e535aee8a328372f9de2cc844065ae12c048d9 Mon Sep 17 00:00:00 2001
From: cpovirk <cpovirk@google.com>
Date: Tue, 8 Oct 2019 13:17:32 -0700
Subject: [PATCH] Make enclosingClass return null for module symbols.

This solves a problem with UnusedNestedClass on module-info.java files.

Fixes https://github.com/google/error-prone/issues/1240
Somewhat relevant to https://github.com/google/guava-beta-checker/issues/8

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=273590160
---
 .../google/errorprone/util/ASTHelpers.java    |  3 +-
 .../bugpatterns/UnusedNestedClassTest.java    | 18 ++++++++++++
 .../ErrorProneInMemoryFileManager.java        | 29 +++++++++++++++++++
 3 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java b/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java
index 0b8b99c0dbe..9ee352f0a6b 100644
--- a/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java
+++ b/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java
@@ -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}. */
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/UnusedNestedClassTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/UnusedNestedClassTest.java
index f17072f37e3..621869b572b 100644
--- a/core/src/test/java/com/google/errorprone/bugpatterns/UnusedNestedClassTest.java
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/UnusedNestedClassTest.java
@@ -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;
@@ -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();
+  }
 }
diff --git a/test_helpers/src/main/java/com/google/errorprone/ErrorProneInMemoryFileManager.java b/test_helpers/src/main/java/com/google/errorprone/ErrorProneInMemoryFileManager.java
index 060e7973af5..8430fc9221c 100644
--- a/test_helpers/src/main/java/com/google/errorprone/ErrorProneInMemoryFileManager.java
+++ b/test_helpers/src/main/java/com/google/errorprone/ErrorProneInMemoryFileManager.java
@@ -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;
@@ -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);