-
Notifications
You must be signed in to change notification settings - Fork 291
[MTP] Improve performance of validating command line options #5655
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
d88d0ce
1443d5e
85ffb88
39d6db5
78e96f8
975d2d6
86c987c
2e2ee02
07a4700
6c344de
3ba2fb0
e1369e7
780fd3b
fe102f2
cae2a1b
97c8299
63ab4f7
3f0ce4e
1cc37c3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,14 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using System; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using System.Collections.Generic; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using System.Diagnostics.CodeAnalysis; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using System.Globalization; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using System.Linq; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using System.Text; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using System.Threading.Tasks; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using Microsoft.Testing.Platform.Extensions; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using Microsoft.Testing.Platform.Extensions.CommandLine; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using Microsoft.Testing.Platform.Helpers; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -101,40 +109,69 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| HashSet<string> systemOptionNames = new(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| foreach (var provider in systemOptionsByProvider) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var stringBuilder = new StringBuilder(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| foreach (string reservedOption in invalidReservedOptions) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| foreach (var option in provider.Value) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Evangelink marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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 (var provider in extensionOptionsByProvider) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Evangelink marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| foreach (var option in provider.Value) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (systemOptionNames.Contains(option.Name)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| stringBuilder ??= new StringBuilder(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| stringBuilder.AppendLine(string.Format(CultureInfo.InvariantCulture, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Check failure on line 130 in src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Youssef1313 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| PlatformResources.CommandLineOptionIsReserved, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Check failure on line 131 in src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Youssef1313 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| option.Name, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Check failure on line 132 in src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Youssef1313 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| provider.Key.DisplayName)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+140
to
152
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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)); | |
| } | |
| } | |
| } | |
| // Aggregate reserved options by name and track all offending providers | |
| Dictionary<string, List<ICommandLineOptionsProvider>> reservedOptionToProviders = new(); | |
| foreach (KeyValuePair<ICommandLineOptionsProvider, IReadOnlyCollection<CommandLineOption>> kvp in extensionOptionsByProvider) | |
| { | |
| ICommandLineOptionsProvider provider = kvp.Key; | |
| foreach (CommandLineOption option in kvp.Value) | |
| { | |
| if (systemOptionNames.Contains(option.Name)) | |
| { | |
| if (!reservedOptionToProviders.TryGetValue(option.Name, out List<ICommandLineOptionsProvider>? providers)) | |
| { | |
| providers = new List<ICommandLineOptionsProvider>(); | |
| reservedOptionToProviders[option.Name] = providers; | |
| } | |
| providers.Add(provider); | |
| } | |
| } | |
| } | |
| StringBuilder? stringBuilder = null; | |
| foreach (KeyValuePair<string, List<ICommandLineOptionsProvider>> kvp in reservedOptionToProviders) | |
| { | |
| string reservedOption = kvp.Key; | |
| stringBuilder ??= new StringBuilder(); | |
| IEnumerable<string> faultyProvidersDisplayNames = kvp.Value.Select(p => p.DisplayName); | |
| stringBuilder.AppendLine(string.Format( | |
| CultureInfo.InvariantCulture, | |
| PlatformResources.CommandLineOptionIsReserved, | |
| reservedOption, | |
| string.Join("', '", faultyProvidersDisplayNames))); | |
| } |
Check failure on line 148 in src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs
Azure Pipelines / microsoft.testfx (Build MacOS Debug)
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs#L148
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs(148,1): error IDE0055: (NETCORE_ENGINEERING_TELEMETRY=Build) Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055)
Check failure on line 148 in src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs
Azure Pipelines / microsoft.testfx (Build Linux Debug)
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs#L148
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs(148,1): error IDE0055: (NETCORE_ENGINEERING_TELEMETRY=Build) Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055)
Check failure on line 148 in src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs
Azure Pipelines / microsoft.testfx (Build Linux Release)
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs#L148
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs(148,1): error IDE0055: (NETCORE_ENGINEERING_TELEMETRY=Build) Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055)
Check failure on line 148 in src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs
Azure Pipelines / microsoft.testfx (Build MacOS Release)
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs#L148
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs(148,1): error IDE0055: (NETCORE_ENGINEERING_TELEMETRY=Build) Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055)
Check failure on line 148 in src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs
Azure Pipelines / microsoft.testfx
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs#L148
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs(148,1): error IDE0055: (NETCORE_ENGINEERING_TELEMETRY=Build) Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055)
Youssef1313 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Youssef1313 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Youssef1313 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Youssef1313 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Youssef1313 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Youssef1313 marked this conversation as resolved.
Show resolved
Hide resolved
Check failure on line 163 in src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs
Azure Pipelines / microsoft.testfx (Build MacOS Debug)
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs#L163
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs(163,1): error IDE0055: (NETCORE_ENGINEERING_TELEMETRY=Build) Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055)
Check failure on line 163 in src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs
Azure Pipelines / microsoft.testfx (Build Linux Debug)
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs#L163
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs(163,1): error IDE0055: (NETCORE_ENGINEERING_TELEMETRY=Build) Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055)
Check failure on line 163 in src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs
Azure Pipelines / microsoft.testfx (Build Linux Release)
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs#L163
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs(163,1): error IDE0055: (NETCORE_ENGINEERING_TELEMETRY=Build) Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055)
Check failure on line 163 in src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs
Azure Pipelines / microsoft.testfx (Build MacOS Release)
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs#L163
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.cs(163,1): error IDE0055: (NETCORE_ENGINEERING_TELEMETRY=Build) Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055)
Youssef1313 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Copilot
AI
Mar 31, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ValidateOptionsAreNotDuplicated tracks providers in a List and counts duplicates based on total entries, which will incorrectly report “multiple extensions” if a single provider returns the same option name more than once (and can also print the same provider name multiple times). Track distinct providers per option (e.g., HashSet) and base the duplication decision / formatted provider list on the distinct-provider count.
Uh oh!
There was an error while loading. Please reload this page.