Merge pull request #30 from Rutherther/fix/timespanparser

TimeSpanParser unintuitive edge cases

Author: Nihlus

Remora.Commands/Parsers/Builtin/TimeSpanParser.cs

@@ -40,7 +40,7 @@ public class TimeSpanParser : AbstractTypeParser<TimeSpan>
 {
     private static readonly Regex _pattern = new
     (
-        "(?<Years>\\d+(?=y))|(?<Months>\\d+(?=mo))|(?<Weeks>\\d+(?=w))|(?<Days>\\d+(?=d))|(?<Hours>\\d+(?=h))|(?<Minutes>\\d+(?=m))|(?<Seconds>\\d+(?=s))",
+        "^-|(?<Years>\\d+y)|(?<Months>\\d+mo)|(?<Weeks>\\d+w)|(?<Days>\\d+d)|(?<Hours>\\d+h)|(?<Minutes>\\d+m)|(?<Seconds>\\d+s)",
         RegexOptions.Compiled
     );
 
@@ -65,6 +65,11 @@ public override ValueTask<Result<TimeSpan>> TryParseAsync(string? value, Cancell
             return new ValueTask<Result<TimeSpan>>(new ParsingError<TimeSpan>(value));
         }
 
+        if (value.Length != matches.Sum(x => x.Length))
+        {
+            return new ValueTask<Result<TimeSpan>>(new ParsingError<TimeSpan>(value));
+        }
+
         var timeSpan = TimeSpan.Zero;
         foreach (var match in matches.Cast<Match>())
         {
@@ -74,8 +79,14 @@ public override ValueTask<Result<TimeSpan>> TryParseAsync(string? value, Cancell
                 .Skip(1)
                 .Select(g => (g.Name, g.Value));
 
-            foreach (var (key, groupValue) in groups)
+            foreach (var (key, groupRawValue) in groups)
             {
+                var groupValue = groupRawValue[..^1];
+                if (key is "Months")
+                {
+                    groupValue = groupValue[..^1];
+                }
+
                 if (!double.TryParse(groupValue, out var parsedGroupValue))
                 {
                     return new ValueTask<Result<TimeSpan>>(new ParsingError<TimeSpan>(value));
@@ -123,6 +134,11 @@ public override ValueTask<Result<TimeSpan>> TryParseAsync(string? value, Cancell
             }
         }
 
+        if (value[0] == '-')
+        {
+            timeSpan = -timeSpan;
+        }
+
         return new ValueTask<Result<TimeSpan>>(timeSpan);
     }
 }

Tests/Remora.Commands.Tests/Parsers/TimeSpanParserTests.cs

@@ -24,6 +24,7 @@
 using System.Collections.Generic;
 using System.Threading.Tasks;
 using Remora.Commands.Parsers;
+using Remora.Commands.Results;
 using Xunit;
 
 namespace Remora.Commands.Tests.Parsers;
@@ -86,9 +87,25 @@ public class TimeSpanParserTests
 
                 return then - now + TimeSpan.FromMinutes(1);
             })
+        ],
+        [
+            "-1m",
+            new Func<TimeSpan>(() => -TimeSpan.FromMinutes(1))
         ]
     ];
 
+    /// <summary>
+    /// Gets a set of various test cases that aren't parseable.
+    /// </summary>
+    public static IEnumerable<object?[]> InvalidCases =>
+    [
+        ["x60sx"],
+        ["1m-60s"],
+        ["s"],
+        [string.Empty],
+        [null],
+    ];
+
     /// <summary>
     /// Tests whether the parser can successfully parse various inputs.
     /// </summary>
@@ -106,4 +123,21 @@ public async Task CanParse(string value, Func<TimeSpan> expected)
         Assert.True(result.IsSuccess);
         Assert.Equal(expected(), result.Entity);
     }
+
+    /// <summary>
+    /// Tests whether the parser outputs errors in case of malformed inputs.
+    /// </summary>
+    /// <param name="value">The value to parse.</param>
+    /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+    [Theory]
+    [MemberData(nameof(InvalidCases))]
+    public async Task DoesntAcceptInvalidInput(string? value)
+    {
+        var parser = new TimeSpanParser();
+
+        var result = await parser.TryParseAsync(value);
+
+        Assert.False(result.IsSuccess);
+        Assert.IsType<ParsingError<TimeSpan>>(result.Error);
+    }
 }