Further enhancements to parseForTypeSignatures (#1478)

We generalize the method so it is useful for parsing either method
argument types or generic type arguments. (The latter required handling
wildcards.) Also factor out some common logic, and make the method
public so it can be used from other projects.

core/src/main/java/com/ibm/wala/types/generics/TypeSignature.java

@@ -13,7 +13,6 @@
 import com.ibm.wala.types.TypeReference;
 import com.ibm.wala.util.debug.Assertions;
 import java.util.ArrayList;
-import java.util.Iterator;
 
 /**
  * UNDER CONSTRUCTION.
@@ -81,13 +80,21 @@ public static TypeSignature make(String s) throws IllegalArgumentException {
   public abstract boolean isBaseType();
 
   /**
-   * @param typeSigs TypeSignature*
-   * @return tokenize it
+   * Split a string of consecutive type signatures (TypeSignature*) into its top-level type
+   * signatures. The string should start with either {@code (} or {@code <} and have a respective
+   * matching {@code )} or {@code >}.
+   *
+   * @param typeSigs a string of consecutive type signatures
+   * @return an array of top-level type signatures
    */
-  static String[] parseForTypeSignatures(String typeSigs) throws IllegalArgumentException {
+  public static String[] parseForTypeSignatures(String typeSigs) throws IllegalArgumentException {
     ArrayList<String> sigs = new ArrayList<>(10);
+    char start = typeSigs.charAt(0);
+    if (start != '(' && start != '<') {
+      throw new IllegalArgumentException(
+          "illegal start of TypeSignature " + typeSigs + ", must be '(' or '<'");
+    }
     if (typeSigs.length() < 2) {
-      // TODO: check this?
       throw new IllegalArgumentException("illegal string of TypeSignature " + typeSigs);
     }
 
@@ -124,20 +131,16 @@ static String[] parseForTypeSignatures(String typeSigs) throws IllegalArgumentEx
         case TypeReference.ClassTypeCode:
           {
             int off = i - 1;
-            int depth = 0;
-            while (typeSigs.charAt(i++) != ';' || depth > 0) {
-              if (typeSigs.charAt(i - 1) == '<') {
-                depth++;
-              }
-              if (typeSigs.charAt(i - 1) == '>') {
-                depth--;
-              }
-            }
+            i = getEndIndexOfClassType(typeSigs, i);
             sigs.add(typeSigs.substring(off, i));
             continue;
           }
         case TypeReference.ArrayTypeCode:
           {
+            int arrayStart = i - 1;
+            while (typeSigs.charAt(i) == TypeReference.ArrayTypeCode) {
+              i++;
+            }
             switch (typeSigs.charAt(i)) {
               case TypeReference.BooleanTypeCode:
               case TypeReference.ByteTypeCode:
@@ -147,23 +150,14 @@ static String[] parseForTypeSignatures(String typeSigs) throws IllegalArgumentEx
               case TypeReference.FloatTypeCode:
               case TypeReference.DoubleTypeCode:
               case TypeReference.CharTypeCode:
-                sigs.add(typeSigs.substring(i - 1, i + 1));
+                sigs.add(typeSigs.substring(arrayStart, i + 1));
                 i++;
                 break;
               case 'T':
               case TypeReference.ClassTypeCode:
-                int off = i - 1;
-                i++;
-                int depth = 0;
-                while (typeSigs.charAt(i++) != ';' || depth > 0) {
-                  if (typeSigs.charAt(i - 1) == '<') {
-                    depth++;
-                  }
-                  if (typeSigs.charAt(i - 1) == '>') {
-                    depth--;
-                  }
-                }
-                sigs.add(typeSigs.substring(off, i));
+                i++; // to skip 'L' or 'T'
+                i = getEndIndexOfClassType(typeSigs, i);
+                sigs.add(typeSigs.substring(arrayStart, i));
                 break;
               default:
                 Assertions.UNREACHABLE("BANG " + typeSigs.charAt(i));
@@ -178,20 +172,35 @@ static String[] parseForTypeSignatures(String typeSigs) throws IllegalArgumentEx
             sigs.add(typeSigs.substring(off, i));
             continue;
           }
+        case (byte) '*': // unbounded wildcard
+          sigs.add("*");
+          break;
+        case (byte) '-': // bounded wildcard
+        case (byte) '+': // bounded wildcard
+          int boundedStart = i - 1;
+          i++; // to skip 'L'
+          i = getEndIndexOfClassType(typeSigs, i);
+          sigs.add(typeSigs.substring(boundedStart, i));
+          break;
         case (byte) ')': // end of parameter list
-          int size = sigs.size();
-          if (size == 0) {
-            return null;
-          }
-          Iterator<String> it = sigs.iterator();
-          String[] result = new String[size];
-          for (int j = 0; j < size; j++) {
-            result[j] = it.next();
-          }
-          return result;
+        case (byte) '>': // end of type argument list
+          return sigs.toArray(new String[sigs.size()]);
         default:
-          assert false : "bad type signature list " + typeSigs;
+          throw new IllegalArgumentException("bad type signature list " + typeSigs);
+      }
+    }
+  }
+
+  private static int getEndIndexOfClassType(String typeSigs, int i) {
+    int depth = 0;
+    while (typeSigs.charAt(i++) != ';' || depth > 0) {
+      if (typeSigs.charAt(i - 1) == '<') {
+        depth++;
+      }
+      if (typeSigs.charAt(i - 1) == '>') {
+        depth--;
       }
     }
+    return i;
   }
 }

