Fix Separated parser (#55)

Fixes #48
sebastienros · Nov 27, 2021 · 1b2fe20 · 1b2fe20
1 parent b306212
commit 1b2fe20
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 21 deletions.
diff --git a/Parlot.sln b/Parlot.sln
@@ -1,7 +1,7 @@
 
 Microsoft Visual Studio Solution File, Format Version 12.00
-# Visual Studio Version 16
-VisualStudioVersion = 16.0.30709.64
+# Visual Studio Version 17
+VisualStudioVersion = 17.1.31903.286
 MinimumVisualStudioVersion = 15.0.26124.0
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{0D1E6480-3C81-4951-8F44-BF74398BA8D4}"
 EndProject
@@ -16,6 +16,8 @@ EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{0EA81138-0646-4AFD-96C6-83A94D568C83}"
 	ProjectSection(SolutionItems) = preProject
 		.editorconfig = .editorconfig
+		.github\workflows\build.yml = .github\workflows\build.yml
+		.github\workflows\publish.yml = .github\workflows\publish.yml
 		README.md = README.md
 	EndProjectSection
 EndProject

diff --git a/docs/parsers.md b/docs/parsers.md
@@ -438,7 +438,7 @@ null // success
 
 ### Separated
 
-Matches all occurrences of a parser that are separated by another one.
+Matches all occurrences of a parser that are separated by another one. If a separator is not followed by a value, it is not consumed.
 
 ```
 Parser<List<T>> Separated<U, T>(Parser<U> separator, Parser<T> parser)

diff --git a/src/Parlot/Fluent/Separated.cs b/src/Parlot/Fluent/Separated.cs
@@ -23,42 +23,50 @@ public override bool Parse(ParseContext context, ref ParseResult<List<T>> result
             List<T> results = null;
 
             var start = 0;
-            var end = 0;
+            var end = context.Scanner.Cursor.Position;
 
             var first = true;
             var parsed = new ParseResult<T>();
             var separatorResult = new ParseResult<U>();
 
             while (true)
             {
+                if (!first)
+                {
+                    if (!_separator.Parse(context, ref separatorResult))
+                    {
+                        break;
+                    }
+                }
+
                 if (!_parser.Parse(context, ref parsed))
                 {
                     if (!first)
                     {
+                        // A separator was found, but not followed by another value.
+                        // It's still succesful if there was one value parsed, but we reset the cursor to before the separator
+                        context.Scanner.Cursor.ResetPosition(end);
                         break;
                     }
 
-                    // A parser that returns false is reponsible for resetting the position.
-                    // Nothing to do here since the inner parser is already failing and resetting it.
                     return false;
                 }
-
+                else
+                {
+                    end = context.Scanner.Cursor.Position;
+                }
+
                 if (first)
                 {
+                    results = new List<T>();
                     start = parsed.Start;
+                    first = false;
                 }
-
-                end = parsed.End;
-                results ??= new List<T>();
+
                 results.Add(parsed.Value);
-
-                if (!_separator.Parse(context, ref separatorResult))
-                {
-                    break;
-                }
             }
 
-            result = new ParseResult<List<T>>(start, end, results);
+            result = new ParseResult<List<T>>(start, end.Offset, results);
             return true;
         }
 
@@ -68,6 +76,8 @@ public CompilationResult Compile(CompilationContext context)
 
             var success = context.DeclareSuccessVariable(result, false);
             var value = context.DeclareValueVariable(result, Expression.New(typeof(List<T>)));
+
+            var end = context.DeclarePositionVariable(result);
 
             // value = new List<T>();
             //
@@ -79,6 +89,7 @@ public CompilationResult Compile(CompilationContext context)
             //   {
             //      success = true;
             //      value.Add(parse1.Value);
+            //      end = currenPosition;
             //   }
             //   else
             //   {
@@ -93,6 +104,8 @@ public CompilationResult Compile(CompilationContext context)
             //   }
             // }
             // 
+            // resetPosition(end);
+            // 
 
             var parserCompileResult = _parser.Build(context);
             var breakLabel = Expression.Label("break");
@@ -110,7 +123,8 @@ public CompilationResult Compile(CompilationContext context)
                                 context.DiscardResult
                                 ? Expression.Empty()
                                 : Expression.Call(value, typeof(List<T>).GetMethod("Add"), parserCompileResult.Value),
-                                Expression.Assign(success, Expression.Constant(true))
+                                Expression.Assign(success, Expression.Constant(true)),
+                                Expression.Assign(end, context.Position())
                                 ),
                             Expression.Break(breakLabel)
                             ),
@@ -127,7 +141,8 @@ public CompilationResult Compile(CompilationContext context)
                             Expression.Break(breakLabel)
                             )
                         ),
-                    breakLabel)
+                    breakLabel),
+                context.ResetPosition(end)
                 );
 
             result.Body.Add(block);

diff --git a/test/Parlot.Tests/CompileTests.cs b/test/Parlot.Tests/CompileTests.cs
@@ -210,13 +210,14 @@ public void ShouldCompileBetweens()
         }
 
         [Fact]
-        public void ShouldCompileSeparatedChar()
+        public void ShouldcompiledSeparated()
         {
             var parser = Separated(Terms.Char(','), Terms.Decimal()).Compile();
 
-            Assert.Null(parser.Parse(""));
             Assert.Single(parser.Parse("1"));
+            Assert.Equal(2, parser.Parse("1,2").Count);
             Assert.Null(parser.Parse(",1,"));
+            Assert.Null(parser.Parse(""));
 
             var result = parser.Parse("1, 2,3");
 
@@ -225,6 +226,18 @@ public void ShouldCompileSeparatedChar()
             Assert.Equal(3, result[2]);
         }
 
+        [Fact]
+        public void SeparatedShouldNotBeConsumedIfNotFollowedByValueCompiled()
+        {
+            // This test ensures that the separator is not consumed if there is no valid net value.
+
+            var parser = Separated(Terms.Char(','), Terms.Decimal()).AndSkip(Terms.Char(',')).And(Terms.Identifier()).Then(x => true).Compile();
+
+            Assert.False(parser.Parse("1"));
+            Assert.False(parser.Parse("1,"));
+            Assert.True(parser.Parse("1,x"));
+        }
+
         [Fact]
         public void ShouldCompileExpressionParser()
         {

diff --git a/test/Parlot.Tests/FluentTests.cs b/test/Parlot.Tests/FluentTests.cs
@@ -582,9 +582,10 @@ public void SeparatedShouldSplit()
         {
             var parser = Separated(Terms.Char(','), Terms.Decimal());
 
-            Assert.Null(parser.Parse(""));
             Assert.Single(parser.Parse("1"));
+            Assert.Equal(2, parser.Parse("1,2").Count);
             Assert.Null(parser.Parse(",1,"));
+            Assert.Null(parser.Parse(""));
 
             var result = parser.Parse("1, 2,3");
 
@@ -593,6 +594,18 @@ public void SeparatedShouldSplit()
             Assert.Equal(3, result[2]);
         }
 
+        [Fact]
+        public void SeparatedShouldNotBeConsumedIfNotFollowedByValue()
+        {
+            // This test ensures that the separator is not consumed if there is no valid net value.
+
+            var parser = Separated(Terms.Char(','), Terms.Decimal()).AndSkip(Terms.Char(',')).And(Terms.Identifier()).Then(x => true);
+
+            Assert.False(parser.Parse("1"));
+            Assert.False(parser.Parse("1,"));
+            Assert.True(parser.Parse("1,x"));
+        }
+
         [Fact]
         public void ShouldSkipWhiteSpace()
         {