parsers: stop Choice from looping on a nested choice parser

src/Ramstack.Parsing/Parser.Choice.cs

@@ -18,24 +18,21 @@ public static Parser<T> Choice<T>(params Parser<T>[] parsers)
 
         var list = new List<Parser<T>>();
 
-        while (true)
+        foreach (var parser in parsers)
         {
-            foreach (var parser in parsers)
+            switch (parser)
             {
-                switch (parser)
-                {
-                    case ChoiceParser<T> p:
-                        list.AddRange(p.Parsers);
-                        break;
+                // A named choice is a diagnostics boundary and stays a single alternative.
+                // Nested unnamed choices were flattened when they were created,
+                // so one pass expands the whole tree.
+                case ChoiceParser<T> { Name: null } p:
+                    list.AddRange(p.Parsers);
+                    break;
 
-                    default:
-                        list.Add(parser);
-                        break;
-                }
+                default:
+                    list.Add(parser);
+                    break;
             }
-
-            if (list.Count == parsers.Length)
-                break;
         }
 
         if (list.Count == 1)
@@ -66,10 +63,10 @@ public static Parser<T> Choice<T>(params Parser<T>[] parsers)
                         typeof(T) == typeof(Unit) ? p.Void() : p)
                         );
             }
-
-            parsers = list.ToArray();
         }
 
+        parsers = list.ToArray();
+
         if (parsers.Length == 1)
             return parsers[0];
 

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

@@ -1,3 +1,5 @@
+using System.Reflection;
+
 using static Ramstack.Parsing.Parser;
 
 namespace Ramstack.Parsing;
@@ -69,4 +71,64 @@ public void Choice_UnnamedParser_ReportsAlternatives()
             parser.Parse("?").ErrorMessage,
             Is.EqualTo("(1:1) Expected 'a' or '('"));
     }
+
+    [Test]
+    public void Choice_NestedParser_FlattensAlternatives()
+    {
+        var parser1 =
+            Choice(
+                Choice(
+                    L('a'),
+                    L('b').Between(L('('), L(')'))),
+            L('c'));
+
+        var parser2 =
+            L('a')
+                .Or(L('b').Between(L('('), L(')')))
+                .Or(L('c'));
+
+        Assert.That(parser1.Parse("a").Value, Is.EqualTo('a'));
+        Assert.That(parser1.Parse("(b)").Value, Is.EqualTo('b'));
+        Assert.That(parser1.Parse("c").Value, Is.EqualTo('c'));
+
+        Assert.That(parser2.Parse("a").Value, Is.EqualTo('a'));
+        Assert.That(parser2.Parse("(b)").Value, Is.EqualTo('b'));
+        Assert.That(parser2.Parse("c").Value, Is.EqualTo('c'));
+    }
+
+    [Test]
+    public void Choice_NestedNamedParser_KeepsName()
+    {
+        var p = Choice(L('a'), L('b').Between(L('('), L(')'))).As("letter");
+        var parser = Choice(p, L('c'));
+
+        Assert.That(
+            parser.Parse("?").ErrorMessage,
+            Is.EqualTo("(1:1) Expected letter or 'c'"));
+    }
+
+    [Test]
+    public void Choice_NestedParsers_NonCharValue_FlattensTheWholeTree()
+    {
+        var p1 = Choice(
+            L("a").Do(_ => 1),
+            L("b").Do(_ => 2));
+
+        var p2 = Choice(
+            p1,
+            L("c").Do(_ => 3));
+
+        var p3 = Choice(
+            L("d").Do(_ => 4),
+            p2);
+
+        var parsers = (Parser<int>[])p3.GetType()
+            .GetProperty("Parsers", BindingFlags.Instance | BindingFlags.Public)!
+            .GetValue(p3)!;
+
+        Assert.That(parsers.Length, Is.EqualTo(4));
+        Assert.That(parsers, Has.None.SameAs(p1));
+        Assert.That(parsers, Has.None.SameAs(p2));
+        Assert.That(p3.Parse("d").Value, Is.EqualTo(4));
+    }
 }