command-line-api icon indicating copy to clipboard operation
command-line-api copied to clipboard

Completion crashes when using `=` syntax for options

Open zivarah opened this issue 2 years ago • 12 comments

System.CommandLine version: 2.0.0-beta4.22272.1

There is a mismatch between how = is handled between splitting up the tokens and processing words for completion, which results in a crash.

using System.CommandLine;
using System.CommandLine.Builder;
using System.CommandLine.Parsing;

RootCommand root = new();
root.AddOption(new Option<string>("--test").AddCompletions("asdf"));

CommandLineBuilder builder = new(root);
builder.UseDefaults();
return await builder.Build().InvokeAsync(args);

Attempting tab completion on CompletionCrashRepro --test=asd results in a crash:

>	System.Linq.dll!System.Linq.ThrowHelper.ThrowNoMatchException() Line 27	C#
 	System.Linq.dll!System.Linq.Enumerable.Last<System.CommandLine.Parsing.Token>(System.Collections.Generic.IEnumerable<System.CommandLine.Parsing.Token> source, System.Func<System.CommandLine.Parsing.Token, bool> predicate) Line 30	C#
 	System.CommandLine.dll!System.CommandLine.Parsing.ParseResult.SymbolToComplete.__WillAcceptAnArgument|42_1(System.CommandLine.Parsing.ParseResult parseResult, int? position, System.CommandLine.Parsing.OptionResult optionResult) Line 305	C#
 	System.CommandLine.dll!System.CommandLine.Parsing.ParseResult.SymbolToComplete.__AllSymbolResultsForCompletion|0() Line 271	C#
 	System.Linq.dll!System.Collections.Generic.LargeArrayBuilder<System.CommandLine.Parsing.SymbolResult>.AddRange(System.Collections.Generic.IEnumerable<System.CommandLine.Parsing.SymbolResult> items) Line 116	C#
 	System.Linq.dll!System.Collections.Generic.EnumerableHelpers.ToArray<System.CommandLine.Parsing.SymbolResult>(System.Collections.Generic.IEnumerable<System.CommandLine.Parsing.SymbolResult> source) Line 85	C#
 	System.Linq.dll!System.Linq.Enumerable.ToArray<System.CommandLine.Parsing.SymbolResult>(System.Collections.Generic.IEnumerable<System.CommandLine.Parsing.SymbolResult> source) Line 17	C#
 	System.CommandLine.dll!System.CommandLine.Parsing.ParseResult.SymbolToComplete(int? position) Line 255	C#
 	System.CommandLine.dll!System.CommandLine.Parsing.ParseResult.GetCompletions(int? position) Line 219	C#
 	System.CommandLine.dll!System.CommandLine.Invocation.SuggestDirectiveResult.Apply(System.CommandLine.Invocation.InvocationContext context) Line 25	C#
 	System.CommandLine.dll!System.CommandLine.Invocation.InvocationPipeline.GetExitCode(System.CommandLine.Invocation.InvocationContext context) Line 98	C#
 	System.CommandLine.dll!System.CommandLine.Invocation.InvocationPipeline.InvokeAsync.__FullInvocationChainAsync|2_0(System.CommandLine.Invocation.InvocationContext context) Line 38	C#
 	System.CommandLine.dll!System.CommandLine.Invocation.InvocationPipeline.InvokeAsync(System.CommandLine.IConsole console) Line 30	C#
 	System.CommandLine.dll!System.CommandLine.Parsing.ParseResultExtensions.InvokeAsync(System.CommandLine.Parsing.ParseResult parseResult, System.CommandLine.IConsole console) Line 27	C#
 	System.CommandLine.dll!System.CommandLine.Parsing.ParserExtensions.InvokeAsync(System.CommandLine.Parsing.Parser parser, string[] args, System.CommandLine.IConsole console) Line 54	C#
 	CompletionCrashRepro.dll!Program.<Main>$(string[] args) Line 15	C#
 	CompletionCrashRepro.dll!Program.<Main>(string[] args)	Unknown

This same crash does not occur if a space is used instead of =: CompletionCrashRepro --test asd, and the completion works as expected.

(Note: I found the easiest way to actually reproduce this was by running this with the arguments of: [suggest:30] "CompletionCrashRepro --test=asd" rather than actually doing tab completion.)

The reason for this is that ParseResult.WillAcceptAnArgument has this line:

var tokenToComplete = parseResult.Tokens.Last(t => t.Value == textCompletionContext.WordToComplete);

However, the = is stripped out in parseResult.Tokens (["--test", "asd"]), but not in textCompletionContext.WordToComplete ("--test=asd"). This results in .Last(...) finding no matches, which crashes.

zivarah avatar Oct 09 '23 15:10 zivarah

I am currently attempting to fix this

