Treat identifiers containing ignorable characters as invalid; Add unit tests for ConverterHelper class.

src/org/jetbrains/java/decompiler/modules/renamer/ConverterHelper.java

@@ -1,6 +1,7 @@
 // Copyright 2000-2017 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
 package org.jetbrains.java.decompiler.modules.renamer;
 
+import org.jetbrains.java.decompiler.code.CodeConstants;
 import org.jetbrains.java.decompiler.main.extern.IIdentifierRenamer;
 
 import java.util.Arrays;
@@ -29,13 +30,48 @@ public class ConverterHelper implements IIdentifierRenamer {
     String value = elementType == Type.ELEMENT_CLASS ? className : element;
     return value == null ||
            value.length() <= 2 ||
-           Character.isDigit(value.charAt(0)) ||
+           !isValidIdentifier(elementType == Type.ELEMENT_METHOD, value) ||
            KEYWORDS.contains(value) ||
            elementType == Type.ELEMENT_CLASS && (
              RESERVED_WINDOWS_NAMESPACE.contains(value.toLowerCase(Locale.US)) ||
              value.length() > 255 - ".class".length());
   }
 
+  /**
+   * Return {@code true} if, and only if identifier passed is compliant to JLS9 section 3.8 AND DOES NOT CONTAINS so-called "ignorable" characters.
+   * Ignorable characters are removed by javac silently during compilation and thus may appear only in specially crafted obfuscated classes.
+   * For more information about "ignorable" characters see <a href="https://bugs.openjdk.java.net/browse/JDK-7144981">JDK-7144981</a>.
+   *
+   * @param identifier Identifier to be checked
+   * @return {@code true} in case {@code identifier} passed can be used as an identifier; {@code false} otherwise.
+   */
+  private static boolean isValidIdentifier(boolean isMethod, String identifier) {
+
+    assert identifier != null : "Null identifier passed to the isValidIdentifier() method.";
+    assert !identifier.isEmpty() : "Empty identifier passed to the isValidIdentifier() method.";
+
+    if (isMethod && (identifier.equals(CodeConstants.INIT_NAME) || identifier.equals(CodeConstants.CLINIT_NAME))) {
+      return true;
+    }
+
+    if (!Character.isJavaIdentifierStart(identifier.charAt(0))) {
+      return false;
+    }
+
+    char[] chars = identifier.toCharArray();
+
+    for(int i = 1; i < chars.length; i++) {
+      char ch = chars[i];
+
+      if ((!Character.isJavaIdentifierPart(ch)) || Character.isIdentifierIgnorable(ch)) {
+        return false;
+      }
+    }
+
+    return true;
+
+  }
+
   // TODO: consider possible conflicts with not renamed classes, fields and methods!
   // We should get all relevant information here.
   @Override

test/org/jetbrains/java/decompiler/ConverterHelperTest.java

