Skip to content

Commit

Permalink
Fixed duplicate reasons are reported for the same rule #2553 (#2696)
Browse files Browse the repository at this point in the history
  • Loading branch information
BernieWhite authored Jan 6, 2025
1 parent db490dd commit 3f74dfd
Show file tree
Hide file tree
Showing 14 changed files with 184 additions and 63 deletions.
2 changes: 2 additions & 0 deletions docs/CHANGELOG-v3.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ What's changed since pre-release v3.0.0-B0351:
[#1828](https://github.com/microsoft/PSRule/issues/1828)
- Fixed directory handling of input paths without trailing slash by @BernieWhite.
[#1842](https://github.com/microsoft/PSRule/issues/1842)
- Fixed duplicate reasons are reported for the same rule by @BernieWhite.
[#2553](https://github.com/microsoft/PSRule/issues/2553)

## v3.0.0-B0351 (pre-release)

Expand Down
35 changes: 17 additions & 18 deletions src/PSRule/Definitions/Expressions/ExpressionContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ internal sealed class ExpressionContext : IExpressionContext, IBindingContext

internal ExpressionContext(RunspaceContext context, ISourceFile source, ResourceKind kind, object current)
{
Context = context;
Context = context ?? throw new ArgumentNullException(nameof(context));
Source = source;
LanguageScope = source.Module;
Kind = kind;
_NameTokenCache = new Dictionary<string, PathExpression>();
_NameTokenCache = [];
Current = current;
}

Expand Down Expand Up @@ -47,43 +47,42 @@ bool IBindingContext.GetPathExpression(string path, out PathExpression expressio

public void Debug(string message, params object[] args)
{
if (RunspaceContext.CurrentThread?.Writer == null)
if (Context.Writer == null)
return;

RunspaceContext.CurrentThread.Writer.WriteDebug(message, args);
Context.Writer.WriteDebug(message, args);
}

public void PushScope(RunspaceScope scope)
{
RunspaceContext.CurrentThread.PushScope(scope);
RunspaceContext.CurrentThread.EnterLanguageScope(Source);
Context.PushScope(scope);
Context.EnterLanguageScope(Source);
}

public void PopScope(RunspaceScope scope)
{
RunspaceContext.CurrentThread.PopScope(scope);
Context.PopScope(scope);
}

public void Reason(IOperand operand, string text, params object[] args)
{
if (string.IsNullOrEmpty(text) || !RunspaceContext.CurrentThread.IsScope(RunspaceScope.Rule))
if (string.IsNullOrEmpty(text) || !Context.IsScope(RunspaceScope.Rule))
return;

_Reason ??= [];
_Reason.Add(new ResultReason(Context.TargetObject?.Path, operand, text, args));
AddReason(new ResultReason(Context.TargetObject?.Path, operand, text, args));
}

public void Reason(string text, params object[] args)
internal ResultReason[] GetReasons()
{
if (string.IsNullOrEmpty(text) || !RunspaceContext.CurrentThread.IsScope(RunspaceScope.Rule))
return;

_Reason ??= [];
_Reason.Add(new ResultReason(Context.TargetObject?.Path, null, text, args));
return _Reason == null || _Reason.Count == 0 ? [] : [.. _Reason];
}

internal ResultReason[] GetReasons()
private void AddReason(ResultReason reason)
{
return _Reason == null || _Reason.Count == 0 ? Array.Empty<ResultReason>() : _Reason.ToArray();
_Reason ??= [];

// Check if the reason already exists
if (!_Reason.Contains(reason))
_Reason.Add(reason);
}
}
2 changes: 1 addition & 1 deletion src/PSRule/Definitions/IDetailedRuleResultV2.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public interface IDetailedRuleResultV2 : IRuleResultV2
/// <summary>
/// Detailed information about the rule result.
/// </summary>
IResultDetailV2 Detail { get; }
IResultDetail Detail { get; }

/// <summary>
/// A set of custom fields bound for the target object.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ namespace PSRule.Definitions;
/// <summary>
/// Detailed information about the rule result.
/// </summary>
public interface IResultDetailV2
public interface IResultDetail
{
/// <summary>
/// Any reasons for the result.
/// </summary>
IEnumerable<IResultReasonV2> Reason { get; }
IEnumerable<IResultReason> Reason { get; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ namespace PSRule.Definitions;
/// <summary>
/// A reason for the rule result.
/// </summary>
public interface IResultReasonV2
public interface IResultReason : IEquatable<IResultReason>
{
/// <summary>
/// The object path that failed.
Expand Down
4 changes: 2 additions & 2 deletions src/PSRule/Definitions/ResultDetail.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

namespace PSRule.Definitions;

internal sealed class ResultDetail : IResultDetailV2
internal sealed class ResultDetail : IResultDetail
{
private readonly IList<ResultReason> _Reason;

Expand All @@ -24,5 +24,5 @@ internal string[] GetReasonStrings()
return _Reason.GetStrings();
}

IEnumerable<IResultReasonV2> IResultDetailV2.Reason => _Reason;
IEnumerable<IResultReason> IResultDetail.Reason => _Reason;
}
63 changes: 45 additions & 18 deletions src/PSRule/Definitions/ResultReason.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,30 @@

namespace PSRule.Definitions;

internal sealed class ResultReason : IResultReasonV2
#nullable enable

/// <summary>
/// A reason for the rule result.
/// </summary>
internal sealed class ResultReason : IResultReason
{
private string _Path;
private string _Formatted;
private string _Message;
private string _FullPath;
private readonly string _ParentPath;

internal ResultReason(string parentPath, IOperand operand, string text, object[] args)
private string? _Path;
private string? _Formatted;
private string? _Message;
private string? _FullPath;

internal ResultReason(string? parentPath, IOperand? operand, string text, object[]? args)
{
_ParentPath = parentPath;
_ParentPath = parentPath ?? string.Empty;
Operand = operand;
_Path = Operand?.Path;
Text = text;
Args = args;
}

internal IOperand Operand { get; }
internal IOperand? Operand { get; }

/// <summary>
/// The object path that failed.
Expand All @@ -31,15 +37,14 @@ public string Path
{
get
{
_Path ??= GetPath();
return _Path;
return _Path ??= GetPath();
}
}

/// <summary>
/// A prefix to add to the object path that failed.
/// </summary>
internal string Prefix
internal string? Prefix
{
get { return Operand?.Prefix; }
set
Expand All @@ -59,21 +64,20 @@ public string FullPath
{
get
{
_FullPath ??= GetFullPath();
return _FullPath;
return _FullPath ??= GetFullPath();
}
}

public string Text { get; }

public object[] Args { get; }
public object[]? Args { get; }

/// <inheritdoc/>
public string Message
{
get
{
_Message ??= Args == null || Args.Length == 0 ? Text : string.Format(Thread.CurrentThread.CurrentCulture, Text, Args);
return _Message;
return _Message ??= Args == null || Args.Length == 0 ? Text : string.Format(Thread.CurrentThread.CurrentCulture, Text, Args);
}
}

Expand All @@ -82,13 +86,23 @@ public override string ToString()
return Format();
}

public override int GetHashCode()
{
return ToString().GetHashCode();
}

public override bool Equals(object obj)
{
return obj is IResultReason other && Equals(other);
}

/// <inheritdoc/>
public string Format()
{
_Formatted ??= string.Concat(
return _Formatted ??= string.Concat(
Operand?.ToString(),
Message
);
return _Formatted;
}

private string GetPath()
Expand All @@ -100,4 +114,17 @@ private string GetFullPath()
{
return Runtime.Operand.JoinPath(_ParentPath, Path);
}

#region IEquatable<IResultReason>

public bool Equals(IResultReason? other)
{
return other != null &&
string.Equals(FullPath, other.FullPath, StringComparison.Ordinal) &&
string.Equals(Message, other.Message, StringComparison.Ordinal);
}

#endregion IEquatable<IResultReason>
}

#nullable restore
2 changes: 1 addition & 1 deletion src/PSRule/Rules/RuleRecord.cs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ internal RuleRecord(ResourceId ruleId, string @ref, TargetObject targetObject, s
[DefaultValue(null)]
[JsonProperty(PropertyName = "detail")]
[YamlMember()]
public IResultDetailV2 Detail => _Detail;
public IResultDetail Detail => _Detail;

/// <summary>
/// Any default properties for the rule.
Expand Down
29 changes: 20 additions & 9 deletions src/PSRule/Runtime/AssertResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,16 @@

namespace PSRule.Runtime;

#nullable enable

/// <summary>
/// The result of a single assertion.
/// </summary>
public sealed class AssertResult : IEquatable<bool>
{
private readonly List<ResultReason> _Reason;
private List<ResultReason>? _Reason;

internal AssertResult(IOperand operand, bool value, string reason, object[] args)
internal AssertResult(IOperand? operand, bool value, string reason, object[] args)
{
Result = value;
if (!Result)
Expand Down Expand Up @@ -54,7 +56,8 @@ internal void AddReason(AssertResult result)
if (result == null || Result || result.Result || result._Reason == null || result._Reason.Count == 0)
return;

_Reason.AddRange(result._Reason);
_Reason ??= [];
_Reason.AddRange(result._Reason.Where(r => !_Reason.Contains(r)));
}

/// <summary>
Expand All @@ -63,13 +66,19 @@ internal void AddReason(AssertResult result)
/// <param name="operand">Identifies the operand that was the reason for the failure.</param>
/// <param name="text">The text of a reason to add. This text should already be localized for the currently culture.</param>
/// <param name="args">Replacement arguments for the format string.</param>
internal void AddReason(IOperand operand, string text, params object[] args)
internal void AddReason(IOperand? operand, string text, params object[]? args)
{
// Ignore reasons if this is a pass.
if (Result || string.IsNullOrEmpty(text))
return;

_Reason.Add(new ResultReason(RunspaceContext.CurrentThread?.TargetObject?.Path, operand, text, args));
_Reason ??= [];

var reason = new ResultReason(RunspaceContext.CurrentThread?.TargetObject?.Path, operand, text, args);
if (_Reason.Contains(reason))
return;

_Reason.Add(reason);
}

/// <summary>
Expand Down Expand Up @@ -164,7 +173,7 @@ public string[] GetReason()
public bool Complete()
{
// Check that the scope is still valid
if (!RunspaceContext.CurrentThread.IsScope(RunspaceScope.Rule))
if (!RunspaceContext.CurrentThread!.IsScope(RunspaceScope.Rule))
throw new RuleException(string.Format(Thread.CurrentThread.CurrentCulture, PSRuleResources.VariableConditionScope, "Assert"));

// Continue
Expand All @@ -179,7 +188,7 @@ public bool Complete()
/// </summary>
public void Ignore()
{
_Reason.Clear();
_Reason?.Clear();
}

/// <inheritdoc/>
Expand All @@ -204,13 +213,15 @@ public bool ToBoolean()
return Result;
}

internal IResultReasonV2[] ToResultReason()
internal IResultReason[] ToResultReason()
{
return _Reason == null || _Reason.Count == 0 ? Array.Empty<IResultReasonV2>() : _Reason.ToArray();
return _Reason == null || _Reason.Count == 0 ? [] : _Reason.ToArray();
}

private bool IsNullOrEmptyReason()
{
return _Reason == null || _Reason.Count == 0;
}
}

#nullable restore
4 changes: 2 additions & 2 deletions src/PSRule/Runtime/RuleConditionHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@ internal static RuleConditionResult Create(IEnumerable<object> value)
private static bool TryBoolean(object o, out bool result)
{
result = false;
if (o is not bool bresult)
if (o is not bool b)
return false;

result = bresult;
result = b;
return true;
}

Expand Down
13 changes: 4 additions & 9 deletions tests/PSRule.Tests/AssertTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,13 @@

namespace PSRule;

[Trait(LANGUAGE, LANGUAGEELEMENT)]
public sealed class AssertTests : ContextBaseTests
[Trait(LANGUAGE, LANGUAGE_ELEMENT)]
public sealed class AssertTests(ITestOutputHelper output) : ContextBaseTests
{
private const string LANGUAGE = "Language";
private const string LANGUAGEELEMENT = "Variable";
private const string LANGUAGE_ELEMENT = "Variable";

private readonly ITestOutputHelper _Output;

public AssertTests(ITestOutputHelper output)
{
_Output = output;
}
private readonly ITestOutputHelper _Output = output;

[Fact]
public void Assertion()
Expand Down
Loading

0 comments on commit 3f74dfd

Please sign in to comment.