dotnet-suggest broken for multiple tools

[Blackclaws]

dotnet-suggest is broken for a lot of standard tools.

My current installed list (according to dotnet-suggest list):

dotnet-ef
dotnet ef
security-scan
dotnet-grpc-cli
dotnet grpc-cli
dotnet-suggest
dotnet suggest
dotnet-grpc
dotnet grpc
reportgenerator

For most of these, except dotnet-suggest and dotnet-grpc, there are any valid suggestions returned. Usually an exception happens.

It seems to me as if dotnet-suggest should catch any exceptions and all output from a tool and only report suggestions if there are any.

Printing the output from the tool completely breaks autocompletion functions for the existing shell shims, at least on zsh.

In general I don't think executing those tools to get completions is a good idea either. It seems that if a tool is not built using System.CommandLine it will inevitably try to make sense of the [suggest] command which doesn't exist.

Running dotnet-suggest get -e on those executables yields:

dotnet-ef

System.IndexOutOfRangeException: Index was outside the bounds of the array.
   at System.String.get_Chars(Int32 index)
   at Microsoft.EntityFrameworkCore.Tools.RootCommand.<>c.<Execute>b__12_0(String a)
   at System.Linq.Enumerable.TakeWhileIterator[TSource](IEnumerable`1 source, Func`2 predicate)+MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at Microsoft.EntityFrameworkCore.Tools.RootCommand.Execute(String[] _)
   at Microsoft.EntityFrameworkCore.Tools.Commands.CommandBase.<>c__DisplayClass0_0.<Configure>b__0(String[] args)
   at Microsoft.DotNet.Cli.CommandLine.CommandLineApplication.Execute(String[] args)
   at Microsoft.EntityFrameworkCore.Tools.Program.Main(String[] args)
Index was outside the bounds of the array.

dotnet-grpc-cli

Error: Unknown command '[suggest:32767]'.

       [suggest:32767] 
       ^^^^^^^^^^^^^^^ No such command

security-scan

