Skip to content

Commit ddc6e86

Browse files
AR-MayAR-May
committed
limit amount of emitted messages by build check
1 parent bab5f9f commit ddc6e86

File tree

6 files changed

+225
-20
lines changed

6 files changed

+225
-20
lines changed

‎src/Build/BuildCheck/Checks/DoubleWritesCheck.cs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,13 @@ internal sealed class DoubleWritesCheck : Check
2929

3030
public override string FriendlyName => "MSBuild.DoubleWritesCheck";
3131

32+
public BuildCheckResultsLimiter? ResultsLimiter;
33+
3234
public override IReadOnlyList<CheckRule> SupportedRules { get; } = [SupportedRule];
3335

3436
public override void Initialize(ConfigurationContext configurationContext)
3537
{
36-
/* This is it - no custom configuration */
38+
ResultsLimiter = new BuildCheckResultsLimiter();
3739
}
3840

3941
public override void RegisterActions(IBuildCheckRegistrationContext registrationContext)
@@ -111,14 +113,15 @@ private void CheckWrite(BuildCheckDataContext<TaskInvocationCheckData> context,
111113

112114
if (_filesWritten.TryGetValue(fileBeingWritten, out (string projectFilePath, string taskName) existingEntry))
113115
{
114-
context.ReportResult(BuildCheckResult.Create(
116+
ResultsLimiter?.ProcessAndReportResult(
117+
context,
115118
SupportedRule,
116119
context.Data.TaskInvocationLocation,
117120
context.Data.TaskName,
118121
existingEntry.taskName,
119122
Path.GetFileName(context.Data.ProjectFilePath),
120123
Path.GetFileName(existingEntry.projectFilePath),
121-
fileBeingWritten));
124+
fileBeingWritten);
122125
}
123126
else
124127
{

‎src/Build/BuildCheck/Checks/NoEnvironmentVariablePropertyCheck.cs‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System;
55
using System.Collections.Generic;
66
using Microsoft.Build.BuildCheck.Infrastructure;
7+
using Microsoft.Build.Experimental.BuildCheck.Infrastructure;
78
using Microsoft.Build.Shared;
89

910
namespace Microsoft.Build.Experimental.BuildCheck.Checks;
@@ -33,6 +34,8 @@ internal sealed class NoEnvironmentVariablePropertyCheck : Check
3334

3435
public override string FriendlyName => "MSBuild.NoEnvironmentVariablePropertyCheck";
3536

37+
public BuildCheckResultsLimiter? ResultsLimiter;
38+
3639
public override IReadOnlyList<CheckRule> SupportedRules { get; } = [SupportedRule];
3740

3841
public override void Initialize(ConfigurationContext configurationContext)
@@ -45,6 +48,8 @@ public override void Initialize(ConfigurationContext configurationContext)
4548
}
4649

4750
CheckScopeClassifier.NotifyOnScopingReadiness += HandleScopeReadiness;
51+
52+
ResultsLimiter = new BuildCheckResultsLimiter();
4853
}
4954

5055
public override void RegisterActions(IBuildCheckRegistrationContext registrationContext) => registrationContext.RegisterEnvironmentVariableReadAction(ProcessEnvironmentVariableReadAction);
@@ -61,10 +66,11 @@ private void ProcessEnvironmentVariableReadAction(BuildCheckDataContext<Environm
6166
}
6267
else if (CheckScopeClassifier.IsActionInObservedScope(_scope, context.Data.EnvironmentVariableLocation.File, context.Data.ProjectFilePath))
6368
{
64-
context.ReportResult(BuildCheckResult.Create(
69+
ResultsLimiter?.ProcessAndReportResult(
70+
context,
6571
SupportedRule,
6672
context.Data.EnvironmentVariableLocation,
67-
GetFormattedMessage(context.Data.EnvironmentVariableName, context.Data.EnvironmentVariableValue)));
73+
GetFormattedMessage(context.Data.EnvironmentVariableName, context.Data.EnvironmentVariableValue));
6874
}
6975

7076
_environmentVariablesCache.Add(identityKey);
@@ -86,10 +92,11 @@ private void HandleScopeReadiness()
8692
continue;
8793
}
8894

