Add tests, require parsing to know simple/cano name

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/SymbolToStrings.java

@@ -61,12 +61,6 @@ public class SymbolToStrings {
             String kind;
             if (sym.isUnresolved()) {
                 kind = "unresolved";
-            } else if (sym.isEnum()) {
-                kind = "enum";
-            } else if (sym.isAnnotation()) {
-                kind = "annot";
-            } else if (sym.isRecord()) {
-                kind = "record";
             } else {
                 kind = "class";
             }

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/AsmSymbolResolver.java

@@ -53,12 +53,7 @@ public class AsmSymbolResolver implements SymbolResolver {
         String internalName = getInternalName(binaryName);
 
         SoftClassReference found = knownStubs.computeIfAbsent(internalName, iname -> {
-            if (!hasCanonicalName(internalName)) {
-                // if the class is anonymous/local, give up
-                return failed;
-            }
-
-            @Nullable URL url = getUrlOfInternalName(internalName);
+            @Nullable URL url = getUrlOfInternalName(iname);
             if (url == null) {
                 return failed;
             }
@@ -66,7 +61,16 @@ public class AsmSymbolResolver implements SymbolResolver {
             return new SoftClassReference(this, iname, new UrlLoader(url), ClassStub.UNKNOWN_ARITY);
         });
 
-        return found == failed ? null : found.get(); // NOPMD CompareObjectsWithEquals
+        if (found == failed) { // NOPMD CompareObjectsWithEquals
+            return null;
+        }
+        ClassStub stub = found.get();
+        if (!stub.hasCanonicalName()) {
+            knownStubs.put(internalName, failed);
+            return null;
+        }
+
+        return stub;
     }
 
     SignatureParser getSigParser() {
@@ -112,21 +116,15 @@ public class AsmSymbolResolver implements SymbolResolver {
        These methods return an unresolved symbol if the url is not found.
      */
 
-    @Nullable JClassSymbol resolveFromInternalNameCannotFail(@Nullable String internalName) {
+    @Nullable ClassStub resolveFromInternalNameCannotFail(@Nullable String internalName) {
         if (internalName == null) {
             return null;
         }
-        return resolveFromInternalNameCannotFail(internalName, 0);
-    }
-
-    // this is for inner + parent classes
-    void registerKnown(@NonNull String internalName, ClassStub innerClass) {
-        SoftClassReference softRef = new SoftClassReference(this, innerClass, internalName);
-        knownStubs.put(internalName, softRef);
+        return resolveFromInternalNameCannotFail(internalName, ClassStub.UNKNOWN_ARITY);
     }
 
     @SuppressWarnings("PMD.CompareObjectsWithEquals") // SoftClassReference
-    @NonNull JClassSymbol resolveFromInternalNameCannotFail(@NonNull String internalName, int observedArity) {
+    @NonNull ClassStub resolveFromInternalNameCannotFail(@NonNull String internalName, int observedArity) {
         return knownStubs.compute(internalName, (iname, prev) -> {
             if (prev != failed && prev != null) {
                 return prev;

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/ClassStub.java

@@ -13,6 +13,7 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Objects;
 import java.util.Set;
 import java.util.regex.Pattern;
 
@@ -43,10 +44,9 @@ final class ClassStub implements JClassSymbol, AsmStub {
     static final int UNKNOWN_ARITY = 0;
 
     private final AsmSymbolResolver resolver;
-    private final String internalName;
     private final Loader loader;
 
-    private Names names;        // lazy (doesn't need parsing)
+    private final Names names;
 
     // all the following are lazy and depend on the parse lock
 
@@ -75,10 +75,15 @@ final class ClassStub implements JClassSymbol, AsmStub {
         assert isValidInternalName(internalName) : internalName;
 
         this.resolver = resolver;
-        this.internalName = internalName;
+        this.names = new Names(internalName);
         this.loader = loader;
 
         this.parseLock = new ParseLock() {
+            // note to devs: to debug the parsing logic you might have
+            // to replace the implementation of toString temporarily,
+            // otherwise an IDE could call toString just to show the item
+            // in the debugger view (which could cause parsing of the class file).
+
             @Override
             protected boolean doParse() throws IOException {
                 try (InputStream instream = loader.getInputStream()) {
@@ -108,6 +113,16 @@ final class ClassStub implements JClassSymbol, AsmStub {
                 fields = Collections.unmodifiableList(fields);
                 memberClasses = Collections.unmodifiableList(memberClasses);
                 enumConstantNames = enumConstantNames == null ? null : Collections.unmodifiableSet(enumConstantNames);
+
+                if (enclosingInfo == EnclosingInfo.NO_ENCLOSING) {
+                    if (names.canonicalName == null || names.simpleName == null) {
+                        // This happens if the simple name contains dollars,
+                        // in which case we might have an enclosing class, and
+                        // we can only tell now (no enclosingInfo) that that's
+                        // not the case.
+                        names.finishOuterClass();
+                    }
+                }
             }
 
             @Override
@@ -134,6 +149,15 @@ final class ClassStub implements JClassSymbol, AsmStub {
         this.signature = new LazyClassSignature(this, signature, superName, interfaces);
     }
 
+    /**
+     * Called if this is an inner class (their simple name cannot be
+     * derived from splitting the internal/binary name on dollars, as
+     * the simple name may itself contain dollars).
+     */
+    void setSimpleName(String simpleName) {
+        this.names.simpleName = simpleName;
+    }
+
     void setModifiers(int accessFlags, boolean fromClassInfo) {
         /*
             A different set of modifiers is contained in the ClassInfo
@@ -328,13 +352,10 @@ final class ClassStub implements JClassSymbol, AsmStub {
     // <editor-fold  defaultstate="collapsed" desc="Names">
 
     public String getInternalName() {
-        return internalName;
+        return getNames().internalName;
     }
 
     private Names getNames() {
-        if (names == null) {
-            this.names = new Names(internalName);
-        }
         return names;
     }
 
@@ -343,10 +364,44 @@ final class ClassStub implements JClassSymbol, AsmStub {
         return getNames().binaryName;
     }
 
-    @Nullable
+    boolean hasCanonicalName() {
+        if (names.canonicalName != null) {
+            return true;
+        }
+        parseLock.ensureParsed();
+        if (isAnonymousClass() || isLocalClass()) {
+            return false;
+        }
+        JClassSymbol enclosing = getEnclosingClass();
+        return enclosing == null // top-level class
+            || enclosing instanceof ClassStub
+            && ((ClassStub) enclosing).hasCanonicalName();
+    }
+
     @Override
     public String getCanonicalName() {
-        return getNames().canonicalName;
+        String canoName = names.canonicalName;
+        if (canoName == null) {
+            parseLock.ensureParsed();
+            canoName = names.canonicalName;
+            if (canoName != null) {
+                return canoName;
+            }
+            JClassSymbol enclosing = getEnclosingClass();
+            if (enclosing == null) {
+                canoName = names.packageName + '.' + getSimpleName();
+                names.canonicalName = canoName;
+                return canoName;
+            }
+            String outerName = enclosing.getCanonicalName();
+            if (outerName == null) {
+                return null; // should not happen
+            }
+            canoName = outerName + '.' + getSimpleName();
+            names.canonicalName = canoName;
+            return canoName;
+        }
+        return canoName;
     }
 
     @Override
@@ -356,7 +411,12 @@ final class ClassStub implements JClassSymbol, AsmStub {
 
     @Override
     public @NonNull String getSimpleName() {
-        return getNames().simpleName;
+        String mySimpleName = names.simpleName;
+        if (mySimpleName == null) {
+            parseLock.ensureParsed();
+            return Objects.requireNonNull(names.simpleName);
+        }
+        return mySimpleName;
     }
 
     @Override
@@ -428,7 +488,7 @@ final class ClassStub implements JClassSymbol, AsmStub {
 
     @Override
     public boolean isLocalClass() {
-        return false; // local classes are not reachable, technically someone can try to fetch them
+        return enclosingInfo.isLocal();
     }
 
     @Override
@@ -441,26 +501,48 @@ final class ClassStub implements JClassSymbol, AsmStub {
 
     static class Names {
 
-        private static final Pattern INNER_DELIMITER = Pattern.compile("\\$(?=\\w)");
         final String binaryName;
-        final String canonicalName;
+        final String internalName;
         final String packageName;
-        final String simpleName;
+        /** If null, the class requires parsing to find out the actual canonical name. */
+        @Nullable String canonicalName;
+        /** If null, the class requires parsing to find out the actual simple name. */
+        @Nullable String simpleName;
 
         Names(String internalName) {
-            int packageEnd = Integer.max(0, internalName.lastIndexOf('/'));
+            assert isValidInternalName(internalName) : internalName;
+            int packageEnd = internalName.lastIndexOf('/');
 
-            binaryName = internalName.replace('/', '.');
-            packageName = binaryName.substring(0, packageEnd);
-            if (binaryName.indexOf('$') >= 0) { // contains a dollar
-                canonicalName = INNER_DELIMITER.matcher(binaryName).replaceAll(".");
+            this.internalName = internalName;
+            this.binaryName = internalName.replace('/', '.');
+            if (packageEnd == -1) {
+                this.packageName = "";
             } else {
-                // fast path
-                canonicalName = binaryName;
+                this.packageName = binaryName.substring(0, packageEnd);
             }
 
-            int lastDot = canonicalName.lastIndexOf('.');
-            simpleName = canonicalName.substring(lastDot + 1);
+            if (binaryName.indexOf('$', packageEnd + 1) >= 0) {
+                // Contains a dollar in class name (after package)
+                // Requires parsing to find out the actual simple name,
+                // this might be an inner class, or simply a class with
+                // a dollar in its name.
+
+                // ASSUMPTION: all JVM languages use the $ convention
+                // to separate inner classes. Java compilers do so but
+                // not necessarily true of all compilers/languages.
+                this.canonicalName = null;
+                this.simpleName = null;
+            } else {
+                // fast path
+                this.canonicalName = binaryName;
+                this.simpleName = binaryName.substring(packageEnd + 1);
+            }
+        }
+
+        public void finishOuterClass() {
+            int packageEnd = internalName.lastIndexOf('/');
+            this.simpleName = binaryName.substring(packageEnd + 1); // if -1, start from 0
+            this.canonicalName = binaryName;
         }
     }
 
@@ -478,6 +560,9 @@ final class ClassStub implements JClassSymbol, AsmStub {
             this.methodDescriptor = methodDescriptor;
         }
 
+        boolean isLocal() {
+            return methodName != null || methodDescriptor != null;
+        }
 
         public @Nullable JClassSymbol getEnclosingClass() {
             return stub;

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/ClassStubBuilder.java

@@ -9,10 +9,14 @@ import org.objectweb.asm.ClassVisitor;
 import org.objectweb.asm.FieldVisitor;
 import org.objectweb.asm.MethodVisitor;
 import org.objectweb.asm.Opcodes;
-
-import net.sourceforge.pmd.lang.java.symbols.internal.asm.Loader.NoUrlLoader;
+import org.objectweb.asm.Type;
 
 
+/**
+ * Populates a {@link ClassStub} by reading a class file. Some info is
+ * known by the ClassStub without parsing (like its internal name), so
+ * we defer parsing until later. The class should be parsed only once.
+ */
 class ClassStubBuilder extends ClassVisitor {
 
     private final ClassStub myStub;
@@ -48,18 +52,30 @@ class ClassStubBuilder extends ClassVisitor {
         return null;
     }
 
+    /**
+     * Visits information about an inner class. This inner class is not necessarily a member of the
+     * class being visited.
+     *
+     * <p>Spec: https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.7.6
+     *
+     * @param innerInternalName the internal name of an inner class (see {@link Type#getInternalName()}).
+     * @param outerName         the internal name of the class to which the inner class belongs (see {@link
+     *                          Type#getInternalName()}). May be {@literal null} for not member classes.
+     * @param innerSimpleName   the (simple) name of the inner class inside its enclosing class. May be
+     *                          {@literal null} for anonymous inner classes.
+     * @param access            the access flags of the inner class as originally
+     *                          declared in the enclosing class.
+     */
     @Override
     public void visitInnerClass(String innerInternalName, @Nullable String outerName, @Nullable String innerSimpleName, int access) {
         if (myInternalName.equals(outerName) && innerSimpleName != null) { // not anonymous
-            ClassStub member = new ClassStub(myStub.getResolver(),
-                                             innerInternalName,
-                                             new NoUrlLoader(myStub.getResolver(), innerInternalName),
-                                             ClassStub.UNKNOWN_ARITY);
-            resolver.registerKnown(innerInternalName, member);
+            ClassStub member = resolver.resolveFromInternalNameCannotFail(innerInternalName, ClassStub.UNKNOWN_ARITY);
             member.setModifiers(access, false);
             myStub.addMemberClass(member);
         } else if (myInternalName.equals(innerInternalName) && outerName != null) {
             // then it's specifying the enclosing class
+            // (myStub is the inner class)
+            myStub.setSimpleName(innerSimpleName);
             myStub.setModifiers(access, false);
             myStub.setOuterClass(outerName, null, null);
             isInnerNonStaticClass = (Opcodes.ACC_STATIC & access) == 0;

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/Loader.java

@@ -27,6 +27,12 @@ abstract class Loader {
         @Nullable InputStream getInputStream() {
             return null;
         }
+
+        @Override
+        public String toString() {
+            return "(failed loader)";
+        }
+
     }
 
     static class UrlLoader extends Loader {
@@ -44,23 +50,10 @@ abstract class Loader {
         InputStream getInputStream() throws IOException {
             return url.openStream();
         }
-    }
-
-    static class NoUrlLoader extends Loader {
-
-        private final AsmSymbolResolver resolver;
-        private final String internalName;
-
-        NoUrlLoader(AsmSymbolResolver resolver, String internalName) {
-            this.resolver = resolver;
-            this.internalName = internalName;
-        }
 
         @Override
-        @Nullable
-        InputStream getInputStream() throws IOException {
-            URL url = resolver.getUrlOfInternalName(internalName);
-            return url != null ? url.openStream() : null;
+        public String toString() {
+            return "(URL loader)";
         }
     }
 

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/internal/asm/SoftClassReference.java

@@ -9,6 +9,9 @@ import java.lang.ref.SoftReference;
 
 import org.checkerframework.checker.nullness.qual.NonNull;
 
+/**
+ * A soft reference over a (possibly loaded class stub).
+ */
 final class SoftClassReference {
 
     private final Loader loader;
@@ -24,14 +27,6 @@ final class SoftClassReference {
         this.observedArity = observedArity;
     }
 
-    SoftClassReference(AsmSymbolResolver resolver, ClassStub stub, String internalName) {
-        this.resolver = resolver;
-        this.loader = stub.getLoader();
-        this.internalName = internalName;
-        this.observedArity = 0;
-        this.ref = new SoftReference<>(stub);
-    }
-
     @SuppressWarnings("PMD.AssignmentInOperand")
     @NonNull ClassStub get() {
         ClassStub c;

pmd-java/src/test/java/javasymbols/testdata/deep/Another$ClassWith$Dollar.java

@@ -0,0 +1,23 @@
+/*
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package javasymbols.testdata.deep;
+
+/**
+ * @author Clément Fournier
+ */
+public class Another$ClassWith$Dollar {
+
+    public static class AnInner$ClassWithDollar {
+
+        public static class ADeeper$ClassWithDollar {
+
+        }
+    }
+
+    // looks like an anonymous class but isn't.
+    public static class DollarsAndNumbers$0 {
+
+    }
+}

pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/symbols/internal/asm/AsmLoaderTest.kt

@@ -7,14 +7,18 @@ package net.sourceforge.pmd.lang.java.symbols.internal.asm
 import io.kotest.core.spec.style.FunSpec
 import io.kotest.inspectors.forExactly
 import io.kotest.matchers.collections.shouldBeEmpty
-import io.kotest.matchers.collections.shouldBeSingleton
 import io.kotest.matchers.collections.shouldContainExactlyInAnyOrder
 import io.kotest.matchers.collections.shouldHaveSize
+import io.kotest.matchers.nulls.shouldBeNull
+import io.kotest.matchers.nulls.shouldNotBeNull
 import io.kotest.matchers.shouldBe
+import io.kotest.matchers.types.shouldBeSameInstanceAs
 import javasymbols.testdata.Enums
 import javasymbols.testdata.NestedClasses
 import javasymbols.testdata.Statics
+import javasymbols.testdata.deep.`Another$ClassWith$Dollar`
 import javasymbols.testdata.impls.GenericClass
+import net.sourceforge.pmd.lang.ast.test.IntelliMarker
 import net.sourceforge.pmd.lang.ast.test.shouldBe
 import net.sourceforge.pmd.lang.java.types.testTypeSystem
 import org.objectweb.asm.Opcodes
@@ -23,15 +27,13 @@ import kotlin.test.assertSame
 /**
  * @author Clément Fournier
  */
-class AsmLoaderTest : FunSpec({
+class AsmLoaderTest : IntelliMarker, FunSpec({
 
 
     // TODO tests:
     //   self-referential typevar bound : class C<T extends C<T>>
     //   self-referential superclass : class C extends Sup<C> implements I<Sup<C>>
-    //   methods with synthetic parameters (ctors of inner classes)
     //   access flags
-
     //   method reference with static ctdecl & zero formal parameters (asInstanceMethod)
 
     val contextClasspath = Classpath {  Thread.currentThread().contextClassLoader.getResource(it) }
@@ -50,11 +52,11 @@ class AsmLoaderTest : FunSpec({
     }
 
     val ts = testTypeSystem
-    val symLoader = AsmSymbolResolver(ts, contextClasspath)
+    fun symLoader() = AsmSymbolResolver(ts, contextClasspath)
 
     test("Generic class") {
 
-        val loaded = symLoader.resolveClassFromBinaryName(GenericClass::class.java.name)!!
+        val loaded = symLoader().resolveClassFromBinaryName(GenericClass::class.java.name)!!
 
         loaded::getTypeParameterCount shouldBe 2
         loaded.typeParameters.let { tparams ->
@@ -76,6 +78,7 @@ class AsmLoaderTest : FunSpec({
 
         val outerName = NestedClasses::class.java.name
 
+        val symLoader = symLoader()
         val outer = symLoader.resolveClassFromBinaryName(outerName)!!
         val inner = symLoader.resolveClassFromBinaryName("$outerName\$Inner")!!
         val iinner = symLoader.resolveClassFromBinaryName("$outerName\$Inner\$IInner")!!
@@ -85,13 +88,13 @@ class AsmLoaderTest : FunSpec({
         inner.declaredClasses.shouldContainExactlyInAnyOrder(iinner)
         iinner.declaredClasses.shouldBeEmpty()
 
-        iinner.enclosingClass.shouldBe(inner)
-        inner.enclosingClass.shouldBe(outer)
-        outer.enclosingClass.shouldBe(null)
+        iinner.enclosingClass.shouldBeSameInstanceAs(inner)
+        inner.enclosingClass.shouldBeSameInstanceAs(outer)
+        outer.enclosingClass.shouldBeNull()
 
-        iinner.enclosingMethod.shouldBe(null)
-        inner.enclosingMethod.shouldBe(null)
-        outer.enclosingMethod.shouldBe(null)
+        iinner.enclosingMethod.shouldBeNull()
+        inner.enclosingMethod.shouldBeNull()
+        outer.enclosingMethod.shouldBeNull()
     }
 
 
@@ -99,16 +102,54 @@ class AsmLoaderTest : FunSpec({
 
         val outerName = Statics::class.java.name
 
-        val inner = symLoader.resolveClassFromBinaryName("$outerName\$ProtectedStatic")!!
+        val inner = symLoader().resolveClassFromBinaryName("$outerName\$ProtectedStatic")!!
 
         inner.modifiers shouldBe (Opcodes.ACC_PROTECTED or Opcodes.ACC_STATIC)
     }
 
+    test("Inner names with dollars") {
+
+        val outerName = `Another$ClassWith$Dollar`::class.java.name
+
+        val inner = symLoader().resolveClassFromBinaryName("$outerName\$AnInner\$ClassWithDollar")!!
+
+        inner.simpleName shouldBe "AnInner\$ClassWithDollar"
+        inner.canonicalName shouldBe "javasymbols.testdata.deep.Another\$ClassWith\$Dollar.AnInner\$ClassWithDollar"
+    }
+
+    test("Deeper inner names with dollars") {
+
+        val outerName = `Another$ClassWith$Dollar`::class.java.name
+
+        val symLoader = symLoader()
+        val deeper = symLoader.resolveClassFromBinaryName("$outerName\$AnInner\$ClassWithDollar\$ADeeper\$ClassWithDollar")!!
+
+        deeper.simpleName shouldBe "ADeeper\$ClassWithDollar"
+        deeper.canonicalName shouldBe "javasymbols.testdata.deep.Another\$ClassWith\$Dollar.AnInner\$ClassWithDollar.ADeeper\$ClassWithDollar"
+
+        symLoader.resolveClassFromCanonicalName(deeper.canonicalName!!) shouldBeSameInstanceAs deeper
+    }
+
+    test("Simple name that looks like an anonymous name but isn't") {
+
+        val klass = `Another$ClassWith$Dollar`.`DollarsAndNumbers$0`::class.java
+        val binaryName = klass.name
+
+        val symLoader = symLoader()
+        val deeper = symLoader.resolveClassFromBinaryName(binaryName).shouldNotBeNull()
+
+        deeper.simpleName shouldBe "DollarsAndNumbers\$0"
+        deeper.canonicalName shouldBe klass.canonicalName!!
+        deeper.binaryName shouldBe binaryName
+
+        symLoader.resolveClassFromCanonicalName(deeper.canonicalName!!) shouldBeSameInstanceAs deeper
+    }
+
     test("Inner class constructors reflect no parameter for the enclosing instance") {
 
         val outerName = NestedClasses::class.java.name
 
-        val inner = symLoader.resolveClassFromBinaryName("$outerName\$Inner")!!
+        val inner = symLoader().resolveClassFromBinaryName("$outerName\$Inner")!!
 
         inner.modifiers shouldBe Opcodes.ACC_PUBLIC
 
@@ -124,6 +165,7 @@ class AsmLoaderTest : FunSpec({
 
         val outerName = Statics::class.java.name
 
+        val symLoader = symLoader()
         val inner = symLoader.resolveClassFromCanonicalName("$outerName.ProtectedStatic")!!
         val second = symLoader.resolveClassFromBinaryName("$outerName\$ProtectedStatic")!!
 
@@ -132,6 +174,7 @@ class AsmLoaderTest : FunSpec({
 
     test("Unresolved class should have object as superclass") {
 
+        val symLoader = symLoader()
         val inner = symLoader.resolveFromInternalNameCannotFail("does/not/exist")!!
         val second = symLoader.resolveFromInternalNameCannotFail("does/not/exist")!!
 
@@ -144,13 +187,13 @@ class AsmLoaderTest : FunSpec({
 
         val outerName = Enums::class.java.name
 
-        val emptyEnum = symLoader.resolveClassFromBinaryName("$outerName\$Empty")!!
+        val emptyEnum = symLoader().resolveClassFromBinaryName("$outerName\$Empty")!!
         emptyEnum::getEnumConstantNames shouldBe emptySet()
 
-        val withConstants = symLoader.resolveClassFromBinaryName("$outerName\$SomeConstants")!!
+        val withConstants = symLoader().resolveClassFromBinaryName("$outerName\$SomeConstants")!!
         withConstants::getEnumConstantNames shouldBe setOf("A", "B")
 
-        val notAnEnum = symLoader.resolveClassFromBinaryName(outerName)!!
+        val notAnEnum = symLoader().resolveClassFromBinaryName(outerName)!!
         notAnEnum::getEnumConstantNames shouldBe null
     }
 })

pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/symbols/internal/asm/BrokenClasspathTest.kt

@@ -10,6 +10,7 @@ import io.kotest.core.spec.style.FunSpec
 import io.kotest.matchers.shouldBe
 import io.kotest.matchers.shouldNotBe
 import net.sourceforge.pmd.lang.ast.test.shouldBe
+import net.sourceforge.pmd.lang.java.symbols.JClassSymbol
 import net.sourceforge.pmd.lang.java.types.JClassType
 import net.sourceforge.pmd.lang.java.types.TypeOps
 import net.sourceforge.pmd.lang.java.types.TypeSystem
@@ -73,6 +74,6 @@ class BrokenClasspathTest : FunSpec({
 
 })
 
-fun TypeSystem.createUnresolvedAsmSymbol(binaryName: String) =
+fun TypeSystem.createUnresolvedAsmSymbol(binaryName: String): JClassSymbol =
     AsmSymbolResolver(this, Classpath.contextClasspath())
-        .resolveFromInternalNameCannotFail(binaryName.replace('.', '/'))
+        .resolveFromInternalNameCannotFail(binaryName.replace('.', '/'))!!

pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/symbols/internal/asm/NamesTest.kt

@@ -15,8 +15,8 @@ class NamesTest : IntelliMarker, FunSpec({
         val names = ClassStub.Names("java/text/NumberFormat\$Style")
 
         names.binaryName shouldBe "java.text.NumberFormat\$Style"
-        names.canonicalName shouldBe "java.text.NumberFormat.Style"
-        names.simpleName shouldBe "Style"
+        names.canonicalName shouldBe null
+        names.simpleName shouldBe null
         names.packageName shouldBe "java.text"
     }
 
@@ -46,9 +46,19 @@ class NamesTest : IntelliMarker, FunSpec({
         val names = ClassStub.Names("javasymbols/testdata/deep/ClassWithDollar\$")
 
         names.binaryName shouldBe "javasymbols.testdata.deep.ClassWithDollar\$"
-        names.canonicalName shouldBe "javasymbols.testdata.deep.ClassWithDollar\$"
-        names.simpleName shouldBe "ClassWithDollar\$"
+        names.canonicalName shouldBe null
+        names.simpleName shouldBe null
         names.packageName shouldBe "javasymbols.testdata.deep"
     }
 
+    test("Test names dollar in package name") {
+
+        val names = ClassStub.Names("\$javasymbols\$/test\$data/de\$ep/ClassWithDollar\$")
+
+        names.binaryName shouldBe "\$javasymbols\$.test\$data.de\$ep.ClassWithDollar\$"
+        names.packageName shouldBe "\$javasymbols\$.test\$data.de\$ep"
+        names.canonicalName shouldBe null
+        names.simpleName shouldBe null
+    }
+
 })

pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/symbols/table/internal/HeaderScopesTest.kt

@@ -329,7 +329,7 @@ class HeaderScopesTest : ProcessorTestSpec({
             )
     }
 
-    parserTest("Import of an unconventional name with dollar") {
+    parserTest("f:Import of an unconventional name with dollar") {
 
         assertNoSemanticErrorsOrWarnings()