samcoppock avatar Oct 23 '23 21:10 samcoppock

Still reproducible in 2.0.0-beta4.23407.1 (https://github.com/dotnet/command-line-api/commit/a045dd54a4c44723c215d992288160eb1401bb7f), although the API is different.

using System.CommandLine;
using System.Threading.Tasks;

internal static class Program
{
    internal static int Main(string[] args)
    {
        return MainAsync(args).GetAwaiter().GetResult();
    }

    internal static async Task<int> MainAsync(string[] args)
    {
        CliRootCommand rootCommand = new CliRootCommand()
        {
            new CliOption<string>("--test")
            {
                CompletionSources = {{ "asdf" }},
            }
        };

        CliConfiguration configuration = new CliConfiguration(rootCommand);
        return await configuration.InvokeAsync(args);
    }
}

KalleOlaviNiemitalo avatar Oct 23 '23 22:10 KalleOlaviNiemitalo

The bug even affects dotnet complete in .NET SDK 6.0.415, but that catches the exception, so you cannot see the stack trace without a debugger.

KalleOlaviNiemitalo avatar Oct 23 '23 22:10 KalleOlaviNiemitalo

Still reproducible in 2.0.0-beta4.23407.1 (a045dd5), although the API is different.

using System.CommandLine;
using System.Threading.Tasks;

internal static class Program
{
    internal static int Main(string[] args)
    {
        return MainAsync(args).GetAwaiter().GetResult();
    }

    internal static async Task<int> MainAsync(string[] args)
    {
        CliRootCommand rootCommand = new CliRootCommand()
        {
            new CliOption<string>("--test")
            {
                CompletionSources = {{ "asdf" }},
            }
        };

        CliConfiguration configuration = new CliConfiguration(rootCommand);
        return await configuration.InvokeAsync(args);
    }
}

Thanks very much for this

I was attempting to use both the code in the first comment ( on this issue ) and the code found here https://learn.microsoft.com/en-us/dotnet/standard/commandline/get-started-tutorial

Both did not work because 'RootCommand' does not exist in 'System.CommandLine' I suspected that 'RootCommand' has been changed to 'CliRootCommand' but could not get it to work just by changing the reference because the AddOption function does not exist on it.

I had given up until I received this.

Why on earth does both the opening post and the tutorial provided by microsoft have broken code - was it significantly refactored recently?

samcoppock avatar Oct 24 '23 12:10 samcoppock

System.CommandLine in NuGet Gallery is still at version 2.0.0-beta4.22272.1, i.e. commit 209b724a3c843253d3071e8348c353b297b0b8b5, published in June 2022. That version does not have the CliRootCommand etc. names yet.

Users are disappointed with the published version lagging so much behind the main branch; see https://github.com/dotnet/command-line-api/discussions/2203 and https://github.com/dotnet/command-line-api/issues/1882.

If you fix the bug and your pull request is merged, I don't know how long it will take before your fix is published to NuGet Gallery.

KalleOlaviNiemitalo avatar Oct 24 '23 12:10 KalleOlaviNiemitalo

ParseResult

how am i supposed to invoke the line 'var tokenToComplete = parseResult.Tokens.Last(t => t.Value == textCompletionContext.WordToComplete);' from ParseResult ?

and where did you get your example code ( ubove from ) - if you wrote it yourself how did you know how to use the library?

samcoppock avatar Oct 24 '23 19:10 samcoppock

@samcoppock, why are you interested in fixing the bug? I get the impression that you aren't using this library in your applications.

KalleOlaviNiemitalo avatar Oct 24 '23 21:10 KalleOlaviNiemitalo

@samcoppock, why are you interested in fixing the bug? I get the impression that you aren't using this library in your applications.

KalleOlaviNiemitalo avatar Oct 24 '23 21:10 KalleOlaviNiemitalo

@samcoppock, why are you interested in fixing the bug? I get the impression that you aren't using this library in your applications.

the project is in the "hacktoberfest" comp so I thought I would have a go at it, and deliberately picked something that's very new to me ( I know nothing about this library and a little about C# ) for career/skill development reasons.

samcoppock avatar Oct 24 '23 21:10 samcoppock

I have successfully reproduced the error and am now working on a fix

samcoppock avatar Oct 25 '23 19:10 samcoppock

Fix and tests done, pull request generated, can someone please review

samcoppock avatar Oct 26 '23 20:10 samcoppock

When i run the code normally -rootCommandResult in the ParseResult constructor is different.

when i run the code normally it is {CommandResult: RequiredExe } ( RequireExe is my program ) when i run it in the debugger it is {CommandResult: testhost }

Would this be causeing the code under test to not run during testing?

If yes what can i do about it - i dont know what to do next

samcoppock avatar Oct 27 '23 17:10 samcoppock