Corrected class name validation to no longer fail for Kotlin classes on class path containing special characters

CHANGELOG.md

@@ -7,6 +7,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2.
 ## Unreleased - 2022-??-??
 ### Fixed
 - Remove duplicated logging frameworks from the Eclipse plugin distribution ([#1868](https://github.com/spotbugs/spotbugs/issues/1868))
+- Corrected class name validation to no longer fail for Kotlin classes on class path containing special characters. ([#1883](https://github.com/spotbugs/spotbugs/issues/1883))
 
 ## 4.5.2 - 2021-12-13
 ### Security

spotbugs/src/main/java/edu/umd/cs/findbugs/util/ClassName.java

@@ -175,9 +175,65 @@ public static String extractSimpleName(@DottedClassName String className) {
      * @return true if it's a valid class name, false otherwise
      */
     public static boolean isValidClassName(String className) {
-        // FIXME: should use a regex
+        return !className.isEmpty() &&
+                (isValidBinaryClassName(className) ||
+                        isValidDottedClassName(className) ||
+                        isValidArrayFieldDescriptor(className) ||
+                        isValidClassFieldDescriptor(className));
+    }
+
+    /**
+     * @see <a href="https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.2.1">
+     *     JVMS (Java SE 8 Edition) 4.2.1. Binary Class and Interface Names
+     *     </a>
+     */
+    private static boolean isValidBinaryClassName(String className) {
+        return className.indexOf('.') == -1 &&
+                className.indexOf('[') == -1 &&
+                className.indexOf(';') == -1;
+    }
+
+    private static boolean isValidDottedClassName(String className) {
+        return className.indexOf('/') == -1 &&
+                className.indexOf('[') == -1 &&
+                className.indexOf(';') == -1;
+    }
+
+    /**
+     * Determines whether a class name is a valid array field descriptor as per
+     * <a href="https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.3.2">
+     * JVMS (Java SE 8 Edition) 4.3.2</a>
+     *
+     * @param className a class name to test for validity - must be non-{@code null} and non-empty.
+     * @return {@code true} if {@code className} is a valid array field descriptor as
+     *          per JVMS 4.3.2, otherwise {@code false}
+     * @throws IndexOutOfBoundsException if {@code className} is empty.
+     * @throws NullPointerException if {@code className} is {@code null}.
+     */
+    private static boolean isValidArrayFieldDescriptor(String className) {
+        String tail = className.substring(1);
+        return className.startsWith("[") &&
+                (isValidArrayFieldDescriptor(tail) ||
+                        isValidClassFieldDescriptor(tail) ||
+                        isValidBaseTypeFieldDescriptor(tail));
+
+    }
+
+    private static boolean isValidClassFieldDescriptor(String className) {
+        return className.startsWith("L") &&
+                className.endsWith(";") &&
+                isValidBinaryClassName(className.substring(1, className.length() - 1));
+    }
 
-        return className.indexOf('(') < 0;
+    private static boolean isValidBaseTypeFieldDescriptor(String className) {
+        return "B".equals(className) ||
+                "C".equals(className) ||
+                "D".equals(className) ||
+                "F".equals(className) ||
+                "I".equals(className) ||
+                "J".equals(className) ||
+                "S".equals(className) ||
+                "Z".equals(className);
     }
 
     /**

spotbugs/src/test/java/edu/umd/cs/findbugs/util/ClassNameTest.java

@@ -95,4 +95,101 @@ public void testMatchedPrefix() {
                     ClassName.matchedPrefixes(searchString, "com.test.TestClass"));
         }
     }
+
+    @Test
+    public void testSimpleBinaryClassNameIsValidClassName() {
+        assertTrue(ClassName.isValidClassName("com/bla/Parent"));
+    }
+
+    @Test
+    public void testSimpleDottedClassNameIsValidClassName() {
+        assertTrue(ClassName.isValidClassName("com.bla.Parent"));
+    }
+
+    @Test
+    public void testInnerClassBinaryClassNameIsValidClassName() {
+        assertTrue(ClassName.isValidClassName("com/bla/Parent$Child"));
+    }
+
+    @Test
+    public void testInnerClassDottedClassNameIsValidClassName() {
+        assertTrue(ClassName.isValidClassName("com.bla.Parent$Child"));
+    }
+
+    @Test
+    public void testJavaStyleAnonymousInnerClassBinaryClassNameIsValidClassName() {
+        assertTrue(ClassName.isValidClassName("com/bla/Parent$Child$1"));
+    }
+
+    @Test
+    public void testJavaStyleAnonymousInnerClassDottedClassNameIsValidClassName() {
+        assertTrue(ClassName.isValidClassName("com.bla.Parent$Child$1"));
+    }
+
+    @Test
+    public void testKotlinStyleAnonymousInnerClassBinaryClassNameIsValidClassName() {
+        assertTrue(ClassName.isValidClassName("com/bla/Parent$function$variable$1"));
+    }
+
+    @Test
+    public void testKotlinStyleAnonymousInnerClassDottedClassNameIsValidClassName() {
+        assertTrue(ClassName.isValidClassName("com.bla.Parent$function$variable$1"));
+    }
+
+    @Test
+    public void testBinaryClassNameContainingAllowedSpecialCharactersIsValidClassName() {
+        assertTrue(ClassName.isValidClassName("com/bla/Parent$function!@#%^&*,?{}]()$variable$1"));
+    }
+
+    @Test
+    public void testDottedClassNameContainingAllowedSpecialCharactersIsValidClassName() {
+        assertTrue(ClassName.isValidClassName("com.bla.Parent$function!@#%^&*,?{}]()$variable$1"));
+    }
+
+    @Test
+    public void testFieldDescriptorClassNameIsValidClassName() {
+        assertTrue(ClassName.isValidClassName("Lcom/bla/Parent;"));
+    }
+
+    @Test
+    public void testFieldDescriptorOneDimensionalArrayClassNameIsValidClassName() {
+        assertTrue(ClassName.isValidClassName("[Lcom/bla/Parent;"));
+    }
+
+    @Test
+    public void testFieldDescriptorOneDimensionalPrimitiveArrayClassNameIsValidClassName() {
+        assertTrue(ClassName.isValidClassName("[I"));
+    }
+
+    @Test
+    public void testFieldDescriptorTwoDimensionalArrayClassNameIsValidClassName() {
+        assertTrue(ClassName.isValidClassName("[[Lcom/bla/Parent;"));
+    }
+
+    @Test
+    public void testFieldDescriptorClassNameContainingAllowedSpecialCharactersIsValidClassName() {
+        assertTrue(ClassName.isValidClassName("[[Lcom/bla/Parent$function!@#%^&*,?{}]()$variable$1;"));
+    }
+
+    @Test
+    public void testFieldDescriptorClassNameContainingDotsIsInvalidClassName() {
+        assertFalse(ClassName.isValidClassName("Lcom.bla.Parent;"));
+    }
+
+    @Test
+    public void testClassNameContainingBothSlashesAndDotsIsInvalidClassName() {
+        assertFalse(ClassName.isValidClassName("com.bla/Parent"));
+    }
+
+    @Test
+    public void testBinaryClassNameContainingDisallowedSpecialCharactersIsInvalidClassName() {
+        assertFalse(ClassName.isValidClassName("com/bla/Parent$function$variable$1;"));
+        assertFalse(ClassName.isValidClassName("com/bla/Parent$function;$variable$1"));
+        assertFalse(ClassName.isValidClassName("com/bla/Parent$function[$variable$1"));
+    }
+
+    @Test
+    public void testEmptyStringIsInvalidClassName() {
+        assertFalse(ClassName.isValidClassName(("")));
+    }
 }