89-
context.ReportResult(BuildCheckResult.Create(
95+
ResultsLimiter?.ProcessAndReportResult(
96+
context,
9097
SupportedRule,
9198
context.Data.EnvironmentVariableLocation,
92-
GetFormattedMessage(context.Data.EnvironmentVariableName, context.Data.EnvironmentVariableValue)));
99+
GetFormattedMessage(context.Data.EnvironmentVariableName, context.Data.EnvironmentVariableValue));
93100
}
94101

95102
CheckScopeClassifier.NotifyOnScopingReadiness -= HandleScopeReadiness;

‎src/Build/BuildCheck/Checks/PropertiesUsageCheck.cs‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ internal class PropertiesUsageCheck : InternalCheck
3737

3838
public override IReadOnlyList<CheckRule> SupportedRules => SupportedRulesList;
3939

40+
public BuildCheckResultsLimiter? ResultsLimiter;
41+
4042
private const string _allowUninitPropsInConditionsKey = "AllowUninitializedPropertiesInConditions";
4143
private bool _allowUninitPropsInConditions = false;
4244
// Each check can have it's scope and enablement
@@ -89,6 +91,8 @@ public override void Initialize(ConfigurationContext configurationContext)
8991
{
9092
_allowUninitPropsInConditions = allowUninitPropsInConditionsRule1 ?? allowUninitPropsInConditionsRule2 ?? false;
9193
}
94+
95+
ResultsLimiter = new BuildCheckResultsLimiter();
9296
}
9397

