Merge pull request #748 from jonsequitur/FileSystemInfo-binding

FileSystemInfo binding

Author: jonsequitur

src/System.CommandLine.Tests/Binding/BindingTestCase.cs

@@ -1,20 +1,20 @@
 // Copyright (c) .NET Foundation and contributors. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
-using System.Collections.Generic;
-
 namespace System.CommandLine.Tests.Binding
 {
     public class BindingTestCase
     {
         private readonly Action<object> _assertBoundValue;
-
-        public BindingTestCase(
+        
+        private BindingTestCase(
             string commandLine,
             Type parameterType,
-            Action<object> assertBoundValue)
+            Action<object> assertBoundValue,
+            string variationName)
         {
             _assertBoundValue = assertBoundValue;
+            VariationName = variationName;
             CommandLine = commandLine;
             ParameterType = parameterType;
         }
@@ -23,23 +23,21 @@ public BindingTestCase(
 
         public Type ParameterType { get; }
 
+        public string VariationName { get; }
+
         public void AssertBoundValue(object value)
         {
             _assertBoundValue(value);
         }
 
         public static BindingTestCase Create<T>(
             string commandLine,
-            Action<T> assertBoundValue) =>
+            Action<T> assertBoundValue, 
+            string variationName = null) =>
             new BindingTestCase(
                 commandLine,
                 typeof(T),
-                o => assertBoundValue((T)o)
-            );
-    }
-
-    public class BindingTestSet : Dictionary<Type, BindingTestCase>
-    {
-        public void Add(BindingTestCase testCase) => Add(testCase.ParameterType, testCase);
+                o => assertBoundValue((T) o),
+                variationName);
     }
-}
+}

src/System.CommandLine.Tests/Binding/BindingTestSet.cs

@@ -0,0 +1,17 @@
+// Copyright (c) .NET Foundation and contributors. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System.Collections.Generic;
+
+namespace System.CommandLine.Tests.Binding
+{
+    public class BindingTestSet : Dictionary<(Type type, string variationName), BindingTestCase>
+    {
+        public void Add(BindingTestCase testCase)
+        {
+            Add((testCase.ParameterType, testCase.VariationName), testCase);
+        }
+
+        public BindingTestCase this[Type type] => base[(type, null)];
+    }
+}

src/System.CommandLine.Tests/Binding/ModelBindingCommandHandlerTests.cs

@@ -10,6 +10,7 @@
 using System.Reflection;
 using System.Threading.Tasks;
 using FluentAssertions;
+using FluentAssertions.Execution;
 using Xunit;
 
 namespace System.CommandLine.Tests.Binding
@@ -251,6 +252,28 @@ public async Task When_argument_type_is_not_known_until_binding_then_int_paramet
             received.Should().Be(123);
         }
 
+        [Fact]
+        public void When_argument_type_is_more_specific_than_parameter_type_then_parameter_is_bound_correctly()
+        {
+            FileSystemInfo received = null;
+
+            var root = new RootCommand
+            {
+                new Option<DirectoryInfo>("-f")
+            };
+            root.Handler = CommandHandler.Create<FileSystemInfo>(f => received = f);
+            var path = $"{Directory.GetCurrentDirectory()}{Path.DirectorySeparatorChar}";
+
+            root.Invoke($"-f {path}");
+
+            received.Should()
+                    .BeOfType<DirectoryInfo>()
+                    .Which
+                    .FullName
+                    .Should()
+                    .Be(path);
+        }
+
         [Theory]
         [InlineData(typeof(ClassWithCtorParameter<int>), false)]
         [InlineData(typeof(ClassWithCtorParameter<int>), true)]
@@ -260,10 +283,23 @@ public async Task When_argument_type_is_not_known_until_binding_then_int_paramet
         [InlineData(typeof(ClassWithCtorParameter<string>), true)]
         [InlineData(typeof(ClassWithSetter<string>), false)]
         [InlineData(typeof(ClassWithSetter<string>), true)]
+
         [InlineData(typeof(FileInfo), false)]
         [InlineData(typeof(FileInfo), true)]
         [InlineData(typeof(FileInfo[]), false)]
         [InlineData(typeof(FileInfo[]), true)]
+        
+        [InlineData(typeof(DirectoryInfo), false)]
+        [InlineData(typeof(DirectoryInfo), true)]
+        [InlineData(typeof(DirectoryInfo[]), false)]
+        [InlineData(typeof(DirectoryInfo[]), true)]
+        
+        [InlineData(typeof(FileSystemInfo), true, nameof(ExistingFile))]
+        [InlineData(typeof(FileSystemInfo), true, nameof(ExistingDirectory))]
+        [InlineData(typeof(FileSystemInfo), true, nameof(NonexistentPathWithTrailingSlash))]
+        [InlineData(typeof(FileSystemInfo), true, nameof(NonexistentPathWithTrailingAltSlash))]
+        [InlineData(typeof(FileSystemInfo), true, nameof(NonexistentPathWithoutTrailingSlash))]
+
         [InlineData(typeof(string[]), false)]
         [InlineData(typeof(string[]), true)]
         [InlineData(typeof(List<string>), false)]
