R: clean up some code

jonsequitur · jonsequitur · commit 7146ca4b45df · 2020-04-19T10:05:39.000-07:00
diff --git a/src/System.CommandLine.Rendering.Tests/ConsoleRendererTests.cs b/src/System.CommandLine.Rendering.Tests/ConsoleRendererTests.cs
@@ -11,7 +11,6 @@
 using static System.CommandLine.Rendering.TestTerminal;
 using static System.Environment;
 using System.CommandLine.Rendering.Views;
-using System.CommandLine.Tests;
 using System.CommandLine.Tests.Utility;
 
 namespace System.CommandLine.Rendering.Tests
diff --git a/src/System.CommandLine.Rendering.Tests/TextSpanVisitorTests.cs b/src/System.CommandLine.Rendering.Tests/TextSpanVisitorTests.cs
@@ -2,7 +2,6 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using System.Collections.Generic;
-using System.CommandLine.Tests;
 using System.CommandLine.Tests.Utility;
 using FluentAssertions;
 using System.Linq;
diff --git a/src/System.CommandLine.Rendering.Tests/Views/ContentViewTests.cs b/src/System.CommandLine.Rendering.Tests/Views/ContentViewTests.cs
@@ -3,7 +3,6 @@
 
 using System.Collections.Generic;
 using System.CommandLine.Rendering.Views;
-using System.CommandLine.Tests;
 using System.CommandLine.Tests.Utility;
 using System.Drawing;
 using FluentAssertions;
diff --git a/src/System.CommandLine.Rendering.Tests/Views/ContentView{T}Tests.cs b/src/System.CommandLine.Rendering.Tests/Views/ContentView{T}Tests.cs
@@ -2,7 +2,6 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using System.CommandLine.Rendering.Views;
-using System.CommandLine.Tests;
 using System.CommandLine.Tests.Utility;
 using System.Drawing;
 using FluentAssertions;
diff --git a/src/System.CommandLine.Rendering.Tests/Views/GridViewTests.cs b/src/System.CommandLine.Rendering.Tests/Views/GridViewTests.cs
@@ -3,7 +3,6 @@
 
 using System.CommandLine.IO;
 using System.CommandLine.Rendering.Views;
-using System.CommandLine.Tests;
 using System.CommandLine.Tests.Utility;
 using System.Drawing;
 using FluentAssertions;
diff --git a/src/System.CommandLine.Rendering.Tests/Views/LayoutViewTests.cs b/src/System.CommandLine.Rendering.Tests/Views/LayoutViewTests.cs
@@ -2,7 +2,6 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using System.CommandLine.Rendering.Views;
-using System.CommandLine.Tests;
 using System.CommandLine.Tests.Utility;
 using FluentAssertions;
 using Xunit;
diff --git a/src/System.CommandLine.Rendering.Tests/Views/ScreenViewTests.cs b/src/System.CommandLine.Rendering.Tests/Views/ScreenViewTests.cs
@@ -3,7 +3,6 @@
 
 using System.Collections.Generic;
 using System.CommandLine.Rendering.Views;
-using System.CommandLine.Tests;
 using System.CommandLine.Tests.Utility;
 using System.Linq;
 using System.Threading;
diff --git a/src/System.CommandLine.Rendering.Tests/Views/StackLayoutViewTests.cs b/src/System.CommandLine.Rendering.Tests/Views/StackLayoutViewTests.cs
@@ -2,7 +2,6 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using System.CommandLine.Rendering.Views;
-using System.CommandLine.Tests;
 using System.CommandLine.Tests.Utility;
 using System.Drawing;
 using FluentAssertions;
diff --git a/src/System.CommandLine.Suggest.Tests/DotnetSuggestEndToEndTests.cs b/src/System.CommandLine.Suggest.Tests/DotnetSuggestEndToEndTests.cs
@@ -2,7 +2,6 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using System.CommandLine.Invocation;
-using System.CommandLine.Tests;
 using System.CommandLine.Tests.Utility;
 using System.IO;
 using FluentAssertions;
