Imporve key uniqueness in signature map

policeman-tools · Mar 31, 2018 · 5772c9f · 5772c9f
1 parent 8a4dd7e
commit 5772c9f
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 10 deletions.
diff --git a/src/main/java/de/thetaphi/forbiddenapis/Signatures.java b/src/main/java/de/thetaphi/forbiddenapis/Signatures.java
@@ -104,6 +104,18 @@ public Signatures(RelatedClassLookup lookup, Logger logger, boolean failOnUnreso
     this.failOnUnresolvableSignatures = failOnUnresolvableSignatures;
   }
 
+  static String getKey(String internalClassName) {
+    return "c\000" + internalClassName;
+  }
+
+  static String getKey(String internalClassName, String field) {
+    return "f\000" + internalClassName + '\000' + field;
+  }
+
+  static String getKey(String internalClassName, Method method) {
+    return "m\000" + internalClassName + '\000' + method;
+  }
+
   /** Adds the method signature to the list of disallowed methods. The Signature is checked against the given ClassLoader. */
   private void addSignature(final String line, final String defaultMessage, final UnresolvableReporting report, final Set<String> missingClasses) throws ParseException,IOException {
     final String clazz, field, signature;
@@ -175,7 +187,7 @@ private void addSignature(final String line, final String defaultMessage, final
         for (final Method m : c.methods) {
           if (m.getName().equals(method.getName()) && Arrays.equals(m.getArgumentTypes(), method.getArgumentTypes())) {
             found = true;
-            signatures.put(c.className + '\000' + m, printout);
+            signatures.put(getKey(c.className, m), printout);
             // don't break when found, as there may be more covariant overrides!
           }
         }
@@ -189,11 +201,11 @@ private void addSignature(final String line, final String defaultMessage, final
           report.parseFailed(logger, "Field not found", signature);
           return;
         }
-        signatures.put(c.className + '\000' + field, printout);
+        signatures.put(getKey(c.className, field), printout);
       } else {
         assert field == null && method == null;
         // only add the signature as class name
-        signatures.put(c.className, printout);
+        signatures.put(getKey(c.className), printout);
       }
     }
   }
@@ -314,7 +326,7 @@ public String checkType(Type type) {
     if (type.getSort() != Type.OBJECT) {
       return null; // we don't know this type, just pass!
     }
-    final String printout = signatures.get(type.getInternalName());
+    final String printout = signatures.get(getKey(type.getInternalName()));
     if (printout != null) {
       return printout;
     }
@@ -328,11 +340,11 @@ public String checkType(Type type) {
   }
 
   public String checkMethod(String internalClassName, Method method) {
-    return signatures.get(internalClassName + '\000' + method);
+    return signatures.get(getKey(internalClassName, method));
   }
 
   public String checkField(String internalClassName, String field) {
-    return signatures.get(internalClassName + '\000' + field);
+    return signatures.get(getKey(internalClassName, field));
   }
 
   public static String fixTargetVersion(String name) throws ParseException {

diff --git a/src/test/java/de/thetaphi/forbiddenapis/CheckerSetupTest.java b/src/test/java/de/thetaphi/forbiddenapis/CheckerSetupTest.java
@@ -26,6 +26,7 @@
 
 import org.junit.Before;
 import org.junit.Test;
+import org.objectweb.asm.commons.Method;
 
 public final class CheckerSetupTest {
 
@@ -44,33 +45,37 @@ public void setUp() {
   public void testEmpty() {
     assertEquals(Collections.emptyMap(), forbiddenSignatures.signatures);
     assertEquals(Collections.emptySet(), forbiddenSignatures.classPatterns);
+    assertTrue(checker.hasNoSignatures());
   }
 
   @Test
   public void testClassSignature() throws Exception {
     checker.parseSignaturesString("java.lang.Object @ Foobar");
-    assertEquals(Collections.singletonMap("java/lang/Object", "java.lang.Object [Foobar]"), forbiddenSignatures.signatures);
+    assertEquals(Collections.singletonMap(Signatures.getKey("java/lang/Object"), "java.lang.Object [Foobar]"), forbiddenSignatures.signatures);
     assertEquals(Collections.emptySet(), forbiddenSignatures.classPatterns);
   }
 
   @Test
   public void testClassPatternSignature() throws Exception {
     checker.parseSignaturesString("java.lang.** @ Foobar");
     assertEquals(Collections.emptyMap(), forbiddenSignatures.signatures);
-    assertEquals(Collections.singleton(new ClassPatternRule("java.lang.**", "Foobar")), forbiddenSignatures.classPatterns);
+    assertEquals(Collections.singleton(new ClassPatternRule("java.lang.**", "Foobar")),
+        forbiddenSignatures.classPatterns);
   }
 
   @Test
   public void testFieldSignature() throws Exception {
     checker.parseSignaturesString("java.lang.String#CASE_INSENSITIVE_ORDER @ Foobar");
-    assertEquals(Collections.singletonMap("java/lang/String\000CASE_INSENSITIVE_ORDER", "java.lang.String#CASE_INSENSITIVE_ORDER [Foobar]"), forbiddenSignatures.signatures);
+    assertEquals(Collections.singletonMap(Signatures.getKey("java/lang/String", "CASE_INSENSITIVE_ORDER"), "java.lang.String#CASE_INSENSITIVE_ORDER [Foobar]"),
+        forbiddenSignatures.signatures);
     assertEquals(Collections.emptySet(), forbiddenSignatures.classPatterns);
   }
 
   @Test
   public void testMethodSignature() throws Exception {
     checker.parseSignaturesString("java.lang.Object#toString() @ Foobar");
-    assertEquals(Collections.singletonMap("java/lang/Object\000toString()Ljava/lang/String;", "java.lang.Object#toString() [Foobar]"), forbiddenSignatures.signatures);
+    assertEquals(Collections.singletonMap(Signatures.getKey("java/lang/Object", new Method("toString", "()Ljava/lang/String;")), "java.lang.Object#toString() [Foobar]"),
+        forbiddenSignatures.signatures);
     assertEquals(Collections.emptySet(), forbiddenSignatures.classPatterns);
   }