When TreatUnmatchedTokensAsErrors is set to false, there should be no errors reported for unmatched tokens (#2208)

* add failing tests

* fix: don't report errors for unmatched tokens for commands with TreatUnmatchedTokensAsErrors set to false

root command TreatUnmatchedTokensAsErrors set to false has precedence over subcommands

* adjust Diagram_directive_writes_parse_diagram test: the error for unmatched tokens is now being reported for the inner most command, as long as TreatUnmatchedTokensAsErrors is true

Author: adamsitnik

src/System.CommandLine.Tests/ParseDirectiveTests.cs

@@ -18,14 +18,17 @@ public DiagramDirectiveTests(ITestOutputHelper output)
             this.output = output;
         }
 
-        [Fact]
-        public async Task Diagram_directive_writes_parse_diagram()
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public async Task Diagram_directive_writes_parse_diagram(bool treatUnmatchedTokensAsErrors)
         {
             var rootCommand = new CliRootCommand();
             var subcommand = new CliCommand("subcommand");
             rootCommand.Subcommands.Add(subcommand);
             var option = new CliOption<int>("-c", "--count");
             subcommand.Options.Add(option);
+            subcommand.TreatUnmatchedTokensAsErrors = treatUnmatchedTokensAsErrors;
 
             CliConfiguration config = new(rootCommand)
             {
@@ -39,10 +42,14 @@ public async Task Diagram_directive_writes_parse_diagram()
 
             await result.InvokeAsync();
 
+            string expected = treatUnmatchedTokensAsErrors
+                ? $"[ {CliRootCommand.ExecutableName} ![ subcommand [ -c <34> ] ] ]   ???--> --nonexistent wat" + Environment.NewLine
+                : $"[ {CliRootCommand.ExecutableName} [ subcommand [ -c <34> ] ] ]   ???--> --nonexistent wat" + Environment.NewLine;
+
             config.Output
                    .ToString()
                    .Should()
-                   .Be($"![ {CliRootCommand.ExecutableName} [ subcommand [ -c <34> ] ] ]   ???--> --nonexistent wat" + Environment.NewLine);
+                   .Be(expected);
         }
 
         [Fact]

src/System.CommandLine.Tests/ParserTests.cs

@@ -1332,6 +1332,59 @@ public void When_a_command_line_has_unmatched_tokens_they_are_not_applied_to_sub
             result.UnmatchedTokens.Should().BeEquivalentTo("unmatched-token");
         }
 
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public void When_a_command_line_has_unmatched_tokens_the_parse_result_action_should_depend_on_parsed_command_TreatUnmatchedTokensAsErrors(bool treatUnmatchedTokensAsErrors)
+        {
+            CliRootCommand rootCommand = new();
+            CliCommand subcommand = new("vstest")
+            {
+                new CliOption<string>("--Platform"),
+                new CliOption<string>("--Framework"),
+                new CliOption<string[]>("--logger")
+            };
+            subcommand.TreatUnmatchedTokensAsErrors = treatUnmatchedTokensAsErrors;
+            rootCommand.Subcommands.Add(subcommand);
+
+            var result = rootCommand.Parse("vstest test1.dll test2.dll");
+
+            result.UnmatchedTokens.Should().BeEquivalentTo("test1.dll", "test2.dll");
+
+            if (treatUnmatchedTokensAsErrors)
+            {
+                result.Errors.Should().NotBeEmpty();
+                result.Action.Should().NotBeSameAs(result.CommandResult.Command.Action);
+            }
+            else
+            {
+                result.Errors.Should().BeEmpty();
+                result.Action.Should().BeSameAs(result.CommandResult.Command.Action);
+            }
+        }
+
+        [Fact]
+        public void RootCommand_TreatUnmatchedTokensAsErrors_set_to_false_has_precedence_over_subcommands()
+        {
+            CliRootCommand rootCommand = new();
+            rootCommand.TreatUnmatchedTokensAsErrors = false;
+            CliCommand subcommand = new("vstest")
+            {
+                new CliOption<string>("--Platform"),
+                new CliOption<string>("--Framework"),
+                new CliOption<string[]>("--logger")
+            };
+            subcommand.TreatUnmatchedTokensAsErrors = true; // the default, set to true to make it explicit
+            rootCommand.Subcommands.Add(subcommand);
+
+            var result = rootCommand.Parse("vstest test1.dll test2.dll");
+
+            result.UnmatchedTokens.Should().BeEquivalentTo("test1.dll", "test2.dll");
+
+            result.Errors.Should().BeEmpty();
+            result.Action.Should().BeSameAs(result.CommandResult.Command.Action);
+        }
+
         [Fact]
         public void Parse_can_not_be_called_with_null_args()
         {

src/System.CommandLine/Parsing/ArgumentResult.cs

@@ -107,12 +107,18 @@ public void OnlyTake(int numberOfTokens)
                 nextArgumentIndex++;
             }
 
+            CommandResult rootCommand = parent;
+            while (rootCommand.Parent is CommandResult nextLevel)
+            {
+                rootCommand = nextLevel;
+            }
+
             // When_tokens_are_passed_on_by_custom_parser_on_last_argument_then_they_become_unmatched_tokens
             while (tokensToPass > 0)
             {
                 CliToken unmatched = _tokens[numberOfTokens];
                 _tokens.RemoveAt(numberOfTokens);
-                SymbolResultTree.AddUnmatchedToken(unmatched, parent.Command.TreatUnmatchedTokensAsErrors ? parent : null);
+                SymbolResultTree.AddUnmatchedToken(unmatched, parent, rootCommand);
                 --tokensToPass;
             }
         }

src/System.CommandLine/Parsing/ParseOperation.cs

@@ -359,8 +359,7 @@ private void AddCurrentTokenToUnmatched()
                 return;
             }
 
-            _symbolResultTree.AddUnmatchedToken(CurrentToken,
-                _rootCommandResult.Command.TreatUnmatchedTokensAsErrors ? _rootCommandResult : null);
+            _symbolResultTree.AddUnmatchedToken(CurrentToken, _innermostCommandResult, _rootCommandResult);
         }
 
         private void Validate()

src/System.CommandLine/Parsing/SymbolResultTree.cs

@@ -55,12 +55,17 @@ internal IEnumerable<SymbolResult> GetChildren(SymbolResult parent)
 
         internal void InsertFirstError(ParseError parseError) => (Errors ??= new()).Insert(0, parseError);
 
-        internal void AddUnmatchedToken(CliToken token, CommandResult? commandResult)
+        internal void AddUnmatchedToken(CliToken token, CommandResult commandResult, CommandResult rootCommandResult)
         {
             (UnmatchedTokens ??= new()).Add(token);
 
-            if (commandResult is not null)
+            if (commandResult.Command.TreatUnmatchedTokensAsErrors)
             {
+                if (commandResult != rootCommandResult && !rootCommandResult.Command.TreatUnmatchedTokensAsErrors)
+                {
+                    return;
+                }
+
                 AddError(new ParseError(LocalizationResources.UnrecognizedCommandOrArgument(token.Value), commandResult));
             }
         }