8319463: ClassSignature should have superclass and superinterfaces as ClassTypeSig

Reviewed-by: asotona

src/java.base/share/classes/java/lang/classfile/ClassSignature.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -42,10 +42,10 @@ public sealed interface ClassSignature
     List<Signature.TypeParam> typeParameters();
 
     /** {@return the instantiation of the superclass in this signature} */
-    Signature.RefTypeSig superclassSignature();
+    Signature.ClassTypeSig superclassSignature();
 
     /** {@return the instantiation of the interfaces in this signature} */
-    List<Signature.RefTypeSig> superinterfaceSignatures();
+    List<Signature.ClassTypeSig> superinterfaceSignatures();
 
     /** {@return the raw signature string} */
     String signatureString();
@@ -55,8 +55,8 @@ public sealed interface ClassSignature
      * @param superclassSignature the superclass
      * @param superinterfaceSignatures the interfaces
      */
-    public static ClassSignature of(Signature.RefTypeSig superclassSignature,
+    public static ClassSignature of(Signature.ClassTypeSig superclassSignature,
-                                    Signature.RefTypeSig... superinterfaceSignatures) {
+                                    Signature.ClassTypeSig... superinterfaceSignatures) {
         return of(List.of(), superclassSignature, superinterfaceSignatures);
     }
 
@@ -67,8 +67,8 @@ public sealed interface ClassSignature
      * @param superinterfaceSignatures the interfaces
      */
     public static ClassSignature of(List<Signature.TypeParam> typeParameters,
-                                    Signature.RefTypeSig superclassSignature,
+                                    Signature.ClassTypeSig superclassSignature,
-                                    Signature.RefTypeSig... superinterfaceSignatures) {
+                                    Signature.ClassTypeSig... superinterfaceSignatures) {
         return new SignaturesImpl.ClassSignatureImpl(
                 requireNonNull(typeParameters),
                 requireNonNull(superclassSignature),
@@ -81,6 +81,6 @@ public sealed interface ClassSignature
      * @return class signature
      */
     public static ClassSignature parseFrom(String classSignature) {
-        return new SignaturesImpl().parseClassSignature(requireNonNull(classSignature));
+        return new SignaturesImpl(classSignature).parseClassSignature();
     }
 }

src/java.base/share/classes/java/lang/classfile/MethodSignature.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -109,6 +109,6 @@ public sealed interface MethodSignature
      */
     public static MethodSignature parseFrom(String methodSignature) {
 
-        return new SignaturesImpl().parseMethodSignature(requireNonNull(methodSignature));
+        return new SignaturesImpl(methodSignature).parseMethodSignature();
     }
 }

src/java.base/share/classes/java/lang/classfile/Signature.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -51,7 +51,7 @@ public sealed interface Signature {
      * @return Java type signature
      */
     public static Signature parseFrom(String javaTypeSignature) {
-        return new SignaturesImpl().parseSignature(requireNonNull(javaTypeSignature));
+        return new SignaturesImpl(javaTypeSignature).parseSignature();
     }
 
     /**

src/java.base/share/classes/jdk/internal/classfile/impl/ClassRemapperImpl.java

@@ -291,7 +291,7 @@ public record ClassRemapperImpl(Function<ClassDesc, ClassDesc> mapFunction) impl
         return ClassSignature.of(mapTypeParams(signature.typeParameters()),
                 mapSignature(signature.superclassSignature()),
                 signature.superinterfaceSignatures().stream()
-                        .map(this::mapSignature).toArray(Signature.RefTypeSig[]::new));
+                        .map(this::mapSignature).toArray(Signature.ClassTypeSig[]::new));
     }
 
     MethodSignature mapMethodSignature(MethodSignature signature) {

src/java.base/share/classes/jdk/internal/classfile/impl/SignaturesImpl.java

@@ -26,6 +26,7 @@ package jdk.internal.classfile.impl;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.Collections;
 import java.lang.classfile.ClassSignature;
@@ -35,23 +36,23 @@ import java.lang.classfile.Signature.*;
 
 public final class SignaturesImpl {
 
-    public SignaturesImpl() {
+    public SignaturesImpl(String signature) {
+        this.sig = Objects.requireNonNull(signature);
+        this.sigp = 0;
     }
 
-    private String sig;
+    private final String sig;
     private int sigp;
 
-    public ClassSignature parseClassSignature(String signature) {
+    public ClassSignature parseClassSignature() {
-        this.sig = signature;
-        sigp = 0;
         try {
             List<TypeParam> typeParamTypes = parseParamTypes();
-            RefTypeSig superclass = referenceTypeSig();
+            ClassTypeSig superclass = classTypeSig();
-            ArrayList<RefTypeSig> superinterfaces = null;
+            ArrayList<ClassTypeSig> superinterfaces = null;
             while (sigp < sig.length()) {
                 if (superinterfaces == null)
                     superinterfaces = new ArrayList<>();
-                superinterfaces.add(referenceTypeSig());
+                superinterfaces.add(classTypeSig());
             }
             return new ClassSignatureImpl(typeParamTypes, superclass, null2Empty(superinterfaces));
         } catch (IndexOutOfBoundsException e) {
@@ -59,25 +60,20 @@ public final class SignaturesImpl {
         }
     }
 
-    public MethodSignature parseMethodSignature(String signature) {
+    public MethodSignature parseMethodSignature() {
-        this.sig = signature;
-        sigp = 0;
         try {
             List<TypeParam> typeParamTypes = parseParamTypes();
-            if (sig.charAt(sigp) != '(') throw error("Expected ( at possition %d of signature".formatted(sigp));
+            require('(');
-            sigp++;
             ArrayList<Signature> paramTypes = null;
-            while (sig.charAt(sigp) != ')') {
+            while (!match(')')) {
                 if (paramTypes == null)
-                     paramTypes = new ArrayList<>();
+                    paramTypes = new ArrayList<>();
                 paramTypes.add(typeSig());
             }
-            sigp++;
             Signature returnType = typeSig();
             ArrayList<ThrowableSig> throwsTypes = null;
             while (sigp < sig.length()) {
-                if (sig.charAt(sigp) != '^') throw error("Expected ^ at possition %d of signature".formatted(sigp));
+                require('^');
-                sigp++;
                 if (throwsTypes == null)
                     throwsTypes = new ArrayList<>();
                 var t = referenceTypeSig();
@@ -92,12 +88,10 @@ public final class SignaturesImpl {
         }
     }
 
-    public Signature parseSignature(String signature) {
+    public Signature parseSignature() {
-        this.sig = signature;
-        sigp = 0;
         try {
             var s = typeSig();
-            if (sigp == signature.length())
+            if (sigp == sig.length())
                 return s;
         } catch (IndexOutOfBoundsException e) {
         }
@@ -106,26 +100,23 @@ public final class SignaturesImpl {
 
     private List<TypeParam> parseParamTypes() {
         ArrayList<TypeParam> typeParamTypes = null;
-        if (sig.charAt(sigp) == '<') {
+        if (match('<')) {
-            sigp++;
             typeParamTypes = new ArrayList<>();
-            while (sig.charAt(sigp) != '>') {
+            // cannot have empty <>
-                int sep = sig.indexOf(":", sigp);
+            do {
-                String name = sig.substring(sigp, sep);
+                String name = sig.substring(sigp, requireIdentifier());
                 RefTypeSig classBound = null;
                 ArrayList<RefTypeSig> interfaceBounds = null;
-                sigp = sep + 1;
+                require(':');
                 if (sig.charAt(sigp) != ':')
                     classBound = referenceTypeSig();
-                while (sig.charAt(sigp) == ':') {
+                while (match(':')) {
-                    sigp++;
                     if (interfaceBounds == null)
                         interfaceBounds = new ArrayList<>();
                     interfaceBounds.add(referenceTypeSig());
                 }
                 typeParamTypes.add(new TypeParamImpl(name, Optional.ofNullable(classBound), null2Empty(interfaceBounds)));
-            }
+            } while (!match('>'));
-            sigp++;
         }
         return null2Empty(typeParamTypes);
     }
@@ -141,38 +132,20 @@ public final class SignaturesImpl {
     }
 
     private RefTypeSig referenceTypeSig() {
-        char c = sig.charAt(sigp++);
+        return switch (sig.charAt(sigp)) {
-        switch (c) {
+            case 'L' -> classTypeSig();
-            case 'L':
+            case 'T' -> {
-                StringBuilder sb = new StringBuilder();
+                sigp++;
-                ArrayList<TypeArg> argTypes = null;
+                var ty = Signature.TypeVarSig.of(sig.substring(sigp, requireIdentifier()));
-                Signature.ClassTypeSig t = null;
+                require(';');
-                char sigch ;
+                yield ty;
-                do {
+            }
-                    switch  (sigch = sig.charAt(sigp++)) {
+            case '[' -> {
-                        case '<' -> {
+                sigp++;
-                            argTypes = new ArrayList<>();
+                yield ArrayTypeSig.of(typeSig());
-                            while (sig.charAt(sigp) != '>')
+            }
-                                argTypes.add(typeArg());
+            default -> throw unexpectedError("a type signature");
-                            sigp++;
+        };
-                        }
-                        case '.',';' -> {
-                            t = new ClassTypeSigImpl(Optional.ofNullable(t), sb.toString(), null2Empty(argTypes));
-                            sb.setLength(0);
-                            argTypes = null;
-                        }
-                        default -> sb.append(sigch);
-                    }
-                } while (sigch != ';');
-                return t;
-            case 'T':
-                int sep = sig.indexOf(';', sigp);
-                var ty = Signature.TypeVarSig.of(sig.substring(sigp, sep));
-                sigp = sep + 1;
-                return ty;
-            case '[': return ArrayTypeSig.of(typeSig());
-        }
-        throw error("Unexpected character %c at possition %d of signature".formatted(c, sigp - 1));
     }
 
     private TypeArg typeArg() {
@@ -187,6 +160,82 @@ public final class SignaturesImpl {
         }
     }
 
+    private ClassTypeSig classTypeSig() {
+        require('L');
+        Signature.ClassTypeSig t = null;
+
+        do {
+            int start = sigp;
+            requireIdentifier();
+            if (t == null) {
+                while (match('/')) {
+                    requireIdentifier();
+                }
+            }
+            String className = sig.substring(start, sigp);
+
+            ArrayList<TypeArg> argTypes;
+            if (match('<')) {
+                // cannot have empty <>
+                argTypes = new ArrayList<>();
+                do {
+                    argTypes.add(typeArg());
+                } while (!match('>'));
+            } else {
+                argTypes = null;
+            }
+
+            boolean end = match(';');
+            if (end || match('.')) {
+                t = new ClassTypeSigImpl(Optional.ofNullable(t), className, null2Empty(argTypes));
+                if (end)
+                    return t;
+            } else {
+                throw unexpectedError(". or ;");
+            }
+        } while (true);
+    }
+
+    /**
+     * Tries to match a character, and moves pointer if it matches.
+     */
+    private boolean match(char c) {
+        if (sigp < sig.length() && sig.charAt(sigp) == c) {
+            sigp++;
+            return true;
+        }
+        return false;
+    }
+
+    /**
+     * Requires a character and moves past it, failing otherwise.
+     */
+    private void require(char c) {
+        if (!match(c))
+            throw unexpectedError(String.valueOf(c));
+    }
+
+    /**
+     * Requires an identifier, moving pointer to next illegal character and returning
+     * its position. Fails if the identifier is empty.
+     */
+    private int requireIdentifier() {
+        int start = sigp;
+        l:
+        while (sigp < sig.length()) {
+            switch (sig.charAt(sigp)) {
+                case '.', ';', '[', '/', '<', '>', ':' -> {
+                    break l;
+                }
+            }
+            sigp++;
+        }
+        if (start == sigp) {
+            throw unexpectedError("an identifier");
+        }
+        return sigp;
+    }
+
     public static record BaseTypeSigImpl(char baseType) implements Signature.BaseTypeSig {
 
         @Override
@@ -271,8 +320,8 @@ public final class SignaturesImpl {
         return sb;
     }
 
-    public static record ClassSignatureImpl(List<TypeParam> typeParameters, RefTypeSig superclassSignature,
+    public static record ClassSignatureImpl(List<TypeParam> typeParameters, ClassTypeSig superclassSignature,
-            List<RefTypeSig> superinterfaceSignatures) implements ClassSignature {
+            List<ClassTypeSig> superinterfaceSignatures) implements ClassSignature {
 
         @Override
         public String signatureString() {
@@ -308,6 +357,12 @@ public final class SignaturesImpl {
         return l == null ? List.of() : Collections.unmodifiableList(l);
     }
 
+    private IllegalArgumentException unexpectedError(String expected) {
+        return error(sigp < sig.length() ? "Unexpected character %c at position %d, expected %s"
+                .formatted(sig.charAt(sigp), sigp, expected)
+                : "Unexpected end of signature at position %d, expected %s".formatted(sigp, expected));
+    }
+
     private IllegalArgumentException error(String message) {
         return new IllegalArgumentException("%s: %s".formatted(message, sig));
     }

test/jdk/jdk/classfile/SignaturesTest.java

@@ -24,7 +24,7 @@
 /*
  * @test
  * @summary Testing Signatures.
- * @bug 8321540
+ * @bug 8321540 8319463
  * @run junit SignaturesTest
  */
 import java.io.IOException;
@@ -183,7 +183,7 @@ class SignaturesTest {
     void testClassSignatureClassDesc() throws IOException {
         var observerCf = ClassFile.of().parse(Path.of(System.getProperty("test.classes"), "SignaturesTest$Observer.class"));
         var sig = observerCf.findAttribute(Attributes.SIGNATURE).orElseThrow().asClassSignature();
-        var innerSig = (ClassTypeSig) ((ClassTypeSig) sig.superclassSignature()) // ArrayList
+        var innerSig = (ClassTypeSig) sig.superclassSignature() // ArrayList
                 .typeArgs().getFirst() // Outer<String>.Inner<Long>
                 .boundType().orElseThrow(); // assert it's exact bound
         assertEquals("Inner", innerSig.className(), "simple name in signature");
@@ -213,9 +213,37 @@ class SignaturesTest {
         LSet<+Kind<**>;>;
         LSet<?Kind<*>;>;
         ()V
+        Ljava/util/Opt<Ljava/lang/Integer;>ional;
+        Lcom/example/Outer<Ljava/lang/String;>.package/Inner<[I>;
+        LSample>;
+        LSample:Other;
+        LOuter<[JTT;>.[Inner;
+        TA:J;
+        LEmpty<>;
+        L
+        Lcom
+        Lcom/example/
+        Lcom/example/Outer<
+        Lcom/example/Outer<Ljava/
+        Lcom/example/Outer<Ljava/lang/String
+        Lcom/example/Outer<Ljava/lang/String;
+        Lcom/example/Outer<Ljava/lang/String;>
+        Lcom/example/Outer<Ljava/lang/String;>.
+        Lcom/example/Outer<Ljava/lang/String;>.Inner<[I>
         """.lines().forEach(assertThrows(Signature::parseFrom));
     }
 
+    @Test
+    void testGoodTypeSignatures() {
+        """
+        Ljava/util/Optional<Ljava/lang/Integer;>;
+        Lcom/example/Outer<Ljava/lang/Integer;>.Inner<[I>;
+        LSample;
+        LOuter<[JTT;>.Inner;
+        LOuter.Inner;
+        """.lines().forEach(Signature::parseFrom);
+    }
+
     @Test
     void testBadClassSignatures() {
         """
@@ -234,6 +262,14 @@ class SignaturesTest {
         <K:LObject;>>LFoo<TK;>;
         <K:LObject;>LFoo<+>;
         ()V
+        <K:Ljava/lang/Object;>Ljava/lang/Object;TK;
+        Ljava/lang/Object;[Ljava/lang/Object;
+        [Ljava/util/Optional<[I>;
+        [I
+        <K:Ljava/lang/Object;>TK;
+        <K;Q:Ljava/lang/Object;>Ljava/lang/Object;
+        <:Ljava/lang/Object;>Ljava/lang/Object;
+        <>Ljava/lang/Object;
         """.lines().forEach(assertThrows(ClassSignature::parseFrom));
     }
 
@@ -259,6 +295,7 @@ class SignaturesTest {
         ()LSet<+Kind<**>;>;
         (LSet<?Kind<*>;>;)V
         <T::LA>()V
+        (TT;I)VI
         """.lines().forEach(assertThrows(MethodSignature::parseFrom));
     }