Skip to content

Commit

Permalink
Merge pull request #3037 from AdmiringWorm/rules-enhancement
Browse files Browse the repository at this point in the history
(#3000) Add additional information in validation messages
  • Loading branch information
gep13 authored Mar 2, 2023
2 parents ee5c820 + 8e30abd commit 18c0d6e
Show file tree
Hide file tree
Showing 19 changed files with 153 additions and 47 deletions.
43 changes: 43 additions & 0 deletions src/chocolatey/RuleResultExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// 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
{
using System.Collections.Generic;
using System.Linq;
using chocolatey.infrastructure.rules;

public static class RuleResultExtensions
{
/// <summary>
/// Extension method used to filter out any rules that hasn't been marked as either unsupported or deprecated.
/// </summary>
/// <param name="ruleResults">The rule results to apply the filter to.</param>
/// <param name="inverse"><c>True</c> if the applied filters should exclude unsupported and deprecated rules; otherwise <c>False</c></param>
/// <returns>The passed in rule results with the applied filters.</returns>
public static IEnumerable<RuleResult> where_unsupported_or_deprecated(this IEnumerable<RuleResult> ruleResults, bool inverse = false)
{
if (!inverse)
{
return ruleResults
.Where(r => r != null && !string.IsNullOrEmpty(r.Id))
.Where(r => r.Id.StartsWith("CHCU") || r.Id.StartsWith("CHCD"));
}
return ruleResults
.Where(r => r != null)
.Where(r => string.IsNullOrEmpty(r.Id) || (!r.Id.StartsWith("CHCU") && !r.Id.StartsWith("CHCD")));
}
}
}
4 changes: 3 additions & 1 deletion src/chocolatey/chocolatey.csproj
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Import Project="..\packages\Microsoft.CodeAnalysis.BannedApiAnalyzers.3.3.3\build\Microsoft.CodeAnalysis.BannedApiAnalyzers.props" Condition="Exists('..\packages\Microsoft.CodeAnalysis.BannedApiAnalyzers.3.3.3\build\Microsoft.CodeAnalysis.BannedApiAnalyzers.props')" />
<PropertyGroup>
Expand Down 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 Expand Up @@ -460,6 +461,7 @@
<Compile Include="NuGetVersionExtensions.cs" />
<Compile Include="ObjectExtensions.cs" />
<Compile Include="Properties\AssemblyInfo.cs" />
<Compile Include="RuleResultExtensions.cs" />
<Compile Include="StringExtensions.cs" />
<Compile Include="TypeExtensions.cs" />
</ItemGroup>
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
56 changes: 45 additions & 11 deletions src/chocolatey/infrastructure.app/services/NugetService.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright © 2017 - 2023 Chocolatey Software, Inc
// Copyright © 2017 - 2023 Chocolatey Software, Inc
// Copyright © 2011 - 2017 RealDimensions Software, LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
Expand Down 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_unsupported_or_deprecated()
.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_unsupported_or_deprecated()
.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_unsupported_or_deprecated(inverse: true);
}

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
}
}
Loading

0 comments on commit 18c0d6e

Please sign in to comment.