@@ -274,9 +310,10 @@ public async Task When_argument_type_is_not_known_until_binding_then_int_paramet
         [InlineData(typeof(List<int>), true)]
         public async Task Handler_method_receives_option_arguments_bound_to_the_specified_type(
             Type type,
-            bool useDelegate)
+            bool useDelegate,
+            string variation = null)
         {
-            var testCase = _bindingCases[type];
+            var testCase = _bindingCases[(type, variation)];
 
             ICommandHandler handler;
             if (!useDelegate)
@@ -318,7 +355,7 @@ public async Task Handler_method_receives_option_arguments_bound_to_the_specifie
 
             var boundValue = ((BoundValueCapturer)invocationContext.InvocationResult).BoundValue;
 
-            boundValue.Should().BeOfType(testCase.ParameterType);
+            boundValue.Should().BeAssignableTo(testCase.ParameterType);
 
             testCase.AssertBoundValue(boundValue);
         }
@@ -443,14 +480,93 @@ public void Apply(InvocationContext context)
                 o => o.Value.Should().Be("123")),
 
             BindingTestCase.Create<FileInfo>(
-                Path.Combine(Directory.GetCurrentDirectory(), "file1.txt"),
-                o => o.FullName.Should().Be(Path.Combine(Directory.GetCurrentDirectory(), "file1.txt"))),
+                Path.Combine(ExistingDirectory(), "file1.txt"),
+                o => o.FullName
+                      .Should()
+                      .Be(Path.Combine(ExistingDirectory(), "file1.txt"))),
 
             BindingTestCase.Create<FileInfo[]>(
-                $"{Path.Combine(Directory.GetCurrentDirectory(), "file1.txt")} {Path.Combine(Directory.GetCurrentDirectory(), "file2.txt")}",
+                $"{Path.Combine(ExistingDirectory(), "file1.txt")} {Path.Combine(ExistingDirectory(), "file2.txt")}",
                 o => o.Select(f => f.FullName)
                       .Should()
-                      .BeEquivalentTo(new[] { Path.Combine(Directory.GetCurrentDirectory(), "file1.txt"), Path.Combine(Directory.GetCurrentDirectory(), "file2.txt") })),
+                      .BeEquivalentTo(new[]
+                      {
+                          Path.Combine(ExistingDirectory(), "file1.txt"),
+                          Path.Combine(ExistingDirectory(), "file2.txt")
+                      })),
+
+            BindingTestCase.Create<DirectoryInfo>(
+                ExistingDirectory(),
+                fsi => fsi.Should()
+                          .BeOfType<DirectoryInfo>()
+                          .Which
+                          .FullName
+                          .Should()
+                          .Be(ExistingDirectory())),
+
+            BindingTestCase.Create<DirectoryInfo[]>(
+                $"{ExistingDirectory()} {ExistingDirectory()}",
+                fsi => fsi.Should()
+                          .BeAssignableTo<IEnumerable<DirectoryInfo>>()
+                          .Which
+                          .Select(d => d.FullName)
+                          .Should()
+                          .BeEquivalentTo(new[]
+                          {
+                              ExistingDirectory(),
+                              ExistingDirectory()
+                          })),
+
+            BindingTestCase.Create<FileSystemInfo>(
+                ExistingFile(),
+                fsi => fsi.Should()
+                          .BeOfType<FileInfo>()
+                          .Which
+                          .FullName
+                          .Should()
+                          .Be(ExistingFile()),
+                variationName: nameof(ExistingFile)),
+
+            BindingTestCase.Create<FileSystemInfo>(
+                ExistingDirectory(),
+                fsi => fsi.Should()
+                          .BeOfType<DirectoryInfo>()
+                          .Which
+                          .FullName
+                          .Should()
+                          .Be(ExistingDirectory()),
+                variationName: nameof(ExistingDirectory)),
+
+            BindingTestCase.Create<FileSystemInfo>(
+                NonexistentPathWithTrailingSlash(),
+                fsi => fsi.Should()
+                          .BeOfType<DirectoryInfo>()
+                          .Which
+                          .FullName
+                          .Should()
+                          .Be(NonexistentPathWithTrailingSlash()),
+                variationName: nameof(NonexistentPathWithTrailingSlash)),
+
+            BindingTestCase.Create<FileSystemInfo>(
+                NonexistentPathWithTrailingAltSlash(),
+                fsi => fsi.Should()
+                          .BeOfType<DirectoryInfo>()
+                          .Which
+                          .FullName
+                          .Should()
+                          .Be(NonexistentPathWithTrailingSlash(), 
+                              "DirectoryInfo replaces Path.AltDirectorySeparatorChar with Path.DirectorySeparatorChar on Windows"),
+                variationName: nameof(NonexistentPathWithTrailingAltSlash)),
+
+            BindingTestCase.Create<FileSystemInfo>(
+                NonexistentPathWithoutTrailingSlash(),
+                fsi => fsi.Should()
+                          .BeOfType<FileInfo>()
+                          .Which
+                          .FullName
+                          .Should()
+                          .Be(NonexistentPathWithoutTrailingSlash()),
+                variationName: nameof(NonexistentPathWithoutTrailingSlash)),
 
             BindingTestCase.Create<string[]>(
                 "one two",
@@ -468,5 +584,23 @@ public void Apply(InvocationContext context)
                 "1 2",
                 o => o.Should().BeEquivalentTo(new List<int> { 1, 2 }))
         };
