Meaningful oneOf and noneOf error labels (#106)

* Added more meaningful error messages for oneOf and noneOf

* Fixed scalaJS unicode regex

build.sbt

@@ -1,5 +1,6 @@
 import scala.collection.mutable
 import sbtcrossproject.Platform
+import org.scalajs.linker.interface.ESVersion
 
 val projectName = "parsley"
 
@@ -53,6 +54,7 @@ Global / onChangedBuildSource := ReloadOnSourceChanges
 
 // See https://github.com/sbt/sbt/issues/1224
 Global / onLoad ~= (_ andThen ("project parsley" :: _))
+//scalaJSLinkerConfig ~= { _.withESFeatures(_.withESVersion(ESVersion.ES2018)) }
 
 Compile / bloopGenerate := None
 Test / bloopGenerate := None
@@ -85,7 +87,8 @@ lazy val parsley = crossProject(JSPlatform, JVMPlatform, NativePlatform)
   .jsSettings(
     crossScalaVersions := List(scala212Version, scala213Version, scala3Version),
     Compile / bloopGenerate := None,
-    Test / bloopGenerate := None
+    Test / bloopGenerate := None,
+    Test / scalaJSLinkerConfig := scalaJSLinkerConfig.value.withESFeatures(_.withESVersion(ESVersion.ES2018))
   )
   .nativeSettings(
     crossScalaVersions := List(scala212Version, scala213Version),

src/main/scala/parsley/character.scala

@@ -1,6 +1,6 @@
 package parsley
 
-import parsley.Parsley.{LazyParsley}
+import parsley.Parsley.{LazyParsley, empty}
 import parsley.combinator.skipMany
 import parsley.implicits.character.charLift
 import parsley.internal.deepembedding
@@ -38,28 +38,56 @@ object character
 
     /**`oneOf(cs)` succeeds if the current character is in the supplied set of characters `cs`.
       * Returns the parsed character. See also `satisfy`.*/
-    def oneOf(cs: Set[Char]): Parsley[Char] = satisfy(cs.contains)
-
-    /**As the dual of `oneOf`, `noneOf(cs)` succeeds if the current character is not in the supplied
-      * set of characters `cs`. Returns the parsed character.*/
-    def noneOf(cs: Set[Char]): Parsley[Char] = satisfy(!cs.contains(_))
+    def oneOf(cs: Set[Char]): Parsley[Char] = cs.size match {
+        case 0 => empty
+        case 1 => char(cs.head)
+        case _ => satisfy(cs.contains).label {
+            val Some(label) = parsley.errors.helpers.combineAsList(cs.map(renderChar).toList)
+            s"one of $label"
+        }
+    }
 
     /**`oneOf(cs)` succeeds if the current character is in the supplied sequence of characters `cs`.
       * Returns the parsed character. See also `satisfy`.*/
     def oneOf(cs: Char*): Parsley[Char] = oneOf(cs.toSet)
 
     /**`oneOf(cs)` succeeds if the current character is in the supplied sequence of characters `cs`.
       * Returns the parsed character. See also `satisfy`.*/
-    def oneOf(cs: NumericRange[Char]): Parsley[Char] = oneOf(cs.toSet)
+    def oneOf(cs: NumericRange[Char]): Parsley[Char] = cs.size match {
+        case 0 => empty
+        case 1 => char(cs.head)
+        case _ if Math.abs(cs(0).toInt - cs(1).toInt) == 1 => satisfy(cs.contains).label {
+            s"one of ${renderChar(cs.min)} to ${renderChar(cs.max)}"
+        }
+        case _ => satisfy(cs.contains)
+    }
 
     /**As the dual of `oneOf`, `noneOf(cs)` succeeds if the current character is not in the supplied
-      * sequence of characters `cs`. Returns the parsed character.*/
-    def noneOf(cs: NumericRange[Char]): Parsley[Char] = noneOf(cs.toSet)
+      * set of characters `cs`. Returns the parsed character.*/
+    def noneOf(cs: Set[Char]): Parsley[Char] = cs.size match {
+        case 0 => anyChar
+        case 1 => satisfy(cs.head != _).label(s"anything except ${renderChar(cs.head)}")
+        case _ => satisfy(!cs.contains(_)).label {
+            val Some(label) = parsley.errors.helpers.combineAsList(cs.map(renderChar).toList)
+            s"anything except $label"
+        }
+    }
 
     /**As the dual of `oneOf`, `noneOf(cs)` succeeds if the current character is not in the supplied
       * sequence of characters `cs`. Returns the parsed character.*/
     def noneOf(cs: Char*): Parsley[Char] = noneOf(cs.toSet)
 
+    /**As the dual of `oneOf`, `noneOf(cs)` succeeds if the current character is not in the supplied
+      * sequence of characters `cs`. Returns the parsed character.*/
+    def noneOf(cs: NumericRange[Char]): Parsley[Char] = cs.size match {
+        case 0 => anyChar
+        case 1 => satisfy(cs.head != _).label(s"anything except ${renderChar(cs.head)}")
+        case _ if Math.abs(cs(0).toInt - cs(1).toInt) == 1 => satisfy(!cs.contains(_)).label {
+            s"anything outside of ${renderChar(cs.min)} to ${renderChar(cs.max)}"
+        }
+        case _ => satisfy(!cs.contains(_))
+    }
+
     /**The parser `anyChar` accepts any kind of character. Returns the accepted character.*/
     val anyChar: Parsley[Char] = satisfy(_ => true).label("any character")
 
@@ -128,4 +156,7 @@ object character
 
     /** Helper function, equivalent to the predicate used by space. Useful for providing to LanguageDef */
     def isSpace(c: Char): Boolean = c == ' ' || c == '\t'
+
+    // Sue me.
+    private def renderChar(c: Char): String = parsley.errors.helpers.renderRawString(s"$c")
 }

src/main/scala/parsley/errors/DefaultErrorBuilder.scala

@@ -1,7 +1,5 @@
 package parsley.errors
 
-import scala.util.matching.Regex
-
 // Turn coverage off, because the tests have their own error builder
 // We might want to test this on its own though
 // $COVERAGE-OFF$
@@ -58,13 +56,8 @@ class DefaultErrorBuilder extends ErrorBuilder[String] with revisions.Revision2
 
     type ExpectedItems = Option[String]
     type Messages = Seq[Message]
-    override def combineExpectedItems(alts: Set[Item]): ExpectedItems = alts.toList.sorted.reverse.filter(_.nonEmpty) match {
-        case Nil => None
-        case List(alt) => Some(alt)
-        case List(alt1, alt2) => Some(s"$alt2 or $alt1")
-        // If the result would contains "," then it's probably nicer to preserve any potential grouping using ";"
-        case any@(alt::alts) if any.exists(_.contains(",")) => Some(s"${alts.reverse.mkString("; ")}; or $alt")
-        case alt::alts => Some(s"${alts.reverse.mkString(", ")}, or $alt")
+    override def combineExpectedItems(alts: Set[Item]): ExpectedItems = {
+        helpers.combineAsList(alts.toList.filter(_.nonEmpty))
     }
     override def combineMessages(alts: Seq[Message]): Messages = alts.filter(_.nonEmpty)
 
@@ -92,18 +85,10 @@ class DefaultErrorBuilder extends ErrorBuilder[String] with revisions.Revision2
     type Raw = String
     type Named = String
     type EndOfInput = String
-    override def raw(item: String): Raw = item match {
-        case "\n"            => "newline"
-        case "\t"            => "tab"
-        case " "             => "space"
-        case Unprintable(up) => f"unprintable character (\\u${up.head.toInt}%04X)"
-        // Do we want this only in unexpecteds?
-        case cs              => "\"" + cs.takeWhile(c => c != '\n' && c != ' ') + "\""
-    }
+    override def raw(item: String): Raw = helpers.renderRawString(item)
     override def named(item: String): Named = item
     override val endOfInput: EndOfInput = "end of input"
 
-    private val Unprintable: Regex = "(\\p{C})".r
     private val Unknown = "unknown parse error"
 }
 // $COVERAGE-ON$

src/main/scala/parsley/errors/helpers.scala

@@ -0,0 +1,25 @@
+package parsley.errors
+
+import scala.util.matching.Regex
+
+private [parsley] object helpers {
+    def renderRawString(s: String): String = s match {
+        case "\n"            => "newline"
+        case "\t"            => "tab"
+        case " "             => "space"
+        case Unprintable(up) => f"unprintable character (\\u${up.head.toInt}%04X)"
+        // Do we want this only in unexpecteds?
+        case cs              => "\"" + cs.takeWhile(c => c != '\n' && c != ' ') + "\""
+    }
+
+    def combineAsList(elems: List[String]): Option[String] = elems.sorted.reverse match {
+        case Nil => None
+        case List(alt) => Some(alt)
+        case List(alt1, alt2) => Some(s"$alt2 or $alt1")
+        // If the result would contains "," then it's probably nicer to preserve any potential grouping using ";"
+        case any@(alt::alts) if any.exists(_.contains(",")) => Some(s"${alts.reverse.mkString("; ")}; or $alt")
+        case alt::alts => Some(s"${alts.reverse.mkString(", ")}, or $alt")
+    }
+
+    private val Unprintable: Regex = "(\\p{C})".r
+}

src/main/scala/parsley/internal/deepembedding/ErrorEmbedding.scala

@@ -15,6 +15,7 @@ private [parsley] final class ErrorLabel[A](_p: =>Parsley[A], private [ErrorLabe
         case ct@CharTok(c) if !ct.expected.contains("") => new CharTok(c, Some(label)).asInstanceOf[Parsley[A]]
         case st@StringTok(s) if !st.expected.contains("") => new StringTok(s, Some(label)).asInstanceOf[Parsley[A]]
         case sat@Satisfy(f) if !sat.expected.contains("") => new Satisfy(f, Some(label)).asInstanceOf[Parsley[A]]
+        // TOOD: The hide property is required to be checked, but there is no test for it
         case ErrorLabel(p, label2) if label2 != "" => ErrorLabel(p, label)
         case _ => this
     }

src/main/scala/parsley/internal/machine/instructions/TokenInstrs.scala

@@ -78,7 +78,8 @@ private [instructions] abstract class WhiteSpaceLike(start: String, end: String,
     }
 
     private final val impl = {
-        if (!lineAllowed) multisOnly(_)
+        if (!lineAllowed && !multiAllowed) spaces(_)
+        else if (!lineAllowed) multisOnly(_)
         else if (!multiAllowed) singlesOnly(_)
         else singlesAndMultis(_)
     }

src/test/scala/parsley/ErrorTests.scala

@@ -3,7 +3,7 @@ package parsley
 import parsley.combinator.{eof, optional}
 import parsley.Parsley._
 import parsley.implicits.character.{charLift, stringLift}
-import parsley.character.{anyChar, digit}
+import parsley.character, character.{anyChar, digit}
 import parsley.unsafe.ErrorLabel
 import parsley.errors.combinator.{fail => pfail, unexpected, amend, entrench, ErrorMethods}
 
@@ -309,4 +309,32 @@ class ErrorTests extends ParsleyTest {
         inside(p.parse("abcd")) { case Failure(TestError((1, 5), _)) => }
         inside(p.parse("abcde")) { case Failure(TestError((1, 2), _)) => }
     }
+
+    "oneOf" should "incorporate range notation into the error" in {
+        inside(character.oneOf('0' to '9').parse("a")) { 
+            case Failure(TestError(_, VanillaError(_, expecteds, _))) => 
+                expecteds should contain only Named("one of \"0\" to \"9\"")
+        }
+    }
+
+    it should "incorporate sets of characters into the error" in {
+        inside(character.oneOf(('0' to '9').toSet).parse("a")) { 
+            case Failure(TestError(_, VanillaError(_, expecteds, _))) => 
+                expecteds should contain only Named("one of \"0\", \"1\", \"2\", \"3\", \"4\", \"5\", \"6\", \"7\", \"8\", or \"9\"")
+        }
+    }
+
+    "noneOf" should "incorporate range notation into the error" in {
+        inside(character.noneOf('0' to '9').parse("8")) { 
+            case Failure(TestError(_, VanillaError(_, expecteds, _))) => 
+                expecteds should contain only Named("anything outside of \"0\" to \"9\"")
+        }
+    }
+
+    it should "incorporate sets of characters into the error" in {
+        inside(character.noneOf(('0' to '9').toSet).parse("8")) { 
+            case Failure(TestError(_, VanillaError(_, expecteds, _))) => 
+                expecteds should contain only Named("anything except \"0\", \"1\", \"2\", \"3\", \"4\", \"5\", \"6\", \"7\", \"8\", or \"9\"")
+        }
+    }
 }