Feat: Improve some of the matching definition parser errors

pact-foundation · Aug 5, 2024 · 7877404 · 7877404
1 parent bf66235
commit 7877404
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 40 deletions.
diff --git a/...rc/main/kotlin/au/com/dius/pact/core/model/matchingrules/expressions/MatcherDefinition.kt b/...rc/main/kotlin/au/com/dius/pact/core/model/matchingrules/expressions/MatcherDefinition.kt
@@ -1,4 +1,4 @@
-package au.com.dius.pact.core.model.matchingrules.expressions;
+package au.com.dius.pact.core.model.matchingrules.expressions
 
 import au.com.dius.pact.core.model.generators.Generator
 import au.com.dius.pact.core.model.matchingrules.BooleanMatcher
@@ -32,6 +32,11 @@ class MatcherDefinitionLexer(expression: String): StringLexer(expression) {
 
   fun matchBoolean() = matchRegex(BOOLEAN_LITERAL).isNotEmpty()
 
+  fun highlightPosition(): String {
+    return if (index > 0) "^".padStart(index + 1)
+    else "^"
+  }
+
   companion object {
     val INTEGER_LITERAL = Regex("^-?\\d+")
     val NUMBER_LITERAL = Regex("^\\d+")
@@ -89,10 +94,16 @@ class MatcherDefinitionParser(private val lexer: MatcherDefinitionLexer) {
     return if (lexer.empty) {
       Result.Ok(definition)
     } else {
-      Result.Err("Error parsing expression: Unexpected characters '${lexer.remainder}' at ${lexer.index}")
+      Result.Err(parseError("Error parsing expression: Unexpected characters at ${lexer.index}"))
     }
   }
 
+  fun parseError(message: String): String {
+    return message +
+      "\n        ${lexer.buffer}" +
+      "\n        ${lexer.highlightPosition()}"
+  }
+
   //  matchingDefinitionExp returns [ MatchingRuleDefinition value ] :
   //      (
   //        'matching' LEFT_BRACKET matchingRule RIGHT_BRACKET
@@ -127,13 +138,13 @@ class MatcherDefinitionParser(private val lexer: MatcherDefinitionLexer) {
                   )
                 }
               } else {
-                Result.Err("Was expecting a ')' at index ${lexer.index}")
+                Result.Err(parseError("Was expecting a ')' at index ${lexer.index}"))
               }
             }
             is Result.Err -> return matchingRuleResult
           }
         } else {
-          Result.Err("Was expecting a '(' at index ${lexer.index}")
+          Result.Err(parseError("Was expecting a '(' at index ${lexer.index}"))
         }
       }
       lexer.matchString("notEmpty") -> {
@@ -144,13 +155,13 @@ class MatcherDefinitionParser(private val lexer: MatcherDefinitionLexer) {
                 Result.Ok(MatchingRuleDefinition(primitiveValueResult.value.first, NotEmptyMatcher, null)
                   .withType(primitiveValueResult.value.second))
               } else {
-                Result.Err("Was expecting a ')' at index ${lexer.index}")
+                Result.Err(parseError("Was expecting a ')' at index ${lexer.index}"))
               }
             }
             is Result.Err -> return primitiveValueResult
           }
         } else {
-          Result.Err("Was expecting a '(' at index ${lexer.index}")
+          Result.Err(parseError("Was expecting a '(' at index ${lexer.index}"))
         }
       }
       lexer.matchString("eachKey") -> {
@@ -160,13 +171,13 @@ class MatcherDefinitionParser(private val lexer: MatcherDefinitionLexer) {
               if (matchChar(')')) {
                 Result.Ok(MatchingRuleDefinition(null, EachKeyMatcher(definitionResult.value), null))
               } else {
-                Result.Err("Was expecting a ')' at index ${lexer.index}")
+                Result.Err(parseError("Was expecting a ')' at index ${lexer.index}"))
               }
             }
             is Result.Err -> return definitionResult
           }
         } else {
-          Result.Err("Was expecting a '(' at index ${lexer.index}")
+          Result.Err(parseError("Was expecting a '(' at index ${lexer.index}"))
         }
       }
       lexer.matchString("eachValue") -> {
@@ -177,13 +188,13 @@ class MatcherDefinitionParser(private val lexer: MatcherDefinitionLexer) {
                 Result.Ok(MatchingRuleDefinition(null, ValueType.Unknown,
                   listOf(Either.A(EachValueMatcher(definitionResult.value))), null))
               } else {
-                Result.Err("Was expecting a ')' at index ${lexer.index}")
+                Result.Err(parseError("Was expecting a ')' at index ${lexer.index}"))
               }
             }
             is Result.Err -> return definitionResult
           }
         } else {
-          Result.Err("Was expecting a '(' at index ${lexer.index}")
+          Result.Err(parseError("Was expecting a '(' at index ${lexer.index}"))
         }
       }
       lexer.matchString("atLeast") -> {
@@ -193,13 +204,13 @@ class MatcherDefinitionParser(private val lexer: MatcherDefinitionLexer) {
               if (matchChar(')')) {
                 Result.Ok(MatchingRuleDefinition("", MinTypeMatcher(lengthResult.value), null))
               } else {
-                Result.Err("Was expecting a ')' at index ${lexer.index}")
+                Result.Err(parseError("Was expecting a ')' at index ${lexer.index}"))
               }
             }
             is Result.Err -> return lengthResult
           }
         } else {
-          Result.Err("Was expecting a '(' at index ${lexer.index}")
+          Result.Err(parseError("Was expecting a '(' at index ${lexer.index}"))
         }
       }
       lexer.matchString("atMost") -> {
@@ -209,16 +220,16 @@ class MatcherDefinitionParser(private val lexer: MatcherDefinitionLexer) {
               if (matchChar(')')) {
                 Result.Ok(MatchingRuleDefinition("", MaxTypeMatcher(lengthResult.value), null))
               } else {
-                Result.Err("Was expecting a ')' at index ${lexer.index}")
+                Result.Err(parseError("Was expecting a ')' at index ${lexer.index}"))
               }
             }
             is Result.Err -> return lengthResult
           }
         } else {
-          Result.Err("Was expecting a '(' at index ${lexer.index}")
+          Result.Err(parseError("Was expecting a '(' at index ${lexer.index}"))
         }
       }
