diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/ClassScope.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/ClassScope.java index 65299a3fce..f9e0a283c4 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/ClassScope.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/ClassScope.java @@ -28,6 +28,8 @@ import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclarator; import net.sourceforge.pmd.lang.java.ast.ASTName; import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix; +import net.sourceforge.pmd.lang.java.ast.ASTPrimitiveType; +import net.sourceforge.pmd.lang.java.ast.ASTReferenceType; import net.sourceforge.pmd.lang.java.ast.ASTType; import net.sourceforge.pmd.lang.java.ast.ASTTypeParameter; import net.sourceforge.pmd.lang.java.ast.ASTTypeParameters; @@ -43,6 +45,20 @@ import net.sourceforge.pmd.lang.symboltable.Scope; */ public class ClassScope extends AbstractJavaScope { + private static final Set PRIMITIVE_TYPES; + + static { + PRIMITIVE_TYPES = new HashSet<>(); + PRIMITIVE_TYPES.add("boolean"); + PRIMITIVE_TYPES.add("char"); + PRIMITIVE_TYPES.add("byte"); + PRIMITIVE_TYPES.add("short"); + PRIMITIVE_TYPES.add("int"); + PRIMITIVE_TYPES.add("long"); + PRIMITIVE_TYPES.add("float"); + PRIMITIVE_TYPES.add("double"); + } + // FIXME - this breaks given sufficiently nested code private static ThreadLocal anonymousInnerClassCounter = new ThreadLocal() { protected Integer initialValue() { @@ -206,7 +222,7 @@ public class ClassScope extends AbstractJavaScope { } } if (isEnum && "valueOf".equals(occurrence.getImage())) { - result.add(createBuiltInMethodDeclaration("valueOf", 1)); + result.add(createBuiltInMethodDeclaration("valueOf", "String")); } return result; } @@ -253,7 +269,7 @@ public class ClassScope extends AbstractJavaScope { * @param parameterCount the parameter count of the method * @return a method name declaration */ - private MethodNameDeclaration createBuiltInMethodDeclaration(final String methodName, final int parameterCount) { + private MethodNameDeclaration createBuiltInMethodDeclaration(final String methodName, final String... parameterTypes) { ASTMethodDeclaration methodDeclaration = new ASTMethodDeclaration(JavaParserTreeConstants.JJTMETHODDECLARATION); methodDeclaration.setPublic(true); methodDeclaration.setScope(this); @@ -270,7 +286,7 @@ public class ClassScope extends AbstractJavaScope { methodDeclarator.jjtAddChild(formalParameters, 0); formalParameters.jjtSetParent(methodDeclarator); - for (int i = 0; i < parameterCount; i++) { + for (int i = 0; i < parameterTypes.length; i++) { ASTFormalParameter formalParameter = new ASTFormalParameter(JavaParserTreeConstants.JJTFORMALPARAMETER); formalParameters.jjtAddChild(formalParameter, i); formalParameter.jjtSetParent(formalParameters); @@ -282,6 +298,23 @@ public class ClassScope extends AbstractJavaScope { variableDeclaratorId.setImage("arg" + i); formalParameter.jjtAddChild(variableDeclaratorId, 1); variableDeclaratorId.jjtSetParent(formalParameter); + + if (PRIMITIVE_TYPES.contains(parameterTypes[i])) { + ASTPrimitiveType primitiveType = new ASTPrimitiveType(JavaParserTreeConstants.JJTPRIMITIVETYPE); + primitiveType.setImage(parameterTypes[i]); + type.jjtAddChild(primitiveType, 0); + primitiveType.jjtSetParent(type); + } else { + ASTReferenceType referenceType = new ASTReferenceType(JavaParserTreeConstants.JJTREFERENCETYPE); + type.jjtAddChild(referenceType, 0); + referenceType.jjtSetParent(type); + + // TODO : this could actually be a primitive array... + ASTClassOrInterfaceType coiType = new ASTClassOrInterfaceType(JavaParserTreeConstants.JJTCLASSORINTERFACETYPE); + coiType.setImage(parameterTypes[i]); + referenceType.jjtAddChild(coiType, 0); + coiType.jjtSetParent(referenceType); + } } MethodNameDeclaration mnd = new MethodNameDeclaration(methodDeclarator); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/MethodNameDeclaration.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/MethodNameDeclaration.java index 2561bad214..e5708081e7 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/MethodNameDeclaration.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/MethodNameDeclaration.java @@ -22,13 +22,13 @@ public class MethodNameDeclaration extends AbstractNameDeclaration { public boolean isVarargs() { ASTFormalParameters params = (ASTFormalParameters) node.jjtGetChild(0); - for (int i = 0; i < ((ASTMethodDeclarator) node).getParameterCount(); i++) { - ASTFormalParameter p = (ASTFormalParameter) params.jjtGetChild(i); - if (p.isVarargs()) { - return true; - } + if (params.getParameterCount() == 0) { + return false; } - return false; + + // If it's a varargs, it HAS to be the last parameter + ASTFormalParameter p = (ASTFormalParameter) params.jjtGetChild(params.getParameterCount() - 1); + return p.isVarargs(); } public ASTMethodDeclarator getMethodNameDeclaratorNode() { @@ -36,7 +36,7 @@ public class MethodNameDeclaration extends AbstractNameDeclaration { } public String getParameterDisplaySignature() { - StringBuilder sb = new StringBuilder("("); + StringBuilder sb = new StringBuilder("("); ASTFormalParameters params = (ASTFormalParameters) node.jjtGetChild(0); // TODO - this can be optimized - add [0] then ,[n] in a loop. // no need to trim at the end @@ -44,7 +44,7 @@ public class MethodNameDeclaration extends AbstractNameDeclaration { ASTFormalParameter p = (ASTFormalParameter) params.jjtGetChild(i); sb.append(p.getTypeNode().getTypeImage()); if (p.isVarargs()) { - sb.append("..."); + sb.append("..."); } sb.append(','); } @@ -82,7 +82,7 @@ public class MethodNameDeclaration extends AbstractNameDeclaration { // Compare vararg if (myParam.isVarargs() != otherParam.isVarargs()) { - return false; + return false; } Node myTypeNode = myParam.getTypeNode().jjtGetChild(0); @@ -118,7 +118,24 @@ public class MethodNameDeclaration extends AbstractNameDeclaration { @Override public int hashCode() { - return node.getImage().hashCode() + ((ASTMethodDeclarator) node).getParameterCount(); + int hash = node.getImage().hashCode() * 31 + ((ASTMethodDeclarator) node).getParameterCount(); + + ASTFormalParameters myParams = (ASTFormalParameters) node.jjtGetChild(0); + for (int i = 0; i < ((ASTMethodDeclarator) node).getParameterCount(); i++) { + ASTFormalParameter myParam = (ASTFormalParameter) myParams.jjtGetChild(i); + Node myTypeNode = myParam.getTypeNode().jjtGetChild(0); + + String myTypeImg; + if (myTypeNode instanceof ASTPrimitiveType) { + myTypeImg = myTypeNode.getImage(); + } else { + myTypeImg = myTypeNode.jjtGetChild(0).getImage(); + } + + hash = hash * 31 + myTypeImg.hashCode(); + } + + return hash; } @Override diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/TypeSet.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/TypeSet.java index 6be744d5db..4c9655810b 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/TypeSet.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/TypeSet.java @@ -9,6 +9,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import net.sourceforge.pmd.lang.java.typeresolution.PMDASMClassLoader; import net.sourceforge.pmd.util.ClasspathClassLoader; @@ -67,6 +68,15 @@ public class TypeSet { * @throws ClassNotFoundException if the class couldn't be found */ Class resolve(String name) throws ClassNotFoundException; + + /** + * Checks if the given class could be resolved by this resolver. + * Notice, that a resolver's ability to resolve a class does not imply + * that the class will actually be found and resolved. + * @param name the name of the class, might be fully classified or not. + * @return whether the class can be resolved + */ + boolean couldResolve(String name); } /** @@ -76,13 +86,22 @@ public class TypeSet { public abstract static class AbstractResolver implements Resolver { /** the class loader. */ protected final PMDASMClassLoader pmdClassLoader; + /** * Creates a new AbstractResolver that uses the given class loader. * @param pmdClassLoader the class loader to use */ - public AbstractResolver(PMDASMClassLoader pmdClassLoader) { + public AbstractResolver(final PMDASMClassLoader pmdClassLoader) { this.pmdClassLoader = pmdClassLoader; } + + public boolean couldResolve(final String name) { + /* + * Resolvers based on this one, will attempt to load the class from + * the class loader, so ask him + */ + return pmdClassLoader.couldResolve(name); + } } /** @@ -90,7 +109,8 @@ public class TypeSet { * explicit import statements. */ public static class ExplicitImportResolver extends AbstractResolver { - private Set importStmts; + private Map importStmts; + /** * Creates a new {@link ExplicitImportResolver}. * @param pmdClassLoader the class loader to use. @@ -98,20 +118,36 @@ public class TypeSet { */ public ExplicitImportResolver(PMDASMClassLoader pmdClassLoader, Set importStmts) { super(pmdClassLoader); - this.importStmts = importStmts; - } - @Override - public Class resolve(String name) throws ClassNotFoundException { - if (name == null) { - throw new ClassNotFoundException(); - } - for (String importStmt : importStmts) { - if (importStmt.endsWith(name)) { - return pmdClassLoader.loadClass(importStmt); + + // unfold imports, to store both FQ and unqualified names mapped to the FQ name + this.importStmts = new HashMap<>(); + for (final String stmt : importStmts) { + if (stmt.endsWith("*")) { + continue; + } + + this.importStmts.put(stmt, stmt); + final int lastDotIdx = stmt.lastIndexOf('.'); + if (lastDotIdx != -1) { + this.importStmts.put(stmt.substring(lastDotIdx + 1), stmt); } } + } + + @Override + public Class resolve(String name) throws ClassNotFoundException { + final String fqName = importStmts.get(name); + if (fqName != null) { + return pmdClassLoader.loadClass(fqName); + } + throw new ClassNotFoundException("Type " + name + " not found"); } + + @Override + public boolean couldResolve(String name) { + return importStmts.containsKey(name); + } } /** @@ -119,6 +155,7 @@ public class TypeSet { */ public static class CurrentPackageResolver extends AbstractResolver { private String pkg; + /** * Creates a new {@link CurrentPackageResolver} * @param pmdClassLoader the class loader to use @@ -128,10 +165,16 @@ public class TypeSet { super(pmdClassLoader); this.pkg = pkg; } + @Override public Class resolve(String name) throws ClassNotFoundException { return pmdClassLoader.loadClass(pkg + '.' + name); } + + @Override + public boolean couldResolve(String name) { + return super.couldResolve(pkg + '.' + name); + } } /** @@ -139,6 +182,13 @@ public class TypeSet { */ // TODO cite the JLS section on implicit imports public static class ImplicitImportResolver extends AbstractResolver { + /* + * They aren't so many to bother about memory, but are used all the time, + * so we worry about performance. On average, you can expect this cache to have ~90% hit ratio + * unless abusing star imports (import on demand) + */ + private static final ConcurrentHashMap> CLASS_CACHE = new ConcurrentHashMap<>(); + /** * Creates a {@link ImplicitImportResolver} * @param pmdClassLoader the class loader @@ -146,9 +196,27 @@ public class TypeSet { public ImplicitImportResolver(PMDASMClassLoader pmdClassLoader) { super(pmdClassLoader); } + @Override public Class resolve(String name) throws ClassNotFoundException { - return pmdClassLoader.loadClass("java.lang." + name); + if (name == null) { + throw new ClassNotFoundException(); + } + + Class clazz = CLASS_CACHE.get(name); + if (clazz != null) { + return clazz; + } + + clazz = pmdClassLoader.loadClass("java.lang." + name); + CLASS_CACHE.putIfAbsent(name, clazz); + + return clazz; + } + + @Override + public boolean couldResolve(String name) { + return super.couldResolve("java.lang." + name); } } @@ -168,11 +236,18 @@ public class TypeSet { } @Override public Class resolve(String name) throws ClassNotFoundException { + if (name == null) { + throw new ClassNotFoundException(); + } + for (String importStmt : importStmts) { if (importStmt.endsWith("*")) { try { - String importPkg = importStmt.substring(0, importStmt.indexOf('*') - 1); - return pmdClassLoader.loadClass(importPkg + '.' + name); + String fqClassName = new StringBuilder(importStmt.length() + name.length()) + .append(importStmt) + .replace(importStmt.length() - 1, importStmt.length(), name) + .toString(); + return pmdClassLoader.loadClass(fqClassName); } catch (ClassNotFoundException cnfe) { // ignored as the class could be imported with the next on demand import... } @@ -180,6 +255,24 @@ public class TypeSet { } throw new ClassNotFoundException("Type " + name + " not found"); } + + @Override + public boolean couldResolve(String name) { + for (String importStmt : importStmts) { + if (importStmt.endsWith("*")) { + String fqClassName = new StringBuilder(importStmt.length() + name.length()) + .append(importStmt) + .replace(importStmt.length() - 1, importStmt.length(), name) + .toString(); + // can any class be resolved / was never attempted? + if (super.couldResolve(fqClassName)) { + return true; + } + } + } + + return false; + } } /** @@ -201,6 +294,7 @@ public class TypeSet { primitiveTypes.put("short", short.class); primitiveTypes.put("char", char.class); } + @Override public Class resolve(String name) throws ClassNotFoundException { if (!primitiveTypes.containsKey(name)) { @@ -208,6 +302,11 @@ public class TypeSet { } return primitiveTypes.get(name); } + + @Override + public boolean couldResolve(String name) { + return primitiveTypes.containsKey(name); + } } /** @@ -221,6 +320,11 @@ public class TypeSet { } throw new ClassNotFoundException(name); } + + @Override + public boolean couldResolve(String name) { + return "void".equals(name); + } } /** @@ -235,6 +339,7 @@ public class TypeSet { public FullyQualifiedNameResolver(PMDASMClassLoader pmdClassLoader) { super(pmdClassLoader); } + @Override public Class resolve(String name) throws ClassNotFoundException { if (name == null) { @@ -281,11 +386,13 @@ public class TypeSet { buildResolvers(); } - for (Resolver resolver : resolvers) { - try { - return resolver.resolve(name); - } catch (ClassNotFoundException cnfe) { - // ignored, maybe another resolver will find the class + for (final Resolver resolver : resolvers) { + if (resolver.couldResolve(name)) { + try { + return resolver.resolve(name); + } catch (ClassNotFoundException cnfe) { + // ignored, maybe another resolver will find the class + } } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/PMDASMClassLoader.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/PMDASMClassLoader.java index 344bef79a7..0ba95138c4 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/PMDASMClassLoader.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/PMDASMClassLoader.java @@ -47,7 +47,7 @@ public final class PMDASMClassLoader extends ClassLoader { private PMDASMClassLoader(ClassLoader parent) { super(parent); } - + /** * A new PMDASMClassLoader is created for each compilation unit, this method * allows to reuse the same PMDASMClassLoader across all the compilation @@ -81,6 +81,18 @@ public final class PMDASMClassLoader extends ClassLoader { } } + /** + * Checks if the class loader could resolve a given class name + * (ie: it doesn't know for sure it will fail). + * Notice, that the ability to resolve a class does not imply + * that the class will actually be found and resolved. + * @param name the name of the class + * @return whether the class can be resolved + */ + public boolean couldResolve(String name) { + return !dontBother.containsKey(name); + } + public synchronized Map getImportedClasses(String name) throws ClassNotFoundException { if (dontBother.containsValue(name)) { throw new ClassNotFoundException(name); diff --git a/src/site/markdown/overview/changelog.md b/src/site/markdown/overview/changelog.md index f3fb7c3c67..c927340850 100644 --- a/src/site/markdown/overview/changelog.md +++ b/src/site/markdown/overview/changelog.md @@ -21,6 +21,7 @@ * [#114](https://github.com/pmd/pmd/pull/114): \[core] Remove multihreading workaround for JRE5, as no PMD version supports running on JRE5 anymore * [#115](https://github.com/pmd/pmd/pull/115): \[java] Simplify lambda parsing * [#116](https://github.com/pmd/pmd/pull/116): \[core] \[java] Improve collection usage +* [#117](https://github.com/pmd/pmd/pull/117): \[java] Improve symboltable performance **Bugfixes:**