dotnet-suggest assumes that dotnet global tools support the [suggest] directive

[KalleOlaviNiemitalo]

GlobalToolsSuggestionRegistration returns suggestion registrations for all executables in ~/.dotnet/tools/, and dotnet-suggest can then run such executables with the [suggest] directive even if they have not been explicitly registered. However, supporting this directive is not a requirement for creating a .NET tool. For example, dotnet-runtimeinfo 1.0.4, dotnet-script 1.1.0, and microsoft.visualstudio.slngen.tool 8.5.0 do not support it. There is a risk that the tool does something unexpected when given '[suggest]' which it does not recognize as a directive. For example, it could create a file with that name, or spend time trying to connect to a computer with that name.

I think the best solution would be:

1. Add something into the DotnetToolSettings.xml file to indicate that the command supports [suggest]. If it becomes possible to have multiple commands in the same tool package, this information should be specific to each command. The GenerateToolsSettingsFile task in .NET SDK generates DotnetToolSettings.xml and could read some property that the System.CommandLine package would set by default.
2. Make dotnet tool install copy this information to some place from which dotnet-suggest can find it, regardless of whether dotnet-suggest has been installed. For example, place it in ~/.dotnet/tools-suggest.json, or copy the whole XML file to tools/.meta/command/DotnetToolSettings.xml.
3. Make dotnet-suggest consult the information from step 2 to ascertain whether the command supports [suggest].

If a tool supports [suggest] but is too old to include this information in its DotnetToolSettings.xml file, then the user can dotnet-suggest register it manually.

I also considered adding some AssemblyMetadataAttribute to the entry point assembly of the command, but that doesn't really seem any easier because the executables in ~/.dotnet/tools/ are shims that don't contain CLR metadata. Also it would not be so easily extensible to wholly unmanaged tools.

[KalleOlaviNiemitalo]

Related #190 and #379.

[baronfel]

I don't know how you do it, but the broad strokes of what you outline is directly in line with what @jonsequitur, @KathleenDollard, and I had been brainstorming for the 'next wave' of suggestions-related features. Kudos!

[KalleOlaviNiemitalo]

There is a security aspect. Using an AssemblyMetadataAttribute or a custom PE section to indicate support for [suggest] would be okay for dotnet tools because the user has taken action to install them and presumably trusts them. But it would not be okay for tab completion when the user has typed the path of some unregistered executable and presses TAB without intending to execute that file right away.

[KalleOlaviNiemitalo]

A new XML element in an assembly manifest as a Win32 resource might be another possibility. However, if it were possible to refer to a separate executable for retrieving the suggestions, then the relative path of that executable would have to be in the manifest, and this relative path would have to be kept valid when files are copied around. Having the suggestion-support information in DotnetToolSettings.xml seems easier because that file is easier to update at install time if paths change.

[KalleOlaviNiemitalo]

The format of DotnetToolSettings.xml doesn't seem to be documented at https://docs.microsoft.com/. The .NET SDK repository has some documentation for it in Tool NuGet package format (not updated for dotnet/sdk#9179), and the code that validates it is in ToolConfigurationDeserializer.cs. In the NuGet wiki, Global Tools NuGet Implementation also links to some specifications, but those links lead to a repository that is private or has been deleted.

dotnet tool install warns if the version number is higher than 1, but it ignores unrecognized attributes and elements. The information about supported directives could thus be added to the Command element without breaking compatibility with older versions of .NET SDK.

Possible approaches for global tools:

• Define a Command/@SupportsSuggestDirective attribute with a Boolean value, and make dotnet tool install --global copy that value into ~/.dotnet/tools-suggest.json.

  DotnetToolSettings.xml:

  <?xml version="1.0" encoding="utf-8"?>
  <DotNetCliTool Version="1">
    <Commands>
      <Command Name="slngen" EntryPoint="slngen.dll" Runner="dotnet" SupportsSuggestDirective="false" />
    </Commands>
  </DotNetCliTool>

  ~/.dotnet/tools-suggest.json:

  { "commands": { "slngen": { "supportsSuggestDirective": false } } }

  Disadvantage: You might later need define something like Command/@SupportsLanguageServerDirective for Using System.CommandLine for powershell completers #1220 and make dotnet tool install --global support that too.

• Define a Command/Suggest/@SupportsSuggestDirective attribute with a Boolean value, and make dotnet tool install --global copy the whole Command/Suggest element into ~/.dotnet/tools-suggest.xml. Future versions of System.CommandLine would then be able to define more attributes and child elements in that, without needing further changes in .NET SDK.

  DotnetToolSettings.xml:

  <?xml version="1.0" encoding="utf-8"?>
  <DotNetCliTool Version="1">
    <Commands>
      <Command Name="slngen" EntryPoint="slngen.dll" Runner="dotnet">
        <Suggest SupportsSuggestDirective="false" />
      </Command>
    </Commands>
  </DotNetCliTool>

  ~/.dotnet/tools-suggest.xml:

  <?xml version="1.0" encoding="utf-8"?>
  <Commands>
    <Command Name="slngen">
      <Suggest SupportsSuggestDirective="false" />
    </Command>
  </Commands>

  Disadvantage: Tool developers might be tempted to abuse the Command/Suggest element for custom metadata that is not related to dotnet-suggest, so that their own applications could then read this metadata from ~/.dotnet/tools-suggest.xml.

