Skip to content

Commit

Permalink
(chocolatey#3000) Update rules to include id and help
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
AdmiringWorm committed Mar 1, 2023
1 parent ee5c820 commit 91f7bfc
Show file tree
Hide file tree
Showing 18 changed files with 107 additions and 45 deletions.
1 change: 1 addition & 0 deletions src/chocolatey/chocolatey.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@
<Compile Include="infrastructure.app\rules\ReadmeMetadataRule.cs" />
<Compile Include="infrastructure.app\rules\RepositoryMetadataRule.cs" />
<Compile Include="infrastructure.app\rules\RequireLicenseAcceptanceMetadataRule.cs" />
<Compile Include="infrastructure.app\rules\RuleIdentifiers.cs" />
<Compile Include="infrastructure.app\rules\ServicableMetadataRule.cs" />
<Compile Include="infrastructure.app\rules\VersionMetadataRule.cs" />
<Compile Include="infrastructure.app\services\RuleService.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ public override IEnumerable<RuleResult> 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));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public override IEnumerable<RuleResult> validate(NuspecReader reader)
{
if (has_element(reader, "frameworkReferences"))
{
yield return new RuleResult(RuleType.Error, "<frameworkReferences> elements are not supported in Chocolatey CLI.");
yield return new RuleResult(RuleType.Error, RuleIdentifiers.UnsupportedElementUsed, "<frameworkReferences> elements are not supported in Chocolatey CLI.");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public IEnumerable<RuleResult> validate(NuspecReader reader)
{
if (!(reader.GetIcon() is null))
{
yield return new RuleResult(RuleType.Error, "<icon> elements are not supported in Chocolatey CLI, use <iconUrl> instead.");
yield return new RuleResult(RuleType.Error, RuleIdentifiers.UnsupportedElementUsed, "<icon> elements are not supported in Chocolatey CLI, use <iconUrl> instead.");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public IEnumerable<RuleResult> validate(NuspecReader reader)
{
if (!(reader.GetLicenseMetadata() is null))
{
yield return new RuleResult(RuleType.Error, "<license> elements are not supported in Chocolatey CLI, use <licenseUrl> instead.");
yield return new RuleResult(RuleType.Error, RuleIdentifiers.UnsupportedElementUsed, "<license> elements are not supported in Chocolatey CLI, use <licenseUrl> instead.");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public override IEnumerable<RuleResult> validate(NuspecReader reader)
{
if (has_element(reader, "packageTypes"))
{
yield return new RuleResult(RuleType.Error, "<packageTypes> elements are not supported in Chocolatey CLI.");
yield return new RuleResult(RuleType.Error, RuleIdentifiers.UnsupportedElementUsed, "<packageTypes> elements are not supported in Chocolatey CLI.");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public IEnumerable<RuleResult> validate(NuspecReader reader)
{
if (!(reader.GetReadme() is null))
{
yield return new RuleResult(RuleType.Error, "<readme> elements are not supported in Chocolatey CLI.");
yield return new RuleResult(RuleType.Error, RuleIdentifiers.UnsupportedElementUsed, "<readme> elements are not supported in Chocolatey CLI.");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public override IEnumerable<RuleResult> validate(NuspecReader reader)

if (has_element(reader, "repository"))
{
yield return new RuleResult(RuleType.Error, "<repository> elements are not supported in Chocolatey CLI, use <packageSourceUrl> instead.");
yield return new RuleResult(RuleType.Error, RuleIdentifiers.UnsupportedElementUsed, "<repository> elements are not supported in Chocolatey CLI, use <packageSourceUrl> instead.");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public IEnumerable<RuleResult> 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.");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public override IEnumerable<RuleResult> 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));
}
}
}
Expand Down
25 changes: 25 additions & 0 deletions src/chocolatey/infrastructure.app/rules/RuleIdentifiers.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// 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 UnsupportedElementUsed = "CHCU0002";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public override IEnumerable<RuleResult> validate(NuspecReader reader)
{
if (has_element(reader, "serviceable"))
{
yield return new RuleResult(RuleType.Error, "<serviceable> elements are not supported in Chocolatey CLI.");
yield return new RuleResult(RuleType.Error, RuleIdentifiers.UnsupportedElementUsed, "<serviceable> elements are not supported in Chocolatey CLI.");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ public override IEnumerable<RuleResult> 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));
}
}
}
Expand Down
54 changes: 44 additions & 10 deletions src/chocolatey/infrastructure.app/services/NugetService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -823,11 +823,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));
Expand Down Expand Up @@ -1398,11 +1399,12 @@ public virtual ConcurrentDictionary<string, PackageResult> 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));
Expand Down Expand Up @@ -1583,26 +1585,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;
}
}
Expand Down
1 change: 1 addition & 0 deletions src/chocolatey/infrastructure.app/services/RuleService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public IEnumerable<RuleResult> validate_rules(string filePath)

return rules
.OrderBy(r => r.Severity)
.ThenBy(r => r.Id)
.ThenBy(r => r.Message);
}

Expand Down
14 changes: 7 additions & 7 deletions src/chocolatey/infrastructure/rules/RuleResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
}
}
3 changes: 2 additions & 1 deletion src/chocolatey/infrastructure/rules/RuleType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public enum RuleType
None = 0,
Error,
Warning,
Information
Information,
Note
}
}
26 changes: 13 additions & 13 deletions tests/chocolatey-tests/commands/choco-pack.Tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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: CHCR0001: $_ is a required element in the package nuspec file."
}

It "Does not create the nuget package" {
Expand Down Expand Up @@ -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" {
Expand Down Expand Up @@ -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: CHCR0001: $_ is a required element in the package nuspec file."
}

It "Does not create the nuget package" {
Expand Down Expand Up @@ -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" {
Expand Down

0 comments on commit 91f7bfc

Please sign in to comment.