From 44e23e7d2e07c9a517074595fa2616c5fc753b00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Wed, 27 Jun 2018 17:52:30 +0200 Subject: [PATCH] Make classloaders final in JTypeQName --- .../java/qname/JavaTypeQualifiedName.java | 20 ++++------------ .../lang/java/qname/QualifiedNameFactory.java | 23 +++++++++++-------- .../java/qname/QualifiedNameResolver.java | 12 +++++----- 3 files changed, 24 insertions(+), 31 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/qname/JavaTypeQualifiedName.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/qname/JavaTypeQualifiedName.java index 0556ca050b..9df796b3ad 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/qname/JavaTypeQualifiedName.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/qname/JavaTypeQualifiedName.java @@ -41,9 +41,9 @@ public final class JavaTypeQualifiedName extends JavaQualifiedName { private Class representedType; private boolean typeLoaded; - private ClassLoader classLoader; + private final ClassLoader classLoader; - JavaTypeQualifiedName(ImmutableList packages, ImmutableList classes, ImmutableList localIndices) { + JavaTypeQualifiedName(ImmutableList packages, ImmutableList classes, ImmutableList localIndices, ClassLoader classLoader) { Objects.requireNonNull(packages); Objects.requireNonNull(classes); Objects.requireNonNull(localIndices); @@ -55,16 +55,8 @@ public final class JavaTypeQualifiedName extends JavaQualifiedName { this.packages = packages; this.classes = classes; this.localIndices = localIndices; - } - - /** - * Sets the classloader to be used when resolving the actual type of this qualified name. - * @see #getType() - */ - JavaTypeQualifiedName withClassLoader(ClassLoader classLoader) { - this.classLoader = classLoader; - return this; + this.classLoader = classLoader; // classLoader may be null } @@ -177,17 +169,15 @@ public final class JavaTypeQualifiedName extends JavaQualifiedName { */ public Class getType() { synchronized (this) { - if (typeLoaded) { - return representedType; - } else { + if (!typeLoaded) { typeLoaded = true; try { representedType = loadType(); } catch (ClassNotFoundException e) { representedType = null; } - return representedType; } + return representedType; } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/qname/QualifiedNameFactory.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/qname/QualifiedNameFactory.java index a074f9f15a..b57e5b2549 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/qname/QualifiedNameFactory.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/qname/QualifiedNameFactory.java @@ -15,6 +15,8 @@ import net.sourceforge.pmd.lang.java.qname.ImmutableList.ListFactory; /** * Static factory methods for JavaQualifiedName. + * These are intended only for tests, even though some deprecated + * APIs use it. May be moved to an internal package? * * @author Clément Fournier * @since 6.1.0 @@ -108,13 +110,10 @@ public final class QualifiedNameFactory { name = '.' + name; // unnamed package, marked by a full stop. See ofString's format below } - JavaTypeQualifiedName qualifiedName = (JavaTypeQualifiedName) ofString(name); - if (qualifiedName != null) { - // Note: this assumes, that clazz has been loaded through the correct classloader, - // specifically through the auxclasspath classloader. - qualifiedName.withClassLoader(clazz.getClassLoader()); - } - return qualifiedName; + // Note: this assumes, that clazz has been loaded through the correct classloader, + // specifically through the auxclasspath classloader. + // But this method should only be used in tests anyway + return (JavaTypeQualifiedName) ofStringWithClassLoader(name, clazz.getClassLoader()); } @@ -146,6 +145,10 @@ public final class QualifiedNameFactory { * @return A qualified name instance corresponding to the parsed string. */ public static JavaQualifiedName ofString(String name) { + return ofStringWithClassLoader(name, null); + } + + private static JavaQualifiedName ofStringWithClassLoader(String name, ClassLoader classLoader) { Matcher matcher = FORMAT.matcher(name); if (!matcher.matches()) { @@ -153,8 +156,8 @@ public final class QualifiedNameFactory { } ImmutableList packages = StringUtils.isBlank(matcher.group(PACKAGES_GROUP_INDEX)) - ? ListFactory.emptyList() - : ListFactory.split(matcher.group(PACKAGES_GROUP_INDEX), "\\."); + ? ListFactory.emptyList() + : ListFactory.split(matcher.group(PACKAGES_GROUP_INDEX), "\\."); String operation = matcher.group(OPERATION_GROUP_INDEX) == null ? null : matcher.group(OPERATION_GROUP_INDEX).substring(1); boolean isLambda = operation != null && COMPILED_LAMBDA_PATTERN.matcher(operation).matches(); @@ -175,7 +178,7 @@ public final class QualifiedNameFactory { } } - JavaTypeQualifiedName parent = new JavaTypeQualifiedName(packages, classes, localIndices); + JavaTypeQualifiedName parent = new JavaTypeQualifiedName(packages, classes, localIndices, classLoader); return operation == null ? parent : new JavaOperationQualifiedName(parent, operation, isLambda); } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/qname/QualifiedNameResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/qname/QualifiedNameResolver.java index babdd7a494..d7f7226a6a 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/qname/QualifiedNameResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/qname/QualifiedNameResolver.java @@ -9,6 +9,7 @@ import static net.sourceforge.pmd.lang.java.qname.JavaTypeQualifiedName.NOTLOCAL import java.util.HashMap; import java.util.Map; import java.util.Stack; +import java.util.concurrent.ConcurrentHashMap; import org.apache.commons.lang3.mutable.MutableInt; @@ -45,7 +46,7 @@ public class QualifiedNameResolver extends JavaParserVisitorReducedAdapter { // Package names to package representation. // Allows reusing the same list instance for the same packages. // Package prefixes are also shared. - private final Map> foundPackages = new HashMap<>(128); + private static final Map> FOUND_PACKAGES = new ConcurrentHashMap<>(128); // The following stacks stack some counter of the // visited classes. A new entry is pushed when @@ -147,7 +148,7 @@ public class QualifiedNameResolver extends JavaParserVisitorReducedAdapter { } final String image = pack.getPackageNameImage(); - ImmutableList fullExisting = foundPackages.get(image); + ImmutableList fullExisting = FOUND_PACKAGES.get(image); if (fullExisting != null) { return fullExisting; @@ -166,7 +167,7 @@ public class QualifiedNameResolver extends JavaParserVisitorReducedAdapter { for (int i = longestPrefix.size(); i < allPacks.length; i++) { longestPrefix = longestPrefix.prepend(allPacks[i]); prefixImage.append(allPacks[i]); - foundPackages.put(prefixImage.toString(), longestPrefix); + FOUND_PACKAGES.put(prefixImage.toString(), longestPrefix); } return longestPrefix; @@ -184,7 +185,7 @@ public class QualifiedNameResolver extends JavaParserVisitorReducedAdapter { * the total number of packages in the package name */ private ImmutableList getLongestPackagePrefix(String acc, int i) { - ImmutableList prefix = foundPackages.get(acc); + ImmutableList prefix = FOUND_PACKAGES.get(acc); if (prefix != null) { return prefix; } @@ -356,8 +357,7 @@ public class QualifiedNameResolver extends JavaParserVisitorReducedAdapter { /** Creates a new class qname from the current context (fields). */ private JavaTypeQualifiedName contextClassQName() { - return new JavaTypeQualifiedName(packages, classNames, localIndices) - .withClassLoader(classLoader); + return new JavaTypeQualifiedName(packages, classNames, localIndices, classLoader); }