[192777] Entity Generation produces fields with Java reserved word names

3 files changed, 64 insertions, 31 deletions

diff --git a/jpa/plugins/org.eclipse.jpt.gen/src/org/eclipse/jpt/gen/internal/GenTable.java b/jpa/plugins/org.eclipse.jpt.gen/src/org/eclipse/jpt/gen/internal/GenTable.java
index 2405ead083..395a51b94e 100644
--- a/jpa/plugins/org.eclipse.jpt.gen/src/org/eclipse/jpt/gen/internal/GenTable.java
+++ b/jpa/plugins/org.eclipse.jpt.gen/src/org/eclipse/jpt/gen/internal/GenTable.java
@@ -62,7 +62,8 @@ class GenTable {
 		String overrideEntityName = this.entityConfig.getOverrideEntityName(this.table);
 		if (overrideEntityName == null) {
 			if (this.entityConfig.convertToCamelCase()) {
-				name = StringTools.convertUnderscoresToCamelCase(name);
+				// camel-casing can convert a name back to a reserved word (e.g. "package_" -> "package")
+				name = NameTools.convertToJavaIdentifier(StringTools.convertUnderscoresToCamelCase(name));
 			}
 		} else {
 			name = overrideEntityName;
@@ -291,15 +292,16 @@ class GenTable {
 	}
 
 	private String configureFieldName(Object o, String fieldName) {
-		fieldName = this.camelCase(fieldName);
+		fieldName = this.camelCaseFieldName(fieldName);
 		fieldName = NameTools.uniqueNameFor(fieldName, this.fieldNames.values());
 		this.fieldNames.put(o, fieldName);
 		return fieldName;
 	}
 
-	private String camelCase(String name) {
+	private String camelCaseFieldName(String name) {
 		return this.entityConfig.convertToCamelCase() ?
-			StringTools.convertUnderscoresToCamelCase(name, false)  // false = don't capitalize first letter
+			// camel-casing can convert a name back to a reserved word (e.g. "package_" -> "package")
+			NameTools.convertToJavaIdentifier(StringTools.convertUnderscoresToCamelCase(name, false))  // false = don't capitalize first letter
 		:
 			name;
 	}
diff --git a/jpa/plugins/org.eclipse.jpt.utility/src/org/eclipse/jpt/utility/internal/NameTools.java b/jpa/plugins/org.eclipse.jpt.utility/src/org/eclipse/jpt/utility/internal/NameTools.java
index 5610f00845..fa8b5609c8 100644
--- a/jpa/plugins/org.eclipse.jpt.utility/src/org/eclipse/jpt/utility/internal/NameTools.java
+++ b/jpa/plugins/org.eclipse.jpt.utility/src/org/eclipse/jpt/utility/internal/NameTools.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2005, 2007 Oracle. All rights reserved.
+ * Copyright (c) 2005, 2008 Oracle. All rights reserved.
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v1.0, which accompanies this distribution
  * and is available at http://www.eclipse.org/legal/epl-v10.html.
@@ -15,7 +15,6 @@ import java.util.Iterator;
 import java.util.Set;
 import org.eclipse.jpt.utility.internal.iterators.ArrayIterator;
 
-
 /**
  * Various helper methods for generating names.
  */
@@ -162,7 +161,7 @@ public final class NameTools {
 				"do",
 				"double",
 				"else",
-				"enum",  // jdk 5.0
+				"enum",  // jdk 1.5
 				"extends",
 				"false",
 				"final",
@@ -221,8 +220,8 @@ public final class NameTools {
 	/**
 	 * Convert the specified string to a valid Java identifier
 	 * by substituting an underscore '_' for any invalid characters
-	 * in the string and capitalizing the string if it is a Java
-	 * reserved word.
+	 * in the string and appending an underscore '_' to the string if
+	 * it is a Java reserved word.
 	 */
 	public static String convertToJavaIdentifier(String string) {
 		return convertToJavaIdentifier(string, '_');
@@ -231,25 +230,27 @@ public final class NameTools {
 	/**
 	 * Convert the specified string to a valid Java identifier
 	 * by substituting the specified character for any invalid characters
-	 * in the string and capitalizing the string if it is a Java
-	 * reserved word.
+	 * in the string and, if necessary, appending the specified character
+	 * to the string until it is not a Java reserved word.
 	 */
 	public static String convertToJavaIdentifier(String string, char c) {
 		if (string.length() == 0) {
 			return string;
 		}
 		if (JAVA_RESERVED_WORDS_SET.contains(string)) {
-			// a reserved words is a valid identifier, we just need to tweak it a bit
-			return StringTools.capitalize(string);
+			// a reserved word is a valid identifier, we just need to tweak it a bit
+			checkCharIsJavaIdentifierPart(c);
+			return convertToJavaIdentifier(string + c, c);
 		}
-		return new String(convertToJavaIdentifierInternal(string.toCharArray(), c));
+		char[] array = string.toCharArray();
+		return (convertToJavaIdentifier_(array, c)) ? new String(array) : string;
 	}
 
 	/**
 	 * Convert the specified string to a valid Java identifier
 	 * by substituting an underscore '_' for any invalid characters
-	 * in the string and capitalizing the string if it is a Java
-	 * reserved word.
+	 * in the string and appending an underscore '_' to the string if
+	 * it is a Java reserved word.
 	 */
 	public static char[] convertToJavaIdentifier(char[] string) {
 		return convertToJavaIdentifier(string, '_');
@@ -258,8 +259,8 @@ public final class NameTools {
 	/**
 	 * Convert the specified string to a valid Java identifier
 	 * by substituting the specified character for any invalid characters
-	 * in the string and capitalizing the string if it is a Java
-	 * reserved word.
+	 * in the string and, if necessary, appending the specified character
+	 * to the string until it is not a Java reserved word.
 	 */
 	public static char[] convertToJavaIdentifier(char[] string, char c) {
 		int length = string.length;
@@ -267,28 +268,45 @@ public final class NameTools {
 			return string;
 		}
 		if (JAVA_RESERVED_WORDS_SET.contains(new String(string))) {
-			// a reserved words is a valid identifier, we just need to tweak it a bit
-			return StringTools.capitalize(string);
+			// a reserved word is a valid identifier, we just need to tweak it a bit
+			checkCharIsJavaIdentifierPart(c);
+			return convertToJavaIdentifier(CollectionTools.add(string, c), c);
 		}
-		return convertToJavaIdentifierInternal(string, c);
+		convertToJavaIdentifier_(string, c);
+		return string;
 	}
 
-	private static char[] convertToJavaIdentifierInternal(char[] string, char c) {
+	/**
+	 * The specified must not be empty.
+	 * Return whether the string was modified.
+	 */
+	private static boolean convertToJavaIdentifier_(char[] string, char c) {
+		boolean mod = false;
 		if ( ! Character.isJavaIdentifierStart(string[0])) {
-			if ( ! Character.isJavaIdentifierStart(c)) {
-				throw new IllegalArgumentException("invalid Java identifier start char: '" + c + "'");
-			}
+			checkCharIsJavaIdentifierStart(c);
 			string[0] = c;
+			mod = true;
 		}
-		if ( ! Character.isJavaIdentifierPart(c)) {
-			throw new IllegalArgumentException("invalid Java identifier part char: '" + c + "'");
-		}
+		checkCharIsJavaIdentifierPart(c);
 		for (int i = string.length; i-- > 1; ) {  // NB: end with 1
 			if ( ! Character.isJavaIdentifierPart(string[i])) {
 				string[i] = c;
+				mod = true;
 			}
 		}
-		return string;
+		return mod;
+	}
+
+	private static void checkCharIsJavaIdentifierStart(char c) {
+		if ( ! Character.isJavaIdentifierStart(c)) {
+			throw new IllegalArgumentException("invalid Java identifier start char: '" + c + "'");
+		}
+	}
+
+	private static void checkCharIsJavaIdentifierPart(char c) {
+		if ( ! Character.isJavaIdentifierPart(c)) {
+			throw new IllegalArgumentException("invalid Java identifier part char: '" + c + "'");
+		}
 	}
 
 
diff --git a/jpa/tests/org.eclipse.jpt.utility.tests/src/org/eclipse/jpt/utility/tests/internal/NameToolsTests.java b/jpa/tests/org.eclipse.jpt.utility.tests/src/org/eclipse/jpt/utility/tests/internal/NameToolsTests.java
index 164fa49e03..52b30b49c3 100644
--- a/jpa/tests/org.eclipse.jpt.utility.tests/src/org/eclipse/jpt/utility/tests/internal/NameToolsTests.java
+++ b/jpa/tests/org.eclipse.jpt.utility.tests/src/org/eclipse/jpt/utility/tests/internal/NameToolsTests.java
@@ -173,7 +173,8 @@ public class NameToolsTests extends TestCase {
 	public void testconvertToJavaIdentifierString() {
 		assertEquals("foo", NameTools.convertToJavaIdentifier("foo"));
 		assertEquals("foo1", NameTools.convertToJavaIdentifier("foo1"));
-		assertEquals("Private", NameTools.convertToJavaIdentifier("private"));
+		assertEquals("private_", NameTools.convertToJavaIdentifier("private"));
+		assertEquals("throw_", NameTools.convertToJavaIdentifier("throw"));
 		assertEquals("_foo", NameTools.convertToJavaIdentifier("1foo"));
 		assertEquals("foo_", NameTools.convertToJavaIdentifier("foo%"));
 		assertEquals("foo__bar__", NameTools.convertToJavaIdentifier("foo  bar  "));
@@ -182,7 +183,8 @@ public class NameToolsTests extends TestCase {
 	public void testconvertToJavaIdentifierStringChar() {
 		assertEquals("foo", NameTools.convertToJavaIdentifier("foo", '$'));
 		assertEquals("foo1", NameTools.convertToJavaIdentifier("foo1", '$'));
-		assertEquals("Private", NameTools.convertToJavaIdentifier("private", '$'));
+		assertEquals("private$", NameTools.convertToJavaIdentifier("private", '$'));
+		assertEquals("throwss", NameTools.convertToJavaIdentifier("throw", 's'));
 		assertEquals("$foo", NameTools.convertToJavaIdentifier("1foo", '$'));
 		assertEquals("foo$", NameTools.convertToJavaIdentifier("foo%", '$'));
 		assertEquals("foo$$bar$$", NameTools.convertToJavaIdentifier("foo  bar  ", '$'));
@@ -209,6 +211,17 @@ public class NameToolsTests extends TestCase {
 		}
 		assertTrue(exCaught);
 
+		exCaught = false;
+		try {
+			String s = NameTools.convertToJavaIdentifier("private", '^');
+			fail("invalid string: \"" + s + "\"");
+		} catch (IllegalArgumentException ex) {
+			if (ex.getMessage().indexOf('^') != -1) {
+				exCaught = true;
+			}
+		}
+		assertTrue(exCaught);
+
 	}
 
 }