9498
private static bool? GetAllowUninitPropsInConditionsConfig(CustomConfigurationData customConfigurationData,
@@ -146,10 +150,11 @@ private void ProcessPropertyWrite(BuildCheckDataContext<PropertyWriteData> conte
146150
{
147151
_uninitializedReadsInScope.Remove(writeData.PropertyName);
148152

149-
context.ReportResult(BuildCheckResult.Create(
153+
ResultsLimiter?.ProcessAndReportResult(
154+
context,
150155
_initializedAfterUsedRule,
151156
uninitInScopeReadLocation,
152-
writeData.PropertyName, writeData.ElementLocation?.LocationString ?? string.Empty));
157+
writeData.PropertyName, writeData.ElementLocation?.LocationString ?? string.Empty);
153158
}
154159

155160
if (CheckScopeClassifier.IsActionInObservedScope(_initializedAfterUseScope,
@@ -158,10 +163,11 @@ private void ProcessPropertyWrite(BuildCheckDataContext<PropertyWriteData> conte
158163
{
159164
_uninitializedReadsOutOfScope.Remove(writeData.PropertyName);
160165

161-
context.ReportResult(BuildCheckResult.Create(
166+
ResultsLimiter?.ProcessAndReportResult(
167+
context,
162168
_initializedAfterUsedRule,
163169
uninitOutScopeReadLocation,
164-
writeData.PropertyName, writeData.ElementLocation?.LocationString ?? string.Empty));
170+
writeData.PropertyName, writeData.ElementLocation?.LocationString ?? string.Empty);
165171
}
166172
}
167173
}
@@ -203,10 +209,11 @@ private void ProcessPropertyRead(BuildCheckDataContext<PropertyReadData> context
203209
readData.ElementLocation, readData.ProjectFilePath))
204210
{
205211
// report immediately
206-
context.ReportResult(BuildCheckResult.Create(
212+
ResultsLimiter?.ProcessAndReportResult(
213+
context,
207214
_usedBeforeInitializedRule,
208215
readData.ElementLocation,
209-
readData.PropertyName));
216+
readData.PropertyName);
210217
}
211218
}
212219
}
@@ -218,21 +225,23 @@ private void DoneWithProject(BuildCheckDataContext<ProjectRequestProcessingDoneD
218225
{
219226
if (propWithLocation.Value != null && !_readProperties.Contains(propWithLocation.Key))
220227
{
221-
context.ReportResult(BuildCheckResult.Create(
228+
ResultsLimiter?.ProcessAndReportResult(
229+
context,
222230
_unusedPropertyRule,
223231
propWithLocation.Value,
224-
propWithLocation.Key));
232+
propWithLocation.Key);
225233
}
226234
}
227235

228236
// Report the remaining uninitialized reads - as if 'initialized after read' check was enabled - we cannot report
229237
// uninitialized reads immediately (instead we wait if they are attempted to be initialized late).
230238
foreach (var uninitializedRead in _uninitializedReadsInScope)
231239
{
232-
context.ReportResult(BuildCheckResult.Create(
240+
ResultsLimiter?.ProcessAndReportResult(
241+
context,
233242
_usedBeforeInitializedRule,
234243
uninitializedRead.Value,
235-
uninitializedRead.Key));
244+
uninitializedRead.Key);
236245
}
237246

238247
_readProperties = new HashSet<string>(MSBuildNameIgnoreCaseComparer.Default);

‎src/Build/BuildCheck/Checks/SharedOutputPathCheck.cs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,11 @@ internal sealed class SharedOutputPathCheck : Check
2525

2626
public override IReadOnlyList<CheckRule> SupportedRules { get; } = [SupportedRule];
2727

28+
public BuildCheckResultsLimiter? ResultsLimiter;
29+
2830
public override void Initialize(ConfigurationContext configurationContext)
2931
{
30-
/* This is it - no custom configuration */
32+
ResultsLimiter = new BuildCheckResultsLimiter();
3133
}
3234

3335
public override void RegisterActions(IBuildCheckRegistrationContext registrationContext)
@@ -81,13 +83,14 @@ private void EvaluatedPropertiesAction(BuildCheckDataContext<EvaluatedProperties
8183

8284
if (_projectsPerOutputPath.TryGetValue(path!, out string? conflictingProject))
8385
{
84-
context.ReportResult(BuildCheckResult.Create(
86+
ResultsLimiter?.ProcessAndReportResult(
87+
context,
8588
SupportedRule,
8689
// Populating precise location tracked via https://github.com/orgs/dotnet/projects/373/views/1?pane=issue&itemId=58661732
8790
ElementLocation.EmptyLocation,
8891
Path.GetFileName(projectPath),
8992
Path.GetFileName(conflictingProject),
90-
path!));
93+
path!);
9194
}
9295
else
9396
{

‎src/Build/BuildCheck/Infrastructure/BuildCheckResultsLimiter.cs‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
using System.Linq;
7+
using System.Text;
8+
using System.Threading.Tasks;
9+
using Microsoft.Build.Experimental.BuildCheck;
10+
using Microsoft.Build.Shared;
11+
12+
namespace Microsoft.Build.Experimental.BuildCheck.Infrastructure;
13+
14+
/// <summary>
15+
/// Wrapper around BuildCheck results message reporting. Allows to centrally control emission of BuildCheck results, limiting results number.
16+
/// </summary>
17+
internal class BuildCheckResultsLimiter
18+
{
19+
private readonly int _maxMessageCountPerRule;
20+
private readonly Dictionary<string, int> _messageCountPerRule = new Dictionary<string, int>();
21+
22+
public BuildCheckResultsLimiter(int maxMessageCount) => _maxMessageCountPerRule = maxMessageCount;
23+
24+
public BuildCheckResultsLimiter() => _maxMessageCountPerRule = 10;
25+
26+
public void ProcessAndReportResult<T>(BuildCheckDataContext<T> context, CheckRule rule, IMSBuildElementLocation location, params string[] messageArgs) where T : CheckData
27+
{
28+
if (!_messageCountPerRule.ContainsKey(rule.Id))
29+
{
30+
_messageCountPerRule[rule.Id] = 0;
31+
}
32+
33+
if (_messageCountPerRule[rule.Id] >= _maxMessageCountPerRule)
34+
{
35+
return;
36+
}
37+
38+
_messageCountPerRule[rule.Id]++;
39+
context.ReportResult(BuildCheckResult.Create(
40+
rule,
41+
location,
42+
messageArgs));
43+
}
44+
}

‎src/BuildCheck.UnitTests/BuildCheckResultsLimiter_Tests.cs‎

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
using System.Linq;
7+
using System.Text;
8+
using System.Threading.Tasks;
9+
using Microsoft.Build.Construction;
10+
using Microsoft.Build.Experimental.BuildCheck;
11+
using Microsoft.Build.Experimental.BuildCheck.Infrastructure;
12+
using Shouldly;
13+
using Xunit;
14+
15+
namespace Microsoft.Build.BuildCheck.UnitTests;
16+
17+
public class BuildCheckResultsLimiter_Tests
18+
{
19+
private readonly VeryVerboseCheckRuleMock _check;
20+
21+
private readonly MockBuildCheckRegistrationContext _registrationContext;
22+
23+
public BuildCheckResultsLimiter_Tests()
24+
{
25+
_check = new VeryVerboseCheckRuleMock("VerboseCheckRuleMock");
26+
_check.Initialize();
27+
_registrationContext = new MockBuildCheckRegistrationContext();
28+
_check.RegisterActions(_registrationContext);
29+
}
30+
31+
private EvaluatedPropertiesCheckData MakeEvaluatedPropertiesAction(
32+
string projectFile,
33+
Dictionary<string, string>? evaluatedProperties,
34+
IReadOnlyDictionary<string, (string EnvVarValue, string File, int Line, int Column)>? evaluatedEnvVars)
35+
{
36+
return new EvaluatedPropertiesCheckData(
37+
projectFile,
38+
null,
39+
evaluatedProperties ?? new Dictionary<string, string>());
40+
}
41+
42+
[Fact]
43+
public void TestBuildCheckMessageDispatcher()
44+
{
45+
string projectFile = "/fake/project.proj";
46+
_registrationContext.TriggerEvaluatedPropertiesAction(MakeEvaluatedPropertiesAction(
47+
projectFile,
48+
null,
49+
null));
50+
51+
_registrationContext.Results.Count.ShouldBe(
52+
Math.Max(VeryVerboseCheckRuleMock.Rule1Count, VeryVerboseCheckRuleMock.MaxCountPerRule) +
53+
Math.Max(VeryVerboseCheckRuleMock.Rule2Count, VeryVerboseCheckRuleMock.MaxCountPerRule) +
54+
Math.Max(VeryVerboseCheckRuleMock.Rule3Count, VeryVerboseCheckRuleMock.MaxCountPerRule));
55+
}
56+
57+
private sealed class VeryVerboseCheckRuleMock : Check
58+
{
59+
public static CheckRule SupportedRule1 = new CheckRule(
60+
"V001",
61+
"Rule1",
62+
"Description",
63+
"Message format: {0}",
64+
new CheckConfiguration());
65+
66+
public static CheckRule SupportedRule2 = new CheckRule(
67+
"V002",
68+
"Rule2",
69+
"Description",
70+
"Message format: {0}",
71+
new CheckConfiguration());
72+
73+
public static CheckRule SupportedRule3 = new CheckRule(
74+
"V003",
75+
"Rule3",
76+
"Description",
77+
"Message format: {0}",
78+
new CheckConfiguration());
79+
80+
public const int Rule1Count = 3;
81+
public const int Rule2Count = 20;
82+
public const int Rule3Count = 2;
83+
public const int MaxCountPerRule = 3;
84+
85+
public BuildCheckResultsLimiter? ResultsLimiter;
86+
87+
internal VeryVerboseCheckRuleMock(string friendlyName)
88+
{
89+
FriendlyName = friendlyName;
90+
}
91+
92+
public override string FriendlyName { get; }
93+
94+
public override IReadOnlyList<CheckRule> SupportedRules { get; } = new List<CheckRule>() { SupportedRule1, SupportedRule2, SupportedRule3 };
95+
96+
public override void Initialize(ConfigurationContext configurationContext)
97+
{
98+
ResultsLimiter = new BuildCheckResultsLimiter(MaxCountPerRule);
99+
}
100+
101+
public void Initialize()
102+
{
103+
ResultsLimiter = new BuildCheckResultsLimiter(3);
104+
}
105+
106+
public override void RegisterActions(IBuildCheckRegistrationContext registrationContext)
107+
{
108+
registrationContext.RegisterEvaluatedPropertiesAction(EvaluatedPropertiesAction);
109+
}
110+
111+
private void EvaluatedPropertiesAction(BuildCheckDataContext<EvaluatedPropertiesCheckData> context)
112+
{
113+
for (int i = 0; i < Rule1Count; i++)
114+
{
115+
ResultsLimiter?.ProcessAndReportResult(
116+
context,
117+
SupportedRule1,
118+
ElementLocation.EmptyLocation,
119+
"Argument for the message format");
120+
}
121+
for (int i = 0; i < Rule2Count; i++)
122+
{
123+
ResultsLimiter?.ProcessAndReportResult(
124+
context,
125+
SupportedRule2,
126+
ElementLocation.EmptyLocation,
127+
"Argument for the message format");
128+
}
129+
for (int i = 0; i < Rule3Count; i++)
130+
{
131+
ResultsLimiter?.ProcessAndReportResult(
132+
context,
133+
SupportedRule3,
134+
ElementLocation.EmptyLocation,
135+
"Argument for the message format");
136+
}
137+
}
138+
}
139+
}

0 commit comments

Comments
 (0)