From b514b85f39c2462c86ac19e1aced1ba27a0620a6 Mon Sep 17 00:00:00 2001
From: Eike Kettner <eike.kettner@posteo.de>
Date: Mon, 8 Mar 2021 10:26:39 +0100
Subject: [PATCH] Improve parser error messages a bit

---
 .../docspell/query/js/JSItemQueryParser.scala |  2 +-
 .../scala/docspell/query/ParseFailure.scala   | 57 ++++++++++++++-----
 2 files changed, 45 insertions(+), 14 deletions(-)

diff --git a/modules/query/js/src/main/scala/docspell/query/js/JSItemQueryParser.scala b/modules/query/js/src/main/scala/docspell/query/js/JSItemQueryParser.scala
index 68f7cd5e..a58371cd 100644
--- a/modules/query/js/src/main/scala/docspell/query/js/JSItemQueryParser.scala
+++ b/modules/query/js/src/main/scala/docspell/query/js/JSItemQueryParser.scala
@@ -18,7 +18,7 @@ object JSItemQueryParser {
         new Failure(
           fr.input,
           fr.failedAt,
-          js.Array(fr.messages.toList.toSeq.map(_.msg): _*)
+          js.Array(fr.messages.toList.toSeq.map(_.render): _*)
         )
       )
       .orNull
diff --git a/modules/query/shared/src/main/scala/docspell/query/ParseFailure.scala b/modules/query/shared/src/main/scala/docspell/query/ParseFailure.scala
index 05235c03..4562d68c 100644
--- a/modules/query/shared/src/main/scala/docspell/query/ParseFailure.scala
+++ b/modules/query/shared/src/main/scala/docspell/query/ParseFailure.scala
@@ -18,48 +18,79 @@ final case class ParseFailure(
 ) {
 
   def render: String = {
-    val items = messages.map(_.msg).toList.mkString(", ")
+    val items = messages.map(_.render).toList.mkString(", ")
     s"Failed to read input near $failedAt: $input\nDetails: $items"
   }
 }
 
 object ParseFailure {
 
-  final case class Message(offset: Int, msg: String)
+  sealed trait Message {
+    def offset: Int
+    def render: String
+  }
+  final case class SimpleMessage(offset: Int, msg: String) extends Message {
+    def render: String =
+       s"Failed at $offset: $msg"
+  }
+  final case class ExpectMessage(offset: Int, expected: List[String], exhaustive: Boolean) extends Message {
+    def render: String = {
+      val opts = expected.mkString(", ")
+      val dots = if (exhaustive) "" else "…"
+      s"Expected: ${opts}${dots}"
+    }
+  }
 
   private[query] def fromError(input: String)(pe: Parser.Error): ParseFailure =
     ParseFailure(
       input,
       pe.failedAtOffset,
-      Parser.Expectation.unify(pe.expected).map(expectationToMsg)
+      packMsg(Parser.Expectation.unify(pe.expected).map(expectationToMsg))
     )
 
+  private[query] def packMsg(msg: Nel[Message]): Nel[Message] = {
+    val expectMsg = combineExpected(msg.collect({ case em: ExpectMessage => em }))
+      .sortBy(_.offset).headOption
+
+    val simpleMsg = msg.collect({ case sm: SimpleMessage => sm })
+
+    Nel.fromListUnsafe((simpleMsg ++ expectMsg).sortBy(_.offset))
+  }
+
+  private[query] def combineExpected(msg: List[ExpectMessage]): List[ExpectMessage] =
+    msg.groupBy(_.offset).map({ case (offset, es) =>
+      ExpectMessage(offset, es.flatMap(_.expected).distinct.sorted, es.forall(_.exhaustive))
+    }).toList
+
   private[query] def expectationToMsg(e: Parser.Expectation): Message =
     e match {
       case StartOfString(offset) =>
-        Message(offset, "Expected start of string")
+        SimpleMessage(offset, "Expected start of string")
 
       case FailWith(offset, message) =>
-        Message(offset, message)
+        SimpleMessage(offset, message)
 
       case InRange(offset, lower, upper) =>
-        if (lower == upper) Message(offset, s"Expected character: $lower")
-        else Message(offset, s"Expected character from range: [$lower .. $upper]")
+        if (lower == upper) ExpectMessage(offset, List(lower.toString), true)
+        else {
+          val expect = s"${lower}-${upper}"
+          ExpectMessage(offset, List(expect), true)
+        }
 
       case Length(offset, expected, actual) =>
-        Message(offset, s"Expected input of length $expected, but got $actual")
+        SimpleMessage(offset, s"Expected input of length $expected, but got $actual")
 
       case ExpectedFailureAt(offset, matched) =>
-        Message(offset, s"Expected failing, but matched '$matched'")
+        SimpleMessage(offset, s"Expected failing, but matched '$matched'")
 
       case EndOfString(offset, length) =>
-        Message(offset, s"Expected end of string at length: $length")
+        SimpleMessage(offset, s"Expected end of string at length: $length")
 
       case Fail(offset) =>
-        Message(offset, s"Failed to parse near $offset")
+        SimpleMessage(offset, s"Failed to parse near $offset")
 
       case OneOfStr(offset, strs) =>
-        val options = strs.mkString(", ")
-        Message(offset, s"Expected one of the following strings: $options")
+        val options = strs.take(8)
+        ExpectMessage(offset, options.take(7), options.size < 8)
     }
 }