core/src/test/java/com/ibm/wala/types/generics/TypeSignatureTest.java

@@ -0,0 +1,74 @@
+package com.ibm.wala.types.generics;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.arrayContaining;
+import static org.hamcrest.Matchers.is;
+
+import com.ibm.wala.classLoader.IClass;
+import com.ibm.wala.classLoader.IMethod;
+import com.ibm.wala.classLoader.ShrikeCTMethod;
+import com.ibm.wala.core.tests.callGraph.CallGraphTestUtil;
+import com.ibm.wala.core.tests.util.TestConstants;
+import com.ibm.wala.ipa.callgraph.AnalysisScope;
+import com.ibm.wala.ipa.cha.ClassHierarchy;
+import com.ibm.wala.ipa.cha.ClassHierarchyException;
+import com.ibm.wala.ipa.cha.ClassHierarchyFactory;
+import com.ibm.wala.shrike.shrikeCT.InvalidClassFileException;
+import java.io.IOException;
+import org.junit.jupiter.api.Test;
+
+public class TypeSignatureTest {
+
+  @Test
+  void basicTypeArguments() {
+    assertThat(
+        TypeSignature.parseForTypeSignatures("<Ljava/lang/String;ILjava/lang/Object;>"),
+        arrayContaining(is("Ljava/lang/String;"), is("I"), is("Ljava/lang/Object;")));
+  }
+
+  @Test
+  void multiDimArray() {
+    assertThat(
+        TypeSignature.parseForTypeSignatures("<[[Ljava/lang/String;[[[J>"),
+        arrayContaining(is("[[Ljava/lang/String;"), is("[[[J")));
+  }
+
+  @Test
+  void wildcards() {
+    assertThat(
+        TypeSignature.parseForTypeSignatures("<B*J>"), arrayContaining(is("B"), is("*"), is("J")));
+    assertThat(
+        TypeSignature.parseForTypeSignatures("<+Ljava/lang/Object;>"),
+        arrayContaining(is("+Ljava/lang/Object;")));
+    assertThat(
+        TypeSignature.parseForTypeSignatures("<-Ljava/lang/Double;BB>"),
+        arrayContaining(is("-Ljava/lang/Double;"), is("B"), is("B")));
+  }
+
+  @Test
+  void testAllGenericMethodSigs()
+      throws IOException, ClassHierarchyException, InvalidClassFileException {
+    AnalysisScope scope =
+        CallGraphTestUtil.makeJ2SEAnalysisScope(
+            TestConstants.WALA_TESTDATA, "J2SEClassHierarchyExclusions.txt");
+    ClassHierarchy cha = ClassHierarchyFactory.make(scope);
+    for (IClass klass : cha) {
+      for (IMethod m : klass.getDeclaredMethods()) {
+        if (m instanceof ShrikeCTMethod) {
+          ShrikeCTMethod method = (ShrikeCTMethod) m;
+          MethodTypeSignature methodTypeSignature = method.getMethodTypeSignature();
+          if (methodTypeSignature != null) {
+            String typeSigStr = methodTypeSignature.toString();
+            for (int i = 0; i < typeSigStr.length(); i++) {
+              if ((typeSigStr.charAt(i) == '<' && i != 0) || typeSigStr.charAt(i) == '(') {
+                // parsing will automatically end at the matching '>' or ')'
+                // this is just testing for crashes
+                TypeSignature.parseForTypeSignatures(typeSigStr.substring(i));
+              }
+            }
+          }
+        }
+      }
+    }
+  }
+}