From 3c67094e0823ad84169f6d71981286881744aadb Mon Sep 17 00:00:00 2001 From: rameel Date: Wed, 23 Sep 2026 01:07:36 +0500 Subject: [PATCH 1/2] parsers: keep Choice alternatives in order when merging character classes Merge only contiguous runs of character class parsers and keep each merged parser in its place. Previously every character class was merged into one parser and moved to the front, so a later character class could win before an earlier alternative and consume only one character. --- src/Ramstack.Parsing/Parser.Choice.cs | 38 +++++++++++-------- .../ParsersTests.Choice.cs | 30 +++++++++++++++ .../ParsersTests.Choice_Opt.cs | 26 +++++++++---- 3 files changed, 71 insertions(+), 23 deletions(-) diff --git a/src/Ramstack.Parsing/Parser.Choice.cs b/src/Ramstack.Parsing/Parser.Choice.cs index 313bc79..82295c1 100644 --- a/src/Ramstack.Parsing/Parser.Choice.cs +++ b/src/Ramstack.Parsing/Parser.Choice.cs @@ -40,28 +40,34 @@ public static Parser Choice(params Parser[] 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)(object)( - typeof(T) == typeof(Unit) ? p.Void() : p) - ); + list[i] = (Parser)(object)( + typeof(T) == typeof(Unit) ? p.Void() : p); + + list.RemoveRange(i + 1, j - i - 1); } } diff --git a/tests/Ramstack.Parsing.Tests/ParsersTests.Choice.cs b/tests/Ramstack.Parsing.Tests/ParsersTests.Choice.cs index fe33076..5bac874 100644 --- a/tests/Ramstack.Parsing.Tests/ParsersTests.Choice.cs +++ b/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)); + } } diff --git a/tests/Ramstack.Parsing.Tests/ParsersTests.Choice_Opt.cs b/tests/Ramstack.Parsing.Tests/ParsersTests.Choice_Opt.cs index 0cae6b9..4d12325 100644 --- a/tests/Ramstack.Parsing.Tests/ParsersTests.Choice_Opt.cs +++ b/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[])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[])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] From 75fcf4d8d8222f6ff38083bd8013a76a8433dec0 Mon Sep 17 00:00:00 2001 From: rameel Date: Wed, 23 Sep 2026 01:19:56 +0500 Subject: [PATCH 2/2] characters: check the pattern argument instead of its parameter name --- src/Ramstack.Parsing/Utilities/CharClassParser.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Ramstack.Parsing/Utilities/CharClassParser.cs b/src/Ramstack.Parsing/Utilities/CharClassParser.cs index 4f5ca88..064e4a6 100644 --- a/src/Ramstack.Parsing/Utilities/CharClassParser.cs +++ b/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(); var p = 0;