Skip to content

Commit 3f74dfd

Browse files
authored
Fixed duplicate reasons are reported for the same rule microsoft#2553 (microsoft#2696)
1 parent db490dd commit 3f74dfd

14 files changed

+184
-63
lines changed

docs/CHANGELOG-v3.md

+2
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ What's changed since pre-release v3.0.0-B0351:
4040
[#1828](https://github.com/microsoft/PSRule/issues/1828)
4141
- Fixed directory handling of input paths without trailing slash by @BernieWhite.
4242
[#1842](https://github.com/microsoft/PSRule/issues/1842)
43+
- Fixed duplicate reasons are reported for the same rule by @BernieWhite.
44+
[#2553](https://github.com/microsoft/PSRule/issues/2553)
4345

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

src/PSRule/Definitions/Expressions/ExpressionContext.cs

+17-18
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@ internal sealed class ExpressionContext : IExpressionContext, IBindingContext
1515

1616
internal ExpressionContext(RunspaceContext context, ISourceFile source, ResourceKind kind, object current)
1717
{
18-
Context = context;
18+
Context = context ?? throw new ArgumentNullException(nameof(context));
1919
Source = source;
2020
LanguageScope = source.Module;
2121
Kind = kind;
22-
_NameTokenCache = new Dictionary<string, PathExpression>();
22+
_NameTokenCache = [];
2323
Current = current;
2424
}
2525

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

4848
public void Debug(string message, params object[] args)
4949
{
50-
if (RunspaceContext.CurrentThread?.Writer == null)
50+
if (Context.Writer == null)
5151
return;
5252

53-
RunspaceContext.CurrentThread.Writer.WriteDebug(message, args);
53+
Context.Writer.WriteDebug(message, args);
5454
}
5555

5656
public void PushScope(RunspaceScope scope)
5757
{
58-
RunspaceContext.CurrentThread.PushScope(scope);
59-
RunspaceContext.CurrentThread.EnterLanguageScope(Source);
58+
Context.PushScope(scope);
59+
Context.EnterLanguageScope(Source);
6060
}
6161

6262
public void PopScope(RunspaceScope scope)
6363
{
64-
RunspaceContext.CurrentThread.PopScope(scope);
64+
Context.PopScope(scope);
6565
}
6666

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

72-
_Reason ??= [];
73-
_Reason.Add(new ResultReason(Context.TargetObject?.Path, operand, text, args));
72+
AddReason(new ResultReason(Context.TargetObject?.Path, operand, text, args));
7473
}
7574

76-
public void Reason(string text, params object[] args)
75+
internal ResultReason[] GetReasons()
7776
{
78-
if (string.IsNullOrEmpty(text) || !RunspaceContext.CurrentThread.IsScope(RunspaceScope.Rule))
79-
return;
80-
81-
_Reason ??= [];
82-
_Reason.Add(new ResultReason(Context.TargetObject?.Path, null, text, args));
77+
return _Reason == null || _Reason.Count == 0 ? [] : [.. _Reason];
8378
}
8479

85-
internal ResultReason[] GetReasons()
80+
private void AddReason(ResultReason reason)
8681
{
87-
return _Reason == null || _Reason.Count == 0 ? Array.Empty<ResultReason>() : _Reason.ToArray();
82+
_Reason ??= [];
83+
84+
// Check if the reason already exists
85+
if (!_Reason.Contains(reason))
86+
_Reason.Add(reason);
8887
}
8988
}

src/PSRule/Definitions/IDetailedRuleResultV2.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public interface IDetailedRuleResultV2 : IRuleResultV2
1818
/// <summary>
1919
/// Detailed information about the rule result.
2020
/// </summary>
21-
IResultDetailV2 Detail { get; }
21+
IResultDetail Detail { get; }
2222

2323
/// <summary>
2424
/// A set of custom fields bound for the target object.

src/PSRule/Definitions/IResultDetailV2.cs src/PSRule/Definitions/IResultDetail.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@ namespace PSRule.Definitions;
66
/// <summary>
77
/// Detailed information about the rule result.
88
/// </summary>
9-
public interface IResultDetailV2
9+
public interface IResultDetail
1010
{
1111
/// <summary>
1212
/// Any reasons for the result.
1313
/// </summary>
14-
IEnumerable<IResultReasonV2> Reason { get; }
14+
IEnumerable<IResultReason> Reason { get; }
1515
}

src/PSRule/Definitions/IResultReasonV2.cs src/PSRule/Definitions/IResultReason.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ namespace PSRule.Definitions;
66
/// <summary>
77
/// A reason for the rule result.
88
/// </summary>
9-
public interface IResultReasonV2
9+
public interface IResultReason : IEquatable<IResultReason>
1010
{
1111
/// <summary>
1212
/// The object path that failed.

src/PSRule/Definitions/ResultDetail.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
namespace PSRule.Definitions;
55

6-
internal sealed class ResultDetail : IResultDetailV2
6+
internal sealed class ResultDetail : IResultDetail
77
{
88
private readonly IList<ResultReason> _Reason;
99

@@ -24,5 +24,5 @@ internal string[] GetReasonStrings()
2424
return _Reason.GetStrings();
2525
}
2626

27-
IEnumerable<IResultReasonV2> IResultDetailV2.Reason => _Reason;
27+
IEnumerable<IResultReason> IResultDetail.Reason => _Reason;
2828
}

src/PSRule/Definitions/ResultReason.cs

+45-18
Original file line numberDiff line numberDiff line change
@@ -5,24 +5,30 @@
55

66
namespace PSRule.Definitions;
77

8-
internal sealed class ResultReason : IResultReasonV2
8+
#nullable enable
9+
10+
/// <summary>
11+
/// A reason for the rule result.
12+
/// </summary>
13+
internal sealed class ResultReason : IResultReason
914
{
10-
private string _Path;
11-
private string _Formatted;
12-
private string _Message;
13-
private string _FullPath;
1415
private readonly string _ParentPath;
1516

16-
internal ResultReason(string parentPath, IOperand operand, string text, object[] args)
17+
private string? _Path;
18+
private string? _Formatted;
19+
private string? _Message;
20+
private string? _FullPath;
21+
22+
internal ResultReason(string? parentPath, IOperand? operand, string text, object[]? args)
1723
{
18-
_ParentPath = parentPath;
24+
_ParentPath = parentPath ?? string.Empty;
1925
Operand = operand;
2026
_Path = Operand?.Path;
2127
Text = text;
2228
Args = args;
2329
}
2430

25-
internal IOperand Operand { get; }
31+
internal IOperand? Operand { get; }
2632

2733
/// <summary>
2834
/// The object path that failed.
@@ -31,15 +37,14 @@ public string Path
3137
{
3238
get
3339
{
34-
_Path ??= GetPath();
35-
return _Path;
40+
return _Path ??= GetPath();
3641
}
3742
}
3843

3944
/// <summary>
4045
/// A prefix to add to the object path that failed.
4146
/// </summary>
42-
internal string Prefix
47+
internal string? Prefix
4348
{
4449
get { return Operand?.Prefix; }
4550
set
@@ -59,21 +64,20 @@ public string FullPath
5964
{
6065
get
6166
{
62-
_FullPath ??= GetFullPath();
63-
return _FullPath;
67+
return _FullPath ??= GetFullPath();
6468
}
6569
}
6670

6771
public string Text { get; }
6872

69-
public object[] Args { get; }
73+
public object[]? Args { get; }
7074

75+
/// <inheritdoc/>
7176
public string Message
7277
{
7378
get
7479
{
75-
_Message ??= Args == null || Args.Length == 0 ? Text : string.Format(Thread.CurrentThread.CurrentCulture, Text, Args);
76-
return _Message;
80+
return _Message ??= Args == null || Args.Length == 0 ? Text : string.Format(Thread.CurrentThread.CurrentCulture, Text, Args);
7781
}
7882
}
7983

@@ -82,13 +86,23 @@ public override string ToString()
8286
return Format();
8387
}
8488

89+
public override int GetHashCode()
90+
{
91+
return ToString().GetHashCode();
92+
}
93+
94+
public override bool Equals(object obj)
95+
{
96+
return obj is IResultReason other && Equals(other);
97+
}
98+
99+
/// <inheritdoc/>
85100
public string Format()
86101
{
87-
_Formatted ??= string.Concat(
102+
return _Formatted ??= string.Concat(
88103
Operand?.ToString(),
89104
Message
90105
);
91-
return _Formatted;
92106
}
93107

94108
private string GetPath()
@@ -100,4 +114,17 @@ private string GetFullPath()
100114
{
101115
return Runtime.Operand.JoinPath(_ParentPath, Path);
102116
}
117+
118+
#region IEquatable<IResultReason>
119+
120+
public bool Equals(IResultReason? other)
121+
{
122+
return other != null &&
123+
string.Equals(FullPath, other.FullPath, StringComparison.Ordinal) &&
124+
string.Equals(Message, other.Message, StringComparison.Ordinal);
125+
}
126+
127+
#endregion IEquatable<IResultReason>
103128
}
129+
130+
#nullable restore

src/PSRule/Rules/RuleRecord.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ internal RuleRecord(ResourceId ruleId, string @ref, TargetObject targetObject, s
191191
[DefaultValue(null)]
192192
[JsonProperty(PropertyName = "detail")]
193193
[YamlMember()]
194-
public IResultDetailV2 Detail => _Detail;
194+
public IResultDetail Detail => _Detail;
195195

196196
/// <summary>
197197
/// Any default properties for the rule.

src/PSRule/Runtime/AssertResult.cs

+20-9
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,16 @@
77

88
namespace PSRule.Runtime;
99

10+
#nullable enable
11+
1012
/// <summary>
1113
/// The result of a single assertion.
1214
/// </summary>
1315
public sealed class AssertResult : IEquatable<bool>
1416
{
15-
private readonly List<ResultReason> _Reason;
17+
private List<ResultReason>? _Reason;
1618

17-
internal AssertResult(IOperand operand, bool value, string reason, object[] args)
19+
internal AssertResult(IOperand? operand, bool value, string reason, object[] args)
1820
{
1921
Result = value;
2022
if (!Result)
@@ -54,7 +56,8 @@ internal void AddReason(AssertResult result)
5456
if (result == null || Result || result.Result || result._Reason == null || result._Reason.Count == 0)
5557
return;
5658

57-
_Reason.AddRange(result._Reason);
59+
_Reason ??= [];
60+
_Reason.AddRange(result._Reason.Where(r => !_Reason.Contains(r)));
5861
}
5962

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

72-
_Reason.Add(new ResultReason(RunspaceContext.CurrentThread?.TargetObject?.Path, operand, text, args));
75+
_Reason ??= [];
76+
77+
var reason = new ResultReason(RunspaceContext.CurrentThread?.TargetObject?.Path, operand, text, args);
78+
if (_Reason.Contains(reason))
79+
return;
80+
81+
_Reason.Add(reason);
7382
}
7483

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

170179
// Continue
@@ -179,7 +188,7 @@ public bool Complete()
179188
/// </summary>
180189
public void Ignore()
181190
{
182-
_Reason.Clear();
191+
_Reason?.Clear();
183192
}
184193

185194
/// <inheritdoc/>
@@ -204,13 +213,15 @@ public bool ToBoolean()
204213
return Result;
205214
}
206215

207-
internal IResultReasonV2[] ToResultReason()
216+
internal IResultReason[] ToResultReason()
208217
{
209-
return _Reason == null || _Reason.Count == 0 ? Array.Empty<IResultReasonV2>() : _Reason.ToArray();
218+
return _Reason == null || _Reason.Count == 0 ? [] : _Reason.ToArray();
210219
}
211220

212221
private bool IsNullOrEmptyReason()
213222
{
214223
return _Reason == null || _Reason.Count == 0;
215224
}
216225
}
226+
227+
#nullable restore

src/PSRule/Runtime/RuleConditionHelper.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,10 @@ internal static RuleConditionResult Create(IEnumerable<object> value)
3838
private static bool TryBoolean(object o, out bool result)
3939
{
4040
result = false;
41-
if (o is not bool bresult)
41+
if (o is not bool b)
4242
return false;
4343

44-
result = bresult;
44+
result = b;
4545
return true;
4646
}
4747

tests/PSRule.Tests/AssertTests.cs

+4-9
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,13 @@
1313

1414
namespace PSRule;
1515

16-
[Trait(LANGUAGE, LANGUAGEELEMENT)]
17-
public sealed class AssertTests : ContextBaseTests
16+
[Trait(LANGUAGE, LANGUAGE_ELEMENT)]
17+
public sealed class AssertTests(ITestOutputHelper output) : ContextBaseTests
1818
{
1919
private const string LANGUAGE = "Language";
20-
private const string LANGUAGEELEMENT = "Variable";
20+
private const string LANGUAGE_ELEMENT = "Variable";
2121

22-
private readonly ITestOutputHelper _Output;
23-
24-
public AssertTests(ITestOutputHelper output)
25-
{
26-
_Output = output;
27-
}
22+
private readonly ITestOutputHelper _Output = output;
2823

2924
[Fact]
3025
public void Assertion()

0 commit comments

Comments
 (0)