Skip to content

Commit 94e1db5

Browse files
committed
limit amount of emitted messages by build check
1 parent bab5f9f commit 94e1db5

File tree

6 files changed

+101
-6
lines changed

6 files changed

+101
-6
lines changed

src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -573,7 +573,10 @@ public CheckWrapper Initialize(Check ba, ConfigurationContext configContext)
573573
throw new BuildCheckConfigurationException(
574574
$"The Check '{ba.FriendlyName}' failed to initialize: {e.Message}", e);
575575
}
576-
return new CheckWrapper(ba);
576+
577+
CheckWrapper wrapper = new(ba);
578+
wrapper.Initialize();
579+
return wrapper;
577580
}
578581

579582
public CheckWrapper? MaterializedCheck { get; set; }

src/Build/BuildCheck/Infrastructure/BuildEventsProcessor.cs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -242,10 +242,6 @@ private static void ReportResult(
242242
return;
243243
}
244244

245-
BuildEventArgs eventArgs = result.ToEventArgs(config.Severity);
246-
247-
eventArgs.BuildEventContext = checkContext.BuildEventContext;
248-
249-
checkContext.DispatchBuildEvent(eventArgs);
245+
checkWrapper.ReportResult(result, checkContext, config);
250246
}
251247
}

src/Build/BuildCheck/Infrastructure/CheckWrapper.cs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System.Linq;
88
using Microsoft.Build.BackEnd.Logging;
99
using Microsoft.Build.Experimental.BuildCheck;
10+
using Microsoft.Build.Framework;
1011

1112
namespace Microsoft.Build.Experimental.BuildCheck.Infrastructure;
1213

@@ -17,6 +18,16 @@ internal sealed class CheckWrapper
1718
{
1819
private readonly Stopwatch _stopwatch = new Stopwatch();
1920

21+
/// <summary>
22+
/// Maximum amount of messages that could be sent per check rule.
23+
/// </summary>
24+
public const int MaxMessageCountPerRule = 10;
25+
26+
/// <summary>
27+
/// Keeps track of number of reports sent per rule.
28+
/// </summary>
29+
private Dictionary<string, int>? _reportsCountPerRule;
30+
2031
public CheckWrapper(Check check)
2132
{
2233
Check = check;
@@ -29,6 +40,11 @@ public CheckWrapper(Check check)
2940
// In such case - configuration will be same for all projects. So we do not need to store it per project in a collection.
3041
internal CheckConfigurationEffective? CommonConfig { get; private set; }
3142

43+
internal void Initialize()
44+
{
45+
_reportsCountPerRule = new Dictionary<string, int>();
46+
}
47+
3248
// start new project
3349
internal void StartNewProject(
3450
string fullProjectPath,
@@ -52,6 +68,33 @@ internal void StartNewProject(
5268
}
5369
}
5470

71+
public void ReportResult(BuildCheckResult result, ICheckContext checkContext, CheckConfigurationEffective config)
72+
{
73+
if (_reportsCountPerRule is not null)
74+
{
75+
if (!_reportsCountPerRule.ContainsKey(result.CheckRule.Id))
76+
{
77+
_reportsCountPerRule[result.CheckRule.Id] = 0;
78+
}
79+
_reportsCountPerRule[result.CheckRule.Id]++;
80+
81+
if (_reportsCountPerRule[result.CheckRule.Id] == MaxMessageCountPerRule + 1)
82+
{
83+
checkContext.DispatchAsCommentFromText(MessageImportance.Normal, $"The check '{Check.FriendlyName}' has exceeded the maximum number of results allowed for the rule '{result.CheckRule.Id}'. Any additional results will not be displayed.");
84+
return;
85+
}
86+
87+
if (_reportsCountPerRule[result.CheckRule.Id] > MaxMessageCountPerRule + 1)
88+
{
89+
return;
90+
}
91+
}
92+
93+
BuildEventArgs eventArgs = result.ToEventArgs(config.Severity);
94+
eventArgs.BuildEventContext = checkContext.BuildEventContext;
95+
checkContext.DispatchBuildEvent(eventArgs);
96+
}
97+
5598
// to be used on eval node (BuildCheckDataSource.check)
5699
internal void Uninitialize()
57100
{

src/BuildCheck.UnitTests/EndToEndTests.cs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,30 @@ public void PropertiesUsageAnalyzerTest(bool buildInOutOfProcessNode)
6262
Regex.Matches(output, "BC0203 .* Property").Count.ShouldBe(2);
6363
}
6464

65+
66+
[Theory]
67+
[InlineData(true)]
68+
[InlineData(false)]
69+
public void WarningsCountExceedsLimitTest(bool buildInOutOfProcessNode)
70+
{
71+
PrepareSampleProjectsAndConfig(
72+
buildInOutOfProcessNode,
73+
out TransientTestFile projectFile,
74+
"PropsCheckTestWithLimit.csproj");
75+
76+
string output = RunnerUtilities.ExecBootstrapedMSBuild($"{projectFile.Path} -check", out bool success);
77+
_env.Output.WriteLine(output);
78+
_env.Output.WriteLine("=========================");
79+
success.ShouldBeTrue(output);
80+
81+
output.ShouldMatch(@"has exceeded the maximum number of results allowed for the rule");
82+
83+
// each finding should be found just once - but reported twice, due to summary
84+
Regex.Matches(output, "BC0202: .* Property").Count.ShouldBe(2);
85+
Regex.Matches(output, "BC0203: .* Property").Count.ShouldBe(20);
86+
}
87+
88+
6589
[Theory]
6690
[InlineData(true, true)]
6791
[InlineData(false, true)]
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<Project DefaultTargets="PrintEnvVar">
2+
<PropertyGroup>
3+
<MyProp01>$(MyProp01)</MyProp01>
4+
<MyProp02>$(MyProp02)</MyProp02>
5+
<MyProp03>$(MyProp03)</MyProp03>
6+
<MyProp04>$(MyProp04)</MyProp04>
7+
<MyProp05>$(MyProp05)</MyProp05>
8+
<MyProp06>$(MyProp06)</MyProp06>
9+
<MyProp07>$(MyProp07)</MyProp07>
10+
<MyProp08>$(MyProp08)</MyProp08>
11+
<MyProp09>$(MyProp09)</MyProp09>
12+
<MyProp10>$(MyProp10)</MyProp10>
13+
<MyProp11>$(MyProp11)</MyProp11>
14+
</PropertyGroup>
15+
16+
<Target Name="PrintEnvVar">
17+
<Message Text="MyPropT2 has value $(MyPropT2)" Importance="High" />
18+
<PropertyGroup>
19+
<MyPropT2>SomeValue</MyPropT2>
20+
</PropertyGroup>
21+
</Target>
22+
</Project>

src/MSBuild/XMake.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2721,6 +2721,13 @@ private static bool ProcessCommandLineSwitches(
27212721

27222722
isBuildCheckEnabled = IsBuildCheckEnabled(commandLineSwitches);
27232723

2724+
// BuildCheck is not compatible with node reusing, see #10317.
2725+
// Disable node reuse when build check is on.
2726+
if (isBuildCheckEnabled)
2727+
{
2728+
enableNodeReuse = false;
2729+
}
2730+
27242731
inputResultsCaches = ProcessInputResultsCaches(commandLineSwitches);
27252732

27262733
outputResultsCache = ProcessOutputResultsCache(commandLineSwitches);

0 commit comments

Comments
 (0)