Bug 522137: refactor JavaScriptCodeFactory
no functional changes
Change-Id: I4a0d064482134f796757145b9111a60a21b3ba4f
diff --git a/plugins/org.eclipse.ease.lang.javascript.nashorn/src/org/eclipse/ease/lang/javascript/nashorn/NashornScriptEngine.java b/plugins/org.eclipse.ease.lang.javascript.nashorn/src/org/eclipse/ease/lang/javascript/nashorn/NashornScriptEngine.java
index 8299e4e..e80a12c 100755
--- a/plugins/org.eclipse.ease.lang.javascript.nashorn/src/org/eclipse/ease/lang/javascript/nashorn/NashornScriptEngine.java
+++ b/plugins/org.eclipse.ease.lang.javascript.nashorn/src/org/eclipse/ease/lang/javascript/nashorn/NashornScriptEngine.java
@@ -24,7 +24,7 @@
import org.eclipse.ease.ScriptObjectType;
import org.eclipse.ease.debugging.model.EaseDebugVariable;
import org.eclipse.ease.debugging.model.EaseDebugVariable.Type;
-import org.eclipse.ease.lang.javascript.JavaScriptHelper;
+import org.eclipse.ease.lang.javascript.JavaScriptCodeFactory;
public class NashornScriptEngine extends AbstractReplScriptEngine {
@@ -63,7 +63,7 @@
@Override
protected void internalSetVariable(final String name, final Object content) {
- if (!JavaScriptHelper.isSaveName(name))
+ if (!JavaScriptCodeFactory.isSaveName(name))
throw new RuntimeException("\"" + name + "\" is not a valid JavaScript variable name");
fEngine.put(name, content);
diff --git a/plugins/org.eclipse.ease.lang.javascript.rhino/src/org/eclipse/ease/lang/javascript/rhino/RhinoScriptEngine.java b/plugins/org.eclipse.ease.lang.javascript.rhino/src/org/eclipse/ease/lang/javascript/rhino/RhinoScriptEngine.java
index 5b56acb..0fc5a63 100644
--- a/plugins/org.eclipse.ease.lang.javascript.rhino/src/org/eclipse/ease/lang/javascript/rhino/RhinoScriptEngine.java
+++ b/plugins/org.eclipse.ease.lang.javascript.rhino/src/org/eclipse/ease/lang/javascript/rhino/RhinoScriptEngine.java
@@ -42,7 +42,7 @@
import org.eclipse.ease.debugging.ScriptStackTrace;
import org.eclipse.ease.debugging.model.EaseDebugVariable;
import org.eclipse.ease.debugging.model.EaseDebugVariable.Type;
-import org.eclipse.ease.lang.javascript.JavaScriptHelper;
+import org.eclipse.ease.lang.javascript.JavaScriptCodeFactory;
import org.eclipse.ease.tools.RunnableWithResult;
import org.eclipse.swt.widgets.Display;
import org.mozilla.javascript.Context;
@@ -399,7 +399,7 @@
runInJobContext(new RunnableWithResult<Boolean>() {
@Override
public Boolean runWithTry() throws Throwable {
- if (!JavaScriptHelper.isSaveName(name))
+ if (!JavaScriptCodeFactory.isSaveName(name))
throw new RuntimeException("\"" + name + "\" is not a valid JavaScript variable name");
final Scriptable scope = fScope;
diff --git a/plugins/org.eclipse.ease.lang.javascript.rhino/src/org/eclipse/ease/lang/javascript/rhino/debugger/RhinoDebuggerEngine.java b/plugins/org.eclipse.ease.lang.javascript.rhino/src/org/eclipse/ease/lang/javascript/rhino/debugger/RhinoDebuggerEngine.java
index f1f2cb8..cc92498 100644
--- a/plugins/org.eclipse.ease.lang.javascript.rhino/src/org/eclipse/ease/lang/javascript/rhino/debugger/RhinoDebuggerEngine.java
+++ b/plugins/org.eclipse.ease.lang.javascript.rhino/src/org/eclipse/ease/lang/javascript/rhino/debugger/RhinoDebuggerEngine.java
@@ -28,7 +28,7 @@
import org.eclipse.ease.debugging.events.debugger.ThreadTerminatedEvent;
import org.eclipse.ease.debugging.model.EaseDebugTarget;
import org.eclipse.ease.debugging.model.EaseDebugVariable;
-import org.eclipse.ease.lang.javascript.JavaScriptHelper;
+import org.eclipse.ease.lang.javascript.JavaScriptCodeFactory;
import org.eclipse.ease.lang.javascript.rhino.RhinoScriptEngine;
import org.eclipse.ease.lang.javascript.rhino.debugger.RhinoDebugger.RhinoDebugFrame;
import org.eclipse.ease.lang.javascript.rhino.debugger.model.RhinoDebugTarget;
@@ -138,7 +138,7 @@
}
public void setVariable(String name, Object content, Scriptable scope) {
- if (!JavaScriptHelper.isSaveName(name))
+ if (!JavaScriptCodeFactory.isSaveName(name))
throw new RuntimeException("\"" + name + "\" is not a valid JavaScript variable name");
final Object jsOut = internaljavaToJS(content, scope);
diff --git a/plugins/org.eclipse.ease.lang.javascript/src/org/eclipse/ease/lang/javascript/JavaScriptCodeFactory.java b/plugins/org.eclipse.ease.lang.javascript/src/org/eclipse/ease/lang/javascript/JavaScriptCodeFactory.java
index 5b0f0c5..793c688 100644
--- a/plugins/org.eclipse.ease.lang.javascript/src/org/eclipse/ease/lang/javascript/JavaScriptCodeFactory.java
+++ b/plugins/org.eclipse.ease.lang.javascript/src/org/eclipse/ease/lang/javascript/JavaScriptCodeFactory.java
@@ -15,8 +15,11 @@
import java.lang.reflect.Field;
import java.lang.reflect.Method;
-import java.util.ArrayList;
+import java.util.Arrays;
import java.util.List;
+import java.util.Random;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
import org.eclipse.ease.AbstractCodeFactory;
import org.eclipse.ease.IScriptEngine;
@@ -28,76 +31,14 @@
public class JavaScriptCodeFactory extends AbstractCodeFactory {
- public static List<String> RESERVED_KEYWORDS = new ArrayList<>();
+ private static final List<String> RESERVED_KEYWORDS = Arrays.asList("abstract", "arguments", "boolean", "break", "byte", "case", "catch", "char", "class",
+ "const", "continue", "debugger", "default", "delete", "do", "double", "else", "enum", "eval", "export", "extends", "false", "final", "finally",
+ "float", "for", "function", "goto", "if", "implements", "import", "in", "instanceof", "int", "interface", "let", "long", "native", "new", "null",
+ "package", "private", "protected", "public", "return", "short", "static", "super", "switch", "synchronized", "this", "throw", "throws", "transient",
+ "true", "try", "typeof", "var", "void", "volatile", "while", "with", "yield");
- static {
- RESERVED_KEYWORDS.add("abstract");
- RESERVED_KEYWORDS.add("arguments");
- RESERVED_KEYWORDS.add("boolean");
- RESERVED_KEYWORDS.add("break");
- RESERVED_KEYWORDS.add("byte");
- RESERVED_KEYWORDS.add("case");
- RESERVED_KEYWORDS.add("catch");
- RESERVED_KEYWORDS.add("char");
- RESERVED_KEYWORDS.add("class");
- RESERVED_KEYWORDS.add("const");
- RESERVED_KEYWORDS.add("continue");
- RESERVED_KEYWORDS.add("debugger");
- RESERVED_KEYWORDS.add("default");
- RESERVED_KEYWORDS.add("delete");
- RESERVED_KEYWORDS.add("do");
- RESERVED_KEYWORDS.add("double");
- RESERVED_KEYWORDS.add("else");
- RESERVED_KEYWORDS.add("enum");
- RESERVED_KEYWORDS.add("eval");
- RESERVED_KEYWORDS.add("export");
- RESERVED_KEYWORDS.add("extends");
- RESERVED_KEYWORDS.add("false");
- RESERVED_KEYWORDS.add("final");
- RESERVED_KEYWORDS.add("finally");
- RESERVED_KEYWORDS.add("float");
- RESERVED_KEYWORDS.add("for");
- RESERVED_KEYWORDS.add("function");
- RESERVED_KEYWORDS.add("goto");
- RESERVED_KEYWORDS.add("if");
- RESERVED_KEYWORDS.add("implements");
- RESERVED_KEYWORDS.add("import");
- RESERVED_KEYWORDS.add("in");
- RESERVED_KEYWORDS.add("instanceof");
- RESERVED_KEYWORDS.add("int");
- RESERVED_KEYWORDS.add("interface");
- RESERVED_KEYWORDS.add("let");
- RESERVED_KEYWORDS.add("long");
- RESERVED_KEYWORDS.add("native");
- RESERVED_KEYWORDS.add("new");
- RESERVED_KEYWORDS.add("null");
- RESERVED_KEYWORDS.add("package");
- RESERVED_KEYWORDS.add("private");
- RESERVED_KEYWORDS.add("protected");
- RESERVED_KEYWORDS.add("public");
- RESERVED_KEYWORDS.add("return");
- RESERVED_KEYWORDS.add("short");
- RESERVED_KEYWORDS.add("static");
- RESERVED_KEYWORDS.add("super");
- RESERVED_KEYWORDS.add("switch");
- RESERVED_KEYWORDS.add("synchronized");
- RESERVED_KEYWORDS.add("this");
- RESERVED_KEYWORDS.add("throw");
- RESERVED_KEYWORDS.add("throws");
- RESERVED_KEYWORDS.add("transient");
- RESERVED_KEYWORDS.add("true");
- RESERVED_KEYWORDS.add("try");
- RESERVED_KEYWORDS.add("typeof");
- RESERVED_KEYWORDS.add("var");
- RESERVED_KEYWORDS.add("void");
- RESERVED_KEYWORDS.add("volatile");
- RESERVED_KEYWORDS.add("while");
- RESERVED_KEYWORDS.add("with");
- RESERVED_KEYWORDS.add("yield");
- }
-
- private static boolean isValidMethodName(final String methodName) {
- return JavaScriptHelper.isSaveName(methodName) && !RESERVED_KEYWORDS.contains(methodName);
+ public static boolean isSaveName(final String identifier) {
+ return Pattern.matches("[a-zA-Z_$][a-zA-Z0-9_$]*", identifier) && !RESERVED_KEYWORDS.contains(identifier);
}
@Override
@@ -105,13 +46,8 @@
final StringBuilder code = new StringBuilder();
code.append("new Packages.").append(clazz.getName()).append('(');
- if (parameters != null) {
- for (final String parameter : parameters)
- code.append(parameter).append(", ");
-
- if (parameters.length > 0)
- code.delete(code.length() - 2, code.length());
- }
+ if (parameters != null)
+ code.append(Arrays.asList(parameters).stream().collect(Collectors.joining(", ")));
code.append(')');
@@ -125,7 +61,29 @@
@Override
public String getSaveVariableName(final String variableName) {
- return JavaScriptHelper.getSaveName(variableName);
+ // check if name is already valid
+ if (isSaveName(variableName))
+ return variableName;
+
+ // not valid, convert string to valid format
+ final StringBuilder buffer = new StringBuilder(variableName.replaceAll("[^a-zA-Z0-9_$]", "_"));
+
+ // check for valid first character
+ if (buffer.length() > 0) {
+ final char start = buffer.charAt(0);
+ if (((start < 65) || ((start > 90) && (start < 97)) || (start > 122)) && (start != '_'))
+ buffer.insert(0, '_');
+ } else {
+ // buffer is empty, create a random string of lowercase letters
+ buffer.append('_');
+ for (int index = 0; index < new Random().nextInt(20); index++)
+ buffer.append('a' + new Random().nextInt(26));
+ }
+
+ if (RESERVED_KEYWORDS.contains(buffer.toString()))
+ return "_" + buffer.toString();
+
+ return buffer.toString();
}
private StringBuilder verifyParameters(Method method, final List<Parameter> parameters, String indent) {
@@ -210,7 +168,7 @@
// append method aliases
for (final String alias : getMethodAliases(method)) {
- if (!isValidMethodName(alias)) {
+ if (!isSaveName(alias)) {
Logger.error(PluginConstants.PLUGIN_ID,
"The method name \"" + alias + "\" from the module \"" + identifier + "\" can not be wrapped because it's name is reserved");
@@ -246,7 +204,7 @@
// build function declarations
for (final String name : getMethodNames(method)) {
- if (!isValidMethodName(name)) {
+ if (!isSaveName(name)) {
Logger.error(PluginConstants.PLUGIN_ID,
"The method name \"" + name + "\" from the module \"" + moduleVariable + "\" can not be wrapped because it's name is reserved");
diff --git a/plugins/org.eclipse.ease.lang.javascript/src/org/eclipse/ease/lang/javascript/JavaScriptHelper.java b/plugins/org.eclipse.ease.lang.javascript/src/org/eclipse/ease/lang/javascript/JavaScriptHelper.java
index 3e9d97b..d65dd0f 100644
--- a/plugins/org.eclipse.ease.lang.javascript/src/org/eclipse/ease/lang/javascript/JavaScriptHelper.java
+++ b/plugins/org.eclipse.ease.lang.javascript/src/org/eclipse/ease/lang/javascript/JavaScriptHelper.java
@@ -1,8 +1,5 @@
package org.eclipse.ease.lang.javascript;
-import java.util.Random;
-import java.util.regex.Pattern;
-
import org.eclipse.ease.service.IScriptService;
import org.eclipse.ease.service.ScriptService;
import org.eclipse.ease.service.ScriptType;
@@ -12,31 +9,9 @@
/** Script type identifier for JavaScript. Must match with the script type 'name' from plugin.xml. */
public static final String SCRIPT_TYPE_JAVASCRIPT = "JavaScript";
- public static String getSaveName(final String identifier) {
- // check if name is already valid
- if (isSaveName(identifier))
- return identifier;
-
- // not valid, convert string to valid format
- final StringBuilder buffer = new StringBuilder(identifier.replaceAll("[^a-zA-Z0-9_$]", "_"));
-
- // check for valid first character
- if (buffer.length() > 0) {
- final char start = buffer.charAt(0);
- if (((start < 65) || ((start > 90) && (start < 97)) || (start > 122)) && (start != '_'))
- buffer.insert(0, '_');
- } else {
- // buffer is empty, create a random string of lowercase letters
- buffer.append('_');
- for (int index = 0; index < new Random().nextInt(20); index++)
- buffer.append('a' + new Random().nextInt(26));
- }
-
- return buffer.toString();
- }
-
- public static boolean isSaveName(final String identifier) {
- return Pattern.matches("[a-zA-Z_$][a-zA-Z0-9_$]*", identifier);
+ @Deprecated
+ private JavaScriptHelper() {
+ throw new IllegalArgumentException("Not meant to be called");
}
/**
diff --git a/tests/org.eclipse.ease.lang.javascript.test/src/org/eclipse/ease/lang/javascript/JavaScriptCodeFactoryTest.java b/tests/org.eclipse.ease.lang.javascript.test/src/org/eclipse/ease/lang/javascript/JavaScriptCodeFactoryTest.java
index af678b6..57a15b2 100644
--- a/tests/org.eclipse.ease.lang.javascript.test/src/org/eclipse/ease/lang/javascript/JavaScriptCodeFactoryTest.java
+++ b/tests/org.eclipse.ease.lang.javascript.test/src/org/eclipse/ease/lang/javascript/JavaScriptCodeFactoryTest.java
@@ -14,8 +14,10 @@
package org.eclipse.ease.lang.javascript;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
public class JavaScriptCodeFactoryTest {
@@ -27,9 +29,71 @@
}
@Test
- public void testCommentCreator() {
+ @DisplayName("createCommentedString() creates single line comment")
+ public void createCommentedString_creates_single_line_comment() {
assertEquals("// Comment", fFactory.createCommentedString("Comment", false));
+ }
+
+ @Test
+ @DisplayName("createCommentedString() creates multi line comment")
+ public void createCommentedString_creates_multi_line_comment() {
assertEquals(String.format("// Multi%n// Line%n// Comment"), fFactory.createCommentedString("Multi\nLine\nComment", false));
assertEquals(String.format("/**%n * Multi%n * Line%n * Comment%n */%n"), fFactory.createCommentedString("Multi\nLine\nComment", true));
}
+
+ @Test
+ @DisplayName("createCommentedString() creates block comment")
+ public void createCommentedString_creates_block_comment() {
+ assertEquals(String.format("/**%n * Multi%n * Line%n * Comment%n */%n"), fFactory.createCommentedString("Multi\nLine\nComment", true));
+ }
+
+ @Test
+ @DisplayName("getNullString() == 'null'")
+ public void getNullString_equals_null() {
+ assertEquals("null", fFactory.getNullString());
+ }
+
+ @Test
+ @DisplayName("classInstantiation() creates default instance")
+ public void classInstantiation_creates_default_instance() {
+ assertEquals(String.format("new Packages.%s()", getClass().getName()), fFactory.classInstantiation(getClass(), null));
+ }
+
+ @Test
+ @DisplayName("classInstantiation() creates instance with parameters")
+ public void classInstantiation_creates_instance_with_parameters() {
+ final String[] parameters = new String[] { "one", "2" };
+ assertEquals(String.format("new Packages.%s(%s, %s)", getClass().getName(), parameters[0], parameters[1]),
+ fFactory.classInstantiation(getClass(), parameters));
+ }
+
+ @Test
+ @DisplayName("getSaveVariableName() does not alter save name")
+ public void getSaveVariableName_does_not_alter_save_name() {
+ assertEquals("this_is_2021", fFactory.getSaveVariableName("this_is_2021"));
+ }
+
+ @Test
+ @DisplayName("getSaveVariableName() replaces invalid characters")
+ public void getSaveVariableName_replaces_invalid_characters() {
+ assertEquals("this_is_2021__", fFactory.getSaveVariableName("this is-2021 :"));
+ }
+
+ @Test
+ @DisplayName("getSaveVariableName() prefixes numbers with _")
+ public void getSaveVariableName_prefixes_numbers_with_underscore() {
+ assertEquals("_123", fFactory.getSaveVariableName("123"));
+ }
+
+ @Test
+ @DisplayName("getSaveVariableName() creates random string for invalid characters")
+ public void getSaveVariableName_creates_random_string_for_invalid_characters() {
+ assertFalse(fFactory.getSaveVariableName("!§%&/()=?").isEmpty());
+ }
+
+ @Test
+ @DisplayName("getSaveVariableName() prefixes keywords with _")
+ public void getSaveVariableName_prefixes_keywords_with_underscore() {
+ assertEquals("_for", fFactory.getSaveVariableName("for"));
+ }
}
