[MTP] Improve performance of validating command line options

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

@@ -127,40 +127,66 @@
         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();
-
-        IEnumerable<string> invalidReservedOptions = allSystemOptions.Intersect(allExtensionOptions);
-        if (invalidReservedOptions.Any())
+        // Create a HashSet of all system option names for faster lookup
+        var systemOptionNames = new HashSet<string>();
+        foreach (KeyValuePair<ICommandLineOptionsProvider, IReadOnlyCollection<CommandLineOption>> provider in systemOptionsByProvider)
         {
-            var stringBuilder = new StringBuilder();
-            foreach (string reservedOption in invalidReservedOptions)
+            foreach (CommandLineOption option in provider.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)));
+                systemOptionNames.Add(option.Name);
             }
+        }
 
-            return ValidationResult.Invalid(stringBuilder.ToTrimmedString());
+        StringBuilder? stringBuilder = null;
+        foreach (KeyValuePair<ICommandLineOptionsProvider, IReadOnlyCollection<CommandLineOption>> provider in extensionOptionsByProvider)
+        {
+            foreach (CommandLineOption option in provider.Value)
+            {
+                if (systemOptionNames.Contains(option.Name))
+                {
+                    stringBuilder ??= new StringBuilder();
+                    stringBuilder.AppendLine(string.Format(CultureInfo.InvariantCulture, PlatformResources.CommandLineOptionIsReserved, option.Name, provider.Key.DisplayName));
+                }
+            }
         }
 
-        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 providers
+        var optionNameToProviders = new Dictionary<string, List<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 List<ICommandLineOptionsProvider>? providers))
+                {
+                    providers = new List<ICommandLineOptionsProvider>();
+                    optionNameToProviders[name] = providers;
+                }
 
+                providers.Add(provider);
+            }
+        }
+
+        // Check for duplications
         StringBuilder? stringBuilder = null;
-        foreach (string duplicatedOption in duplications)
+        foreach (KeyValuePair<string, List<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
@@ -173,10 +199,28 @@
         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));
@@ -192,7 +236,7 @@
         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.
@@ -207,19 +251,22 @@
 
             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();
     }
@@ -280,7 +327,23 @@
     }
 
     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
+    {
+        // Use a more efficient approach to trim without creating unnecessary intermediate strings
+        string result = stringBuilder.ToString();
+        int end = result.Length;
+
+        // Find the last non-whitespace char
+        while (end > 0)
+        {
+            char c = result[end - 1];
+            if (c is not ('\r' or '\n'))
+            {
+                break;
+            }
+
+            end--;
+        }
+
+        return end == result.Length ? result : result.Substring(0, end);
+    }
 }

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,38 @@ 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;
+        }
+
+        public string Uid { get; } = nameof(PlatformCommandLineProvider);
+
+        /// <inheritdoc />
+        public string Version { get; } = AppVersion.DefaultSemVer;
+
+        /// <inheritdoc />
+        public string DisplayName { get; }
+
+        /// <inheritdoc />
+        public string Description { get; } = "Built-in 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;
+    }
 }