Fail on keywords used as parameters for Groovy translation

testware/integration-tests/src/test/java/org/opencypher/gremlin/client/BytecodeCypherGremlinClientTest.java

@@ -27,6 +27,7 @@
 import org.opencypher.gremlin.rules.GremlinServerExternalResource;
 import org.opencypher.gremlin.translation.translator.Translator;
 
+@SuppressWarnings("Duplicates")
 public class BytecodeCypherGremlinClientTest {
 
     @ClassRule
@@ -83,4 +84,26 @@ public void invalidSyntax() {
         assertThat(throwable)
             .hasMessageContaining("Invalid input");
     }
+
+    @Test
+    public void invalidParameter() {
+        String cypher = "RETURN $`🐼`";
+        Map<String, ?> parameters = singletonMap("🐼", 0);
+        CypherResultSet resultSet = client.submit(cypher, parameters);
+        Throwable throwable = catchThrowable(resultSet::all);
+
+        assertThat(throwable)
+            .hasMessageContaining("Invalid parameter name: 🐼");
+    }
+
+    @Test
+    public void groovyKeywordParameter() {
+        String cypher = "RETURN $goto";
+        Map<String, ?> parameters = singletonMap("goto", 0);
+        CypherResultSet resultSet = client.submit(cypher, parameters);
+        Throwable throwable = catchThrowable(resultSet::all);
+
+        assertThat(throwable)
+            .hasMessageContaining("Invalid parameter name: goto");
+    }
 }

testware/integration-tests/src/test/java/org/opencypher/gremlin/client/GroovyCypherGremlinClientTest.java

@@ -27,6 +27,7 @@
 import org.opencypher.gremlin.rules.GremlinServerExternalResource;
 import org.opencypher.gremlin.translation.translator.Translator;
 
+@SuppressWarnings("Duplicates")
 public class GroovyCypherGremlinClientTest {
 
     @ClassRule
@@ -94,4 +95,15 @@ public void invalidParameter() {
         assertThat(throwable)
             .hasMessageContaining("Invalid parameter name: 🐼");
     }
+
+    @Test
+    public void groovyKeywordParameter() {
+        String cypher = "RETURN $goto";
+        Map<String, ?> parameters = singletonMap("goto", 0);
+        CypherResultSet resultSet = client.submit(cypher, parameters);
+        Throwable throwable = catchThrowable(resultSet::all);
+
+        assertThat(throwable)
+            .hasMessageContaining("Invalid parameter name: goto");
+    }
 }

tinkerpop/cypher-gremlin-server-client/src/main/java/org/opencypher/gremlin/client/BytecodeCypherGremlinClient.java

@@ -62,10 +62,15 @@ public CompletableFuture<CypherResultSet> submitAsync(String cypher, Map<String,
         }
 
         Translator<Bytecode, P> translator = translatorSupplier.get();
-        Bytecode bytecode = ast.buildTranslation(translator);
+        Bytecode bytecode;
+        try {
+            bytecode = ast.buildTranslation(translator);
+        } catch (Exception e) {
+            return completedFuture(exceptional(e));
+        }
+
         CompletableFuture<ResultSet> resultSetFuture = client.submitAsync(bytecode);
         ReturnNormalizer returnNormalizer = ReturnNormalizer.create(ast.getReturnTypes());
-
         return resultSetFuture
             .thenApply(ResultSet::iterator)
             .thenApply(resultIterator -> new CypherResultSet(

translation/src/main/java/org/opencypher/gremlin/translation/GroovyIdentifiers.java

@@ -0,0 +1,56 @@
+/*
+ * Copyright (c) 2018 "Neo4j, Inc." [https://neo4j.com]
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.opencypher.gremlin.translation;
+
+import static java.lang.Character.isJavaIdentifierPart;
+import static java.util.Arrays.asList;
+
+import java.util.HashSet;
+import java.util.Set;
+
+public final class GroovyIdentifiers {
+    private GroovyIdentifiers() {
+    }
+
+    private static final Set<String> GROOVY_KEYWORDS = new HashSet<>(asList(
+        "as", "assert", "break", "case", "catch", "class", "const", "continue",
+        "def", "default", "do", "else", "enum", "extends", "false", "finally",
+        "for", "goto", "if", "implements", "import", "in", "instanceof", "interface",
+        "new", "null", "package", "return", "super", "switch", "this", "throw",
+        "throws", "trait", "true", "try", "while"
+    ));
+
+    public static boolean isValidIdentifier(String value) {
+        char[] chars = value.toCharArray();
+        int length = chars.length;
+        if (length == 0) {
+            return false;
+        }
+        if (!isJavaIdentifierStart(chars[0])) {
+            return false;
+        }
+        for (int i = 1; i < length; i++) {
+            if (!isJavaIdentifierPart(chars[i])) {
+                return false;
+            }
+        }
+        return !GROOVY_KEYWORDS.contains(value);
+    }
+
+    private static boolean isJavaIdentifierStart(char c) {
+        return Character.isJavaIdentifierStart(c) && Character.getType(c) != Character.CURRENCY_SYMBOL;
+    }
+}

translation/src/main/java/org/opencypher/gremlin/translation/bytecode/BytecodeGremlinBindings.java

@@ -15,6 +15,7 @@
  */
 package org.opencypher.gremlin.translation.bytecode;
 
+import static org.opencypher.gremlin.translation.GroovyIdentifiers.isValidIdentifier;
 import static org.opencypher.gremlin.translation.Tokens.NULL;
 
 import java.util.Optional;
@@ -24,6 +25,10 @@
 public class BytecodeGremlinBindings implements GremlinBindings {
     @Override
     public Object bind(String name, Object value) {
-        return new Binding<>(name, Optional.ofNullable(value).orElse(NULL));
+        if (isValidIdentifier(name)) {
+            return new Binding<>(name, Optional.ofNullable(value).orElse(NULL));
+        } else {
+            throw new IllegalArgumentException("Invalid parameter name: " + name);
+        }
     }
 }

translation/src/main/java/org/opencypher/gremlin/translation/groovy/GroovyGremlinBindings.java

@@ -15,12 +15,11 @@
  */
 package org.opencypher.gremlin.translation.groovy;
 
-import static java.lang.Character.isJavaIdentifierPart;
+import static org.opencypher.gremlin.translation.GroovyIdentifiers.isValidIdentifier;
 
 import org.opencypher.gremlin.translation.GremlinBindings;
 
 public class GroovyGremlinBindings implements GremlinBindings {
-
     @Override
     public Object bind(String name, Object value) {
         if (isValidIdentifier(name)) {
@@ -29,25 +28,4 @@ public Object bind(String name, Object value) {
             throw new IllegalArgumentException("Invalid parameter name: " + name);
         }
     }
-
-    private static boolean isValidIdentifier(String value) {
-        char[] chars = value.toCharArray();
-        int length = chars.length;
-        if (length == 0) {
-            return false;
-        }
-        if (!isJavaIdentifierStart(chars[0])) {
-            return false;
-        }
-        for (int i = 1; i < length; i++) {
-            if (!isJavaIdentifierPart(chars[i])) {
-                return false;
-            }
-        }
-        return true;
-    }
-
-    private static boolean isJavaIdentifierStart(char c) {
-        return Character.isJavaIdentifierStart(c) && Character.getType(c) != Character.CURRENCY_SYMBOL;
-    }
 }