(build) Refactor task arguments for readability and maintainability

arturcic · arturcic · commit 297004be7bdc · 2024-10-09T23:54:14.000+02:00
This commit improves the arguments presented by each task in our system, switching from directly using TaskArgument with constant data to an attribute approach. This change will make our code easier to read and maintain, whilst preserving the functionality of the tasks. The attribute classes provide a more explicit specification of the argument options, leading to a better understanding of the system requirements.
diff --git a/build/artifacts/Tasks/ArtifactsDotnetToolTest.cs b/build/artifacts/Tasks/ArtifactsDotnetToolTest.cs
@@ -4,9 +4,9 @@ namespace Artifacts.Tasks;
 
 [TaskName(nameof(ArtifactsDotnetToolTest))]
 [TaskDescription("Tests the dotnet global tool in docker container")]
-[TaskArgument(Arguments.DockerRegistry, Constants.DockerHub, Constants.GitHub)]
-[TaskArgument(Arguments.DockerDotnetVersion, Constants.VersionCurrent, Constants.VersionLatest)]
-[TaskArgument(Arguments.DockerDistro, Constants.AlpineLatest, Constants.DebianLatest, Constants.UbuntuLatest)]
+[DockerRegistryArgument]
+[DockerDotnetArgument]
+[DockerDistroArgument]
 [IsDependentOn(typeof(ArtifactsPrepare))]
 public class ArtifactsDotnetToolTest : FrostingTask<BuildContext>
 {
diff --git a/build/artifacts/Tasks/ArtifactsMsBuildCoreTest.cs b/build/artifacts/Tasks/ArtifactsMsBuildCoreTest.cs
@@ -4,9 +4,9 @@ namespace Artifacts.Tasks;
 
 [TaskName(nameof(ArtifactsMsBuildCoreTest))]
 [TaskDescription("Tests the msbuild package in docker container")]
-[TaskArgument(Arguments.DockerRegistry, Constants.DockerHub, Constants.GitHub)]
-[TaskArgument(Arguments.DockerDotnetVersion, Constants.VersionCurrent, Constants.VersionLatest)]
-[TaskArgument(Arguments.DockerDistro, Constants.AlpineLatest, Constants.DebianLatest, Constants.UbuntuLatest)]
+[DockerRegistryArgument]
+[DockerDotnetArgument]
+[DockerDistroArgument]
 [IsDependentOn(typeof(ArtifactsPrepare))]
 public class ArtifactsMsBuildCoreTest : FrostingTask<BuildContext>
 {
diff --git a/build/artifacts/Tasks/ArtifactsNativeTest.cs b/build/artifacts/Tasks/ArtifactsNativeTest.cs
@@ -4,9 +4,9 @@ namespace Artifacts.Tasks;
 
 [TaskName(nameof(ArtifactsNativeTest))]
 [TaskDescription("Tests the native executables in docker container")]
-[TaskArgument(Arguments.DockerRegistry, Constants.DockerHub, Constants.GitHub)]
-[TaskArgument(Arguments.DockerDotnetVersion, Constants.VersionCurrent, Constants.VersionLatest)]
-[TaskArgument(Arguments.DockerDistro, Constants.AlpineLatest, Constants.DebianLatest, Constants.UbuntuLatest)]
+[DockerRegistryArgument]
+[DockerDotnetArgument]
+[DockerDistroArgument]
 [IsDependentOn(typeof(ArtifactsPrepare))]
 public class ArtifactsNativeTest : FrostingTask<BuildContext>
 {
diff --git a/build/artifacts/Tasks/ArtifactsPrepare.cs b/build/artifacts/Tasks/ArtifactsPrepare.cs
@@ -4,9 +4,9 @@ namespace Artifacts.Tasks;
 
 [TaskName(nameof(ArtifactsPrepare))]
 [TaskDescription("Pulls the docker images needed for testing the artifacts")]
-[TaskArgument(Arguments.DockerRegistry, Constants.DockerHub, Constants.GitHub)]
-[TaskArgument(Arguments.DockerDotnetVersion, Constants.VersionCurrent, Constants.VersionLatest)]
-[TaskArgument(Arguments.DockerDistro, Constants.AlpineLatest, Constants.DebianLatest, Constants.UbuntuLatest)]
+[DockerRegistryArgument]
+[DockerDotnetArgument]
+[DockerDistroArgument]
 public class ArtifactsPrepare : FrostingTask<BuildContext>
 {
     public override bool ShouldRun(BuildContext context)
diff --git a/build/artifacts/Tasks/ArtifactsTest.cs b/build/artifacts/Tasks/ArtifactsTest.cs
@@ -4,9 +4,9 @@ namespace Artifacts.Tasks;
 
 [TaskName(nameof(ArtifactsTest))]
 [TaskDescription("Tests packages in docker container")]
-[TaskArgument(Arguments.DockerRegistry, Constants.DockerHub, Constants.GitHub)]
-[TaskArgument(Arguments.DockerDotnetVersion, Constants.VersionCurrent, Constants.VersionLatest)]
-[TaskArgument(Arguments.DockerDistro, Constants.AlpineLatest, Constants.DebianLatest, Constants.UbuntuLatest)]
+[DockerRegistryArgument]
+[DockerDotnetArgument]
+[DockerDistroArgument]
 [IsDependentOn(typeof(ArtifactsNativeTest))]
 [IsDependentOn(typeof(ArtifactsDotnetToolTest))]
 [IsDependentOn(typeof(ArtifactsMsBuildCoreTest))]
diff --git a/build/build/Tasks/Test/UnitTest.cs b/build/build/Tasks/Test/UnitTest.cs
@@ -8,7 +8,7 @@ namespace Build.Tasks;
 
 [TaskName(nameof(UnitTest))]
 [TaskDescription("Run the unit tests")]
-[TaskArgument(Arguments.DotnetTarget, Constants.VersionCurrent, Constants.VersionLatest)]
+[DotnetArgument]
 [IsDependentOn(typeof(Build))]
 public class UnitTest : FrostingTask<BuildContext>
 {
diff --git a/build/common/Addins/GitVersion/GitVersionAliases.cs b/build/common/Addins/GitVersion/GitVersionAliases.cs
@@ -54,10 +54,7 @@ public static class GitVersionAliases
     [CakeMethodAlias]
     public static GitVersion GitVersion(this ICakeContext context)
     {
-        if (context == null)
-        {
-            throw new ArgumentNullException(nameof(context));
-        }
+        ArgumentNullException.ThrowIfNull(context);
 
         return GitVersion(context, new GitVersionSettings());
     }
@@ -104,10 +101,7 @@ public static GitVersion GitVersion(this ICakeContext context)
     [CakeMethodAlias]
     public static GitVersion GitVersion(this ICakeContext context, GitVersionSettings settings)
     {
-        if (context == null)
-        {
-            throw new ArgumentNullException(nameof(context));
-        }
+        ArgumentNullException.ThrowIfNull(context);
 
         var gitVersionRunner = new GitVersionRunner(context.FileSystem, context.Environment, context.ProcessRunner, context.Tools, context.Log);
         return gitVersionRunner.Run(settings);
diff --git a/build/common/Addins/GitVersion/GitVersionRunner.cs b/build/common/Addins/GitVersion/GitVersionRunner.cs
@@ -8,7 +8,7 @@ namespace Common.Addins.GitVersion;
 /// </summary>
 public sealed class GitVersionRunner : Tool<GitVersionSettings>
 {
-    private readonly ICakeLog log;
+    private readonly ICakeLog _log;
 
     /// <summary>
     /// Initializes a new instance of the <see cref="GitVersionRunner"/> class.
@@ -23,7 +23,7 @@ public GitVersionRunner(
         ICakeEnvironment environment,
         IProcessRunner processRunner,
         IToolLocator tools,
-        ICakeLog log) : base(fileSystem, environment, processRunner, tools) => this.log = log;
+        ICakeLog log) : base(fileSystem, environment, processRunner, tools) => this._log = log;
 
     /// <summary>
     /// Runs GitVersion and processes the results.
@@ -32,31 +32,28 @@ public GitVersionRunner(
     /// <returns>A task with the GitVersion results.</returns>
     public GitVersion Run(GitVersionSettings settings)
     {
-        if (settings == null)
-        {
-            throw new ArgumentNullException(nameof(settings));
-        }
+        ArgumentNullException.ThrowIfNull(settings);
 
         var output = string.Empty;
         Run(settings, GetArguments(settings), new ProcessSettings { RedirectStandardOutput = true }, process =>
         {
             output = string.Join("\n", process.GetStandardOutput());
-            if (log.Verbosity < Verbosity.Diagnostic)
+            if (this._log.Verbosity < Verbosity.Diagnostic)
             {
                 var errors = Regex.Matches(output, @"( *ERROR:? [^\n]*)\n([^\n]*)")
                     .SelectMany(match => new[] { match.Groups[1].Value, match.Groups[2].Value });
                 foreach (var error in errors)
                 {
-                    log.Error(error);
+                    this._log.Error(error);
                 }
             }
         });
 
         if (!settings.OutputTypes.Contains(GitVersionOutput.Json))
             return new GitVersion();
 
-        var jsonStartIndex = output.IndexOf("{", StringComparison.Ordinal);
-        var jsonEndIndex = output.IndexOf("}", StringComparison.Ordinal);
+        var jsonStartIndex = output.IndexOf('{');
+        var jsonEndIndex = output.IndexOf('}');
         var json = output.Substring(jsonStartIndex, jsonEndIndex - jsonStartIndex + 1);
 
         return JsonConvert.DeserializeObject<GitVersion>(json) ?? new GitVersion();
@@ -119,7 +116,7 @@ private ProcessArgumentBuilder GetArguments(GitVersionSettings settings)
             }
             else
             {
-                log.Warning("If you leave the branch name for GitVersion unset, it will fallback to the default branch for the repository.");
+                this._log.Warning("If you leave the branch name for GitVersion unset, it will fallback to the default branch for the repository.");
             }
 
             if (!string.IsNullOrWhiteSpace(settings.Commit))
@@ -146,7 +143,7 @@ private ProcessArgumentBuilder GetArguments(GitVersionSettings settings)
             builder.Append("-nofetch");
         }
 
-        var verbosity = settings.Verbosity ?? log.Verbosity;
+        var verbosity = settings.Verbosity ?? this._log.Verbosity;
 
         if (verbosity != Verbosity.Normal)
         {
diff --git a/build/common/Utilities/Constants.cs b/build/common/Utilities/Constants.cs
@@ -21,18 +21,20 @@ public static class Constants
     public const string GitHub = "github";
     public const string DockerHubRegistry = "docker.io";
     public const string GitHubContainerRegistry = "ghcr.io";
+    public static readonly string[] DockerRegistries = [DockerHub, GitHub];
 
     public const string Arm64 = "arm64";
     public const string Amd64 = "amd64";
 
+    public static readonly string[] Architectures = [Amd64, Arm64];
+
     public const string AlpineLatest = "alpine.3.20";
     public const string CentosStreamLatest = "centos.stream.9";
     public const string DebianLatest = "debian.12";
     public const string FedoraLatest = "fedora.40";
     public const string Ubuntu2004 = "ubuntu.20.04";
     public const string Ubuntu2204 = "ubuntu.22.04";
     public const string Ubuntu2404 = "ubuntu.24.04";
-    public const string UbuntuLatest = Ubuntu2404;
 
     public const string DockerDistroLatest = DebianLatest;
 
diff --git a/build/common/Utilities/Extensions.cs b/build/common/Utilities/Extensions.cs
@@ -13,32 +13,23 @@ where baseType.IsAssignableFrom(type) && info.IsClass && !info.IsAbstract
 
     public static string GetTaskDescription(this Type task)
     {
-        if (task is null)
-        {
-            throw new ArgumentNullException(nameof(task));
-        }
+        ArgumentNullException.ThrowIfNull(task);
 
         var attribute = task.GetCustomAttribute<TaskDescriptionAttribute>();
         return attribute != null ? attribute.Description : string.Empty;
     }
 
     public static string GetTaskName(this Type task)
     {
-        if (task is null)
-        {
-            throw new ArgumentNullException(nameof(task));
-        }
+        ArgumentNullException.ThrowIfNull(task);
 
         var attribute = task.GetCustomAttribute<TaskNameAttribute>();
         return attribute != null ? attribute.Name : task.Name;
     }
 
     public static string GetTaskArguments(this Type task)
     {
-        if (task is null)
-        {
-            throw new ArgumentNullException(nameof(task));
-        }
+        ArgumentNullException.ThrowIfNull(task);
 
         var attributes = task.GetCustomAttributes<TaskArgumentAttribute>().ToArray();
         if (attributes.Length != 0)
diff --git a/build/common/Utilities/TaskArgumentsAttribute.cs b/build/common/Utilities/TaskArgumentsAttribute.cs
@@ -10,3 +10,23 @@ public class TaskArgumentAttribute(string name, params string[] possibleValues)
     public string Name { get; } = name;
     public string[] PossibleValues { get; } = possibleValues;
 }
+
+[AttributeUsage(AttributeTargets.Class, Inherited = false, AllowMultiple = true)]
+public class DotnetArgumentAttribute()
+    : TaskArgumentAttribute(Arguments.DotnetTarget, Constants.Frameworks);
+
+[AttributeUsage(AttributeTargets.Class, Inherited = false, AllowMultiple = true)]
+public class DockerDotnetArgumentAttribute()
+    : TaskArgumentAttribute(Arguments.DockerDotnetVersion, Constants.Frameworks);
+
+[AttributeUsage(AttributeTargets.Class, Inherited = false, AllowMultiple = true)]
+public class DockerDistroArgumentAttribute()
+    : TaskArgumentAttribute(Arguments.DockerDistro, Constants.DockerDistrosToBuild);
+
+[AttributeUsage(AttributeTargets.Class, Inherited = false, AllowMultiple = true)]
+public class DockerRegistryArgumentAttribute()
+    : TaskArgumentAttribute(Arguments.DockerRegistry, Constants.DockerRegistries);
+
+[AttributeUsage(AttributeTargets.Class, Inherited = false, AllowMultiple = true)]
+public class ArchitectureArgumentAttribute()
+    : TaskArgumentAttribute(Arguments.Architecture, Constants.Architectures);
diff --git a/build/docker/Tasks/DockerBuild.cs b/build/docker/Tasks/DockerBuild.cs
@@ -4,10 +4,10 @@ namespace Docker.Tasks;
 
 [TaskName(nameof(DockerBuild))]
 [TaskDescription("Build the docker images containing the GitVersion Tool")]
-[TaskArgument(Arguments.DockerRegistry, Constants.DockerHub, Constants.GitHub)]
-[TaskArgument(Arguments.DockerDotnetVersion, Constants.VersionCurrent, Constants.VersionLatest)]
-[TaskArgument(Arguments.DockerDistro, Constants.AlpineLatest, Constants.DebianLatest, Constants.UbuntuLatest)]
-[TaskArgument(Arguments.Architecture, Constants.Amd64, Constants.Arm64)]
+[DockerRegistryArgument]
+[DockerDotnetArgument]
+[DockerDistroArgument]
+[ArchitectureArgument]
 public class DockerBuild : FrostingTask<BuildContext>
 {
     public override bool ShouldRun(BuildContext context)
diff --git a/build/docker/Tasks/DockerHubReadmePublish.cs b/build/docker/Tasks/DockerHubReadmePublish.cs
@@ -8,6 +8,7 @@ namespace Docker.Tasks;
 [IsDependentOn(typeof(DockerHubReadmePublishInternal))]
 [TaskDescription("Publish the DockerHub updated README.md")]
 public class DockerHubReadmePublish : FrostingTask<BuildContext>;
+
 [TaskName(nameof(DockerHubReadmePublishInternal))]
 [TaskDescription("Publish the DockerHub updated README.md")]
 public class DockerHubReadmePublishInternal : FrostingTask<BuildContext>
diff --git a/build/docker/Tasks/DockerManifest.cs b/build/docker/Tasks/DockerManifest.cs
@@ -4,9 +4,9 @@ namespace Docker.Tasks;
 
 [TaskName(nameof(DockerManifest))]
 [TaskDescription("Publish the docker manifest containing the images for amd64 and arm64")]
-[TaskArgument(Arguments.DockerRegistry, Constants.DockerHub, Constants.GitHub)]
-[TaskArgument(Arguments.DockerDotnetVersion, Constants.VersionCurrent, Constants.VersionLatest)]
-[TaskArgument(Arguments.DockerDistro, Constants.AlpineLatest, Constants.DebianLatest, Constants.UbuntuLatest)]
+[DockerRegistryArgument]
+[DockerDotnetArgument]
+[DockerDistroArgument]
 [IsDependentOn(typeof(DockerManifestInternal))]
 public class DockerManifest : FrostingTask<BuildContext>
 {
diff --git a/build/docker/Tasks/DockerPublish.cs b/build/docker/Tasks/DockerPublish.cs
@@ -4,10 +4,10 @@ namespace Docker.Tasks;
 
 [TaskName(nameof(DockerPublish))]
 [TaskDescription("Publish the docker images containing the GitVersion Tool")]
-[TaskArgument(Arguments.DockerRegistry, Constants.DockerHub, Constants.GitHub)]
-[TaskArgument(Arguments.DockerDotnetVersion, Constants.VersionCurrent, Constants.VersionLatest)]
-[TaskArgument(Arguments.DockerDistro, Constants.AlpineLatest, Constants.DebianLatest, Constants.UbuntuLatest)]
-[TaskArgument(Arguments.Architecture, Constants.Amd64, Constants.Arm64)]
+[DockerRegistryArgument]
+[DockerDotnetArgument]
+[DockerDistroArgument]
+[ArchitectureArgument]
 [IsDependentOn(typeof(DockerPublishInternal))]
 public class DockerPublish : FrostingTask<BuildContext>
 {
diff --git a/build/docker/Tasks/DockerTest.cs b/build/docker/Tasks/DockerTest.cs
@@ -4,10 +4,10 @@ namespace Docker.Tasks;
 
 [TaskName(nameof(DockerTest))]
 [TaskDescription("Test the docker images containing the GitVersion Tool")]
-[TaskArgument(Arguments.DockerRegistry, Constants.DockerHub, Constants.GitHub)]
-[TaskArgument(Arguments.DockerDotnetVersion, Constants.VersionCurrent, Constants.VersionLatest)]
-[TaskArgument(Arguments.DockerDistro, Constants.AlpineLatest, Constants.DebianLatest, Constants.UbuntuLatest)]
-[TaskArgument(Arguments.Architecture, Constants.Amd64, Constants.Arm64)]
+[DockerRegistryArgument]
+[DockerDotnetArgument]
+[DockerDistroArgument]
+[ArchitectureArgument]
 [IsDependentOn(typeof(DockerBuild))]
 public class DockerTest : FrostingTask<BuildContext>
 {

Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@ namespace Build.Tasks;`
`8`	`8`
`9`	`9`	`[TaskName(nameof(UnitTest))]`
`10`	`10`	`[TaskDescription("Run the unit tests")]`
`11`		`-[TaskArgument(Arguments.DotnetTarget, Constants.VersionCurrent, Constants.VersionLatest)]`
	`11`	`+[DotnetArgument]`
`12`	`12`	`[IsDependentOn(typeof(Build))]`
`13`	`13`	`public class UnitTest : FrostingTask<BuildContext>`
`14`	`14`	`{`
Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@ namespace Common.Addins.GitVersion;`
`8`	`8`	`/// </summary>`
`9`	`9`	`public sealed class GitVersionRunner : Tool<GitVersionSettings>`
`10`	`10`	`{`
`11`		`- private readonly ICakeLog log;`
	`11`	`+ private readonly ICakeLog _log;`
`12`	`12`
`13`	`13`	`/// <summary>`
`14`	`14`	`/// Initializes a new instance of the <see cref="GitVersionRunner"/> class.`
`@@ -23,7 +23,7 @@ public GitVersionRunner(`
`23`	`23`	`ICakeEnvironment environment,`
`24`	`24`	`IProcessRunner processRunner,`
`25`	`25`	`IToolLocator tools,`
`26`		`- ICakeLog log) : base(fileSystem, environment, processRunner, tools) => this.log = log;`
	`26`	`+ ICakeLog log) : base(fileSystem, environment, processRunner, tools) => this._log = log;`
`27`	`27`
`28`	`28`	`/// <summary>`
`29`	`29`	`/// Runs GitVersion and processes the results.`
`@@ -32,31 +32,28 @@ public GitVersionRunner(`
`32`	`32`	`/// <returns>A task with the GitVersion results.</returns>`
`33`	`33`	`public GitVersion Run(GitVersionSettings settings)`
`34`	`34`	`{`
`35`		`- if (settings == null)`
`36`		`- {`
`37`		`- throw new ArgumentNullException(nameof(settings));`
`38`		`- }`
	`35`	`+ ArgumentNullException.ThrowIfNull(settings);`
`39`	`36`
`40`	`37`	`var output = string.Empty;`
`41`	`38`	`Run(settings, GetArguments(settings), new ProcessSettings { RedirectStandardOutput = true }, process =>`
`42`	`39`	`{`
`43`	`40`	`output = string.Join("\n", process.GetStandardOutput());`
`44`		`- if (log.Verbosity < Verbosity.Diagnostic)`
	`41`	`+ if (this._log.Verbosity < Verbosity.Diagnostic)`
`45`	`42`	`{`
`46`	`43`	`var errors = Regex.Matches(output, @"( ERROR:? [^\n])\n([^\n]*)")`
`47`	`44`	`.SelectMany(match => new[] { match.Groups[1].Value, match.Groups[2].Value });`
`48`	`45`	`foreach (var error in errors)`
`49`	`46`	`{`
`50`		`- log.Error(error);`
	`47`	`+ this._log.Error(error);`
`51`	`48`	`}`
`52`	`49`	`}`
`53`	`50`	`});`
`54`	`51`
`55`	`52`	`if (!settings.OutputTypes.Contains(GitVersionOutput.Json))`
`56`	`53`	`return new GitVersion();`
`57`	`54`
`58`		`- var jsonStartIndex = output.IndexOf("{", StringComparison.Ordinal);`
`59`		`- var jsonEndIndex = output.IndexOf("}", StringComparison.Ordinal);`
	`55`	`+ var jsonStartIndex = output.IndexOf('{');`
	`56`	`+ var jsonEndIndex = output.IndexOf('}');`
`60`	`57`	`var json = output.Substring(jsonStartIndex, jsonEndIndex - jsonStartIndex + 1);`
`61`	`58`
`62`	`59`	`return JsonConvert.DeserializeObject<GitVersion>(json) ?? new GitVersion();`
`@@ -119,7 +116,7 @@ private ProcessArgumentBuilder GetArguments(GitVersionSettings settings)`
`119`	`116`	`}`
`120`	`117`	`else`
`121`	`118`	`{`
`122`		`- log.Warning("If you leave the branch name for GitVersion unset, it will fallback to the default branch for the repository.");`
	`119`	`+ this._log.Warning("If you leave the branch name for GitVersion unset, it will fallback to the default branch for the repository.");`
`123`	`120`	`}`
`124`	`121`
`125`	`122`	`if (!string.IsNullOrWhiteSpace(settings.Commit))`
`@@ -146,7 +143,7 @@ private ProcessArgumentBuilder GetArguments(GitVersionSettings settings)`
`146`	`143`	`builder.Append("-nofetch");`
`147`	`144`	`}`
`148`	`145`
`149`		`- var verbosity = settings.Verbosity ?? log.Verbosity;`
	`146`	`+ var verbosity = settings.Verbosity ?? this._log.Verbosity;`
`150`	`147`
`151`	`148`	`if (verbosity != Verbosity.Normal)`
`152`	`149`	`{`