Merge pull request #810 from jonsequitur/fixes-for-723

jonsequitur · web-flow · commit dde07c52b377 · 2020-03-04T14:50:31.000-08:00
Fixes for 723
diff --git a/SECURITY.md b/SECURITY.md
@@ -1,15 +1,15 @@
-# Security Policy
-
-## Supported Versions
-
-The .NET Core and ASP.NET Core support policy, including supported versions can be found at the [.NET Core Support Policy Page](https://dotnet.microsoft.com/platform/support/policy/dotnet-core).
-
-## Reporting a Vulnerability
-
-Security issues and bugs should be reported privately to the Microsoft Security Response Center (MSRC), either by emailing secure@microsoft.com or via the portal at https://msrc.microsoft.com. 
-You should receive a response within 24 hours. If for some reason you do not, please follow up via email to ensure we received your 
-original message. Further information, including the MSRC PGP key, can be found in the [MSRC Report an Issue FAQ](https://www.microsoft.com/en-us/msrc/faqs-report-an-issue).
-
-Reports via MSRC may qualify for the .NET Core Bug Bounty. Details of the .NET Core Bug Bounty including terms and conditions are at [https://aka.ms/corebounty](https://aka.ms/corebounty).
-
+# Security Policy
+
+## Supported Versions
+
+The .NET Core and ASP.NET Core support policy, including supported versions can be found at the [.NET Core Support Policy Page](https://dotnet.microsoft.com/platform/support/policy/dotnet-core).
+
+## Reporting a Vulnerability
+
+Security issues and bugs should be reported privately to the Microsoft Security Response Center (MSRC), either by emailing secure@microsoft.com or via the portal at https://msrc.microsoft.com. 
+You should receive a response within 24 hours. If for some reason you do not, please follow up via email to ensure we received your 
+original message. Further information, including the MSRC PGP key, can be found in the [MSRC Report an Issue FAQ](https://www.microsoft.com/en-us/msrc/faqs-report-an-issue).
+
+Reports via MSRC may qualify for the .NET Core Bug Bounty. Details of the .NET Core Bug Bounty including terms and conditions are at [https://aka.ms/corebounty](https://aka.ms/corebounty).
+
 Please do not open issues for anything you think might have a security implication.
diff --git a/src/System.CommandLine.Rendering.Tests/ViewRenderingTests.cs b/src/System.CommandLine.Rendering.Tests/ViewRenderingTests.cs
@@ -41,7 +41,6 @@ public void In_Ansi_mode_ConsoleView_keeps_track_of_position_so_that_multiple_Wr
             var renderer = new ConsoleRenderer(
                 _terminal,
                 OutputMode.Ansi);
-
             
             var view = new StringsView(new[] {
                 "1",
diff --git a/src/System.CommandLine.Tests/ArgumentTests.cs b/src/System.CommandLine.Tests/ArgumentTests.cs
@@ -3,6 +3,7 @@
 
 using System.Collections.Generic;
 using System.CommandLine.Builder;
+using System.CommandLine.Help;
 using System.CommandLine.Invocation;
 using System.CommandLine.Parsing;
 using System.IO;
@@ -120,7 +121,7 @@ public void GetDefaultValue_can_return_null()
             }
 
             [Fact]
-            public void Validation_failure_message_can_be_specified()
+            public void Validation_failure_message_can_be_specified_when_parsing_tokens()
             {
                 var argument = new Argument<FileSystemInfo>(result =>
                 {
@@ -138,6 +139,46 @@ public void Validation_failure_message_can_be_specified()
                         .Be("oops!");
             }
 
+            [Fact]
+            public void Validation_failure_message_can_be_specified_when_evaluating_default_argument_value()
+            {
+                var argument = new Argument<FileSystemInfo>(result =>
+                {
+                    result.ErrorMessage = "oops!";
+                    return null;
+                }, true);
+
+                argument.Parse("")
+                        .Errors
+                        .Should()
+                        .ContainSingle(e => e.SymbolResult.Symbol == argument)
+                        .Which
+                        .Message
+                        .Should()
+                        .Be("oops!");
+            }
+
+            [Fact]
+            public void Validation_failure_message_can_be_specified_when_evaluating_default_option_value()
+            {
+                var option = new Option<FileSystemInfo>(
+                    "-x",
+                    result =>
+                    {
+                        result.ErrorMessage = "oops!";
+                        return null;
+                    }, true);
+
+                option.Parse("")
+                      .Errors
+                      .Should()
+                      .ContainSingle()
+                      .Which
+                      .Message
+                      .Should()
+                      .Be("oops!");
+            }
+
             [Fact]
             public void custom_parsing_of_scalar_value_from_an_argument_with_one_token()
             {
@@ -309,6 +350,42 @@ public void Default_value_and_custom_argument_parser_can_be_used_together()
                     .Should()
                     .Be(123);
             }
+            
+            [Fact]
+            public void Multiple_command_arguments_can_have_custom_parse_delegates()
+            {
+                var root = new RootCommand
+                {
+                    new Argument<FileInfo[]>("from", argumentResult =>
+                    {
+                        argumentResult.ErrorMessage = "nope";
+                        return null;
+                    }, true)
+                    {
+                        Arity = new ArgumentArity(0, 2)
+                    },
+                    new Argument<DirectoryInfo>("to", argumentResult =>
+                    {
+                        argumentResult.ErrorMessage = "UH UH";
+                        return null;
+                    }, true)
+                    {
+                        Arity = ArgumentArity.ExactlyOne
+                    }
+                };
+
+                var result = root.Parse("a.txt b.txt /path/to/dir");
+
+                result.Errors
+                      .Select(e => e.Message)
+                      .Should()
+                      .Contain("nope");
+
+                result.Errors
+                      .Select(e => e.Message)
+                      .Should()
+                      .Contain("UH UH");
+            }
 
             [Fact]
             public void When_custom_conversion_fails_then_an_option_does_not_accept_further_arguments()
diff --git a/src/System.CommandLine.Tests/OptionTests.cs b/src/System.CommandLine.Tests/OptionTests.cs
@@ -325,14 +325,6 @@ public void Option_T_default_value_is_validated()
                 .Select(e => e.Message)
                 .Should()
                 .BeEquivalentTo(new[] { "ERR" });
-
-            option
-                .Parse("")
-                .Errors
-                .Select(e => e.Message)
-                .Should()
-                .BeEquivalentTo(new[] { "ERR" });
-
         }
    
         protected override Symbol CreateSymbol(string name) => new Option(name);
diff --git a/src/System.CommandLine.Tests/ParseDiagramTests.cs b/src/System.CommandLine.Tests/ParseDiagramTests.cs
@@ -2,6 +2,7 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using System.CommandLine.Parsing;
+using System.IO;
 using FluentAssertions;
 using Xunit;
 
@@ -89,7 +90,6 @@ public void Parse_diagram_identifies_options_where_default_values_have_been_appl
                 }
             };
 
-
             var result = rootCommand.Parse("-w 9000");
 
             var diagram = result.Diagram();
@@ -114,5 +114,22 @@ public void Parse_diagram_indicates_which_tokens_were_applied_to_which_command_a
                   .Should()
                   .Be("[ the-command [ first <one> ] [ second <two> ] [ third <three> <four> <five> ] ]");
         }
+        
+        [Fact]
+        public void Parse_diagram_indicates_which_tokens_were_applied_to_which_command_argument_for_sequences_of_complex_types()
+        {
+            var command = new Command("the-command")
+            {
+                new Argument<FileInfo> { Name = "first" },
+                new Argument<FileInfo> { Name = "second" },
+                new Argument<FileInfo[]> { Name = "third" }
+            };
+
+            var result = command.Parse("one two three four five");
+
+            result.Diagram()
+                  .Should()
+                  .Be("[ the-command [ first <one> ] [ second <two> ] [ third <three> <four> <five> ] ]");
+        }
     }
 }
diff --git a/src/System.CommandLine.Tests/ParserTests.MultipleArguments.cs b/src/System.CommandLine.Tests/ParserTests.MultipleArguments.cs
@@ -135,7 +135,7 @@ public void Multiple_arguments_of_unspecified_type_are_parsed_correctly()
                     }
                 };
 
-                var result = root.Parse("src.txt", "dest.txt");
+                var result = root.Parse("src.txt dest.txt");
 
                 result.RootCommandResult
                       .GetArgumentValueOrDefault("source")
@@ -147,7 +147,6 @@ public void Multiple_arguments_of_unspecified_type_are_parsed_correctly()
                       .Should()
                       .Be("dest.txt");
             }