-      else -> Result.Err("Was expecting a matching rule definition type at index ${lexer.index}")
+      else -> Result.Err(parseError("Was expecting a matching rule definition type at index ${lexer.index}"))
     }
   }
 

diff --git a/...in/kotlin/au/com/dius/pact/core/model/matchingrules/expressions/MatchingRuleDefinition.kt b/...in/kotlin/au/com/dius/pact/core/model/matchingrules/expressions/MatchingRuleDefinition.kt
@@ -120,15 +120,20 @@ data class MatchingRuleDefinition(
      */
     @JvmStatic
     fun parseMatchingRuleDefinition(expression: String): Result<MatchingRuleDefinition, String> {
-      val lexer = MatcherDefinitionLexer(expression)
-      val parser = MatcherDefinitionParser(lexer)
-      return when (val result = parser.matchingDefinition()) {
-        is Result.Ok -> if (result.value == null) {
-          Result.Err("Error parsing expression")
-        } else {
-          Result.Ok(result.value!!)
+      return if (expression.isEmpty()) {
+        Result.Err("Error parsing expression: expression is empty")
+      } else {
+        val lexer = MatcherDefinitionLexer(expression)
+        val parser = MatcherDefinitionParser(lexer)
+        when (val result = parser.matchingDefinition()) {
+          is Result.Ok -> if (result.value == null) {
+            Result.Err("Error parsing expression")
+          } else {
+            Result.Ok(result.value!!)
+          }
+
+          is Result.Err -> Result.Err("Error parsing expression: ${result.error}")
         }
-        is Result.Err -> Result.Err("Error parsing expression: ${result.error}")
       }
     }
   }

diff --git a/...au/com/dius/pact/core/model/matchingrules/expressions/MatchingDefinitionParserSpec.groovy b/...au/com/dius/pact/core/model/matchingrules/expressions/MatchingDefinitionParserSpec.groovy
@@ -21,15 +21,14 @@ import spock.lang.Specification
 class MatchingDefinitionParserSpec extends Specification {
   def 'if the string does not start with a valid matching definition'() {
     expect:
-    MatchingRuleDefinition.parseMatchingRuleDefinition(expression) instanceof Result.Err
+    MatchingRuleDefinition.parseMatchingRuleDefinition(expression).errorValue() == message
 
     where:
 
-    expression << [
-      '',
-      'a, b, c',
-      'matching some other text'
-    ]
+    expression                 | message
+    ''                         | 'Error parsing expression: expression is empty'
+    'a, b, c'                  | 'Error parsing expression: Was expecting a matching rule definition type at index 0\n        a, b, c\n        ^'
+    'matching some other text' | 'Error parsing expression: Was expecting a \'(\' at index 9\n        matching some other text\n                 ^'
   }
 
   def 'parse type matcher'() {
@@ -62,13 +61,12 @@ class MatchingDefinitionParserSpec extends Specification {
 
   def 'invalid number matcher'() {
     expect:
-    MatchingRuleDefinition.parseMatchingRuleDefinition(expression) instanceof Result.Err
+    MatchingRuleDefinition.parseMatchingRuleDefinition(expression).errorValue() == error
 
     where:
 
-    expression << [
-      'matching(integer,100.101)'
-    ]
+    expression | error
+    'matching(integer,100.101)' | 'Error parsing expression: Was expecting a \')\' at index 20\n        matching(integer,100.101)\n                            ^'
   }
 
   def 'parse datetime matcher'() {
@@ -238,12 +236,12 @@ class MatchingDefinitionParserSpec extends Specification {
     where:
 
     expression     | error
-    'atLeast'      | 'Error parsing expression: Was expecting a \'(\' at index 7'
+    'atLeast'      | 'Error parsing expression: Was expecting a \'(\' at index 7\n        atLeast\n               ^'
     'atLeast('     | 'Error parsing expression: Was expecting an unsigned number at index 8'
     'atLeast()'    | 'Error parsing expression: Was expecting an unsigned number at index 8'
-    'atLeast(100'  | 'Error parsing expression: Was expecting a \')\' at index 11'
+    'atLeast(100'  | 'Error parsing expression: Was expecting a \')\' at index 11\n        atLeast(100\n                   ^'
     'atLeast(-10)' | 'Error parsing expression: Was expecting an unsigned number at index 8'
-    'atLeast(0.1)' | 'Error parsing expression: Was expecting a \')\' at index 9'
+    'atLeast(0.1)' | 'Error parsing expression: Was expecting a \')\' at index 9\n        atLeast(0.1)\n                 ^'
   }
 
   def 'parse atMost matcher'() {
@@ -265,11 +263,11 @@ class MatchingDefinitionParserSpec extends Specification {
     where:
 
     expression    | error
-    'atMost'      | 'Error parsing expression: Was expecting a \'(\' at index 6'
+    'atMost'      | 'Error parsing expression: Was expecting a \'(\' at index 6\n        atMost\n              ^'
     'atMost('     | 'Error parsing expression: Was expecting an unsigned number at index 7'
     'atMost()'    | 'Error parsing expression: Was expecting an unsigned number at index 7'
-    'atMost(100'  | 'Error parsing expression: Was expecting a \')\' at index 10'
+    'atMost(100'  | 'Error parsing expression: Was expecting a \')\' at index 10\n        atMost(100\n                  ^'
     'atMost(-10)' | 'Error parsing expression: Was expecting an unsigned number at index 7'
-    'atMost(0.1)' | 'Error parsing expression: Was expecting a \')\' at index 8'
+    'atMost(0.1)' | 'Error parsing expression: Was expecting a \')\' at index 8\n        atMost(0.1)\n                ^'
   }
 }
diff --git a/core/support/src/main/kotlin/au/com/dius/pact/core/support/parsers/StringLexer.kt b/core/support/src/main/kotlin/au/com/dius/pact/core/support/parsers/StringLexer.kt
@@ -2,7 +2,7 @@ package au.com.dius.pact.core.support.parsers
 
 import au.com.dius.pact.core.support.Result
 
-open class StringLexer(private val buffer: String) {
+open class StringLexer(val buffer: String) {
   var index = 0
     private set