Merge pull request #251 from LinneaAndersson/4.2-javacc-parse-params

LinneaAndersson · web-flow · commit eac99b6f00b6 · 2020-10-21T11:24:24.000+02:00
Use JavaCC for parameter parsing
diff --git a/build.gradle b/build.gradle
@@ -69,7 +69,7 @@ ext {
 
     argparse4jVersion = '0.7.0'
     junitVersion = '4.12'
-    evaluatorVersion = '3.5.14'
+    evaluatorVersion = '4.1.1'
     neo4jJavaDriverVersion = '4.2.0-alpha01'
     findbugsVersion = '3.0.0'
     jansiVersion = '1.13'
diff --git a/cypher-shell/build.gradle b/cypher-shell/build.gradle
@@ -54,9 +54,9 @@ dependencies {
         exclude(group: 'org.neo4j', module: 'neo4j-native')
         exclude(group: 'org.neo4j', module: 'neo4j-logging')
         exclude(group: 'org.neo4j', module: 'neo4j-procedure-api')
-        exclude(group: 'org.neo4j', module: 'neo4j-kernel-api')
         exclude(group: 'org.eclipse.collections')
     }
+    compile("org.neo4j:neo4j-cypher-javacc-parser:$evaluatorVersion")
     compile "org.neo4j.driver:neo4j-java-driver:$neo4jJavaDriverVersion"
     compileOnly "com.google.code.findbugs:annotations:$findbugsVersion"
     compile "org.fusesource.jansi:jansi:$jansiVersion"
diff --git a/cypher-shell/src/main/java/org/neo4j/shell/Connector.java b/cypher-shell/src/main/java/org/neo4j/shell/Connector.java
@@ -2,8 +2,8 @@
 
 import javax.annotation.Nonnull;
 
-import org.neo4j.function.ThrowingAction;
 import org.neo4j.shell.exception.CommandException;
+import org.neo4j.shell.exception.ThrowingAction;
 
 /**
  * An object with the ability to connect and disconnect.
diff --git a/cypher-shell/src/main/java/org/neo4j/shell/CypherShell.java b/cypher-shell/src/main/java/org/neo4j/shell/CypherShell.java
@@ -8,12 +8,12 @@
 import org.neo4j.driver.exceptions.DiscoveryException;
 import org.neo4j.driver.exceptions.Neo4jException;
 import org.neo4j.driver.exceptions.ServiceUnavailableException;
-import org.neo4j.function.ThrowingAction;
 import org.neo4j.shell.commands.Command;
 import org.neo4j.shell.commands.CommandExecutable;
 import org.neo4j.shell.commands.CommandHelper;
 import org.neo4j.shell.exception.CommandException;
 import org.neo4j.shell.exception.ExitException;
+import org.neo4j.shell.exception.ThrowingAction;
 import org.neo4j.shell.prettyprint.LinePrinter;
 import org.neo4j.shell.prettyprint.PrettyConfig;
 import org.neo4j.shell.prettyprint.PrettyPrinter;
diff --git a/cypher-shell/src/main/java/org/neo4j/shell/Main.java b/cypher-shell/src/main/java/org/neo4j/shell/Main.java
@@ -10,12 +10,12 @@
 
 import org.neo4j.driver.exceptions.AuthenticationException;
 import org.neo4j.driver.exceptions.Neo4jException;
-import org.neo4j.function.ThrowingAction;
 import org.neo4j.shell.build.Build;
 import org.neo4j.shell.cli.CliArgHelper;
 import org.neo4j.shell.cli.CliArgs;
 import org.neo4j.shell.commands.CommandHelper;
 import org.neo4j.shell.exception.CommandException;
+import org.neo4j.shell.exception.ThrowingAction;
 import org.neo4j.shell.log.AnsiLogger;
 import org.neo4j.shell.log.Logger;
 import org.neo4j.shell.prettyprint.PrettyConfig;
diff --git a/cypher-shell/src/main/java/org/neo4j/shell/ParameterMap.java b/cypher-shell/src/main/java/org/neo4j/shell/ParameterMap.java
@@ -1,6 +1,6 @@
 package org.neo4j.shell;
 
-import org.neo4j.cypher.internal.evaluator.EvaluationException;
+import org.neo4j.shell.exception.ParameterException;
 import org.neo4j.shell.state.ParamValue;
 
 import javax.annotation.Nonnull;
@@ -15,7 +15,7 @@ public interface ParameterMap {
      * @param valueString to interpret the value from
      * @return the evaluated value
      */
-    Object setParameter(@Nonnull String name, @Nonnull String valueString) throws EvaluationException;
+    Object setParameter(@Nonnull String name, @Nonnull String valueString) throws ParameterException;
 
     /**
      * @return map of all currently set variables and their values
diff --git a/cypher-shell/src/main/java/org/neo4j/shell/ShellParameterMap.java b/cypher-shell/src/main/java/org/neo4j/shell/ShellParameterMap.java
@@ -1,13 +1,18 @@
 package org.neo4j.shell;
 
+import java.io.StringReader;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.stream.Collectors;
 import javax.annotation.Nonnull;
 
+import org.neo4j.cypher.internal.ast.factory.LiteralInterpreter;
 import org.neo4j.cypher.internal.evaluator.EvaluationException;
 import org.neo4j.cypher.internal.evaluator.Evaluator;
 import org.neo4j.cypher.internal.evaluator.ExpressionEvaluator;
+import org.neo4j.cypher.internal.parser.javacc.Cypher;
+import org.neo4j.cypher.internal.parser.javacc.ParseException;
+import org.neo4j.shell.exception.ParameterException;
 import org.neo4j.shell.prettyprint.CypherVariablesFormatter;
 import org.neo4j.shell.state.ParamValue;
 
@@ -17,15 +22,29 @@
 public class ShellParameterMap implements ParameterMap
 {
     private final Map<String, ParamValue> queryParams = new HashMap<>();
+    private LiteralInterpreter interpreter = new LiteralInterpreter();
     private ExpressionEvaluator evaluator = Evaluator.expressionEvaluator();
 
+
     @Override
-    public Object setParameter( @Nonnull String name, @Nonnull String valueString ) throws EvaluationException
+    public Object setParameter( @Nonnull String name, @Nonnull String valueString ) throws ParameterException
     {
         String parameterName = CypherVariablesFormatter.unescapedCypherVariable( name );
-        Object value = evaluator.evaluate( valueString, Object.class );
-        queryParams.put( parameterName, new ParamValue( valueString, value ) );
-        return value;
+        try {
+            Object value = new Cypher<>( interpreter,
+                                         ParameterException.FACTORY,
+                                         new StringReader( valueString ) ).Expression();
+            queryParams.put( parameterName, new ParamValue( valueString, value ) );
+            return value;
+        } catch (ParseException | UnsupportedOperationException e) {
+            try {
+                Object value = evaluator.evaluate( valueString, Object.class );
+                queryParams.put( parameterName, new ParamValue( valueString, value ) );
+                return value;
+            } catch (EvaluationException e1) {
+                throw new ParameterException( e1.getMessage() );
+            }
+        }
     }
 
     @Nonnull
diff --git a/cypher-shell/src/main/java/org/neo4j/shell/cli/AddParamArgumentAction.java b/cypher-shell/src/main/java/org/neo4j/shell/cli/AddParamArgumentAction.java
@@ -7,8 +7,8 @@
 
 import java.util.Map;
 
-import org.neo4j.cypher.internal.evaluator.EvaluationException;
 import org.neo4j.shell.ParameterMap;
+import org.neo4j.shell.exception.ParameterException;
 import org.neo4j.shell.util.ParameterSetter;
 
 /**
@@ -63,7 +63,7 @@ protected void onWrongNumberOfArguments()
     }
 
     @Override
-    protected void onEvaluationException( EvaluationException e )
+    protected void onParameterException( ParameterException e )
     {
         throw new RuntimeException( e.getMessage(), e );
     }
diff --git a/cypher-shell/src/main/java/org/neo4j/shell/commands/Param.java b/cypher-shell/src/main/java/org/neo4j/shell/commands/Param.java
@@ -1,18 +1,14 @@
 package org.neo4j.shell.commands;
 
-import org.neo4j.cypher.internal.evaluator.EvaluationException;
 import org.neo4j.shell.ParameterMap;
 import org.neo4j.shell.exception.CommandException;
+import org.neo4j.shell.exception.ParameterException;
 import org.neo4j.shell.log.AnsiFormattedText;
 import org.neo4j.shell.util.ParameterSetter;
 
 import javax.annotation.Nonnull;
 import java.util.Collections;
 import java.util.List;
-import java.util.function.BiPredicate;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-
 
 /**
  * This command sets a variable to a name, for use as query parameter.
@@ -72,7 +68,7 @@ protected void onWrongNumberOfArguments() throws CommandException
     }
 
     @Override
-    protected void onEvaluationException( EvaluationException e ) throws CommandException
+    protected void onParameterException( ParameterException e ) throws CommandException
     {
         throw new CommandException( e.getMessage(), e );
     }
diff --git a/cypher-shell/src/main/java/org/neo4j/shell/exception/ParameterException.java b/cypher-shell/src/main/java/org/neo4j/shell/exception/ParameterException.java
@@ -0,0 +1,21 @@
+package org.neo4j.shell.exception;
+
+import org.neo4j.cypher.internal.ast.factory.ASTExceptionFactory;
+
+public class ParameterException extends IllegalArgumentException {
+    public ParameterException(String msg) {
+        super(msg);
+    }
+
+    public static final ASTExceptionFactory FACTORY = new ASTExceptionFactory() {
+        @Override
+        public Exception syntaxException(Exception e) {
+            return new ParameterException(e.getMessage());
+        }
+
+        @Override
+        public Exception invalidUnicodeLiteral(String s) {
+            return new ParameterException(s);
+        }
+    };
+}
diff --git a/cypher-shell/src/main/java/org/neo4j/shell/exception/ThrowingAction.java b/cypher-shell/src/main/java/org/neo4j/shell/exception/ThrowingAction.java
@@ -0,0 +1,17 @@
+package org.neo4j.shell.exception;
+
+/**
+ * An action that takes no parameters and returns no values, but may have a side-effect and may throw an exception.
+ *
+ * @param <E> The type of exception this action may throw.
+ */
+@FunctionalInterface
+public interface ThrowingAction<E extends Exception>
+{
+    /**
+     * Apply the action for some or all of its side-effects to take place, possibly throwing an exception.
+     *
+     * @throws E the exception that performing this action may throw.
+     */
+    void apply() throws E;
+}
diff --git a/cypher-shell/src/main/java/org/neo4j/shell/state/BoltStateHandler.java b/cypher-shell/src/main/java/org/neo4j/shell/state/BoltStateHandler.java
@@ -29,14 +29,14 @@
 import org.neo4j.driver.internal.Scheme;
 import org.neo4j.driver.summary.DatabaseInfo;
 import org.neo4j.driver.summary.ResultSummary;
-import org.neo4j.function.ThrowingAction;
 import org.neo4j.shell.ConnectionConfig;
 import org.neo4j.shell.Connector;
 import org.neo4j.shell.DatabaseManager;
 import org.neo4j.shell.TransactionHandler;
 import org.neo4j.shell.TriFunction;
 import org.neo4j.shell.build.Build;
 import org.neo4j.shell.exception.CommandException;
+import org.neo4j.shell.exception.ThrowingAction;
 import org.neo4j.shell.log.NullLogging;
 
 import static org.neo4j.shell.util.Versions.isPasswordChangeRequiredException;
diff --git a/cypher-shell/src/main/java/org/neo4j/shell/util/ParameterSetter.java b/cypher-shell/src/main/java/org/neo4j/shell/util/ParameterSetter.java
@@ -5,8 +5,8 @@
 import java.util.regex.Pattern;
 import javax.annotation.Nonnull;
 
-import org.neo4j.cypher.internal.evaluator.EvaluationException;
 import org.neo4j.shell.ParameterMap;
+import org.neo4j.shell.exception.ParameterException;
 
 /**
  * Shared logic to parse parameters and set them in a ParameterMap
@@ -28,7 +28,7 @@ protected ParameterSetter( ParameterMap parameterMap )
 
     protected abstract void onWrongUsage() throws E;
     protected abstract void onWrongNumberOfArguments() throws E;
-    protected abstract void onEvaluationException(EvaluationException e) throws E;
+    protected abstract void onParameterException(ParameterException e) throws E;
 
     public void execute(@Nonnull final String argString) throws E {
         Matcher lambdaMapMatcher = lambdaMapPattern.matcher( argString);
@@ -41,13 +41,13 @@ public void execute(@Nonnull final String argString) throws E {
                 onWrongNumberOfArguments();
             }
         }
-        catch ( EvaluationException e )
+        catch ( ParameterException e )
         {
-            onEvaluationException(e);
+            onParameterException(e);
         }
     }
 
-    private boolean assignIfValidParameter(@Nonnull String argString) throws EvaluationException
+    private boolean assignIfValidParameter(@Nonnull String argString) throws ParameterException
     {
         return setParameterIfItMatchesPattern(argString, lambdaPattern, assignIfValidParameter())
                || setParameterIfItMatchesPattern(argString, argPattern, assignIfValidParameter())
@@ -56,7 +56,7 @@ private boolean assignIfValidParameter(@Nonnull String argString) throws Evaluat
     }
 
     private boolean setParameterIfItMatchesPattern(@Nonnull String argString, Pattern pattern,
-                                                   BiPredicate<String, Matcher> matchingFunction) throws EvaluationException
+                                                   BiPredicate<String, Matcher> matchingFunction) throws ParameterException
     {
         Matcher matcher = pattern.matcher(argString);
         if (matchingFunction.test(argString, matcher)) {
diff --git a/cypher-shell/src/test/java/org/neo4j/shell/CypherShellTest.java b/cypher-shell/src/test/java/org/neo4j/shell/CypherShellTest.java
@@ -7,7 +7,6 @@
 
 import java.util.Optional;
 
-import org.neo4j.cypher.internal.evaluator.EvaluationException;
 import org.neo4j.driver.Driver;
 import org.neo4j.driver.Record;
 import org.neo4j.driver.Session;
@@ -17,6 +16,7 @@
 import org.neo4j.shell.commands.CommandExecutable;
 import org.neo4j.shell.commands.CommandHelper;
 import org.neo4j.shell.exception.CommandException;
+import org.neo4j.shell.exception.ParameterException;
 import org.neo4j.shell.log.Logger;
 import org.neo4j.shell.prettyprint.LinePrinter;
 import org.neo4j.shell.prettyprint.PrettyPrinter;
@@ -112,7 +112,7 @@ public void verifyDelegationOfTransactionMethods() throws CommandException {
     }
 
     @Test
-    public void setWhenOfflineShouldWork() throws EvaluationException, CommandException
+    public void setWhenOfflineShouldWork() throws ParameterException, CommandException
     {
         CypherShell shell = new OfflineTestShell(logger, mockedBoltStateHandler, mockedPrettyPrinter);
         when(mockedBoltStateHandler.isConnected()).thenReturn(false);
@@ -134,7 +134,7 @@ public void executeOfflineThrows() throws CommandException {
     }
 
     @Test
-    public void setParamShouldAddParamWithSpecialCharactersAndValue() throws EvaluationException, CommandException {
+    public void setParamShouldAddParamWithSpecialCharactersAndValue() throws ParameterException, CommandException {
         Value value = mock(Value.class);
         Record recordMock = mock(Record.class);
         BoltResult boltResult = new ListBoltResult(asList(recordMock), mock(ResultSummary.class));
@@ -151,7 +151,7 @@ public void setParamShouldAddParamWithSpecialCharactersAndValue() throws Evaluat
     }
 
     @Test
-    public void setParamShouldAddParam() throws EvaluationException, CommandException {
+    public void setParamShouldAddParam() throws ParameterException, CommandException {
         Value value = mock(Value.class);
         Record recordMock = mock(Record.class);
         BoltResult boltResult = mock(ListBoltResult.class);
@@ -243,7 +243,7 @@ public void shouldReturnNothingOnStrangeCommand() {
 
 
     @Test
-    public void setParameterDoesNotTriggerByBoltError() throws EvaluationException, CommandException {
+    public void setParameterDoesNotTriggerByBoltError() throws ParameterException, CommandException {
         // given
         when(mockedBoltStateHandler.runCypher(anyString(), anyMap())).thenReturn(Optional.empty());
         CypherShell shell = new CypherShell(logger, mockedBoltStateHandler, mockedPrettyPrinter, new ShellParameterMap());
diff --git a/cypher-shell/src/test/java/org/neo4j/shell/ShellParameterMapTest.java b/cypher-shell/src/test/java/org/neo4j/shell/ShellParameterMapTest.java
@@ -3,7 +3,7 @@
 import org.junit.Before;
 import org.junit.Test;
 
-import org.neo4j.cypher.internal.evaluator.EvaluationException;
+import org.neo4j.shell.exception.ParameterException;
 import org.neo4j.shell.state.ParamValue;
 
 import static junit.framework.TestCase.assertTrue;
@@ -26,21 +26,21 @@ public void newParamMapShouldBeEmpty() {
     }
 
     @Test
-    public void setParamShouldAddParamWithSpecialCharactersAndValue() throws EvaluationException {
+    public void setParamShouldAddParamWithSpecialCharactersAndValue() throws ParameterException {
         Object result = parameterMap.setParameter("`bo``b`", "99");
         assertEquals(99L, result);
         assertEquals(99L, parameterMap.allParameterValues().get("bo`b"));
     }
 
     @Test
-    public void setParamShouldAddParam() throws EvaluationException {
+    public void setParamShouldAddParam() throws ParameterException {
         Object result = parameterMap.setParameter("`bob`", "99");
         assertEquals(99L, result);
         assertEquals(99L, parameterMap.allParameterValues().get("bob"));
     }
 
     @Test
-    public void getUserInput() throws EvaluationException {
+    public void getUserInput() throws ParameterException {
         parameterMap.setParameter("`bob`", "99");
         assertEquals( new ParamValue( "99", 99L ), parameterMap.getAllAsUserInput().get("bob"));
     }
diff --git a/cypher-shell/src/test/java/org/neo4j/shell/commands/ParamTest.java b/cypher-shell/src/test/java/org/neo4j/shell/commands/ParamTest.java

Original file line number	Diff line number	Diff line change
`@@ -7,8 +7,8 @@`
`7`	`7`
`8`	`8`	`import java.util.Map;`
`9`	`9`
`10`		`-import org.neo4j.cypher.internal.evaluator.EvaluationException;`
`11`	`10`	`import org.neo4j.shell.ParameterMap;`
	`11`	`+import org.neo4j.shell.exception.ParameterException;`
`12`	`12`	`import org.neo4j.shell.util.ParameterSetter;`
`13`	`13`
`14`	`14`	`/**`
`@@ -63,7 +63,7 @@ protected void onWrongNumberOfArguments()`
`63`	`63`	`}`
`64`	`64`
`65`	`65`	`@Override`
`66`		`- protected void onEvaluationException( EvaluationException e )`
	`66`	`+ protected void onParameterException( ParameterException e )`
`67`	`67`	`{`
`68`	`68`	`throw new RuntimeException( e.getMessage(), e );`
`69`	`69`	`}`