-
         }
     }
 }
diff --git a/src/System.CommandLine.Tests/ParserTests.cs b/src/System.CommandLine.Tests/ParserTests.cs
@@ -1371,6 +1371,45 @@ public void When_an_option_with_a_default_value_is_not_matched_then_the_option_r
                   .BeTrue();
         }
 
+        [Fact]
+        public void When_an_option_with_a_default_value_is_not_matched_then_there_are_no_tokens()
+        {
+            var option = new Option<string>(
+                "-o", 
+                () => "the-default");
+
+            var command = new Command("command")
+            {
+                option
+            };
+
+            var result = command.Parse("command");
+
+            result.FindResultFor(option)
+                  .Token
+                  .Should()
+                  .BeNull();
+        }
+
+        [Fact]
+        public void When_an_argument_with_a_default_value_is_not_matched_then_there_are_no_tokens()
+        {
+            var argument = new Argument<string>(
+                "o", 
+                () => "the-default");
+
+            var command = new Command("command")
+            {
+                argument
+            };
+            var result = command.Parse("command");
+
+            result.FindResultFor(argument)
+                  .Tokens
+                  .Should()
+                  .BeEmpty();
+        }
+
         [Fact]
         public void Command_default_argument_value_does_not_override_parsed_value()
         {
diff --git a/src/System.CommandLine.Tests/SymbolResultTests.cs b/src/System.CommandLine.Tests/SymbolResultTests.cs
@@ -27,28 +27,6 @@ public void An_option_with_a_default_value_and_no_explicitly_provided_argument_h
             result.Tokens.Should().BeEmpty();
         }
 
