From 8fc53736dcb6768a336dcec52fd91dc5a57a0b55 Mon Sep 17 00:00:00 2001 From: cushon Date: Mon, 6 Aug 2018 12:59:10 -0700 Subject: [PATCH] Prepare get{Symbol,Type}FromString for modular compilations There's no long a single top-level name->class map. Instead, if the compilation has modules enabled, we need to scan individual modules. This change is an initial step towards full JDK 10 support. MOE_MIGRATED_REVID=207593004 --- .../com/google/errorprone/VisitorState.java | 113 ++++++++++++------ .../google/errorprone/util/ASTHelpers.java | 45 ++++--- .../threadsafety/ImmutableCheckerTest.java | 2 +- 3 files changed, 102 insertions(+), 58 deletions(-) diff --git a/check_api/src/main/java/com/google/errorprone/VisitorState.java b/check_api/src/main/java/com/google/errorprone/VisitorState.java index e146fecef93..5fc0e2b5f2a 100644 --- a/check_api/src/main/java/com/google/errorprone/VisitorState.java +++ b/check_api/src/main/java/com/google/errorprone/VisitorState.java @@ -15,8 +15,8 @@ */ package com.google.errorprone; - import com.google.common.base.Optional; +import com.google.common.base.Splitter; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; @@ -27,6 +27,7 @@ import com.google.errorprone.util.ErrorProneTokens; import com.sun.source.tree.Tree; import com.sun.source.util.TreePath; +import com.sun.tools.javac.code.Kinds.Kind; import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Symbol.ClassSymbol; import com.sun.tools.javac.code.Symbol.CompletionFailure; @@ -36,13 +37,13 @@ import com.sun.tools.javac.code.Type.ArrayType; import com.sun.tools.javac.code.Type.ClassType; import com.sun.tools.javac.code.Types; +import com.sun.tools.javac.comp.Modules; import com.sun.tools.javac.main.JavaCompiler; import com.sun.tools.javac.parser.Tokens.Token; import com.sun.tools.javac.tree.JCTree; import com.sun.tools.javac.tree.JCTree.JCCompilationUnit; import com.sun.tools.javac.tree.TreeMaker; import com.sun.tools.javac.util.Context; -import com.sun.tools.javac.util.Convert; import com.sun.tools.javac.util.Name; import com.sun.tools.javac.util.Names; import com.sun.tools.javac.util.Options; @@ -183,17 +184,6 @@ public Type getTypeFromString(String typeStr) { } } - /** Infers a module symbol for the given flat class name. */ - // TODO(cushon): decide how to provide actual -source 9 module support - public ModuleSymbol inferModule(Name flatName) { - Symtab symtab = getSymtab(); - ModuleSymbol result = symtab.inferModule(Convert.packagePart(flatName)); - if (result != null) { - return result; - } - return symtab.java_base == symtab.noModule ? symtab.noModule : symtab.unnamedModule; - } - private Type getTypeFromStringInternal(String typeStr) { validateTypeStr(typeStr); if (isPrimitiveType(typeStr)) { @@ -202,52 +192,97 @@ private Type getTypeFromStringInternal(String typeStr) { if (isVoidType(typeStr)) { return getVoidType(); } - Name typeName = getName(typeStr); + // Fast path if the type's symbol is available. + ClassSymbol classSymbol = (ClassSymbol) getSymbolFromString(typeStr); + if (classSymbol != null) { + return classSymbol.asType(); + } + // Otherwise, fall back to JavaCompiler#resolveIdent. + // TODO(cushon): this isn't compatible with modular compilations. + return resolveIdent(typeStr); + } + + private Type resolveIdent(String typeStr) { + JavaCompiler compiler = JavaCompiler.instance(context); + Symbol sym = compiler.resolveIdent(getSymtab().noModule, typeStr); + if (!(sym instanceof ClassSymbol)) { + return null; + } + Type type = ((ClassSymbol) sym).asType(); try { - ClassSymbol typeSymbol = getSymtab().getClass(inferModule(typeName), typeName); - if (typeSymbol == null) { - JavaCompiler compiler = JavaCompiler.instance(context); - Symbol sym = compiler.resolveIdent(inferModule(typeName), typeStr); - if (!(sym instanceof ClassSymbol)) { - return null; - } - typeSymbol = (ClassSymbol) sym; - } - Type type = typeSymbol.asType(); - // Throws CompletionFailure if the source/class file for this type is not available. - // This is hacky but the best way I can think of to handle this case. type.complete(); - if (type.isErroneous()) { - return null; - } - return type; } catch (CompletionFailure failure) { - // Ignoring completion error is problematic in general, but in this case we're ignoring a - // completion error for a type that was directly requested, not one that was discovered - // during the compilation. return null; } + if (type.isErroneous()) { + return null; + } + return type; } /** * @param symStr the string representation of a symbol * @return the Symbol object, or null if it cannot be found */ + // TODO(cushon): deal with binary compat issues and return ClassSymbol public Symbol getSymbolFromString(String symStr) { - try { - Name symName = getName(symStr); - Symbol result = getSymtab().getClass(inferModule(symName), symName); + symStr = inferBinaryName(symStr); + Name name = getName(symStr); + Modules modules = Modules.instance(context); + boolean modular = modules.getDefaultModule() != getSymtab().noModule; + if (!modular) { + return getSymbolFromString(getSymtab().noModule, name); + } + for (ModuleSymbol msym : Modules.instance(context).allModules()) { + ClassSymbol result = getSymbolFromString(msym, name); if (result != null) { - // Force a completion failure if the type is not available. - result.complete(); + // TODO(cushon): the path where we iterate over all modules is probably slow. + // Try to learn some lessons from JDK-8189747, and consider disallowing this case and + // requiring users to call the getSymbolFromString(ModuleSymbol, Name) overload instead. return result; } + } + return null; + } + + public ClassSymbol getSymbolFromString(ModuleSymbol msym, Name name) { + ClassSymbol result = getSymtab().getClass(msym, name); + if (result == null || result.kind == Kind.ERR || !result.exists()) { + return null; + } + try { + result.complete(); } catch (CompletionFailure failure) { // Ignoring completion error is problematic in general, but in this case we're ignoring a // completion error for a type that was directly requested, not one that was discovered // during the compilation. + return null; } - return null; + return result; + } + + /** + * Given a canonical class name, infers the binary class name using case conventions. For example, + * give {@code com.example.Outer.Inner} returns {@code com.example.Outer$Inner}. + */ + // TODO(cushon): consider migrating call sites to use binary names and removing this code. + // (But then we'd probably want error handling for probably-incorrect canonical names, + // so it may not end up being a performance win.) + private static String inferBinaryName(String classname) { + StringBuilder sb = new StringBuilder(); + boolean first = true; + char sep = '.'; + for (String bit : Splitter.on('.').split(classname)) { + if (!first) { + sb.append(sep); + } + sb.append(bit); + if (Character.isUpperCase(bit.charAt(0))) { + sep = '$'; + } + first = false; + } + return sb.toString(); } /** Build an instance of a Type. */ 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 77e247a31f0..5a6cb7c539d 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 @@ -640,11 +640,28 @@ private static boolean isFinal(Symbol symbol) { * @return true if the symbol is annotated with given type. */ public static boolean hasAnnotation(Symbol sym, String annotationClass, VisitorState state) { + if (sym == null) { + return false; + } + // normalize to non-binary names + annotationClass = annotationClass.replace('$', '.'); Name annotationName = state.getName(annotationClass); - Symbol annotationSym; - synchronized (state.context) { - annotationSym = - state.getSymtab().enterClass(state.inferModule(annotationName), annotationName); + if (!isInherited(state, annotationClass)) { + return hasAttribute(sym, annotationName); + } + while (sym instanceof ClassSymbol) { + if (hasAttribute(sym, annotationName)) { + return true; + } + sym = ((ClassSymbol) sym).getSuperclass().tsym; + } + return false; + } + + private static boolean isInherited(VisitorState state, String annotationName) { + Symbol annotationSym = state.getSymbolFromString(annotationName); + if (annotationSym == null) { + return false; } try { annotationSym.complete(); @@ -653,21 +670,13 @@ public static boolean hasAnnotation(Symbol sym, String annotationClass, VisitorS // if it's present directly } Symbol inheritedSym = state.getSymtab().inheritedType.tsym; + return annotationSym.attribute(inheritedSym) != null; + } - if ((sym == null) || (annotationSym == null)) { - return false; - } - if ((sym instanceof ClassSymbol) && (annotationSym.attribute(inheritedSym) != null)) { - while (sym != null) { - if (sym.attribute(annotationSym) != null) { - return true; - } - sym = ((ClassSymbol) sym).getSuperclass().tsym; - } - return false; - } else { - return sym.attribute(annotationSym) != null; - } + private static boolean hasAttribute(Symbol sym, Name annotationName) { + return sym.getRawAttributes() + .stream() + .anyMatch(a -> a.type.tsym.getQualifiedName().equals(annotationName)); } /** diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableCheckerTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableCheckerTest.java index e50f4aae629..b2375015176 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableCheckerTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableCheckerTest.java @@ -1419,8 +1419,8 @@ public void incompleteClassPath() throws Exception { .addSourceLines( "Test.java", "import " + ClassPathTest.class.getCanonicalName() + ";", + "// BUG: Diagnostic contains: not annotated", "class Test extends ClassPathTest {", - " // BUG: Diagnostic contains: non-final", " int x;", "}") .setArgs(Arrays.asList("-cp", libJar.toString()))