don't pass null to constructor

support edge case where param source for ERXStringUtilities.safeIdentifierName is null which would throw a NPE
wocommunity · Mar 12, 2015 · 5bb806a · 5bb806a
1 parent 431ab3a
commit 5bb806a
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 7 deletions.
diff --git a/Frameworks/Core/ERExtensions/Sources/er/extensions/foundation/ERXStringUtilities.java b/Frameworks/Core/ERExtensions/Sources/er/extensions/foundation/ERXStringUtilities.java
@@ -2053,15 +2053,13 @@ public static boolean regionMatches(StringBuffer str, int toffset, String other,
      */
     public static String safeIdentifierName(String source, String prefix, char replacement)
     {
-    	StringBuilder b;
+    	StringBuilder b = new StringBuilder();
     	// Add prefix if source does not start with valid character
-        if (source == null || source.length() == 0 || Character.isJavaIdentifierStart(source.charAt(0))) {
-            b = new StringBuilder(source);
-        } else {
-        	b = new StringBuilder(prefix);
-        	b.append(source);
+        if (source == null || source.length() == 0 || !Character.isJavaIdentifierStart(source.charAt(0))) {
+            b.append(prefix);
         }
-
+        b.append(source);
+
         for (int i = 0; i < b.length(); i++) {
             char c = b.charAt(i);
             if ( ! Character.isJavaIdentifierPart(c)) {

diff --git a/Tests/ERXTest/Sources/er/extensions/foundation/ERXStringUtilitiesTest.java b/Tests/ERXTest/Sources/er/extensions/foundation/ERXStringUtilitiesTest.java
@@ -1,6 +1,7 @@
 package er.extensions.foundation;
 
 import static org.junit.Assert.assertEquals;
+import junit.framework.Assert;
 import junit.framework.JUnit4TestAdapter;
 
 import org.junit.After;
@@ -254,4 +255,38 @@ public void testLevenshteinDistance() {
 					ERXStringUtilities.levenshteinDistance(l.s1, l.s2));
 		}
 	}
+
+	@Test
+	public void testSafeIdentifierName() {
+		String safeJavaIdentifierStart = "IamSafe";
+		String safeJavaIdentifierStartWithUnsafeChars = "Iam Nearly+Safe";
+		String unsafeJavaIdentifierStart = "0safe";
+		String nullIdentifierStart = null;
+		String emptyIdentifierStart = "";
+		String prefix = "prefix";
+		char replacement = '_';
+
+		String resultWithSafeStart = ERXStringUtilities.safeIdentifierName(safeJavaIdentifierStart, prefix, replacement);
+		String resultWithSafeStartUnsafeContent = ERXStringUtilities.safeIdentifierName(safeJavaIdentifierStartWithUnsafeChars, prefix, replacement);
+		String resultWithUnsafeStart = ERXStringUtilities.safeIdentifierName(unsafeJavaIdentifierStart, prefix, replacement);
+		String resultWithNullStart = ERXStringUtilities.safeIdentifierName(nullIdentifierStart, prefix, replacement);
+		String resultWithEmptyStart = ERXStringUtilities.safeIdentifierName(emptyIdentifierStart, prefix, replacement);
+
+		Assert.assertEquals(safeJavaIdentifierStart, resultWithSafeStart);
+
+		Assert.assertNotSame(safeJavaIdentifierStartWithUnsafeChars, resultWithSafeStartUnsafeContent);
+		Assert.assertEquals("Expected 2 replacements for unsafe characters", 2, resultWithSafeStartUnsafeContent.replaceAll("[^_]", "").length());
+		Assert.assertFalse("Did not expect prefix as identifier starts with safe character.", resultWithSafeStartUnsafeContent.contains(prefix));
+
+		Assert.assertNotSame(unsafeJavaIdentifierStart, resultWithUnsafeStart);
+		Assert.assertFalse("Expected no replacement for unsafe characters", resultWithUnsafeStart.contains("_"));
+		Assert.assertTrue("Did expect prefix as identifier starts with unsafe character.", resultWithUnsafeStart.contains(prefix));
+
+		Assert.assertNotSame(nullIdentifierStart, resultWithNullStart);
+		Assert.assertTrue("Did expect 'null' as identifier was null.", resultWithNullStart.contains("null"));
+		Assert.assertTrue("Did expect prefix as identifier was null.", resultWithNullStart.contains(prefix));
+
+		Assert.assertNotSame(emptyIdentifierStart, resultWithEmptyStart);
+		Assert.assertEquals(prefix, resultWithEmptyStart);
+	}
 }