-        [Fact]
-        public void Default_values_are_reevaluated_and_not_cached_between_parses()
-        {
-            var option =
-                new Option("-x")
-                {
-                    Argument = new Argument
-                    {
-                        Arity = ArgumentArity.ExactlyOne
-                    }
-                };
-
-            var i = 0;
-            option.Argument.SetDefaultValueFactory(() => ++i);
-
-            var result1 = option.Parse("");
-            var result2 = option.Parse("");
-
-            result1.ValueForOption<int>("x").Should().Be(1);
-            result2.ValueForOption<int>("x").Should().Be(2);
-        }
-
         [Fact]
         public void HasOption_can_be_used_to_check_the_presence_of_an_option()
         {
diff --git a/src/System.CommandLine.Tests/UseHelpTests.cs b/src/System.CommandLine.Tests/UseHelpTests.cs
@@ -5,7 +5,6 @@
 using System.CommandLine.Invocation;
 using System.CommandLine.IO;
 using System.CommandLine.Parsing;
-using System.IO;
 using System.Threading.Tasks;
 using FluentAssertions;
 using Xunit;
diff --git a/src/System.CommandLine/Parsing/ArgumentResult.cs b/src/System.CommandLine/Parsing/ArgumentResult.cs
@@ -26,6 +26,11 @@ internal ArgumentResult(
 
         internal ParseError CustomError(Argument argument)
         {
+            if (!string.IsNullOrEmpty(ErrorMessage))
+            {
+                return new ParseError(ErrorMessage, this);
+            }
+
             foreach (var symbolValidator in argument.Validators)
             {
                 var errorMessage = symbolValidator(this);
@@ -55,9 +60,25 @@ internal virtual ArgumentConversionResult Convert(
 
             if (parentResult.UseDefaultValueFor(argument))
             {
-                var defaultValueFor = parentResult.GetDefaultValueFor(argument);
-
-                return ArgumentConversionResult.Success(argument, defaultValueFor);
+                if (argument is Argument arg)
+                {
+                    var argumentResult = new ArgumentResult(arg, Parent);
+
+                    var defaultValue = arg.GetDefaultValue(argumentResult);
+
+                    if (string.IsNullOrEmpty(argumentResult.ErrorMessage))
+                    {
+                        return ArgumentConversionResult.Success(
+                            argument,
+                            defaultValue);
+                    }
+                    else
+                    {
+                        return ArgumentConversionResult.Failure(
+                            argument,
+                            argumentResult.ErrorMessage);
+                    }
+                }
             }
 
             if (argument is Argument a &&
@@ -107,8 +128,5 @@ bool ShouldCheckArity()
                        optionResult.IsImplicit);
             }
         }
-
-        internal override object CreateDefaultArgumentResultAndGetItsValue(Argument argument) => 
-            argument.GetDefaultValue(this);
     }
 }
diff --git a/src/System.CommandLine/Parsing/OptionResult.cs b/src/System.CommandLine/Parsing/OptionResult.cs
@@ -12,18 +12,18 @@ public class OptionResult : SymbolResult
 
         internal OptionResult(
             IOption option,
-            Token token,
-            CommandResult parent) :
+            Token token = null,
+            CommandResult parent = null) :
             base(option ?? throw new ArgumentNullException(nameof(option)),
                  parent)
         {
             Option = option;
-            Token = token ?? throw new ArgumentNullException(nameof(token));
+            Token = token;
         }
 
         public IOption Option { get; }
 
