From c707ca4c1b28207fb77dbc92bb62e83d6bdeb8be Mon Sep 17 00:00:00 2001 From: AdmiringWorm Date: Tue, 14 Feb 2023 09:09:12 +0100 Subject: [PATCH] (#3000) Update rules to include id and help This commit updates the rule results and its implemented rules to include an identifier and optionally a url to where more information can be found about the rule. This is done to make it easier in the future to add/update additional functionality in extensions. --- src/chocolatey/chocolatey.csproj | 1 + .../rules/EmptyOrInvalidUrlMetadataRule.cs | 4 +- .../rules/FrameWorkReferencesMetadataRule.cs | 2 +- .../rules/IconMetadataRule.cs | 2 +- .../rules/LicenseMetadataRule.cs | 2 +- .../rules/PackageTypesMetadataRule.cs | 2 +- .../rules/ReadmeMetadataRule.cs | 2 +- .../rules/RepositoryMetadataRule.cs | 2 +- .../RequireLicenseAcceptanceMetadataRule.cs | 2 +- .../rules/RequiredMetadataRule.cs | 2 +- .../rules/RuleIdentifiers.cs | 26 +++++++++ .../rules/ServicableMetadataRule.cs | 2 +- .../rules/VersionMetadataRule.cs | 6 +-- .../services/NugetService.cs | 54 +++++++++++++++---- .../services/RuleService.cs | 1 + .../infrastructure/rules/RuleResult.cs | 14 ++--- .../infrastructure/rules/RuleType.cs | 3 +- .../commands/choco-pack.Tests.ps1 | 26 ++++----- 18 files changed, 108 insertions(+), 45 deletions(-) create mode 100644 src/chocolatey/infrastructure.app/rules/RuleIdentifiers.cs diff --git a/src/chocolatey/chocolatey.csproj b/src/chocolatey/chocolatey.csproj index 9da5742410..9de84cfc1a 100644 --- a/src/chocolatey/chocolatey.csproj +++ b/src/chocolatey/chocolatey.csproj @@ -225,6 +225,7 @@ + diff --git a/src/chocolatey/infrastructure.app/rules/EmptyOrInvalidUrlMetadataRule.cs b/src/chocolatey/infrastructure.app/rules/EmptyOrInvalidUrlMetadataRule.cs index 53c2f3b9e7..e18badd9a2 100644 --- a/src/chocolatey/infrastructure.app/rules/EmptyOrInvalidUrlMetadataRule.cs +++ b/src/chocolatey/infrastructure.app/rules/EmptyOrInvalidUrlMetadataRule.cs @@ -43,11 +43,11 @@ public override IEnumerable validate(NuspecReader reader) if (string.IsNullOrWhiteSpace(value)) { - yield return new RuleResult(RuleType.Error, "The {0} element in the package nuspec file cannot be empty.".format_with(item)); + yield return new RuleResult(RuleType.Error, RuleIdentifiers.EmptyRequiredElement, "The {0} element in the package nuspec file cannot be empty.".format_with(item)); } else if (!Uri.TryCreate(value, UriKind.Absolute, out _)) { - yield return new RuleResult(RuleType.Error, "'{0}' is not a valid URL for the {1} element in the package nuspec file.".format_with(value, item)); + yield return new RuleResult(RuleType.Error, RuleIdentifiers.InvalidTypeElement, "'{0}' is not a valid URL for the {1} element in the package nuspec file.".format_with(value, item)); } } } diff --git a/src/chocolatey/infrastructure.app/rules/FrameWorkReferencesMetadataRule.cs b/src/chocolatey/infrastructure.app/rules/FrameWorkReferencesMetadataRule.cs index d9b0fd6dd5..313fc19850 100644 --- a/src/chocolatey/infrastructure.app/rules/FrameWorkReferencesMetadataRule.cs +++ b/src/chocolatey/infrastructure.app/rules/FrameWorkReferencesMetadataRule.cs @@ -25,7 +25,7 @@ public override IEnumerable validate(NuspecReader reader) { if (has_element(reader, "frameworkReferences")) { - yield return new RuleResult(RuleType.Error, " elements are not supported in Chocolatey CLI."); + yield return new RuleResult(RuleType.Error, RuleIdentifiers.UnsupportedElementUsed, " elements are not supported in Chocolatey CLI."); } } } diff --git a/src/chocolatey/infrastructure.app/rules/IconMetadataRule.cs b/src/chocolatey/infrastructure.app/rules/IconMetadataRule.cs index d9a4600931..6167463676 100644 --- a/src/chocolatey/infrastructure.app/rules/IconMetadataRule.cs +++ b/src/chocolatey/infrastructure.app/rules/IconMetadataRule.cs @@ -25,7 +25,7 @@ public IEnumerable validate(NuspecReader reader) { if (!(reader.GetIcon() is null)) { - yield return new RuleResult(RuleType.Error, " elements are not supported in Chocolatey CLI, use instead."); + yield return new RuleResult(RuleType.Error, RuleIdentifiers.UnsupportedElementUsed, " elements are not supported in Chocolatey CLI, use instead."); } } } diff --git a/src/chocolatey/infrastructure.app/rules/LicenseMetadataRule.cs b/src/chocolatey/infrastructure.app/rules/LicenseMetadataRule.cs index 9588057e33..074d1b8077 100644 --- a/src/chocolatey/infrastructure.app/rules/LicenseMetadataRule.cs +++ b/src/chocolatey/infrastructure.app/rules/LicenseMetadataRule.cs @@ -26,7 +26,7 @@ public IEnumerable validate(NuspecReader reader) { if (!(reader.GetLicenseMetadata() is null)) { - yield return new RuleResult(RuleType.Error, " elements are not supported in Chocolatey CLI, use instead."); + yield return new RuleResult(RuleType.Error, RuleIdentifiers.UnsupportedElementUsed, " elements are not supported in Chocolatey CLI, use instead."); } } } diff --git a/src/chocolatey/infrastructure.app/rules/PackageTypesMetadataRule.cs b/src/chocolatey/infrastructure.app/rules/PackageTypesMetadataRule.cs index ae260842fd..bb3e8f8f3b 100644 --- a/src/chocolatey/infrastructure.app/rules/PackageTypesMetadataRule.cs +++ b/src/chocolatey/infrastructure.app/rules/PackageTypesMetadataRule.cs @@ -25,7 +25,7 @@ public override IEnumerable validate(NuspecReader reader) { if (has_element(reader, "packageTypes")) { - yield return new RuleResult(RuleType.Error, " elements are not supported in Chocolatey CLI."); + yield return new RuleResult(RuleType.Error, RuleIdentifiers.UnsupportedElementUsed, " elements are not supported in Chocolatey CLI."); } } } diff --git a/src/chocolatey/infrastructure.app/rules/ReadmeMetadataRule.cs b/src/chocolatey/infrastructure.app/rules/ReadmeMetadataRule.cs index f34bf52c5b..090b86ee7d 100644 --- a/src/chocolatey/infrastructure.app/rules/ReadmeMetadataRule.cs +++ b/src/chocolatey/infrastructure.app/rules/ReadmeMetadataRule.cs @@ -25,7 +25,7 @@ public IEnumerable validate(NuspecReader reader) { if (!(reader.GetReadme() is null)) { - yield return new RuleResult(RuleType.Error, " elements are not supported in Chocolatey CLI."); + yield return new RuleResult(RuleType.Error, RuleIdentifiers.UnsupportedElementUsed, " elements are not supported in Chocolatey CLI."); } } } diff --git a/src/chocolatey/infrastructure.app/rules/RepositoryMetadataRule.cs b/src/chocolatey/infrastructure.app/rules/RepositoryMetadataRule.cs index 16b6745117..84e15400fe 100644 --- a/src/chocolatey/infrastructure.app/rules/RepositoryMetadataRule.cs +++ b/src/chocolatey/infrastructure.app/rules/RepositoryMetadataRule.cs @@ -29,7 +29,7 @@ public override IEnumerable validate(NuspecReader reader) if (has_element(reader, "repository")) { - yield return new RuleResult(RuleType.Error, " elements are not supported in Chocolatey CLI, use instead."); + yield return new RuleResult(RuleType.Error, RuleIdentifiers.UnsupportedElementUsed, " elements are not supported in Chocolatey CLI, use instead."); } } } diff --git a/src/chocolatey/infrastructure.app/rules/RequireLicenseAcceptanceMetadataRule.cs b/src/chocolatey/infrastructure.app/rules/RequireLicenseAcceptanceMetadataRule.cs index 9ce1854674..ef58e0585e 100644 --- a/src/chocolatey/infrastructure.app/rules/RequireLicenseAcceptanceMetadataRule.cs +++ b/src/chocolatey/infrastructure.app/rules/RequireLicenseAcceptanceMetadataRule.cs @@ -25,7 +25,7 @@ public IEnumerable validate(NuspecReader reader) { if (string.IsNullOrWhiteSpace(reader.GetLicenseUrl()) && reader.GetRequireLicenseAcceptance()) { - yield return new RuleResult(RuleType.Error, "Enabling license acceptance requires a license url."); + yield return new RuleResult(RuleType.Error, RuleIdentifiers.MissingElementOnRequiringLicenseAcceptance, "Enabling license acceptance requires a license url."); } } } diff --git a/src/chocolatey/infrastructure.app/rules/RequiredMetadataRule.cs b/src/chocolatey/infrastructure.app/rules/RequiredMetadataRule.cs index e17a2219aa..578c7a74d6 100644 --- a/src/chocolatey/infrastructure.app/rules/RequiredMetadataRule.cs +++ b/src/chocolatey/infrastructure.app/rules/RequiredMetadataRule.cs @@ -35,7 +35,7 @@ public override IEnumerable validate(NuspecReader reader) { if (string.IsNullOrWhiteSpace(get_element_value(reader, item))) { - yield return new RuleResult(RuleType.Error, "{0} is a required element in the package nuspec file.".format_with(item)); + yield return new RuleResult(RuleType.Error, RuleIdentifiers.EmptyRequiredElement, "{0} is a required element in the package nuspec file.".format_with(item)); } } } diff --git a/src/chocolatey/infrastructure.app/rules/RuleIdentifiers.cs b/src/chocolatey/infrastructure.app/rules/RuleIdentifiers.cs new file mode 100644 index 0000000000..1f4bc2d09e --- /dev/null +++ b/src/chocolatey/infrastructure.app/rules/RuleIdentifiers.cs @@ -0,0 +1,26 @@ +// Copyright © 2023-Present Chocolatey Software, Inc +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +namespace chocolatey.infrastructure.app.rules +{ + internal static class RuleIdentifiers + { + public const string EmptyRequiredElement = "CHCR0001"; + public const string InvalidTypeElement = "CHCU0001"; + public const string MissingElementOnRequiringLicenseAcceptance = "CHCR0002"; + public const string MissingRequiredElement = "CHCR0003"; + public const string UnsupportedElementUsed = "CHCU0002"; + } +} diff --git a/src/chocolatey/infrastructure.app/rules/ServicableMetadataRule.cs b/src/chocolatey/infrastructure.app/rules/ServicableMetadataRule.cs index 46c7ea4f92..ea6fbeb834 100644 --- a/src/chocolatey/infrastructure.app/rules/ServicableMetadataRule.cs +++ b/src/chocolatey/infrastructure.app/rules/ServicableMetadataRule.cs @@ -25,7 +25,7 @@ public override IEnumerable validate(NuspecReader reader) { if (has_element(reader, "serviceable")) { - yield return new RuleResult(RuleType.Error, " elements are not supported in Chocolatey CLI."); + yield return new RuleResult(RuleType.Error, RuleIdentifiers.UnsupportedElementUsed, " elements are not supported in Chocolatey CLI."); } } } diff --git a/src/chocolatey/infrastructure.app/rules/VersionMetadataRule.cs b/src/chocolatey/infrastructure.app/rules/VersionMetadataRule.cs index c5f8429fff..1cc290c5b7 100644 --- a/src/chocolatey/infrastructure.app/rules/VersionMetadataRule.cs +++ b/src/chocolatey/infrastructure.app/rules/VersionMetadataRule.cs @@ -26,11 +26,11 @@ public override IEnumerable validate(NuspecReader reader) { var version = get_element_value(reader, "version"); - // We need to check for the $version$ substitution value, - // as it will not be replaced before the package gets created + // We need to check for the $version$ substitution value, as it will not be replaced + // before the package gets created if (!string.IsNullOrEmpty(version) && !version.is_equal_to("$version$") && !NuGetVersion.TryParse(version, out _)) { - yield return new RuleResult(RuleType.Error, "'{0}' is not a valid version string in the package nuspec file.".format_with(version)); + yield return new RuleResult(RuleType.Error, RuleIdentifiers.InvalidTypeElement, "'{0}' is not a valid version string in the package nuspec file.".format_with(version)); } } } diff --git a/src/chocolatey/infrastructure.app/services/NugetService.cs b/src/chocolatey/infrastructure.app/services/NugetService.cs index 774122e7e9..5ffcd358b1 100644 --- a/src/chocolatey/infrastructure.app/services/NugetService.cs +++ b/src/chocolatey/infrastructure.app/services/NugetService.cs @@ -840,11 +840,12 @@ Version was specified as '{0}'. It is possible that version packageResult.Messages.Add(new ResultMessage(ResultType.Debug, ApplicationParameters.Messages.ContinueChocolateyAction)); var elementsList = _ruleService.validate_rules(manifestPath) - .Where(r => r.Severity == infrastructure.rules.RuleType.Error) - .Select(r => r.Message) + .Where(r => r.Severity == infrastructure.rules.RuleType.Error && !string.IsNullOrEmpty(r.Id)) + .Where(r => r.Id.StartsWith("CHCU") || r.Id.StartsWith("CHCD")) + .Select(r => "{0}: {1}".format_with(r.Id, r.Message)) .ToList(); - if (elementsList.Any()) + if (elementsList.Count > 0) { var message = "Issues found with nuspec elements\r\n" + elementsList.join("\r\n"); packageResult.Messages.Add(new ResultMessage(ResultType.Warn, message)); @@ -1399,11 +1400,12 @@ public virtual ConcurrentDictionary upgrade_run(Chocolate upgradePackageResult.Messages.Add(new ResultMessage(ResultType.Debug, ApplicationParameters.Messages.ContinueChocolateyAction)); var elementsList = _ruleService.validate_rules(manifestPath) - .Where(r => r.Severity == infrastructure.rules.RuleType.Error) - .Select(r => r.Message) + .Where(r => r.Severity == infrastructure.rules.RuleType.Error && !string.IsNullOrEmpty(r.Id)) + .Where(r => r.Id.StartsWith("CHCU") || r.Id.StartsWith("CHCD")) + .Select(r => "{0}: {1}".format_with(r.Id, r.Message)) .ToList(); - if (elementsList.Any()) + if (elementsList.Count > 0) { var message = "Issues found with nuspec elements\r\n" + elementsList.join("\r\n"); packageResult.Messages.Add(new ResultMessage(ResultType.Warn, message)); @@ -1584,26 +1586,58 @@ private void validate_nuspec(string nuspecFilePath, ChocolateyConfiguration conf if (!config.PackCommand.PackThrowOnUnsupportedElements) { - results = results.Where(r => !r.Message.contains("not supported")); + results = results.Where(r => !r.Id.StartsWith("CHCD") && !r.Id.StartsWith("CHCU")); } var hasErrors = false; foreach (var rule in results) { + var message = string.IsNullOrEmpty(rule.Id) + ? rule.Message + : "{0}: {1}".format_with(rule.Id, rule.Message); + switch (rule.Severity) { case infrastructure.rules.RuleType.Error: - this.Log().Error("ERROR: " + rule.Message); + this.Log().Error("ERROR: " + message); + + if (!string.IsNullOrEmpty(rule.HelpUrl)) + { + this.Log().Error(" See {0}", rule.HelpUrl); + } + hasErrors = true; break; case infrastructure.rules.RuleType.Warning: - this.Log().Warn("WARNING: " + rule.Message); + this.Log().Warn("WARNING: " + message); + + if (!string.IsNullOrEmpty(rule.HelpUrl)) + { + this.Log().Warn(" See {0}", rule.HelpUrl); + } + break; case infrastructure.rules.RuleType.Information: - this.Log().Info("INFO: " + rule.Message); + this.Log().Info("INFO: " + message); + + if (!string.IsNullOrEmpty(rule.HelpUrl)) + { + this.Log().Info(" See {0}", rule.HelpUrl); + } + + break; + + case infrastructure.rules.RuleType.Note: + this.Log().Info("NOTE: " + message); + + if (!string.IsNullOrEmpty(rule.HelpUrl)) + { + this.Log().Info(" See {0}", rule.HelpUrl); + } + break; } } diff --git a/src/chocolatey/infrastructure.app/services/RuleService.cs b/src/chocolatey/infrastructure.app/services/RuleService.cs index dad2d53321..2c6c40ad40 100644 --- a/src/chocolatey/infrastructure.app/services/RuleService.cs +++ b/src/chocolatey/infrastructure.app/services/RuleService.cs @@ -46,6 +46,7 @@ public IEnumerable validate_rules(string filePath) return rules .OrderBy(r => r.Severity) + .ThenBy(r => r.Id) .ThenBy(r => r.Message); } diff --git a/src/chocolatey/infrastructure/rules/RuleResult.cs b/src/chocolatey/infrastructure/rules/RuleResult.cs index 434c5c4c3c..3ad07810fd 100644 --- a/src/chocolatey/infrastructure/rules/RuleResult.cs +++ b/src/chocolatey/infrastructure/rules/RuleResult.cs @@ -15,18 +15,18 @@ namespace chocolatey.infrastructure.rules { - public readonly struct RuleResult + public sealed class RuleResult { - public static readonly RuleResult Success = new RuleResult(RuleType.None, string.Empty); - - public RuleResult(RuleType severity, string message) + public RuleResult(RuleType severity, string id, string message) { Severity = severity; + Id = id; Message = message; } - public readonly RuleType Severity; - - public readonly string Message; + public string HelpUrl { get; set; } + public string Id { get; private set; } + public string Message { get; private set; } + public RuleType Severity { get; set; } } } diff --git a/src/chocolatey/infrastructure/rules/RuleType.cs b/src/chocolatey/infrastructure/rules/RuleType.cs index b274885751..b7c114ab97 100644 --- a/src/chocolatey/infrastructure/rules/RuleType.cs +++ b/src/chocolatey/infrastructure/rules/RuleType.cs @@ -20,6 +20,7 @@ public enum RuleType None = 0, Error, Warning, - Information + Information, + Note } } diff --git a/tests/chocolatey-tests/commands/choco-pack.Tests.ps1 b/tests/chocolatey-tests/commands/choco-pack.Tests.ps1 index 48bd2f99e7..424b21e634 100644 --- a/tests/chocolatey-tests/commands/choco-pack.Tests.ps1 +++ b/tests/chocolatey-tests/commands/choco-pack.Tests.ps1 @@ -15,17 +15,17 @@ $emptyFailures = @( ) # Elements that will return an invalid failure (usually due to serialization) $invalidFailures = @( - @{id = 'projectUrl'; message = "ERROR: 'invalid project url' is not a valid URL for the projectUrl element in the package nuspec file." } - @{id = 'projectSourceUrl'; message = "ERROR: 'invalid project source url' is not a valid URL for the projectSourceUrl element in the package nuspec file." } - @{id = 'docsUrl'; message = "ERROR: 'invalid docs url' is not a valid URL for the docsUrl element in the package nuspec file." } - @{id = 'bugTrackerUrl'; message = "ERROR: 'invalid bug tracker url' is not a valid URL for the bugTrackerUrl element in the package nuspec file." } - @{id = 'mailingListUrl'; message = "ERROR: 'invalid mailing list url' is not a valid URL for the mailingListUrl element in the package nuspec file." } - @{id = 'iconUrl'; message = "ERROR: 'invalid icon url' is not a valid URL for the iconUrl element in the package nuspec file." } - @{id = 'licenseUrl'; message = "ERROR: 'invalid license url' is not a valid URL for the licenseUrl element in the package nuspec file." } - @{id = "version"; message = "ERROR: 'INVALID' is not a valid version string in the package nuspec file." } + @{id = 'projectUrl'; message = "ERROR: CHCU0001: 'invalid project url' is not a valid URL for the projectUrl element in the package nuspec file." } + @{id = 'projectSourceUrl'; message = "ERROR: CHCU0001: 'invalid project source url' is not a valid URL for the projectSourceUrl element in the package nuspec file." } + @{id = 'docsUrl'; message = "ERROR: CHCU0001: 'invalid docs url' is not a valid URL for the docsUrl element in the package nuspec file." } + @{id = 'bugTrackerUrl'; message = "ERROR: CHCU0001: 'invalid bug tracker url' is not a valid URL for the bugTrackerUrl element in the package nuspec file." } + @{id = 'mailingListUrl'; message = "ERROR: CHCU0001: 'invalid mailing list url' is not a valid URL for the mailingListUrl element in the package nuspec file." } + @{id = 'iconUrl'; message = "ERROR: CHCU0001: 'invalid icon url' is not a valid URL for the iconUrl element in the package nuspec file." } + @{id = 'licenseUrl'; message = "ERROR: CHCU0001: 'invalid license url' is not a valid URL for the licenseUrl element in the package nuspec file." } + @{id = "version"; message = "ERROR: CHCU0001: 'INVALID' is not a valid version string in the package nuspec file." } @{id = "no-content"; message = "Cannot create a package that has no dependencies nor content." } # This is a message from NuGet.Client, we may want to take ownership of it eventually. @{id = "id"; message = "The package ID 'invalid id' contains invalid characters. Examples of valid package IDs include 'MyPackage' and 'MyPackage.Sample'." } # This is a message from NuGet.Client, we may want to take ownership of it eventually. - @{id = "requirelicenseacceptance"; message = "ERROR: Enabling license acceptance requires a license url." } + @{id = "requirelicenseacceptance"; message = "ERROR: CHCR0002: Enabling license acceptance requires a license url." } ) Describe "choco pack" -Tag Chocolatey, PackCommand { @@ -153,7 +153,7 @@ Describe "choco pack" -Tag Chocolatey, PackCommand { } It "Displays required error message for <_>" -ForEach $missingFailures { - $Output.Lines | Should -Contain "ERROR: $_ is a required element in the package nuspec file." + $Output.Lines | Should -Contain "ERROR: CHCR0003: $_ is a required element in the package nuspec file." } It "Does not create the nuget package" { @@ -188,7 +188,7 @@ Describe "choco pack" -Tag Chocolatey, PackCommand { } It "Displays empty error message for <_>" -ForEach $emptyFailures { - $Output.Lines | Should -Contain "ERROR: The $_ element in the package nuspec file cannot be empty." + $Output.Lines | Should -Contain "ERROR: CHCR0001: The $_ element in the package nuspec file cannot be empty." } It "Does not create the nuget package" { @@ -255,7 +255,7 @@ Describe "choco pack" -Tag Chocolatey, PackCommand { } It "Displays empty error message for <_>" -ForEach $missingFailures { - $Output.Lines | Should -Contain "ERROR: $_ is a required element in the package nuspec file." + $Output.Lines | Should -Contain "ERROR: CHCR0003: $_ is a required element in the package nuspec file." } It "Does not create the nuget package" { @@ -568,7 +568,7 @@ Describe "choco pack" -Tag Chocolatey, PackCommand { } It 'Shows an error about the unsupported nuspec metadata element "<_>"' -TestCases $testCases { - $Output.String | Should -Match "ERROR: $_ elements are not supported in Chocolatey CLI" + $Output.String | Should -Match "ERROR: CHCU0002 $_ elements are not supported in Chocolatey CLI" } It "Should not output message about license url being deprecated" {