Open KalleOlaviNiemitalo opened 2 years ago
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!
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.
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.
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 https://github.com/dotnet/sdk/issues/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 https://github.com/dotnet/command-line-api/issues/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.
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.
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.
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.
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.
If so, I think it would be a good feature
Yes.
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:
[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.dotnet tool install
copy this information to some place from whichdotnet-suggest
can find it, regardless of whetherdotnet-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.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 candotnet-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.