diff --git a/src/System.CommandLine/ArgumentArity.cs b/src/System.CommandLine/ArgumentArity.cs
@@ -4,7 +4,6 @@
 using System.Collections;
 using System.CommandLine.Binding;
 using System.CommandLine.Parsing;
-using System.Linq;
 
 namespace System.CommandLine
 {
@@ -59,21 +58,9 @@ internal static FailedArgumentConversionArityResult Validate(
 
             if (tokenCount > maximumNumberOfValues)
             {
-                if (maximumNumberOfValues == 1)
-                {
-                    return new TooManyArgumentsConversionResult(
-                        argument,
-                        symbolResult.ValidationMessages.ExpectsOneArgument(symbolResult));
-                }
-                else
-                {
-                    return new TooManyArgumentsConversionResult(
-                        argument,
-                        symbolResult.ValidationMessages.ExpectsFewerArguments(
-                            symbolResult.Tokens.Last(),
-                            tokenCount,
-                            maximumNumberOfValues));
-                }
+                return new TooManyArgumentsConversionResult(
+                    argument,
+                    symbolResult.ValidationMessages.ExpectsOneArgument(symbolResult));
             }
 
             return null;
diff --git a/src/System.CommandLine/Binding/ArgumentConverter.cs b/src/System.CommandLine/Binding/ArgumentConverter.cs
@@ -38,13 +38,6 @@ internal static ArgumentConversionResult ConvertObject(
             Type type,
             object value)
         {
-            if (value == null &&
-                type == typeof(bool))
-            {
-                // the presence of the parsed symbol is treated as true
-                return new SuccessfulArgumentConversionResult(argument, true);
-            }
-
             switch (value)
             {
                 case string singleValue:
@@ -59,9 +52,6 @@ internal static ArgumentConversionResult ConvertObject(
 
                 case IReadOnlyCollection<string> manyValues:
                     return ConvertStrings(argument, type, manyValues);
-
-                case null:
-                    break;
             }
 
             return None(argument);
@@ -93,16 +83,9 @@ private static ArgumentConversionResult ConvertString(
 
             if (_converters.TryGetValue(type, out var convert))
             {
-                try
-                {
-                    return Success(
-                        argument,
-                        convert(value));
-                }
-                catch (Exception)
-                {
-                    return Failure(argument, type, value);
-                }
+                return Success(
+                    argument,
+                    convert(value));
             }
 
             if (type.TryFindConstructorWithSingleParameterOfType(
@@ -121,7 +104,7 @@ private static ArgumentConversionResult ConvertString(
 
         public static ArgumentConversionResult ConvertStrings(
             IArgument argument,
-            Type type, 
+            Type type,
             IReadOnlyCollection<string> arguments)
         {
             if (type == null)
@@ -134,45 +117,26 @@ public static ArgumentConversionResult ConvertStrings(
                 throw new ArgumentNullException(nameof(arguments));
             }
 
-            Type itemType;
+            var itemType = type == typeof(string)
+                               ? typeof(string)
+                               : GetItemTypeIfEnumerable(type);
 
-            if (type == typeof(string))
-            {
-                // don't treat items as char
-                itemType = typeof(string);
-            }
-            else
-            {
-                itemType = GetItemTypeIfEnumerable(type);
-            }
-
-            var allParseResults = arguments
-                                  .Select(arg => ConvertString(argument, itemType, arg))
-                                  .ToArray();
+            var successfulParseResults = arguments
+                                         .Select(arg => ConvertString(argument, itemType, arg))
+                                         .OfType<SuccessfulArgumentConversionResult>();
 
-            var successfulParseResults = allParseResults
-                                         .OfType<SuccessfulArgumentConversionResult>()
-                                         .ToArray();
+            var list = (IList) Activator.CreateInstance(typeof(List<>).MakeGenericType(itemType));
 
-            if (successfulParseResults.Length == arguments.Count)
+            foreach (var parseResult in successfulParseResults)
             {
-                var list = (IList)Activator.CreateInstance(typeof(List<>).MakeGenericType(itemType));
-
-                foreach (var parseResult in successfulParseResults)
-                {
-                    list.Add(parseResult.Value);
-                }
+                list.Add(parseResult.Value);
+            }
 
-                var value = type.IsArray
-                                ? (object)Enumerable.ToArray((dynamic)list)
-                                : list;
+            var value = type.IsArray
+                            ? (object) Enumerable.ToArray((dynamic) list)
+                            : list;
 
-                return Success(argument, value);
-            }
-            else
-            {
-                return allParseResults.OfType<FailedArgumentConversionResult>().First();
-            }
+            return Success(argument, value);
         }
 
         private static Type GetItemTypeIfEnumerable(Type type)
@@ -312,12 +276,6 @@ internal static ArgumentConversionResult ConvertIfNeeded(
                         type,
                         Array.Empty<string>());
 
-                case TooManyArgumentsConversionResult _:
-                    return conversionResult;
-
-                case MissingArgumentConversionResult _:
-                    return conversionResult;
-
                 default:
                     return conversionResult;
             }
diff --git a/src/System.CommandLine/Parsing/ArgumentResult.cs b/src/System.CommandLine/Parsing/ArgumentResult.cs
@@ -8,19 +8,19 @@ namespace System.CommandLine.Parsing
 {
     public class ArgumentResult : SymbolResult
     {
+        private ArgumentConversionResult _conversionResult;
+
         internal ArgumentResult(
             IArgument argument,
             SymbolResult parent) : base(argument, parent)
         {
             Argument = argument;
         }
 
-        internal ArgumentConversionResult ConversionResult;
-
         public IArgument Argument { get; }
 
-        internal ArgumentConversionResult ArgumentConversionResult =>
-            ConversionResult ??= Convert(Argument);
+        internal ArgumentConversionResult GetArgumentConversionResult() => 
+            _conversionResult ??= Convert(Argument);
 
         public override string ToString() => $"{GetType().Name} {Argument.Name}: {string.Join(" ", Tokens.Select(t => $"<{t.Value}>"))}";
 
@@ -31,8 +31,9 @@ internal ParseError CustomError(Argument argument)
                 return new ParseError(ErrorMessage, this);
             }
 
-            foreach (var symbolValidator in argument.Validators)
+            for (var i = 0; i < argument.Validators.Count; i++)
             {
+                var symbolValidator = argument.Validators[i];
                 var errorMessage = symbolValidator(this);
 
                 if (!string.IsNullOrWhiteSpace(errorMessage))
@@ -58,9 +59,9 @@ internal virtual ArgumentConversionResult Convert(
                 return failedResult;
             }
 
-            if (parentResult.UseDefaultValueFor(argument))
+            if (argument is Argument arg)
             {
-                if (argument is Argument arg)
+                if (parentResult.UseDefaultValueFor(argument))
                 {
                     var argumentResult = new ArgumentResult(arg, Parent);
 
@@ -79,54 +80,48 @@ internal virtual ArgumentConversionResult Convert(
                             argumentResult.ErrorMessage);
                     }
                 }
-            }
 
-            if (argument is Argument a &&
-                a.ConvertArguments != null)
-            {
-                if (ConversionResult != null)
+                if (arg.ConvertArguments != null)
                 {
-                    return ConversionResult;
-                }
+                    if (_conversionResult != null)
+                    {
+                        return _conversionResult;
+                    }
 
-                var success = a.ConvertArguments(this, out var value);
+                    var success = arg.ConvertArguments(this, out var value);
 
-                if (value is ArgumentConversionResult conversionResult)
-                {
-                    return conversionResult;
-                }
+                    if (value is ArgumentConversionResult conversionResult)
+                    {
+                        return conversionResult;
+                    }
 
-                if (success)
-                {
-                    return ArgumentConversionResult.Success(argument, value);
-                }
+                    if (success)
+                    {
+                        return ArgumentConversionResult.Success(argument, value);
+                    }
 
-                return ArgumentConversionResult.Failure(
-                    argument, 
-                    ErrorMessage ?? $"Invalid: {parentResult.Token()} {string.Join(" ", Tokens.Select(t => t.Value))}");
+                    return ArgumentConversionResult.Failure(
+                        argument,
+                        ErrorMessage ?? $"Invalid: {parentResult.Token()} {string.Join(" ", Tokens.Select(t => t.Value))}");
+                }
             }
 
             switch (argument.Arity.MaximumNumberOfValues)
             {
-                case 0:
-                    return ArgumentConversionResult.Success(argument, null);
-
                 case 1:
                     return ArgumentConversionResult.Success(
-                        argument, 
+                        argument,
                         Tokens.Select(t => t.Value).SingleOrDefault());
 
                 default:
                     return ArgumentConversionResult.Success(
-                        argument, 
+                        argument,
                         Tokens.Select(t => t.Value).ToArray());
             }
 
-            bool ShouldCheckArity()
-            {
-                return !(parentResult is OptionResult optionResult &&
-                       optionResult.IsImplicit);
-            }
+            bool ShouldCheckArity() =>
+                !(parentResult is OptionResult optionResult &&
+                  optionResult.IsImplicit);
         }
     }
 }
diff --git a/src/System.CommandLine/Parsing/ArgumentResultExtensions.cs b/src/System.CommandLine/Parsing/ArgumentResultExtensions.cs
@@ -11,9 +11,8 @@ public static object GetValueOrDefault(this ArgumentResult argumentResult) =>
             argumentResult.GetValueOrDefault<object>();
 
         public static T GetValueOrDefault<T>(this ArgumentResult argumentResult) =>
-            argumentResult
-                .ArgumentConversionResult
-                .ConvertIfNeeded(argumentResult, typeof(T))
-                .GetValueOrDefault<T>();
+            argumentResult.GetArgumentConversionResult()
+                          .ConvertIfNeeded(argumentResult, typeof(T))
+                          .GetValueOrDefault<T>();
     }
 }
diff --git a/src/System.CommandLine/Parsing/ParseResultExtensions.cs b/src/System.CommandLine/Parsing/ParseResultExtensions.cs
@@ -125,7 +125,7 @@ argumentResult.Argument is Argument argument &&
                     builder.Append(" ");
                 }
 
