From 787740484476fac4d68a50de5ff0562e467d25e2 Mon Sep 17 00:00:00 2001 From: Ronald Holshausen Date: Mon, 5 Aug 2024 16:17:47 +1000 Subject: [PATCH] Feat: Improve some of the matching definition parser errors --- .../expressions/MatcherDefinition.kt | 41 ++++++++++++------- .../expressions/MatchingRuleDefinition.kt | 21 ++++++---- .../MatchingDefinitionParserSpec.groovy | 30 +++++++------- .../pact/core/support/parsers/StringLexer.kt | 2 +- 4 files changed, 54 insertions(+), 40 deletions(-) diff --git a/core/model/src/main/kotlin/au/com/dius/pact/core/model/matchingrules/expressions/MatcherDefinition.kt b/core/model/src/main/kotlin/au/com/dius/pact/core/model/matchingrules/expressions/MatcherDefinition.kt index 8aee3e6d7..1817f78fe 100644 --- a/core/model/src/main/kotlin/au/com/dius/pact/core/model/matchingrules/expressions/MatcherDefinition.kt +++ b/core/model/src/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/core/model/src/main/kotlin/au/com/dius/pact/core/model/matchingrules/expressions/MatchingRuleDefinition.kt b/core/model/src/main/kotlin/au/com/dius/pact/core/model/matchingrules/expressions/MatchingRuleDefinition.kt index 13940da8f..7a0d044c6 100644 --- a/core/model/src/main/kotlin/au/com/dius/pact/core/model/matchingrules/expressions/MatchingRuleDefinition.kt +++ b/core/model/src/main/kotlin/au/com/dius/pact/core/model/matchingrules/expressions/MatchingRuleDefinition.kt @@ -120,15 +120,20 @@ data class MatchingRuleDefinition( */ @JvmStatic fun parseMatchingRuleDefinition(expression: String): Result { - 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/core/model/src/test/groovy/au/com/dius/pact/core/model/matchingrules/expressions/MatchingDefinitionParserSpec.groovy b/core/model/src/test/groovy/au/com/dius/pact/core/model/matchingrules/expressions/MatchingDefinitionParserSpec.groovy index 48f3d3728..0a604326d 100644 --- a/core/model/src/test/groovy/au/com/dius/pact/core/model/matchingrules/expressions/MatchingDefinitionParserSpec.groovy +++ b/core/model/src/test/groovy/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 index 206d54138..4388b5183 100644 --- 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