\n and \t handling - fixes #52

allegro · Jun 27, 2024 · a682a83 · a682a83
1 parent 49ee4b0
commit a682a83
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 15 deletions.
diff --git a/src/main/java/pl/allegro/tech/opel/MethodCallExpressionNode.java b/src/main/java/pl/allegro/tech/opel/MethodCallExpressionNode.java
@@ -6,7 +6,11 @@
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.lang.reflect.Parameter;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
 import java.util.function.Function;
 import java.util.stream.Collectors;
@@ -36,13 +40,6 @@ static MethodCallExpressionNode create(OpelNode subject, OpelNode identifier, Op
         return new MethodCallExpressionNode(subject, ((IdentifierExpressionNode) identifier).getIdentifier(), Optional.of((ArgumentsListExpressionNode) arguments), implicitConversion, methodExecutionFilter);
     }
 
-    static MethodCallExpressionNode create(OpelNode subject, OpelNode identifier, ImplicitConversion implicitConversion, MethodExecutionFilter methodExecutionFilter) {
-        if (!(identifier instanceof IdentifierExpressionNode)) {
-            throw new IllegalArgumentException("Cannot create from OpelNode because identifier is of wrong node type " + identifier.getClass().getSimpleName());
-        }
-        return new MethodCallExpressionNode(subject, ((IdentifierExpressionNode) identifier).getIdentifier(), Optional.empty(), implicitConversion, methodExecutionFilter);
-    }
-
     @Override
     public CompletableFuture<?> getValue(EvalContext context) {
         return FutureUtil.sequence(

diff --git a/src/main/java/pl/allegro/tech/opel/OpelParser.java b/src/main/java/pl/allegro/tech/opel/OpelParser.java
@@ -7,10 +7,14 @@
 import org.parboiled.annotations.SuppressSubnodes;
 
 import java.math.BigDecimal;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 @BuildParseTree
 public class OpelParser extends BaseParser<OpelNode> {
 
+    private final Pattern whitespacePattern = Pattern.compile("\\\\([nt])");
+
     final ImplicitConversion implicitConversion;
     final OpelNodeFactory nodeFactory;
 
@@ -343,8 +347,14 @@ protected Rule fromStringLiteral(String string) {
         return String(string).label("'" + string + "'");
     }
 
+    /**
+     * Escapes special characters in string literals.
+     * Explicitly converts \t and \n to their corresponding whitespace characters.
+     */
     protected String escapeString(String string) {
+        Matcher matcher = whitespacePattern.matcher(string);
         StringBuilder result = new StringBuilder();
+        while (matcher.find()) matcher.appendReplacement(result, matcher.group(1));
         for (int i = 0; i < string.length(); i++) {
             if (string.charAt(i) == '\\') {
                 i++;

diff --git a/src/test/groovy/pl/allegro/tech/opel/MethodCallExpressionNodeSpec.groovy b/src/test/groovy/pl/allegro/tech/opel/MethodCallExpressionNodeSpec.groovy
@@ -1,6 +1,7 @@
 package pl.allegro.tech.opel
 
 import spock.lang.Specification
+import spock.lang.Unroll
 
 import java.lang.reflect.Method
 import java.util.concurrent.ExecutionException
@@ -78,6 +79,25 @@ class MethodCallExpressionNodeSpec extends Specification {
         ex.cause.message == 'Can\'t call \'method\' on null'
     }
 
+    @Unroll
+    def "test"() {
+        given:
+        def arguments = Optional.of(new ArgumentsListExpressionNode([valueNode(startsWith)]))
+        def methodCallNode = new MethodCallExpressionNode(valueNode(string), 'startsWith', arguments, new ImplicitConversion(), MethodExecutionFilters.ALLOW_ALL)
+
+        expect:
+        methodCallNode.getValue().get() == result
+
+        where:
+        string   | startsWith || result
+        'test'   | 't'        || true
+        'test'   | 'a'        || false
+        'test'   | '\t'       || false
+        'test'   | '\\t'      || false
+        '\ttest' | '\t'       || true
+        '\ttest' | 't'        || false
+    }
+
     class Foo {
         def method(a, b) {
             a + b

diff --git a/src/test/groovy/pl/allegro/tech/opel/OpelEngineIntegrationSpec.groovy b/src/test/groovy/pl/allegro/tech/opel/OpelEngineIntegrationSpec.groovy
@@ -7,7 +7,6 @@ import java.util.concurrent.CompletableFuture
 import java.util.concurrent.ExecutionException
 
 import static OpelEngineBuilder.create
-import static pl.allegro.tech.opel.TestUtil.constFunctionReturning
 import static pl.allegro.tech.opel.TestUtil.functions
 
 class OpelEngineIntegrationSpec extends Specification {
@@ -74,9 +73,9 @@ xyz'"""
     def 'should evaluate expression with whitespaces "#input"'() {
         given:
         def engine = create()
-                    .withImplicitConversion(String, BigDecimal, { string -> new BigDecimal(string) })
-                    .withImplicitConversion(BigDecimal, String, { decimal -> decimal.toPlainString() })
-                    .build()
+                .withImplicitConversion(String, BigDecimal, { string -> new BigDecimal(string) })
+                .withImplicitConversion(BigDecimal, String, { decimal -> decimal.toPlainString() })
+                .build()
 
         expect:
         engine.eval(input).get() == expResult
@@ -109,9 +108,9 @@ xyz'"""
     def 'should evaluate expression which starts with whitespace(s): "#input"'() {
         given:
         def engine = create()
-                    .withImplicitConversion(String, BigDecimal, { string -> new BigDecimal(string) })
-                    .withImplicitConversion(BigDecimal, String, { decimal -> decimal.toPlainString() })
-                    .build()
+                .withImplicitConversion(String, BigDecimal, { string -> new BigDecimal(string) })
+                .withImplicitConversion(BigDecimal, String, { decimal -> decimal.toPlainString() })
+                .build()
 
         expect:
         engine.eval(input).get() == expResult
@@ -305,4 +304,20 @@ xyz'"""
         counter1 == 1
         counter2 == 0
     }
+
+    @Unroll
+    def 'should handle slash character'() {
+        given:
+        def engine = create().build()
+
+        expect:
+        engine.eval(input).get() == expResult
+
+        where:
+        input                      || expResult
+        /'atest'.startsWith('\a')/ || true //\a is not a special character, so it's just unecessary escape
+        /'test'.startsWith('\t')/  || false //\t is replaced by tab
+        /'test'.startsWith('\n')/  || false //\n is replaced by CL character
+        /'test'.startsWith('\r')/  || false //\r is replaced by RF character
+    }
 }