Skip to content

Commit

Permalink
Prepare get{Symbol,Type}FromString for modular compilations
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@cushon
cushon committed Aug 6, 2018
1 parent 08c6c79 commit 8fc5373
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 58 deletions.
113 changes: 74 additions & 39 deletions check_api/src/main/java/com/google/errorprone/VisitorState.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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)) {
Expand All @@ -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. */
Expand Down
45 changes: 27 additions & 18 deletions check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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));
}

/**
Expand Down
2 changes: 1 addition & 1 deletion core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableCheckerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> {",
" // BUG: Diagnostic contains: non-final",
" int x;",
"}")
.setArgs(Arrays.asList("-cp", libJar.toString()))
Expand Down

0 comments on commit 8fc5373

Please sign in to comment.