Unhandled exception. System.InvalidOperationException: Invalid solution file path: ''
   at Microsoft.CodeAnalysis.MSBuild.DiagnosticReporter.Report(DiagnosticReportingMode mode, String message, Func`2 createException)
   at Microsoft.CodeAnalysis.MSBuild.PathResolver.TryGetAbsoluteSolutionPath(String path, String baseDirectory, DiagnosticReportingMode reportingMode, String& absolutePath)
   at Microsoft.CodeAnalysis.MSBuild.MSBuildProjectLoader.LoadSolutionInfoAsync(String solutionFilePath, IProgress`1 progress, ILogger msbuildLogger, CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.MSBuild.MSBuildWorkspace.OpenSolutionAsync(String solutionFilePath, ILogger msbuildLogger, IProgress`1 progress, CancellationToken cancellationToken)
   at SecurityCodeScan.Tool.Program.Main(String[] args) in D:\a\security-code-scan\security-code-scan\SecurityCodeScan.Tool\Program.cs:line 296
   at SecurityCodeScan.Tool.Program.<Main>(String[] args)
╔═╗┌─┐┌─┐┬ ┬┬─┐┬┌┬┐┬ ┬  ╔═╗┌─┐┌┬┐┌─┐  ╔═╗┌─┐┌─┐┌┐┌
╚═╗├┤ │  │ │├┬┘│ │ └┬┘  ║  │ │ ││├┤   ╚═╗│  ├─┤│││
╚═╝└─┘└─┘└─┘┴└─┴ ┴  ┴   ╚═╝└─┘─┴┘└─┘  ╚═╝└─┘┴ ┴┘└┘

.NET tool by Jaroslav Lobačevski v5.6.3


Loading solution ''%

reportgenerator

2022-09-13T12:34:50: Arguments
2022-09-13T12:34:50:  [suggest:32767]
2022-09-13T12:34:50:  
2022-09-13T12:34:50: No report files specified.
2022-09-13T12:34:50: No target directory specified.

[Blackclaws]

Related: #1777

Though this issue is more about the fact that if dotnet-suggest can't deal with an executable it shouldn't return anything.

[KalleOlaviNiemitalo]

If my command-line app has a custom completion delegate that throws an exception, I want to see the exception so that I can fix it.

Printing the output from the tool completely breaks autocompletion functions for the existing shell shims, at least on zsh.

I expect that suggestions should go to stdout and any exceptions should go to stderr, keeping them separate. Does it not work like that?

[Blackclaws]

If my command-line app has a custom completion delegate that throws an exception, I want to see the exception so that I can fix it.

Sure, in this case adding a --debug flag could help with that.

Printing the output from the tool completely breaks autocompletion functions for the existing shell shims, at least on zsh.

I expect that suggestions should go to stdout and any exceptions should go to stderr, keeping them separate. Does it not work like that?

No, it doesn't. Because dotnet-suggest cannot distinguish exceptions in the tool from normal output from the tool. Exceptions within dotnet-suggest are sent to stderr. The tool error is printed to stdout. Which especially when used in the context of a completion handler for a shell is particularly weird.

[KalleOlaviNiemitalo]

If dotnet-suggest supported a --debug flag, how would I add that flag when I notice that suggestions don't work right? I suppose the easiest way would be to edit the shell function that runs dotnet-suggest.

What prints the tool error to stdout? CommandLineBuilderExtensions.UseExceptionHandler prints the exception to stderr, and SuggestionStore.GetCompletions does not redirect stderr of the child process… but do the foreground color changes in ConsoleExtensions go to stdout anyway? The Platform.IsConsoleRedirectionCheckSupported checks there look strange.

[Blackclaws]

If dotnet-suggest supported a --debug flag, how would I add that flag when I notice that suggestions don't work right? I suppose the easiest way would be to edit the shell function that runs dotnet-suggest.

Just run dotnet-suggest get --debug -e EXECUTABLEPATH -- full-line the same as the completions do

What prints the tool error to stdout? CommandLineBuilderExtensions.UseExceptionHandler prints the exception to stderr, and SuggestionStore.GetCompletions does not redirect stderr of the child process… but do the foreground color changes in ConsoleExtensions go to stdout anyway? The Platform.IsConsoleRedirectionCheckSupported checks there look strange.

I don't know, and it really shouldn't matter. I'm quite sure that those tools simply don't use System.CommandLine at all, hence why dotnet-suggest doesn't do anything useful with them in the first place. However it should ignore non System.CommandLine tools entirely and not execute them.

[jonsequitur]

No, it doesn't. Because dotnet-suggest cannot distinguish exceptions in the tool from normal output from the tool. Exceptions within dotnet-suggest are sent to stderr. The tool error is printed to stdout. Which especially when used in the context of a completion handler for a shell is particularly weird.

This looks like a bug. Exceptions should definitely not be printed to stdout.

[KalleOlaviNiemitalo]

https://github.com/dotnet/efcore/blob/6e5612349c9d763c3986995b7538701ae788434e/src/dotnet-ef/Program.cs#L20-L35 uses Reporter, which writes to stdout.

I'll file a separate issue for the redundant logic in ConsoleExtensions.

[Blackclaws]

No, it doesn't. Because dotnet-suggest cannot distinguish exceptions in the tool from normal output from the tool. Exceptions within dotnet-suggest are sent to stderr. The tool error is printed to stdout. Which especially when used in the context of a completion handler for a shell is particularly weird.

This looks like a bug. Exceptions should definitely not be printed to stdout.

It seems that this is more widespread than one might think though. In general though, any output from the tool that is to be autocompleted should be supressed for dotnet-suggest get

[jonsequitur]

Some of these tools might be using a version of System.CommandLine that isn't compatible with the version of dotnet-suggest that's in use. Support for cursor positioning (i.e. ([suggest:32767]) wasn't available in early versions. And of course 32767 is a very suspicious-looking cursor position.

In the case of dotnet-ef, it doesn't appear to support completions at all, as dotnet-ef [suggest] results in an error (at least on version 6.0.8).

Better debugging support here would be useful. It should probably include some of the diagnostic approaches mentioned in #1085 (comment) and #1648.

Related: #1771.

[Blackclaws]

Some of these tools might be using a version of System.CommandLine that isn't compatible with the version of dotnet-suggest that's in use. Support for cursor positioning (i.e. ([suggest:32767]) wasn't available in early versions. And of course 32767 is a very suspicious-looking cursor position.

In the case of dotnet-ef, it doesn't appear to support completions at all, as dotnet-ef [suggest] results in an error (at least on version 6.0.8).

dotnet-ef isn't written using System.CommandLine at all. Hence its even stranger that dotnet-suggest even picks it up as a supported completion target.

[jonsequitur]

Yeah, I'm curious about how it got registered.

[Blackclaws]

As far as I can tell it registered all already installed tools when I installed it.

[drewburlingame]

Yeah, I'm curious about how it got registered.

Aren't all global tools considered registered by default, thanks to GlobalToolsSuggestionRegistration?