• Define a Command/Suggest/@SupportsSuggestDirective attribute with a Boolean value and make dotnet tool install --global copy the whole file to ~/.dotnet/tools/.meta/command/DotnetToolSettings.xml. Tool developers would be able to define their own elements for custom metadata and would not be tempted to abuse the Command/Suggest element.

  DotnetToolSettings.xml and ~/.dotnet/tools/.meta/slngen/DotnetToolSettings.xml:

  <?xml version="1.0" encoding="utf-8"?>
  <Commands>
    <Command Name="slngen">
      <Suggest SupportsSuggestDirective="false" />
      <Documentation href="https://microsoft.github.io/slngen/" />
    </Command>
  </Commands>

• Define a Command/Suggest/@SupportsSuggestDirective attribute with a Boolean value and make dotnet tool install --global create a ~/.dotnet/tools/.meta/command.json file that points to the path of DotnetToolSettings.xml. This would allow attributes and properties of the Command/Suggest element to reference other files, e.g. an XML file that defines the command-line syntax so that dotnet-suggest need not run the target executable.

  DotnetToolSettings.xml: As above.

  ~/.dotnet/tools/.meta/slngen.json:

  { "toolSettingsPath": "/home/you/.nuget/packages/.dotnet/tools/.store/microsoft.visualstudio.slngen.tool/8.5.0/microsoft.visualstudio.slngen.tool/8.5.0/tools/net5.0/any/DotnetToolSettings.xml" }

Similar features for local tools seem more difficult. AFAIK, to find the local tool that dotnet tool run will run, you first have to parse .config/dotnet-tools.json to get the package name and the requested version, and then parse ~/.dotnet/toolResolverCache/1/package or ~/.nuget/packages/. The toolResolverCache implementation in .NET SDK has no public API, and future .NET SDK versions might use a different directory layout or file format. NuGet has public APIs but I assume they would add latency (why else would there be a toolResolverCache). I think a maintainable solution would be to make .NET SDK implement a dotnet tool get-settings-path command command that outputs the path of DotnetToolSettings.xml, and dotnet-suggest would then run that and parse the DotnetToolSettings.xml file. This would ensure that completions correspond to the tool that dotnet tool run would run, even if the tool resolution algorithm changes in a future version of .NET SDK. If this approach is chosen, then it will be simplest to make dotnet-suggest parse DotnetToolSettings.xml for global tools as well, rather than make dotnet tool install --global copy the information to some other file.

[iSazonov]

There is a security aspect. Using an AssemblyMetadataAttribute or a custom PE section to indicate support for [suggest] would be okay for dotnet tools because the user has taken action to install them and presumably trusts them. But it would not be okay for tab completion when the user has typed the path of some unregistered executable and presses TAB without intending to execute that file right away.

It is not security issue since the application is already accessible in the user session. This is rather an undesirable action. And meta information is the best thing that can solve this problem.

Having the suggestion-support information in DotnetToolSettings.xml seems easier because that file is easier to update at install time if paths change.

What about non dotnet tools? The solution must be for all custom applications.

This XML file could have been generated and distributed with custom application. Even better if it were inside as meta information.
Then even PowerShell could use that XML without running the application.

[KathleenDollard]

I think it would be desirable to solve this in a general way of indicating that an executable supports specific features, rather than solving it specifically for .NET Tools, although .NET Tools is a key scenario.

[KalleOlaviNiemitalo]

It is not security issue since the application is already accessible in the user session.

Perhaps so, if the directory of the executable file is in PATH. But if the file is in an untrusted directory, then it should not be executed to get completions, unless the user has registered it to allow this. Users won't expect such execution.

Imagine a user is typing a command line cd /some/dir && ./some-app and then presses TAB. The shell function that runs dotnet-suggest is unlikely to understand that this command line would change the working directory before resolving ./some-app. The shell function might then run dotnet-suggest ./some-app in the wrong directory. If that directory also contains a some-app executable and is not in PATH, then dotnet-suggest should not execute the file.

Alternatively, imagine a user is editing a shell script that is intended to run by a different user "samuel". This shell script contains a command /home/samuel/bin/clip. If the editor invokes a language service provider to get completions for this command, and the /home/samuel/bin directory is not in PATH of the user who is editing, then the LSP should not execute /home/samuel/bin/clip to get completions.

[KalleOlaviNiemitalo]

Even better if it were inside as meta information.
Then even PowerShell could use that XML without running the application.

Do you mean the XML would describe all the options and subcommands supported by the application? Rather than just state that the application supports the [suggest] directive.

If so, I think it would be a good feature but out of scope for this issue.

[iSazonov]

If so, I think it would be a good feature

Yes.