@@ -0,0 +1,106 @@
+// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
+package org.jetbrains.java.decompiler;
+
+import org.jetbrains.java.decompiler.code.CodeConstants;
+import org.jetbrains.java.decompiler.main.extern.IIdentifierRenamer.Type;
+import org.jetbrains.java.decompiler.modules.renamer.ConverterHelper;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+public class ConverterHelperTest {
+
+  private static final String VALID_CLASS_NAME = "ValidClassName";
+  private static final String VALID_FIELD_NAME = "validFieldName";
+  private static final String VALID_METHOD_NAME = "validMethodName";
+
+  private static final String VALID_FIELD_DESCRIPTOR = "I";
+  private static final String VALID_METHOD_DESCRIPTOR = "()V";
+
+  private ConverterHelper converterHelper;
+
+  @Before
+  public void setUp() {
+    this.converterHelper = new ConverterHelper();
+  }
+
+  @After
+  public void tearDown() {
+    this.converterHelper = null;
+  }
+
+  @Test public void testValidClassName() { doTestClassName(VALID_CLASS_NAME, false); }
+  @Test public void testValidFieldName() { doTestFieldName(VALID_FIELD_NAME, VALID_FIELD_DESCRIPTOR, false); }
+  @Test public void testValidMethodName() { doTestMethodName(VALID_METHOD_NAME, VALID_METHOD_DESCRIPTOR, false); }
+
+  @Test public void testNullClassName() { doTestClassName(null, true); }
+  @Test public void testNullFieldName() { doTestFieldName(null, VALID_FIELD_DESCRIPTOR, true); }
+  @Test public void testNullMethodName() { doTestMethodName(null, VALID_METHOD_DESCRIPTOR, true); }
+
+  @Test public void testEmptyClassName() { doTestClassName("", true); }
+  @Test public void testEmptyFieldName() { doTestFieldName("", VALID_FIELD_DESCRIPTOR, true); }
+  @Test public void testEmptyMethodName() { doTestMethodName("", VALID_METHOD_DESCRIPTOR, true); }
+
+  @Test public void testShortClassName() { doTestClassName("C", true); }
+  @Test public void testShortFieldName() { doTestFieldName("f", VALID_FIELD_DESCRIPTOR, true); }
+  @Test public void testShortMethodName() { doTestMethodName("m", VALID_METHOD_DESCRIPTOR, true); }
+
+  @Test public void testUnderscoreClassName() { doTestClassName("_", true); }
+  @Test public void testUnderscoreFieldName() { doTestFieldName("_", VALID_FIELD_DESCRIPTOR, true); }
+  @Test public void testUnderscoreMethodName() { doTestMethodName("_", VALID_METHOD_DESCRIPTOR, true); }
+
+  @Test public void testKeywordClassName() { doTestClassName("public", true); }
+  @Test public void testKeywordFieldName() { doTestFieldName("public", VALID_FIELD_DESCRIPTOR, true); }
+  @Test public void testKeywordMethodName() { doTestMethodName("public", VALID_METHOD_DESCRIPTOR, true); }
+
+  @Test public void testReservedWindowsNamespaceClassName() { doTestClassName("nul", true); }
+  @Test public void testReservedWindowsNamespaceFieldName() { doTestFieldName("nul", VALID_FIELD_DESCRIPTOR, false); }
+  @Test public void testReservedWindowsNamespaceName() { doTestMethodName("nul", VALID_METHOD_DESCRIPTOR, false); }
+
+  @Test public void testLeadingDigitClassName() { doTestClassName("4identifier", true); }
+  @Test public void testLeadingDigitFieldName() { doTestFieldName("4identifier", VALID_FIELD_DESCRIPTOR, true); }
+  @Test public void testLeadingDigitMethodName() { doTestMethodName("4identifier", VALID_METHOD_DESCRIPTOR, true); }
+
+  @Test public void testInvalidLeadingCharClassName() { doTestClassName("\uFEFFClassName", true); }
+  @Test public void testInvalidLeadingCharFieldName() { doTestFieldName("\uFEFFfieldName", VALID_FIELD_DESCRIPTOR, true); }
+  @Test public void testInvalidLeadingCharMethodName() { doTestMethodName("\uFEFFmethodName", VALID_METHOD_DESCRIPTOR, true); }
+
+  @Test public void testInvalidMiddleCharClassName() { doTestClassName("Class\uFEFFName", true); }
+  @Test public void testInvalidMiddleCharFieldName() { doTestFieldName("field\uFEFFName", VALID_FIELD_DESCRIPTOR, true); }
+  @Test public void testInvalidMiddleCharMethodName() { doTestMethodName("method\uFEFFName", VALID_METHOD_DESCRIPTOR, true); }
+
+  @Test public void testInvalidTrailingCharClassName() { doTestClassName("ClassName\uFEFF", true); }
+  @Test public void testInvalidTrailingCharFieldName() { doTestFieldName("fieldName\uFEFF", VALID_FIELD_DESCRIPTOR, true); }
+  @Test public void testInvalidTrailingCharMethodName() { doTestMethodName("methodName\uFEFF", VALID_METHOD_DESCRIPTOR, true); }
+
+  @Test public void testLtInitGtClassName() { doTestClassName(CodeConstants.INIT_NAME, true); }
+  @Test public void testLtInitGtFieldName() { doTestFieldName(CodeConstants.INIT_NAME, VALID_FIELD_DESCRIPTOR, true); }
+  @Test public void testLtInitGtMethodName() { doTestMethodName(CodeConstants.INIT_NAME, VALID_METHOD_DESCRIPTOR, false); }
+
+  @Test public void testLtClinitGtClassName() { doTestClassName(CodeConstants.CLINIT_NAME, true); }
+  @Test public void testLtClinitGtFieldName() { doTestFieldName(CodeConstants.CLINIT_NAME, VALID_FIELD_DESCRIPTOR, true); }
+  @Test public void testLtClinitGtMethodName() { doTestMethodName(CodeConstants.CLINIT_NAME, VALID_METHOD_DESCRIPTOR, false); }
+
+
+
+  private void doTestClassName(String className, boolean shallBeRenamed) {
+    doTest(Type.ELEMENT_CLASS, className, null, null, shallBeRenamed);
+  }
+
+  private void doTestFieldName(String element, String descriptor, boolean shallBeRenamed) {
+    doTest(Type.ELEMENT_FIELD, VALID_CLASS_NAME, element, descriptor, shallBeRenamed);
+  }
+
+  private void doTestMethodName(String element, String descriptor, boolean shallBeRenamed) {
+    doTest(Type.ELEMENT_METHOD, VALID_CLASS_NAME, element, descriptor, shallBeRenamed);
+  }
+
+  private void doTest(Type elementType, String className, String element, String descriptor, boolean shallBeRenamed) {
+    boolean result = converterHelper.toBeRenamed(elementType, className, element, descriptor);
+    String assertionMessage = shallBeRenamed ? "Identifier { %s, %s, %s, %s } shall be renamed" : "Identifier { %s, %s, %s, %s } shall not be renamed";
+
+    Assert.assertTrue(String.format(assertionMessage, elementType.toString(), className, element, descriptor), result == shallBeRenamed);
+  }
+
+}