From babd5ccf0737ec85ed600b15a228b231e0244720 Mon Sep 17 00:00:00 2001 From: Philipp Ossler Date: Thu, 29 Aug 2024 06:28:54 +0200 Subject: [PATCH 1/5] refactor: Migrate to new test style Align with version 1.17 and migrate the test class to the new test style. This refactoring eases the backporting of upcoming bug fixes. --- .../InterpreterStringExpressionTest.scala | 68 ++++++++++--------- 1 file changed, 36 insertions(+), 32 deletions(-) diff --git a/src/test/scala/org/camunda/feel/impl/interpreter/InterpreterStringExpressionTest.scala b/src/test/scala/org/camunda/feel/impl/interpreter/InterpreterStringExpressionTest.scala index d23bb36a7..a4b438283 100644 --- a/src/test/scala/org/camunda/feel/impl/interpreter/InterpreterStringExpressionTest.scala +++ b/src/test/scala/org/camunda/feel/impl/interpreter/InterpreterStringExpressionTest.scala @@ -16,80 +16,86 @@ */ package org.camunda.feel.impl.interpreter -import org.camunda.feel.impl.FeelIntegrationTest +import org.camunda.feel.impl.{EvaluationResultMatchers, FeelEngineTest, FeelIntegrationTest} import org.camunda.feel.syntaxtree._ import org.scalatest.matchers.should.Matchers import org.scalatest.flatspec.AnyFlatSpec +import scala.collection.immutable.Map + /** @author * Philipp Ossler */ -class InterpreterStringExpressionTest extends AnyFlatSpec with Matchers with FeelIntegrationTest { +class InterpreterStringExpressionTest + extends AnyFlatSpec + with Matchers + with FeelEngineTest + with EvaluationResultMatchers { "A string" should "concatenates to another String" in { - eval(""" "a" + "b" """) should be(ValString("ab")) + evaluateExpression(""" "a" + "b" """) should returnResult("ab") } it should "compare with '='" in { - eval(""" "a" = "a" """) should be(ValBoolean(true)) - eval(""" "a" = "b" """) should be(ValBoolean(false)) + evaluateExpression(""" "a" = "a" """) should returnResult(true) + evaluateExpression(""" "a" = "b" """) should returnResult(false) } it should "compare with '!='" in { - eval(""" "a" != "a" """) should be(ValBoolean(false)) - eval(""" "a" != "b" """) should be(ValBoolean(true)) + evaluateExpression(""" "a" != "a" """) should returnResult(false) + evaluateExpression(""" "a" != "b" """) should returnResult(true) } it should "compare with '<'" in { - eval(""" "a" < "b" """) should be(ValBoolean(true)) - eval(""" "b" < "a" """) should be(ValBoolean(false)) + evaluateExpression(""" "a" < "b" """) should returnResult(true) + evaluateExpression(""" "b" < "a" """) should returnResult(false) } it should "compare with '<='" in { - eval(""" "a" <= "a" """) should be(ValBoolean(true)) - eval(""" "b" <= "a" """) should be(ValBoolean(false)) + evaluateExpression(""" "a" <= "a" """) should returnResult(true) + evaluateExpression(""" "b" <= "a" """) should returnResult(false) } it should "compare with '>'" in { - eval(""" "b" > "a" """) should be(ValBoolean(true)) - eval(""" "a" > "b" """) should be(ValBoolean(false)) + evaluateExpression(""" "b" > "a" """) should returnResult(true) + evaluateExpression(""" "a" > "b" """) should returnResult(false) } it should "compare with '>='" in { - eval(""" "b" >= "b" """) should be(ValBoolean(true)) - eval(""" "a" >= "b" """) should be(ValBoolean(false)) + evaluateExpression(""" "b" >= "b" """) should returnResult(true) + evaluateExpression(""" "a" >= "b" """) should returnResult(false) } it should "compare with null" in { - eval(""" "a" = null """) should be(ValBoolean(false)) - eval(""" null = "a" """) should be(ValBoolean(false)) - eval(""" "a" != null """) should be(ValBoolean(true)) + evaluateExpression(""" "a" = null """) should returnResult(false) + evaluateExpression(""" null = "a" """) should returnResult(false) + evaluateExpression(""" "a" != null """) should returnResult(true) } it should "return not escaped characters" in { - eval(""" "Hello\nWorld" """) should be(ValString("Hello\nWorld")) - eval(" x ", Map("x" -> "Hello\nWorld")) should be(ValString("Hello\nWorld")) + evaluateExpression(""" "Hello\nWorld" """) should returnResult("Hello\nWorld") + evaluateExpression(" x ", Map("x" -> "Hello\nWorld")) should returnResult("Hello\nWorld") - eval(""" "Hello\rWorld" """) should be(ValString("Hello\rWorld")) - eval(" x ", Map("x" -> "Hello\rWorld")) should be(ValString("Hello\rWorld")) + evaluateExpression(""" "Hello\rWorld" """) should returnResult("Hello\rWorld") + evaluateExpression(" x ", Map("x" -> "Hello\rWorld")) should returnResult("Hello\rWorld") - eval(""" "Hello\'World" """) should be(ValString("Hello\'World")) - eval(" x ", Map("x" -> "Hello\'World")) should be(ValString("Hello\'World")) + evaluateExpression(""" "Hello\'World" """) should returnResult("Hello\'World") + evaluateExpression(" x ", Map("x" -> "Hello\'World")) should returnResult("Hello\'World") - eval(""" "Hello\tWorld" """) should be(ValString("Hello\tWorld")) - eval(" x ", Map("x" -> "Hello\tWorld")) should be(ValString("Hello\tWorld")) + evaluateExpression(""" "Hello\tWorld" """) should returnResult("Hello\tWorld") + evaluateExpression(" x ", Map("x" -> "Hello\tWorld")) should returnResult("Hello\tWorld") - eval(""" "Hello\"World" """) should be(ValString("Hello\"World")) - eval(" x ", Map("x" -> "Hello\"World")) should be(ValString("Hello\"World")) + evaluateExpression(""" "Hello\"World" """) should returnResult("Hello\"World") + evaluateExpression(" x ", Map("x" -> "Hello\"World")) should returnResult("Hello\"World") } List( @@ -104,10 +110,8 @@ class InterpreterStringExpressionTest extends AnyFlatSpec with Matchers with Fee .foreach { notEscapeChar => it should s"contains a not escape sequence ($notEscapeChar)" in { - eval(s""" "a $notEscapeChar b" """) should be( - ValString( - s"""a $notEscapeChar b""" - ) + evaluateExpression(s""" "a $notEscapeChar b" """) should returnResult( + s"""a $notEscapeChar b""" ) } } From a1fca734839f2c97be6a9368e4bf5094e9a655b5 Mon Sep 17 00:00:00 2001 From: Philipp Ossler Date: Thu, 29 Aug 2024 06:28:54 +0200 Subject: [PATCH 2/5] refactor: Use parameterized test case (cherry picked from commit c8b01bd09f2cf6fca2bc789a2101001c99778480) --- .../InterpreterStringExpressionTest.scala | 43 ++++++++++++------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/src/test/scala/org/camunda/feel/impl/interpreter/InterpreterStringExpressionTest.scala b/src/test/scala/org/camunda/feel/impl/interpreter/InterpreterStringExpressionTest.scala index a4b438283..a017035d9 100644 --- a/src/test/scala/org/camunda/feel/impl/interpreter/InterpreterStringExpressionTest.scala +++ b/src/test/scala/org/camunda/feel/impl/interpreter/InterpreterStringExpressionTest.scala @@ -20,6 +20,7 @@ import org.camunda.feel.impl.{EvaluationResultMatchers, FeelEngineTest, FeelInte import org.camunda.feel.syntaxtree._ import org.scalatest.matchers.should.Matchers import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.prop.TableDrivenPropertyChecks import scala.collection.immutable.Map @@ -30,7 +31,8 @@ class InterpreterStringExpressionTest extends AnyFlatSpec with Matchers with FeelEngineTest - with EvaluationResultMatchers { + with EvaluationResultMatchers + with TableDrivenPropertyChecks { "A string" should "concatenates to another String" in { @@ -98,22 +100,33 @@ class InterpreterStringExpressionTest evaluateExpression(" x ", Map("x" -> "Hello\"World")) should returnResult("Hello\"World") } - List( - " \' ", - " \\ ", - " \n ", - " \r ", - " \t ", - """ \u269D """, - """ \U101EF """ + private val escapeSequences = Table( + ("Character", "Display name"), + ('\n', "\\n"), + ('\r', "\\r"), + ('\t', "\\t"), + ('\b', "\\b"), + ('\f', "\\f"), + ('\'', "\'"), + ('\\', "\\") ) - .foreach { notEscapeChar => - it should s"contains a not escape sequence ($notEscapeChar)" in { - evaluateExpression(s""" "a $notEscapeChar b" """) should returnResult( - s"""a $notEscapeChar b""" - ) - } + it should "contains an escape sequence" in { + forEvery(escapeSequences) { (character, _) => + evaluateExpression(s" \"a $character b\" ") should returnResult(s"a $character b") } + } + + private val unicodeCharacters = Table( + ("Character", "Display name"), + ('\u269D', "\\u269D"), + ("\\U101EF", "\\U101EF") + ) + + it should "contains unicode characters" in { + forEvery(unicodeCharacters) { (character, _) => + evaluateExpression(s" \"a $character b\" ") should returnResult(s"a $character b") + } + } } From 3adbf4cd0a0ca3a1333d5c222f014a41ee09ce5f Mon Sep 17 00:00:00 2001 From: Philipp Ossler Date: Tue, 24 Sep 2024 10:20:05 +0200 Subject: [PATCH 3/5] refactor: Combine test cases (cherry picked from commit 58efca98cca80dcdb7d9dc28c2331e9991b63956) --- .../InterpreterStringExpressionTest.scala | 42 +++++++------------ 1 file changed, 14 insertions(+), 28 deletions(-) diff --git a/src/test/scala/org/camunda/feel/impl/interpreter/InterpreterStringExpressionTest.scala b/src/test/scala/org/camunda/feel/impl/interpreter/InterpreterStringExpressionTest.scala index a017035d9..b6b05cec4 100644 --- a/src/test/scala/org/camunda/feel/impl/interpreter/InterpreterStringExpressionTest.scala +++ b/src/test/scala/org/camunda/feel/impl/interpreter/InterpreterStringExpressionTest.scala @@ -82,38 +82,24 @@ class InterpreterStringExpressionTest evaluateExpression(""" "a" != null """) should returnResult(true) } - it should "return not escaped characters" in { - - evaluateExpression(""" "Hello\nWorld" """) should returnResult("Hello\nWorld") - evaluateExpression(" x ", Map("x" -> "Hello\nWorld")) should returnResult("Hello\nWorld") - - evaluateExpression(""" "Hello\rWorld" """) should returnResult("Hello\rWorld") - evaluateExpression(" x ", Map("x" -> "Hello\rWorld")) should returnResult("Hello\rWorld") - - evaluateExpression(""" "Hello\'World" """) should returnResult("Hello\'World") - evaluateExpression(" x ", Map("x" -> "Hello\'World")) should returnResult("Hello\'World") - - evaluateExpression(""" "Hello\tWorld" """) should returnResult("Hello\tWorld") - evaluateExpression(" x ", Map("x" -> "Hello\tWorld")) should returnResult("Hello\tWorld") - - evaluateExpression(""" "Hello\"World" """) should returnResult("Hello\"World") - evaluateExpression(" x ", Map("x" -> "Hello\"World")) should returnResult("Hello\"World") - } - private val escapeSequences = Table( - ("Character", "Display name"), - ('\n', "\\n"), - ('\r', "\\r"), - ('\t', "\\t"), - ('\b', "\\b"), - ('\f', "\\f"), - ('\'', "\'"), - ('\\', "\\") + ("Character", "Expected", "Display name"), + ('\n', '\n', "new line"), + ('\r', '\r', "carriage return"), + ('\t', '\t', "tab"), + ('\b', '\b', "backspace"), + ('\f', '\f', "form feed"), + ('\'', '\'', "single quote"), + ("\\\"", '"', "double quote"), + ("\\\\", '\\', "backslash") ) it should "contains an escape sequence" in { - forEvery(escapeSequences) { (character, _) => - evaluateExpression(s" \"a $character b\" ") should returnResult(s"a $character b") + forEvery(escapeSequences) { (character, expected, _) => + val expectedString = s"a $expected b" + + evaluateExpression(s" \"a $character b\" ") should returnResult(expectedString) + evaluateExpression("char", Map("char" -> expectedString)) should returnResult(expectedString) } } From 8eb3bd997289aba12af5fde46d7fb1ef7f494038 Mon Sep 17 00:00:00 2001 From: Philipp Ossler Date: Thu, 29 Aug 2024 07:11:34 +0200 Subject: [PATCH 4/5] test: Verify string literal with regex characters (cherry picked from commit 544c0388bb0ee8e9a3b078f4567941ce684a8f0a) --- .../InterpreterStringExpressionTest.scala | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/test/scala/org/camunda/feel/impl/interpreter/InterpreterStringExpressionTest.scala b/src/test/scala/org/camunda/feel/impl/interpreter/InterpreterStringExpressionTest.scala index b6b05cec4..09f7272bf 100644 --- a/src/test/scala/org/camunda/feel/impl/interpreter/InterpreterStringExpressionTest.scala +++ b/src/test/scala/org/camunda/feel/impl/interpreter/InterpreterStringExpressionTest.scala @@ -115,4 +115,26 @@ class InterpreterStringExpressionTest } } + private val regexCharacters = Table( + ("Character", "Display name"), + ("\\s", "\\s"), + ("\\S", "\\S"), + ("\\d", "\\d"), + ("\\w", "\\w"), + ("\\R", "\\R"), + ("\\h", "\\h"), + ("\\v", "\\v"), + ("\\\n", "\\n"), + ("\\\r", "\\r") + ) + + it should "contains a regex character" in { + forEvery(regexCharacters) { (character, _) => + val expectedString = s"a $character b" + + evaluateExpression(s" \"a $character b\" ") should returnResult(expectedString) + evaluateExpression("char", Map("char" -> expectedString)) should returnResult(expectedString) + } + } + } From 3ac86a78d08c94405beb23c6acd68d5cd78b1f9e Mon Sep 17 00:00:00 2001 From: Philipp Ossler Date: Thu, 29 Aug 2024 10:13:28 +0200 Subject: [PATCH 5/5] fix: Handle escape sequences Correct the handling of escape sequences in string literals. Don't replace escape sequences in regex expressions, for example, \r or \n. In the parser, these sequences start with \\. Same for \s, don't replace it with a whitespace, since this is also a part of a regex. Handle \\ to avoid that the sequence is escaped and returned as \\\\. (cherry picked from commit d266e80173126eaf6dbfee4333be6cb0833c39bf) --- .../camunda/feel/impl/parser/FeelParser.scala | 26 +++++++------------ 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/src/main/scala/org/camunda/feel/impl/parser/FeelParser.scala b/src/main/scala/org/camunda/feel/impl/parser/FeelParser.scala index b9c3b2254..a58166be2 100644 --- a/src/main/scala/org/camunda/feel/impl/parser/FeelParser.scala +++ b/src/main/scala/org/camunda/feel/impl/parser/FeelParser.scala @@ -688,22 +688,16 @@ object FeelParser { }.getOrElse(ConstNull) } - // replace escaped character with the provided replacement private def translateEscapes(input: String): String = { - val escapeMap = Map( - "\\b" -> "\b", - "\\t" -> "\t", - "\\n" -> "\n", - "\\f" -> "\f", - "\\r" -> "\r", - "\\\"" -> "\"", - "\\'" -> "'", - "\\s" -> " " - // Add more escape sequences as needed - ) - - escapeMap.foldLeft(input) { case (result, (escape, replacement)) => - result.replace(escape, replacement) - } + // replace all escape sequences + input + .replaceAll("(?