Better search versions error experience (dotnet#45144)

Forgind · Dec 10, 2024 · 152a5d2 · 152a5d2
2 parents 991efb3 + 7c92aa3
commit 152a5d2
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 11 deletions.
diff --git a/src/Cli/dotnet/commands/dotnet-workload/search/WorkloadSearchVersionsCommandParser.cs b/src/Cli/dotnet/commands/dotnet-workload/search/WorkloadSearchVersionsCommandParser.cs
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.CommandLine;
+using Microsoft.DotNet.Workloads.Workload;
 using Microsoft.DotNet.Workloads.Workload.Search;
 using LocalizableStrings = Microsoft.DotNet.Workloads.Workload.Search.LocalizableStrings;
 
@@ -56,6 +57,15 @@ private static CliCommand ConstructCommand()
                 }
             });
 
+            command.Validators.Add(result =>
+            {
+                var versionArgument = result.GetValue(WorkloadVersionArgument);
+                if (versionArgument is not null && !WorkloadSetVersion.IsWorkloadSetPackageVersion(versionArgument))
+                {
+                    result.AddError(string.Format(CommandLineValidation.LocalizableStrings.UnrecognizedCommandOrArgument, versionArgument));
+                }
+            });
+
             command.SetAction(parseResult => new WorkloadSearchVersionsCommand(parseResult).Execute());
 
             return command;

diff --git a/src/Common/WorkloadSetVersion.cs b/src/Common/WorkloadSetVersion.cs
@@ -8,42 +8,46 @@ namespace Microsoft.DotNet.Workloads.Workload
 {
     static class WorkloadSetVersion
     {
-        public static string ToWorkloadSetPackageVersion(string workloadSetVersion, out SdkFeatureBand sdkFeatureBand)
+        private static string[] SeparateCoreComponents(string workloadSetVersion, out string[] sections)
+        {
+            sections = workloadSetVersion.Split(['-', '+'], 2);
+            return sections[0].Split('.');
+        }
+
+        public static bool IsWorkloadSetPackageVersion(string workloadSetVersion)
         {
-            string[] sections = workloadSetVersion.Split(new char[] { '-', '+' }, 2);
-            string versionCore = sections[0];
-            string? preReleaseOrBuild = sections.Length > 1 ? sections[1] : null;
+            int coreComponentsLength = SeparateCoreComponents(workloadSetVersion, out _).Length;
+            return coreComponentsLength >= 3 && coreComponentsLength <= 4;
+        }
 
-            string[] coreComponents = versionCore.Split('.');
+        public static string ToWorkloadSetPackageVersion(string workloadSetVersion, out SdkFeatureBand sdkFeatureBand)
+        {
+            string[] coreComponents = SeparateCoreComponents(workloadSetVersion, out string[] sections);
             string major = coreComponents[0];
             string minor = coreComponents[1];
             string patch = coreComponents[2];
-
             string packageVersion = $"{major}.{patch}.";
             if (coreComponents.Length == 3)
             {
                 //  No workload set patch version
                 packageVersion += "0";
-
                 //  Use preview specifier (if any) from workload set version as part of SDK feature band
                 sdkFeatureBand = new SdkFeatureBand(workloadSetVersion);
             }
             else
             {
                 //  Workload set version has workload patch version (ie 4 components)
                 packageVersion += coreComponents[3];
-
                 //  Don't include any preview specifiers in SDK feature band
                 sdkFeatureBand = new SdkFeatureBand($"{major}.{minor}.{patch}");
             }
 
-            if (preReleaseOrBuild != null)
+            if (sections.Length > 1)
             {
                 //  Figure out if we split on a '-' or '+'
                 char separator = workloadSetVersion[sections[0].Length];
-                packageVersion += separator + preReleaseOrBuild;
+                packageVersion += separator + sections[1];
             }
-
             return packageVersion;
         }
 

diff --git a/test/dotnet-workload-search.Tests/GivenDotnetWorkloadSearch.cs b/test/dotnet-workload-search.Tests/GivenDotnetWorkloadSearch.cs
@@ -28,6 +28,22 @@ public GivenDotnetWorkloadSearch(ITestOutputHelper log) : base(log)
             _reporter = new BufferedReporter();
         }
 
+        [Theory]
+        [InlineData("--invalidArgument")]
+        [InlineData("notAVersion")]
+        [InlineData("1.2")] // too short
+        [InlineData("1.2.3.4.5")] // too long
+        [InlineData("1.2-3.4")] // numbers after [-, +] don't count
+        public void GivenInvalidArgumentToWorkloadSearchVersionItFailsCleanly(string argument)
+        {
+            _reporter.Clear();
+            var parseResult = Parser.Instance.Parse($"dotnet workload search version {argument}");
+            var workloadResolver = new MockWorkloadResolver(Enumerable.Empty<WorkloadResolver.WorkloadInfo>());
+            var workloadResolverFactory = new MockWorkloadResolverFactory(dotnetPath: null, "9.0.100", workloadResolver);
+            var command = () => new WorkloadSearchVersionsCommand(parseResult, _reporter, workloadResolverFactory);
+            command.Should().Throw<CommandParsingException>();
+        }
+
         [Fact]
         public void GivenNoWorkloadsAreInstalledSearchIsEmpty()
         {