-                switch (argumentResult.ArgumentConversionResult)
+                switch (argumentResult.GetArgumentConversionResult())
                 {
                     case SuccessfulArgumentConversionResult successful:
 
diff --git a/src/System.CommandLine/Parsing/ParseResultVisitor.cs b/src/System.CommandLine/Parsing/ParseResultVisitor.cs
@@ -300,7 +300,7 @@ private void ValidateArgumentResult(ArgumentResult argumentResult)
                 }
             }
 
-            if (argumentResult.ArgumentConversionResult is FailedArgumentConversionResult failed)
+            if (argumentResult.GetArgumentConversionResult() is FailedArgumentConversionResult failed)
             {
                 _errors.Add(
                     new ParseError(
@@ -342,8 +342,7 @@ private void PopulateDefaultValues()
 
                             case Argument argument when argument.HasDefaultValue:
 
-                                var argumentResult = commandResult.GetOrCreateDefaultArgumentResult(
-                                    argument);
+                                var argumentResult = commandResult.GetOrCreateDefaultArgumentResult(argument);
 
                                 commandResult.Children.Add(argumentResult);
                                 _rootCommandResult.AddToSymbolMap(argumentResult);
diff --git a/src/System.CommandLine/Parsing/SymbolResult.cs b/src/System.CommandLine/Parsing/SymbolResult.cs
