parsers: keep Choice alternatives in order

src/Ramstack.Parsing/Parser.Choice.cs

@@ -40,28 +40,34 @@ public static Parser<T> Choice<T>(params Parser<T>[] parsers)
 
         if (typeof(T) == typeof(char) || typeof(T) == typeof(Unit))
         {
-            var count = 0;
-            foreach (var parser in list)
-                if (parser is ICharClassSupport)
-                    count++;
-
-            if (count > 1)
+            // Merge only contiguous runs of character class parsers and keep each
+            // merged parser in place. Merging character classes across a regular
+            // parser would move them ahead of it and change the ordered choice:
+            // the alternative that used to win would lose to a later character class.
+            for (var i = 0; i < list.Count; i++)
             {
+                if (list[i] is not ICharClassSupport)
+                    continue;
+
+                var j = i + 1;
+                while (j < list.Count && list[j] is ICharClassSupport)
+                    j++;
+
+                if (j - i == 1)
+                    continue;
+
                 var @class = new CharClass(CharClassUnicodeCategory.Create(0));
-                for (var i = list.Count - 1; i >= 0; i--)
+                for (var k = i; k < j; k++)
                 {
-                    if (list[i] is ICharClassSupport s)
-                    {
-                        @class = @class.MergeClasses(s.GetCharClass());
-                        list.RemoveAt(i);
-                    }
+                    var s = (ICharClassSupport)list[k];
+                    @class = @class.MergeClasses(s.GetCharClass());
                 }
 
                 var p = Set(@class);
-                list.Insert(0,
-                    (Parser<T>)(object)(
-                        typeof(T) == typeof(Unit) ? p.Void() : p)
-                        );
+                list[i] = (Parser<T>)(object)(
+                    typeof(T) == typeof(Unit) ? p.Void() : p);
+
+                list.RemoveRange(i + 1, j - i - 1);
             }
         }
 

src/Ramstack.Parsing/Utilities/CharClassParser.cs

@@ -19,7 +19,7 @@ public static CharClass Parse(string pattern)
         // if (pattern is ['[', .., ']'])
         //     pattern = pattern[1..^1];
 
-        Argument.ThrowIfNullOrEmpty(nameof(pattern));
+        Argument.ThrowIfNullOrEmpty(pattern);
 
         var elements = new List<CharClassElement>();
         var p = 0;

tests/Ramstack.Parsing.Tests/ParsersTests.Choice.cs

@@ -131,4 +131,34 @@ public void Choice_NestedParsers_NonCharValue_FlattensTheWholeTree()
         Assert.That(parsers, Has.None.SameAs(p2));
         Assert.That(p3.Parse("d").Value, Is.EqualTo(4));
     }
+
+    [Test]
+    public void Choice_LongerLiteralBeforeCharClass_KeepsAlternativeOrder()
+    {
+        var parser = Choice(
+            L("==").Void(),
+            L('=').Void(),
+            L('!').Void());
+
+        Assert.That(parser.Parse("==").Length, Is.EqualTo(2));
+        Assert.That(parser.Parse("==").Success, Is.True);
+
+        Assert.That(parser.Parse("=").Length, Is.EqualTo(1));
+        Assert.That(parser.Parse("!").Length, Is.EqualTo(1));
+    }
+
+    [Test]
+    public void Choice_NonCharClassParserBetweenCharClasses_KeepsAlternativeOrder()
+    {
+        var parser = Choice(
+            L('!').Void(),
+            L("==").Void(),
+            L('=').Void());
+
+        Assert.That(parser.Parse("==").Length, Is.EqualTo(2));
+        Assert.That(parser.Parse("==").Success, Is.True);
+
+        Assert.That(parser.Parse("!").Length, Is.EqualTo(1));
+        Assert.That(parser.Parse("=").Length, Is.EqualTo(1));
+    }
 }

tests/Ramstack.Parsing.Tests/ParsersTests.Choice_Opt.cs

@@ -42,13 +42,16 @@ public void Choice_CharClass_ShouldMerged_2()
         Assert.That(parser.GetType().ToString(), Is.EqualTo("Ramstack.Parsing.Parser+DeferredDiagnosticChoiceParser`1[System.Char]"));
 
         var parsers = (Parser<char>[])parser.GetType().GetField("_parsers", BindingFlags.Instance | BindingFlags.NonPublic)!.GetValue(parser)!;