-        public bool IsImplicit => Token is ImplicitToken;
+        public bool IsImplicit => Token is ImplicitToken || Token is null;
 
         public Token Token { get; }
 
diff --git a/src/System.CommandLine/Parsing/ParseResultExtensions.cs b/src/System.CommandLine/Parsing/ParseResultExtensions.cs
diff --git a/src/System.CommandLine/Parsing/ParseResultVisitor.cs b/src/System.CommandLine/Parsing/ParseResultVisitor.cs
diff --git a/src/System.CommandLine/Parsing/SymbolResult.cs b/src/System.CommandLine/Parsing/SymbolResult.cs
diff --git a/src/System.CommandLine/Parsing/SymbolResultExtensions.cs b/src/System.CommandLine/Parsing/SymbolResultExtensions.cs
diff --git a/src/System.CommandLine/SymbolExtensions.cs b/src/System.CommandLine/SymbolExtensions.cs

Original file line number	Diff line number	Diff line change
`@@ -135,7 +135,7 @@ public void Multiple_arguments_of_unspecified_type_are_parsed_correctly()`
`135`	`135`	`}`
`136`	`136`	`};`
`137`	`137`
`138`		`- var result = root.Parse("src.txt", "dest.txt");`
	`138`	`+ var result = root.Parse("src.txt dest.txt");`
`139`	`139`
`140`	`140`	`result.RootCommandResult`
`141`	`141`	`.GetArgumentValueOrDefault("source")`
`@@ -147,7 +147,6 @@ public void Multiple_arguments_of_unspecified_type_are_parsed_correctly()`
`147`	`147`	`.Should()`
`148`	`148`	`.Be("dest.txt");`
`149`	`149`	`}`
`150`		`-`
`151`	`150`	`}`
`152`	`151`	`}`
`153`	`152`	`}`