From c8b01bd09f2cf6fca2bc789a2101001c99778480 Mon Sep 17 00:00:00 2001 From: Philipp Ossler Date: Thu, 29 Aug 2024 06:28:54 +0200 Subject: [PATCH 1/4] refactor: Use parameterized test case --- .../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 58efca98cca80dcdb7d9dc28c2331e9991b63956 Mon Sep 17 00:00:00 2001 From: Philipp Ossler Date: Tue, 24 Sep 2024 10:20:05 +0200 Subject: [PATCH 2/4] refactor: Combine test cases --- .../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 544c0388bb0ee8e9a3b078f4567941ce684a8f0a Mon Sep 17 00:00:00 2001 From: Philipp Ossler Date: Thu, 29 Aug 2024 07:11:34 +0200 Subject: [PATCH 3/4] test: Verify string literal with regex characters --- .../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 d266e80173126eaf6dbfee4333be6cb0833c39bf Mon Sep 17 00:00:00 2001 From: Philipp Ossler Date: Thu, 29 Aug 2024 10:13:28 +0200 Subject: [PATCH 4/4] 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 \\\\. --- .../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 144a94a7b..4086fa208 100644 --- a/src/main/scala/org/camunda/feel/impl/parser/FeelParser.scala +++ b/src/main/scala/org/camunda/feel/impl/parser/FeelParser.scala @@ -694,22 +694,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("(?