-        Assert.That(parsers.Length, Is.EqualTo(2));
+        Assert.That(parsers.Length, Is.EqualTo(3));
 
-        Assert.That(parsers[0].Name, Is.EqualTo(@"[0-9a-z\p{Nd}]"));
-        Assert.That(parsers[0].GetType().ToString(), Is.EqualTo("Ramstack.Parsing.Parser+RangeParser`2[System.Char,Ramstack.Parsing.Parser+BitVectorSearcher`1[Ramstack.Parsing.Utilities.Block128Bit]]"));
+        Assert.That(parsers[0].Name, Is.EqualTo(@"[a-z]"));
+        Assert.That(parsers[0].GetType().ToString(), Is.EqualTo("Ramstack.Parsing.Parser+RangeParser`2[System.Char,Ramstack.Parsing.Parser+RangeSearcher]"));
 
         Assert.That(parsers[1].Name, Is.EqualTo(@"escape sequence"));
         Assert.That(parsers[1].GetType().ToString(), Is.EqualTo("Ramstack.Parsing.Literal+EscapeSequenceParser`1[System.Char]"));
+
+        Assert.That(parsers[2].Name, Is.EqualTo(@"[0-9a-g\p{Nd}]"));
+        Assert.That(parsers[2].GetType().ToString(), Is.EqualTo("Ramstack.Parsing.Parser+RangeParser`2[System.Char,Ramstack.Parsing.Parser+BitVectorSearcher`1[Ramstack.Parsing.Utilities.Block128Bit]]"));
     }
 
     [Test]
@@ -70,12 +73,21 @@ public void Choice_CharClass_ShouldMerged_3()
 
         var parsers = (Parser<char>[])parser.GetType().GetProperty("Parsers", BindingFlags.Instance | BindingFlags.Public)!.GetValue(parser)!;
 
-        Assert.That(parsers.Length, Is.EqualTo(3));
+        Assert.That(parsers.Length, Is.EqualTo(5));
+
+        Assert.That(parsers[0].Name, Is.EqualTo(@"[a-z]"));
+        Assert.That(parsers[0].GetType().ToString(), Is.EqualTo("Ramstack.Parsing.Parser+RangeParser`2[System.Char,Ramstack.Parsing.Parser+RangeSearcher]"));
 
-        Assert.That(parsers[0].Name, Is.EqualTo(@"[0-9a-z\p{Nd}]"));
-        Assert.That(parsers[0].GetType().ToString(), Is.EqualTo("Ramstack.Parsing.Parser+RangeParser`2[System.Char,Ramstack.Parsing.Parser+BitVectorSearcher`1[Ramstack.Parsing.Utilities.Block128Bit]]"));
         Assert.That(parsers[1].GetType().ToString(), Is.EqualTo("Ramstack.Parsing.Parser+ThenParser`1[System.Char]"));
-        Assert.That(parsers[2].GetType().ToString(), Is.EqualTo("Ramstack.Parsing.Literal+UnicodeEscapeSequenceParser`1[System.Char]"));
+
+        Assert.That(parsers[2].Name, Is.EqualTo(@"[0-7]"));
+        Assert.That(parsers[2].GetType().ToString(), Is.EqualTo("Ramstack.Parsing.Parser+RangeParser`2[System.Char,Ramstack.Parsing.Parser+RangeSearcher]"));
+
+        Assert.That(parsers[3].Name, Is.EqualTo(@"unicode escape"));
+        Assert.That(parsers[3].GetType().ToString(), Is.EqualTo("Ramstack.Parsing.Literal+UnicodeEscapeSequenceParser`1[System.Char]"));
+
+        Assert.That(parsers[4].Name, Is.EqualTo(@"[6-9a-g\p{Nd}]"));
+        Assert.That(parsers[4].GetType().ToString(), Is.EqualTo("Ramstack.Parsing.Parser+RangeParser`2[System.Char,Ramstack.Parsing.Parser+BitVectorSearcher`1[Ramstack.Parsing.Utilities.Block128Bit]]"));
     }
 
     [Test]