[MTP] Improve performance of validating command line options

src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs

@@ -101,40 +101,79 @@ private static ValidationResult ValidateExtensionOptionsDoNotContainReservedOpti
         Dictionary<ICommandLineOptionsProvider, IReadOnlyCollection<CommandLineOption>> extensionOptionsByProvider,
         Dictionary<ICommandLineOptionsProvider, IReadOnlyCollection<CommandLineOption>> systemOptionsByProvider)
     {
-        IEnumerable<string> allExtensionOptions = extensionOptionsByProvider.Values.SelectMany(x => x).Select(x => x.Name).Distinct();
-        IEnumerable<string> allSystemOptions = systemOptionsByProvider.Values.SelectMany(x => x).Select(x => x.Name).Distinct();
+        // Create a HashSet of all system option names for faster lookup
+        var systemOptionNames = new HashSet<string>();
+        foreach (KeyValuePair<ICommandLineOptionsProvider, IReadOnlyCollection<CommandLineOption>> provider in systemOptionsByProvider)
+        {
+            foreach (CommandLineOption option in provider.Value)
+            {
+                systemOptionNames.Add(option.Name);
+            }
+        }
 
-        IEnumerable<string> invalidReservedOptions = allSystemOptions.Intersect(allExtensionOptions);
-        if (invalidReservedOptions.Any())
+        // Aggregate reserved options by name and track all offending providers
+        var reservedOptionToProviderNames = new Dictionary<string, HashSet<string>>();
+        foreach (KeyValuePair<ICommandLineOptionsProvider, IReadOnlyCollection<CommandLineOption>> kvp in extensionOptionsByProvider)
         {
-            var stringBuilder = new StringBuilder();
-            foreach (string reservedOption in invalidReservedOptions)
+            foreach (CommandLineOption option in kvp.Value)
             {
-                IEnumerable<string> faultyProviderNames = extensionOptionsByProvider.Where(tuple => tuple.Value.Any(x => x.Name == reservedOption)).Select(tuple => tuple.Key.DisplayName);
-                stringBuilder.AppendLine(string.Format(CultureInfo.InvariantCulture, PlatformResources.CommandLineOptionIsReserved, reservedOption, string.Join("', '", faultyProviderNames)));
+                if (systemOptionNames.Contains(option.Name))
+                {
+                    if (!reservedOptionToProviderNames.TryGetValue(option.Name, out HashSet<string>? providerNames))
+                    {
+                        providerNames = [];
+                        reservedOptionToProviderNames[option.Name] = providerNames;
+                    }
+
+                    providerNames.Add(kvp.Key.DisplayName);
+                }
             }
+        }
 
-            return ValidationResult.Invalid(stringBuilder.ToTrimmedString());
+        StringBuilder? stringBuilder = null;
+        foreach (KeyValuePair<string, HashSet<string>> kvp in reservedOptionToProviderNames)
+        {
+            stringBuilder ??= new StringBuilder();
+            stringBuilder.AppendLine(string.Format(CultureInfo.InvariantCulture, PlatformResources.CommandLineOptionIsReserved, kvp.Key, string.Join("', '", kvp.Value)));
         }
 
-        return ValidationResult.Valid();
+        return stringBuilder?.Length > 0
+            ? ValidationResult.Invalid(stringBuilder.ToTrimmedString())
+            : ValidationResult.Valid();
     }
 
     private static ValidationResult ValidateOptionsAreNotDuplicated(
         Dictionary<ICommandLineOptionsProvider, IReadOnlyCollection<CommandLineOption>> extensionOptionsByProvider)
     {
-        IEnumerable<string> duplications = extensionOptionsByProvider.Values.SelectMany(x => x)
-            .Select(x => x.Name)
-            .GroupBy(x => x)
-            .Where(x => x.Skip(1).Any())
-            .Select(x => x.Key);
+        // Use a dictionary to track option names and their distinct providers
+        var optionNameToProviders = new Dictionary<string, HashSet<ICommandLineOptionsProvider>>();
+        foreach (KeyValuePair<ICommandLineOptionsProvider, IReadOnlyCollection<CommandLineOption>> kvp in extensionOptionsByProvider)
+        {
+            ICommandLineOptionsProvider provider = kvp.Key;
+            foreach (CommandLineOption option in kvp.Value)
+            {
+                string name = option.Name;
+                if (!optionNameToProviders.TryGetValue(name, out HashSet<ICommandLineOptionsProvider>? providers))
+                {
+                    providers = [];
+                    optionNameToProviders[name] = providers;
+                }
 
+                providers.Add(provider);
+            }
+        }
+
+        // Check for duplications
         StringBuilder? stringBuilder = null;
-        foreach (string duplicatedOption in duplications)
+        foreach (KeyValuePair<string, HashSet<ICommandLineOptionsProvider>> kvp in optionNameToProviders)
         {
-            IEnumerable<string> faultyProvidersDisplayNames = extensionOptionsByProvider.Where(tuple => tuple.Value.Any(x => x.Name == duplicatedOption)).Select(tuple => tuple.Key.DisplayName);
-            stringBuilder ??= new();
-            stringBuilder.AppendLine(string.Format(CultureInfo.InvariantCulture, PlatformResources.CommandLineOptionIsDeclaredByMultipleProviders, duplicatedOption, string.Join("', '", faultyProvidersDisplayNames)));
+            if (kvp.Value.Count > 1)
+            {
+                string duplicatedOption = kvp.Key;
+                stringBuilder ??= new();
+                IEnumerable<string> faultyProvidersDisplayNames = kvp.Value.Select(p => p.DisplayName);
+                stringBuilder.AppendLine(string.Format(CultureInfo.InvariantCulture, PlatformResources.CommandLineOptionIsDeclaredByMultipleProviders, duplicatedOption, string.Join("', '", faultyProvidersDisplayNames)));
+            }
         }
 
         return stringBuilder?.Length > 0
@@ -147,10 +186,28 @@ private static ValidationResult ValidateNoUnknownOptions(
         Dictionary<ICommandLineOptionsProvider, IReadOnlyCollection<CommandLineOption>> extensionOptionsByProvider,
         Dictionary<ICommandLineOptionsProvider, IReadOnlyCollection<CommandLineOption>> systemOptionsByProvider)
     {
+        // Create a HashSet of all valid option names for faster lookup
+        var validOptionNames = new HashSet<string>();
+        foreach (KeyValuePair<ICommandLineOptionsProvider, IReadOnlyCollection<CommandLineOption>> provider in extensionOptionsByProvider)
+        {
+            foreach (CommandLineOption option in provider.Value)
+            {
+                validOptionNames.Add(option.Name);
+            }
+        }
+
+        foreach (KeyValuePair<ICommandLineOptionsProvider, IReadOnlyCollection<CommandLineOption>> provider in systemOptionsByProvider)
+        {
+            foreach (CommandLineOption option in provider.Value)
+            {
+                validOptionNames.Add(option.Name);
+            }
+        }
+
         StringBuilder? stringBuilder = null;
         foreach (CommandLineParseOption optionRecord in parseResult.Options)
         {
-            if (!extensionOptionsByProvider.Union(systemOptionsByProvider).Any(tuple => tuple.Value.Any(x => x.Name == optionRecord.Name)))
+            if (!validOptionNames.Contains(optionRecord.Name))
             {
                 stringBuilder ??= new();
                 stringBuilder.AppendLine(string.Format(CultureInfo.InvariantCulture, PlatformResources.CommandLineUnknownOption, optionRecord.Name));
@@ -166,7 +223,7 @@ private static ValidationResult ValidateOptionsArgumentArity(
         CommandLineParseResult parseResult,
         Dictionary<string, (ICommandLineOptionsProvider Provider, CommandLineOption Option)> providerAndOptionByOptionName)
     {
-        StringBuilder stringBuilder = new();
+        StringBuilder? stringBuilder = null;
         foreach (IGrouping<string, CommandLineParseOption> groupedOptions in parseResult.Options.GroupBy(x => x.Name))
         {
             // getting the arguments count for an option.
@@ -181,19 +238,22 @@ private static ValidationResult ValidateOptionsArgumentArity(
 
             if (arity > option.Arity.Max && option.Arity.Max == 0)
             {
+                stringBuilder ??= new();
                 stringBuilder.AppendLine(string.Format(CultureInfo.InvariantCulture, PlatformResources.CommandLineOptionExpectsNoArguments, optionName, provider.DisplayName, provider.Uid));
             }
             else if (arity < option.Arity.Min)
             {
+                stringBuilder ??= new();
                 stringBuilder.AppendLine(string.Format(CultureInfo.InvariantCulture, PlatformResources.CommandLineOptionExpectsAtLeastArguments, optionName, provider.DisplayName, provider.Uid, option.Arity.Min));
             }
             else if (arity > option.Arity.Max)
             {
+                stringBuilder ??= new();
                 stringBuilder.AppendLine(string.Format(CultureInfo.InvariantCulture, PlatformResources.CommandLineOptionExpectsAtMostArguments, optionName, provider.DisplayName, provider.Uid, option.Arity.Max));
             }
         }
 
-        return stringBuilder.Length > 0
+        return stringBuilder?.Length > 0
             ? ValidationResult.Invalid(stringBuilder.ToTrimmedString())
             : ValidationResult.Valid();
     }
@@ -254,7 +314,13 @@ private static async Task<ValidationResult> ValidateConfigurationAsync(
     }
 
     private static string ToTrimmedString(this StringBuilder stringBuilder)
-#pragma warning disable RS0030 // Do not use banned APIs
-        => stringBuilder.ToString().TrimEnd(Environment.NewLine.ToCharArray());
-#pragma warning restore RS0030 // Do not use banned APIs
+    {
+        // Trim trailing CR/LF characters directly from the StringBuilder to avoid extra allocations
+        while (stringBuilder.Length > 0 && stringBuilder[stringBuilder.Length - 1] is '\r' or '\n')
+        {
+            stringBuilder.Length--;
+        }
+
+        return stringBuilder.ToString();
+    }
 }

test/UnitTests/Microsoft.Testing.Platform.UnitTests/CommandLine/CommandLineHandlerTests.cs

@@ -231,6 +231,91 @@ public async Task ParseAndValidateAsync_UnknownOption_ReturnsFalse()
         Assert.AreEqual("Unknown option '--x'", result.ErrorMessage);
     }
 
+    [TestMethod]
+    public async Task ParseAndValidateAsync_MultipleUnknownOptions_ReportsAll()
+    {
+        // Arrange
+        string[] args = ["--x", "--y"];
+        CommandLineParseResult parseResult = CommandLineParser.Parse(args, new SystemEnvironment());
+
+        ICommandLineOptionsProvider[] extensionCommandLineProvider =
+        [
+            new ExtensionCommandLineProviderMockUnknownOption()
+        ];
+
+        // Act
+        ValidationResult result = await CommandLineOptionsValidator.ValidateAsync(parseResult, _systemCommandLineOptionsProviders,
+            extensionCommandLineProvider, new Mock<ICommandLineOptions>().Object);
+
+        // Assert
+        Assert.IsFalse(result.IsValid);
+        Assert.Contains("Unknown option '--x'", result.ErrorMessage);
+        Assert.Contains("Unknown option '--y'", result.ErrorMessage);
+    }
+
+    [TestMethod]
+    public async Task ParseAndValidateAsync_MultipleReservedOptionsFromDifferentProviders_ReturnsFalse()
+    {
+        // Arrange
+        string[] args = [];
+        CommandLineParseResult parseResult = CommandLineParser.Parse(args, new SystemEnvironment());
+        ICommandLineOptionsProvider[] extensionCommandLineProvider =
+        [
+            new ExtensionCommandLineProviderMockReservedOptions(),
+            new ExtensionCommandLineProviderMockWithNamedOption("help", "Provider2")
+        ];
+
+        // Act
+        ValidationResult result = await CommandLineOptionsValidator.ValidateAsync(parseResult, _systemCommandLineOptionsProviders,
+            extensionCommandLineProvider, new Mock<ICommandLineOptions>().Object);
+
+        // Assert
+        Assert.IsFalse(result.IsValid);
+        Assert.Contains("Option '--help' is reserved and cannot be used by providers: 'help'", result.ErrorMessage);
+    }
+
+    [TestMethod]
+    public async Task ParseAndValidateAsync_DuplicateOptionWithDistinctProviderNames_ReportsAllProviders()
+    {
+        // Arrange
+        string[] args = [];
+        CommandLineParseResult parseResult = CommandLineParser.Parse(args, new SystemEnvironment());
+        ICommandLineOptionsProvider[] extensionCommandLineOptionsProviders =
+        [
+            new ExtensionCommandLineProviderMockWithNamedOption("userOption", "ProviderOne"),
+            new ExtensionCommandLineProviderMockWithNamedOption("userOption", "ProviderTwo")
+        ];
+
+        // Act
+        ValidationResult result = await CommandLineOptionsValidator.ValidateAsync(parseResult, _systemCommandLineOptionsProviders,
+            extensionCommandLineOptionsProviders, new Mock<ICommandLineOptions>().Object);
+
+        // Assert
+        Assert.IsFalse(result.IsValid);
+        Assert.Contains("Option '--userOption' is declared by multiple extensions: 'ProviderOne', 'ProviderTwo'", result.ErrorMessage);
+    }
+
+    [TestMethod]
+    public async Task ParseAndValidateAsync_ValidOptionsWithManyProviders_ReturnsTrue()
+    {
+        // Arrange
+        string[] args = ["--option1", "--option2", "--option3"];
+        CommandLineParseResult parseResult = CommandLineParser.Parse(args, new SystemEnvironment());
+        ICommandLineOptionsProvider[] extensionCommandLineOptionsProviders =
+        [
+            new ExtensionCommandLineProviderMockWithNamedOption("option1", "Provider1"),
+            new ExtensionCommandLineProviderMockWithNamedOption("option2", "Provider2"),
+            new ExtensionCommandLineProviderMockWithNamedOption("option3", "Provider3")
+        ];
+
+        // Act
+        ValidationResult result = await CommandLineOptionsValidator.ValidateAsync(parseResult, _systemCommandLineOptionsProviders,
+            extensionCommandLineOptionsProviders, new Mock<ICommandLineOptions>().Object);
+
+        // Assert
+        Assert.IsTrue(result.IsValid);
+    }
+
     [TestMethod]
     public async Task ParseAndValidateAsync_InvalidValidConfiguration_ReturnsFalse()
     {
@@ -435,4 +520,39 @@ public IReadOnlyCollection<CommandLineOption> GetCommandLineOptions() =>
 
         public Task<ValidationResult> ValidateOptionArgumentsAsync(CommandLineOption commandOption, string[] arguments) => ValidationResult.ValidTask;
     }
+
+    private sealed class ExtensionCommandLineProviderMockWithNamedOption : ICommandLineOptionsProvider
+    {
+        private readonly string _option;
+
+        public ExtensionCommandLineProviderMockWithNamedOption(string optionName, string displayName)
+        {
+            _option = optionName;
+            DisplayName = displayName;
+            Uid = $"TestMock_{displayName}";
+        }
+
+        public string Uid { get; }
+
+        /// <inheritdoc />
+        public string Version { get; } = AppVersion.DefaultSemVer;
+
+        /// <inheritdoc />
+        public string DisplayName { get; }
+
+        /// <inheritdoc />
+        public string Description { get; } = "Test extension command line provider";
+
+        /// <inheritdoc />
+        public Task<bool> IsEnabledAsync() => Task.FromResult(true);
+
+        public IReadOnlyCollection<CommandLineOption> GetCommandLineOptions() =>
+        [
+            new(_option, "Show command line option.", ArgumentArity.ZeroOrOne, false)
+        ];
+
+        public Task<ValidationResult> ValidateCommandLineOptionsAsync(ICommandLineOptions commandLineOptions) => ValidationResult.ValidTask;
+
+        public Task<ValidationResult> ValidateOptionArgumentsAsync(CommandLineOption commandOption, string[] arguments) => ValidationResult.ValidTask;
+    }
 }