From 8bb601df741b96d98d959836c795af9fad5d08bc Mon Sep 17 00:00:00 2001 From: tmat Date: Fri, 6 Feb 2026 12:25:06 -0800 Subject: [PATCH 1/2] Avoid mutable global state --- .../Package/PackageAddCommandDefinition.cs | 2 +- .../ReferenceAddCommandDefinition.cs | 2 +- .../Restore/RestoreCommandDefinition.cs | 2 +- .../Common/CommonArguments.cs | 4 +- .../Common/CommonOptions.cs | 6 +- .../Common/TargetPlatformOptions.cs | 2 +- .../DynamicSymbolExtensions.cs | 55 ------------------- .../ExtendedSymbols.cs | 29 ++++++++++ .../shells/PowershellShellProvider.cs | 2 +- 9 files changed, 39 insertions(+), 65 deletions(-) delete mode 100644 src/System.CommandLine.StaticCompletions/DynamicSymbolExtensions.cs create mode 100644 src/System.CommandLine.StaticCompletions/ExtendedSymbols.cs diff --git a/src/Cli/Microsoft.DotNet.Cli.Definitions/Commands/Package/PackageAddCommandDefinition.cs b/src/Cli/Microsoft.DotNet.Cli.Definitions/Commands/Package/PackageAddCommandDefinition.cs index f23445a5a9e4..bdd7b782eb63 100644 --- a/src/Cli/Microsoft.DotNet.Cli.Definitions/Commands/Package/PackageAddCommandDefinition.cs +++ b/src/Cli/Microsoft.DotNet.Cli.Definitions/Commands/Package/PackageAddCommandDefinition.cs @@ -17,7 +17,7 @@ public class PackageAddCommandDefinition() : PackageAddCommandDefinitionBase(Nam public abstract class PackageAddCommandDefinitionBase : Command { - public static Option CreateVersionOption() => new Option("--version", "-v") + public static Option CreateVersionOption() => new ExtendedOption("--version", "-v") { Description = CommandDefinitionStrings.CmdVersionDescription, HelpName = CommandDefinitionStrings.CmdVersion, diff --git a/src/Cli/Microsoft.DotNet.Cli.Definitions/Commands/Reference/ReferenceAddCommandDefinition.cs b/src/Cli/Microsoft.DotNet.Cli.Definitions/Commands/Reference/ReferenceAddCommandDefinition.cs index 31b17d874860..ea1fdc471457 100644 --- a/src/Cli/Microsoft.DotNet.Cli.Definitions/Commands/Reference/ReferenceAddCommandDefinition.cs +++ b/src/Cli/Microsoft.DotNet.Cli.Definitions/Commands/Reference/ReferenceAddCommandDefinition.cs @@ -31,7 +31,7 @@ internal abstract class ReferenceAddCommandDefinitionBase : Command } }; - public readonly Option FrameworkOption = new("--framework", "-f") + public readonly Option FrameworkOption = new ExtendedOption("--framework", "-f") { Description = CommandDefinitionStrings.ReferenceAddCmdFrameworkDescription, HelpName = CommandDefinitionStrings.CommonCmdFramework, diff --git a/src/Cli/Microsoft.DotNet.Cli.Definitions/Commands/Restore/RestoreCommandDefinition.cs b/src/Cli/Microsoft.DotNet.Cli.Definitions/Commands/Restore/RestoreCommandDefinition.cs index 47ab290a2b85..bd734c999ecb 100644 --- a/src/Cli/Microsoft.DotNet.Cli.Definitions/Commands/Restore/RestoreCommandDefinition.cs +++ b/src/Cli/Microsoft.DotNet.Cli.Definitions/Commands/Restore/RestoreCommandDefinition.cs @@ -112,7 +112,7 @@ public static string RestoreRuntimeArgFunc(IEnumerable rids) return $"-property:RuntimeIdentifiers={string.Join("%3B", convertedRids)}"; } - private static Option> CreateRuntimeOption() => new Option>("--runtime", "-r") + private static Option> CreateRuntimeOption() => new ExtendedOption>("--runtime", "-r") { Description = CommandDefinitionStrings.CmdRuntimeOptionDescription, HelpName = CommandDefinitionStrings.CmdRuntimeOption, diff --git a/src/Cli/Microsoft.DotNet.Cli.Definitions/Common/CommonArguments.cs b/src/Cli/Microsoft.DotNet.Cli.Definitions/Common/CommonArguments.cs index 07b7edc76507..16aaaaef7fb8 100644 --- a/src/Cli/Microsoft.DotNet.Cli.Definitions/Common/CommonArguments.cs +++ b/src/Cli/Microsoft.DotNet.Cli.Definitions/Common/CommonArguments.cs @@ -14,7 +14,7 @@ internal static class CommonArguments public const string PackageIdArgumentName = "packageId"; public static Argument CreateOptionalPackageIdentityArgument(string examplePackage = "Newtonsoft.Json", string exampleVersion = "13.0.3") => - new(PackageIdArgumentName) + new ExtendedArgument(PackageIdArgumentName) { Description = string.Format(CommandDefinitionStrings.PackageIdentityArgumentDescription, examplePackage, exampleVersion), CustomParser = argumentResult => ParsePackageIdentityWithVersionSeparator(argumentResult.Tokens[0]?.Value), @@ -23,7 +23,7 @@ internal static class CommonArguments }; public static Argument CreateRequiredPackageIdentityArgument(string examplePackage = "Newtonsoft.Json", string exampleVersion = "13.0.3") => - new(PackageIdArgumentName) + new ExtendedArgument(PackageIdArgumentName) { Description = string.Format(CommandDefinitionStrings.PackageIdentityArgumentDescription, examplePackage, exampleVersion), CustomParser = argumentResult => ParsePackageIdentityWithVersionSeparator(argumentResult.Tokens[0]?.Value)!.Value, diff --git a/src/Cli/Microsoft.DotNet.Cli.Definitions/Common/CommonOptions.cs b/src/Cli/Microsoft.DotNet.Cli.Definitions/Common/CommonOptions.cs index abac7ed2d280..f13d1ffe357c 100644 --- a/src/Cli/Microsoft.DotNet.Cli.Definitions/Common/CommonOptions.cs +++ b/src/Cli/Microsoft.DotNet.Cli.Definitions/Common/CommonOptions.cs @@ -12,7 +12,7 @@ namespace Microsoft.DotNet.Cli; internal static class CommonOptions { - public static Option CreateYesOption() => new("--yes", "-y") + public static Option CreateYesOption() => new ExtendedOption("--yes", "-y") { Description = CommandDefinitionStrings.YesOptionDescription, Arity = ArgumentArity.Zero, @@ -169,7 +169,7 @@ public static Option CreateHiddenVerbosityOption() => public const string FrameworkOptionName = "--framework"; public static Option CreateFrameworkOption(string description) => - new Option(FrameworkOptionName, "-f") + new ExtendedOption(FrameworkOptionName, "-f") { Description = description, HelpName = CommandDefinitionStrings.FrameworkArgumentName, @@ -196,7 +196,7 @@ public static Option CreateUseCurrentRuntimeOption(string description) => public const string ConfigurationOptionName = "--configuration"; public static Option CreateConfigurationOption(string description) => - new Option(ConfigurationOptionName, "-c") + new ExtendedOption(ConfigurationOptionName, "-c") { Description = description, HelpName = CommandDefinitionStrings.ConfigurationArgumentName, diff --git a/src/Cli/Microsoft.DotNet.Cli.Definitions/Common/TargetPlatformOptions.cs b/src/Cli/Microsoft.DotNet.Cli.Definitions/Common/TargetPlatformOptions.cs index 165c87fab068..82f20c3f6489 100644 --- a/src/Cli/Microsoft.DotNet.Cli.Definitions/Common/TargetPlatformOptions.cs +++ b/src/Cli/Microsoft.DotNet.Cli.Definitions/Common/TargetPlatformOptions.cs @@ -55,7 +55,7 @@ public static IEnumerable RuntimeArgFunc(string rid) } public static Option CreateRuntimeOption(string description) => - new Option(RuntimeOptionName, "-r") + new ExtendedOption(RuntimeOptionName, "-r") { HelpName = CommandDefinitionStrings.RuntimeIdentifierArgumentName, Description = description, diff --git a/src/System.CommandLine.StaticCompletions/DynamicSymbolExtensions.cs b/src/System.CommandLine.StaticCompletions/DynamicSymbolExtensions.cs deleted file mode 100644 index c59d18c7bec1..000000000000 --- a/src/System.CommandLine.StaticCompletions/DynamicSymbolExtensions.cs +++ /dev/null @@ -1,55 +0,0 @@ -namespace System.CommandLine.StaticCompletions; - -/// -/// Extensions for marking options or arguments require dynamic completions. Such symbols get special handling -/// in the static completion generation logic. -/// -public static class DynamicSymbolExtensions -{ - /// - /// The state that is used to track which symbols are dynamic. - /// - private static readonly Dictionary s_dynamicSymbols = []; - - extension(Option option) - { - /// - /// Indicates whether this option requires a dynamic call into the dotnet process to compute completions. - /// - public bool IsDynamic - { - get => s_dynamicSymbols.GetValueOrDefault(option, false); - set => s_dynamicSymbols[option] = value; - } - - /// - /// Mark this option as requiring dynamic completions. - /// - /// - public Option RequiresDynamicCompletion() - { - option.IsDynamic = true; - return option; - } - } - - extension(Argument argument) - { - /// Indicates whether this argument requires a dynamic call into the dotnet process to compute completions. - public bool IsDynamic - { - get => s_dynamicSymbols.GetValueOrDefault(argument, false); - set => s_dynamicSymbols[argument] = value; - } - - /// - /// Mark this argument as requiring dynamic completions. - /// - /// - public Argument RequiresDynamicCompletion() - { - argument.IsDynamic = true; - return argument; - } - } -} diff --git a/src/System.CommandLine.StaticCompletions/ExtendedSymbols.cs b/src/System.CommandLine.StaticCompletions/ExtendedSymbols.cs new file mode 100644 index 000000000000..714446b108bd --- /dev/null +++ b/src/System.CommandLine.StaticCompletions/ExtendedSymbols.cs @@ -0,0 +1,29 @@ +namespace System.CommandLine.StaticCompletions; + +public interface IExtendedSymbol +{ + /// + /// Indicates whether this option requires a dynamic call into the dotnet process to compute completions. + /// + bool IsDynamic { get; set; } +} + +public sealed class ExtendedOption(string name, params string[] aliases) + : Option(name, aliases), IExtendedSymbol +{ + public bool IsDynamic { get; set; } +} + +public sealed class ExtendedArgument(string name) + : Argument(name), IExtendedSymbol +{ + public bool IsDynamic { get; set; } +} + +public static class SymbolExtensions +{ + extension(Symbol symbol) + { + public bool IsDynamic => symbol is IExtendedSymbol { IsDynamic: true }; + } +} diff --git a/src/System.CommandLine.StaticCompletions/shells/PowershellShellProvider.cs b/src/System.CommandLine.StaticCompletions/shells/PowershellShellProvider.cs index 528a78703347..4712d87f12a8 100644 --- a/src/System.CommandLine.StaticCompletions/shells/PowershellShellProvider.cs +++ b/src/System.CommandLine.StaticCompletions/shells/PowershellShellProvider.cs @@ -121,7 +121,7 @@ private static IEnumerable GenerateArgumentCompletions(Argument argument yield break; } - if (argument.IsDynamic) + if (argument is IExtendedSymbol { IsDynamic: true }) { // if the argument is a not-static-friendly argument, we need to call into the app for completions // TODO: not yet supported for powershell From 547e8295e928d84fb32a9e078f3c889a89147117 Mon Sep 17 00:00:00 2001 From: tmat Date: Fri, 6 Feb 2026 12:50:37 -0800 Subject: [PATCH 2/2] Fix --- .../Commands/Reference/ReferenceRemoveCommandDefinition.cs | 4 +++- src/Cli/dotnet/Commands/Reference/ReferenceCommandParser.cs | 6 ------ .../ZshShellProviderTests.cs | 4 ++-- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/Cli/Microsoft.DotNet.Cli.Definitions/Commands/Reference/ReferenceRemoveCommandDefinition.cs b/src/Cli/Microsoft.DotNet.Cli.Definitions/Commands/Reference/ReferenceRemoveCommandDefinition.cs index f1f48ee43d03..ea14450f4c7d 100644 --- a/src/Cli/Microsoft.DotNet.Cli.Definitions/Commands/Reference/ReferenceRemoveCommandDefinition.cs +++ b/src/Cli/Microsoft.DotNet.Cli.Definitions/Commands/Reference/ReferenceRemoveCommandDefinition.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.CommandLine; +using System.CommandLine.StaticCompletions; namespace Microsoft.DotNet.Cli.Commands.Reference.Remove; @@ -17,10 +18,11 @@ internal sealed class ReferenceRemoveCommandDefinition() : ReferenceRemoveComman internal abstract class ReferenceRemoveCommandDefinitionBase : Command { - public static Argument> CreateProjectPathArgument() => new(CommandDefinitionStrings.ReferenceRemoveProjectPathArgumentName) + public static Argument> CreateProjectPathArgument() => new ExtendedArgument>(CommandDefinitionStrings.ReferenceRemoveProjectPathArgumentName) { Description = CommandDefinitionStrings.ReferenceRemoveProjectPathArgumentDescription, Arity = ArgumentArity.OneOrMore, + IsDynamic = true, }; public static Option CreateFrameworkOption() => new("--framework", "-f") diff --git a/src/Cli/dotnet/Commands/Reference/ReferenceCommandParser.cs b/src/Cli/dotnet/Commands/Reference/ReferenceCommandParser.cs index eccd1f478e17..5ae73e73dc9b 100644 --- a/src/Cli/dotnet/Commands/Reference/ReferenceCommandParser.cs +++ b/src/Cli/dotnet/Commands/Reference/ReferenceCommandParser.cs @@ -19,13 +19,7 @@ public static void ConfigureCommand(ReferenceCommandDefinition command) command.AddCommand.SetAction(parseResult => new ReferenceAddCommand(parseResult).Execute()); command.AddCommand.FrameworkOption.AddCompletions(CliCompletion.TargetFrameworksFromProjectFile); - command.ListCommand.SetAction(parseResult => new ReferenceListCommand(parseResult).Execute()); - - var projectPathArgument = command.RemoveCommand.ProjectPathArgument; - projectPathArgument.CompletionSources.Add(CliCompletion.ProjectReferencesFromProjectFile); - projectPathArgument.IsDynamic = true; - command.RemoveCommand.SetAction(parseResult => new ReferenceRemoveCommand(parseResult).Execute()); } } diff --git a/test/System.CommandLine.StaticCompletions.Tests/ZshShellProviderTests.cs b/test/System.CommandLine.StaticCompletions.Tests/ZshShellProviderTests.cs index 5e2ec51d0b03..80ca4a129345 100644 --- a/test/System.CommandLine.StaticCompletions.Tests/ZshShellProviderTests.cs +++ b/test/System.CommandLine.StaticCompletions.Tests/ZshShellProviderTests.cs @@ -42,12 +42,12 @@ public async Task GenericCompletions() [Fact] public async Task DynamicCompletionsGeneration() { - var staticOption = new Option("--static") + var staticOption = new ExtendedOption("--static") { IsDynamic = true }; staticOption.AcceptOnlyFromAmong("1", "2", "3"); - var dynamicArg = new Argument("--dynamic") + var dynamicArg = new ExtendedArgument("--dynamic") { IsDynamic = true };