+
+        private static string NonexistentPathWithoutTrailingSlash()
+        {
+            return Path.Combine(
+                ExistingDirectory(),
+                "does-not-exist");
+        }
+
+        private static string NonexistentPathWithTrailingSlash() => 
+            NonexistentPathWithoutTrailingSlash() + Path.DirectorySeparatorChar;
+        private static string NonexistentPathWithTrailingAltSlash() => 
+            NonexistentPathWithoutTrailingSlash() + Path.AltDirectorySeparatorChar;
+
+        private static string ExistingFile() =>
+            Directory.GetFiles(ExistingDirectory()).FirstOrDefault() ?? 
+            throw new AssertionFailedException("No files found in current directory");
+
+        private static string ExistingDirectory() => Directory.GetCurrentDirectory();
     }
 }

src/System.CommandLine/Binding/ArgumentConverter.cs

@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.CommandLine.Parsing;
 using System.ComponentModel;
+using System.IO;
 using System.Linq;
 using System.Reflection;
 using static System.CommandLine.Binding.ArgumentConversionResult;
@@ -13,6 +14,25 @@ namespace System.CommandLine.Binding
 {
     internal static class ArgumentConverter
     {
+        private static readonly Dictionary<Type, Func<string, object>> _converters = new Dictionary<Type, Func<string, object>>
+        {
+            [typeof(FileSystemInfo)] = value =>
+            {
+                if (Directory.Exists(value))
+                {
+                    return new DirectoryInfo(value);
+                }
+
+                if (value.EndsWith(Path.DirectorySeparatorChar.ToString()) ||
+                    value.EndsWith(Path.AltDirectorySeparatorChar.ToString()))
+                {
+                    return new DirectoryInfo(value);
+                }
+
+                return new FileInfo(value);
+            }
+        };
+
         internal static ArgumentConversionResult ConvertObject(
             IArgument argument,
             Type type,
@@ -54,7 +74,7 @@ private static ArgumentConversionResult ConvertString(
         {
             type ??= typeof(string);
 
-            if (TypeDescriptor.GetConverter(type) is TypeConverter typeConverter)
+            if (TypeDescriptor.GetConverter(type) is { } typeConverter)
             {
                 if (typeConverter.CanConvertFrom(typeof(string)))
                 {
@@ -71,6 +91,20 @@ private static ArgumentConversionResult ConvertString(
                 }
             }
 
+            if (_converters.TryGetValue(type, out var convert))
+            {
+                try
+                {
+                    return Success(
+                        argument,
+                        convert(value));
+                }
+                catch (Exception)
+                {
+                    return Failure(argument, type, value);
+                }
+            }
+
             if (type.TryFindConstructorWithSingleParameterOfType(
                 typeof(string), out (ConstructorInfo ctor, ParameterDescriptor parameterDescriptor) tuple))
             {

src/System.CommandLine/Binding/Binder.cs

@@ -9,7 +9,7 @@ namespace System.CommandLine.Binding
     internal static class Binder
     {
         internal static bool IsMatch(this string parameterName, string alias) =>
-            String.Equals(alias?.RemovePrefix()
+            string.Equals(alias?.RemovePrefix()
                               .Replace("-", ""),
                           parameterName,
                           StringComparison.OrdinalIgnoreCase);

src/System.CommandLine/Option{T}.cs

@@ -80,31 +80,6 @@ public Option(
 
             Argument = new Argument<T>(getDefaultValue);
         }
-        public Option(
-            string alias,
-            T defaultValue,
-            string description = null) : base(alias, description)
-        {
-            if (defaultValue is null)
-            {
-                throw new ArgumentNullException(nameof(defaultValue));
-            }
-
-            Argument = new Argument<T>(() => defaultValue);
-        }
-
-        public Option(
-            string[] aliases,
-            T defaultValue,
-            string description = null) : base(aliases, description)
-        {
-            if (defaultValue is null)
-            {
-                throw new ArgumentNullException(nameof(defaultValue));
-            }
-
-            Argument = new Argument<T>(() => defaultValue);
-        }
 
         public override Argument Argument
         {

src/System.CommandLine/Symbol.cs

@@ -126,12 +126,6 @@ public void AddAlias(string alias)
             }
         }
 
-        protected void ClearAliases()
-        {
-            _aliases.Clear();
-            _rawAliases.Clear();
-        }
-
         public bool HasAlias(string alias)
         {
             if (string.IsNullOrWhiteSpace(alias))