From 8052d79e6df250ea43c8db26a53ce70f8eea108a Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Sat, 4 Jul 2026 12:42:17 -0700 Subject: [PATCH 1/3] Code review --- .../plumelib/reflection/ReflectionPlume.java | 6 +-- .../plumelib/reflection/SignatureRegexes.java | 2 +- .../org/plumelib/reflection/Signatures.java | 4 +- .../plumelib/reflection/TestSignatures.java | 39 +++++++++++++++++++ 4 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/plumelib/reflection/ReflectionPlume.java b/src/main/java/org/plumelib/reflection/ReflectionPlume.java index 11ffd954..77c65dd5 100644 --- a/src/main/java/org/plumelib/reflection/ReflectionPlume.java +++ b/src/main/java/org/plumelib/reflection/ReflectionPlume.java @@ -97,9 +97,9 @@ public static boolean isSubtype(Class sub, Class sup) { * fully-qualified name (in addition to a binary name). * *

If the given name can't be found, this method changes the last '.' to a dollar sign ($) and - * tries again. This accounts for inner classes that are incorrectly passed in fully-qualified - * format instead of binary format. (It should try multiple dollar signs, not just at the last - * position.) + * tries again, repeating with successive dots (from right to left) until the class is found or no + * dots remain. This accounts for inner classes that are incorrectly passed in fully-qualified + * format instead of binary format. * *

Recall the rather odd specification for {@link Class#forName(String)}: the argument is a * binary name for non-arrays, but a field descriptor for arrays. This method uses the same rules, diff --git a/src/main/java/org/plumelib/reflection/SignatureRegexes.java b/src/main/java/org/plumelib/reflection/SignatureRegexes.java index c7c53064..fb4a0a88 100644 --- a/src/main/java/org/plumelib/reflection/SignatureRegexes.java +++ b/src/main/java/org/plumelib/reflection/SignatureRegexes.java @@ -266,7 +266,7 @@ private SignatureRegexes() { /** An anchored regex that matches FieldDescriptorWithoutPackage strings. */ public static final @Regex String FieldDescriptorWithoutPackageRegex = - ANCHORED("(" + FD_PRIMITIVE + "|\\[+" + FD_PRIMITIVE + "|\\[L" + IDENTIFIER + NESTED + ";)"); + ANCHORED("(" + FD_PRIMITIVE + "|\\[+" + FD_PRIMITIVE + "|\\[+L" + IDENTIFIER + NESTED + ";)"); /** An anchored pattern that matches FieldDescriptorWithoutPackage strings. */ public static final Pattern FieldDescriptorWithoutPackagePattern = diff --git a/src/main/java/org/plumelib/reflection/Signatures.java b/src/main/java/org/plumelib/reflection/Signatures.java index 32ae0e1d..973ea5a6 100644 --- a/src/main/java/org/plumelib/reflection/Signatures.java +++ b/src/main/java/org/plumelib/reflection/Signatures.java @@ -98,8 +98,8 @@ private Signatures() { if (!classfilename.endsWith(".class")) { throw new IllegalArgumentException("Bad class file name: " + classfilename); } - @SuppressWarnings("index:assignment") // "/" is not the last character - @IndexFor("classfilename") int start = classfilename.lastIndexOf('/') + 1; + @SuppressWarnings("index:assignment") // the separator is not the last character + @IndexFor("classfilename") int start = Math.max(classfilename.lastIndexOf('/'), classfilename.lastIndexOf(dirSep)) + 1; int end = classfilename.length() - 6; return classfilename.substring(start, end); } diff --git a/src/test/java/org/plumelib/reflection/TestSignatures.java b/src/test/java/org/plumelib/reflection/TestSignatures.java index 74e08ac7..f5c2c6df 100644 --- a/src/test/java/org/plumelib/reflection/TestSignatures.java +++ b/src/test/java/org/plumelib/reflection/TestSignatures.java @@ -5,6 +5,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.Collections; +import org.checkerframework.checker.index.qual.NonNegative; import org.checkerframework.checker.nullness.qual.Nullable; import org.checkerframework.checker.signature.qual.BinaryName; import org.checkerframework.checker.signature.qual.ClassGetName; @@ -161,6 +162,44 @@ void test_isFqBinaryName() { assertTrue(Signatures.isFqBinaryName("pkg.Outer$Inner[][]")); } + /** + * A field descriptor for an array: {@code dims} copies of "[" followed by {@code base}. + * + *

Multidimensional field descriptors are constructed here at run time rather than written as + * source literals, to avoid a crash in the Checker Framework's literal annotator. Its bundled + * FieldDescriptorWithoutPackage regex predates the fix that makes strings such as "[[LMyClass;" + * match, so writing them as literals triggers a {@code BugInCF}. + * + * @param dims the number of array dimensions + * @param base the field descriptor of the element type + * @return a field descriptor for the array type + */ + private static String nDimArray(@NonNegative int dims, String base) { + return "[".repeat(dims) + base; + } + + /** Returns true if the argument has the format of a FieldDescriptorWithoutPackage. */ + @Test + void test_isFieldDescriptorWithoutPackage() { + assertTrue(Signatures.isFieldDescriptorWithoutPackage("I")); + assertTrue(Signatures.isFieldDescriptorWithoutPackage("[[J")); + assertTrue(Signatures.isFieldDescriptorWithoutPackage("[LMyClass;")); + assertTrue(Signatures.isFieldDescriptorWithoutPackage("[LMyClass$22;")); + // Multidimensional arrays of unnamed-package classes (see nDimArray). + assertTrue(Signatures.isFieldDescriptorWithoutPackage(nDimArray(2, "LMyClass;"))); + assertTrue(Signatures.isFieldDescriptorWithoutPackage(nDimArray(3, "LMyClass;"))); + assertTrue(Signatures.isFieldDescriptorWithoutPackage(nDimArray(2, "LMyClass$22;"))); + // Non-arrays of reference types are field descriptors, but not "without package" ones. + assertTrue(!Signatures.isFieldDescriptorWithoutPackage("LMyClass;")); + // A field descriptor whose element type has a package is not "without package". + assertTrue(!Signatures.isFieldDescriptorWithoutPackage("Ljava/lang/Object;")); + assertTrue(!Signatures.isFieldDescriptorWithoutPackage("[Ljava/lang/Object;")); + assertTrue(!Signatures.isFieldDescriptorWithoutPackage(nDimArray(2, "Ljava/lang/Object;"))); + // Not field descriptors at all. + assertTrue(!Signatures.isFieldDescriptorWithoutPackage("int")); + assertTrue(!Signatures.isFieldDescriptorWithoutPackage("MyClass")); + } + /** * Returns true if the argument has the format of an Identifier. The type it refers to might or * might not exist. From 5ec3b290715ad965eec308e803070349f9bb3e32 Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Sun, 5 Jul 2026 15:42:43 -0700 Subject: [PATCH 2/3] Simplify regex --- src/main/java/org/plumelib/reflection/SignatureRegexes.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/plumelib/reflection/SignatureRegexes.java b/src/main/java/org/plumelib/reflection/SignatureRegexes.java index fb4a0a88..96b283e1 100644 --- a/src/main/java/org/plumelib/reflection/SignatureRegexes.java +++ b/src/main/java/org/plumelib/reflection/SignatureRegexes.java @@ -266,7 +266,7 @@ private SignatureRegexes() { /** An anchored regex that matches FieldDescriptorWithoutPackage strings. */ public static final @Regex String FieldDescriptorWithoutPackageRegex = - ANCHORED("(" + FD_PRIMITIVE + "|\\[+" + FD_PRIMITIVE + "|\\[+L" + IDENTIFIER + NESTED + ";)"); + ANCHORED("(\\[+" + FD_PRIMITIVE + "|\\[+L" + IDENTIFIER + NESTED + ";)"); /** An anchored pattern that matches FieldDescriptorWithoutPackage strings. */ public static final Pattern FieldDescriptorWithoutPackagePattern = From 7a3d59598eee74138026ead406f32ac5cbb9cfce Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Sun, 5 Jul 2026 15:48:50 -0700 Subject: [PATCH 3/3] Fix array handling --- src/main/java/org/plumelib/reflection/SignatureRegexes.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/plumelib/reflection/SignatureRegexes.java b/src/main/java/org/plumelib/reflection/SignatureRegexes.java index 96b283e1..c7422600 100644 --- a/src/main/java/org/plumelib/reflection/SignatureRegexes.java +++ b/src/main/java/org/plumelib/reflection/SignatureRegexes.java @@ -266,7 +266,8 @@ private SignatureRegexes() { /** An anchored regex that matches FieldDescriptorWithoutPackage strings. */ public static final @Regex String FieldDescriptorWithoutPackageRegex = - ANCHORED("(\\[+" + FD_PRIMITIVE + "|\\[+L" + IDENTIFIER + NESTED + ";)"); + // Non-arrays of reference types are field descriptors, but not "without package" ones. + ANCHORED("(\\[*" + FD_PRIMITIVE + "|\\[+L" + IDENTIFIER + NESTED + ";)"); /** An anchored pattern that matches FieldDescriptorWithoutPackage strings. */ public static final Pattern FieldDescriptorWithoutPackagePattern =