Skip to content

Commit

Permalink
Fix build check build submission errors (#10227)
Browse files Browse the repository at this point in the history
Fixes #10071

Context
While building with buildcheck enabled and MSBUILDNOINPROCNODE=1
Severity of the rule set to Error, does not reported to the all build:
image
This is since buildcheck error is not added into the _warningsAsErrorsByProject, WarningsAsErrors == null && _warningsAsErrorsByProject == null is true all the time. so HasBuildSubmissionLoggedErrors always return false.
msbuild/src/Build/BackEnd/Components/Logging/LoggingService.cs

Lines 577 to 587 in 843bfa5

 public bool HasBuildSubmissionLoggedErrors(int submissionId) 
 { 
     // Warnings as errors are not tracked if the user did not specify to do so 
     if (WarningsAsErrors == null && _warningsAsErrorsByProject == null) 
     { 
         return false; 
     } 
  
     // Determine if any of the event sinks have logged an error with this submission ID 
     return _buildSubmissionIdsThatHaveLoggedErrors?.Contains(submissionId) == true; 
 } 
Treat warning as errors or message, the buildcheckResultWarning doesn't initialize the code. So when the code of BuildWarningEventArgs is null. ShouldTreatWarningAsError returns false all the time.
msbuild/src/Build/BackEnd/Components/Logging/LoggingService.cs

Lines 1897 to 1908 in a9c95c7

 /// <returns><code>true</code> if the warning should be treated as an error, otherwise <code>false</code>.</returns> 
 private bool ShouldTreatWarningAsError(BuildWarningEventArgs warningEvent) 
 { 
     // This only applies if the user specified /warnaserror from the command-line or added an empty set through the object model 
     if (WarningsAsErrors != null) 
     { 
         // Global warnings as errors apply to all projects.  If the list is empty or contains the code, the warning should be treated as an error 
         if ((WarningsAsErrors.Count == 0 && WarningAsErrorNotOverriden(warningEvent)) || WarningsAsErrors.Contains(warningEvent.Code)) 
         { 
             return true; 
         } 
     } 
Changes Made
when buildEventArgs is BuildErrorEventArgs, treat BuildErrorEventArgs' related warnings as errors

Initialize the code of BuildCheckResultWarning that is inherited from BuildWarningEventArgs

Testing
Manually testing on local now

set MSBUILDNOINPROCNODE=1 and change the build_check.BC0101.Severity= Error
dotnet D:\WORK\msbuild\artifacts\bin\bootstrap\net8.0\MSBuild\MSBuild.dll FooBar.csproj /m:1 -nr:False -restore -analyze

Change build_check.BC0101.Severity= warning
dotnet D:\WORK\msbuild\artifacts\bin\bootstrap\net8.0\MSBuild\MSBuild.dll FooBar.csproj /m:1 -nr:False -restore -analyze -warnaserror
  • Loading branch information
JaynieBai authored Jul 9, 2024
1 parent 74c90d6 commit c2f9b76
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 5 deletions.
7 changes: 7 additions & 0 deletions src/Build/BackEnd/Components/Logging/LoggingService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
using Microsoft.Build.Shared;
using InternalLoggerException = Microsoft.Build.Exceptions.InternalLoggerException;
using LoggerDescription = Microsoft.Build.Logging.LoggerDescription;
using Microsoft.Build.Experimental.BuildCheck;

#nullable disable

Expand Down Expand Up @@ -1592,6 +1593,12 @@ private void RouteBuildEvent(object loggingEvent)
_buildSubmissionIdsThatHaveLoggedErrors.Add(errorEvent.BuildEventContext?.SubmissionId ?? BuildEventContext.InvalidSubmissionId);
}

if (buildEventArgs is BuildCheckResultError checkResultError)
{
// If the specified BuildCheckResultError was issued, an empty ISet<string> signifies that the specified build check warnings should be treated as errors.
AddWarningsAsErrors(checkResultError.BuildEventContext, new HashSet<string>());
}

if (buildEventArgs is ProjectFinishedEventArgs projectFinishedEvent && projectFinishedEvent.BuildEventContext != null)
{
WarningsConfigKey key = GetWarningsConfigKey(projectFinishedEvent);
Expand Down
6 changes: 3 additions & 3 deletions src/Build/BuildCheck/API/BuildCheckResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ internal BuildEventArgs ToEventArgs(BuildAnalyzerResultSeverity severity)
=> severity switch
{
BuildAnalyzerResultSeverity.Suggestion => new BuildCheckResultMessage(this),
BuildAnalyzerResultSeverity.Warning => new BuildCheckResultWarning(this),
BuildAnalyzerResultSeverity.Error => new BuildCheckResultError(this),
BuildAnalyzerResultSeverity.Warning => new BuildCheckResultWarning(this, BuildAnalyzerRule.Id),
BuildAnalyzerResultSeverity.Error => new BuildCheckResultError(this, BuildAnalyzerRule.Id),
_ => throw new ArgumentOutOfRangeException(nameof(severity), severity, null),
};

Expand All @@ -51,7 +51,7 @@ internal BuildEventArgs ToEventArgs(BuildAnalyzerResultSeverity severity)

// Here we will provide different link for built-in rules and custom rules - once we have the base classes differentiated.
public string FormatMessage() =>
_message ??= $"{(Equals(Location ?? ElementLocation.EmptyLocation, ElementLocation.EmptyLocation) ? string.Empty : (Location!.LocationString + ": "))}{BuildAnalyzerRule.Id}: https://aka.ms/buildcheck/codes#{BuildAnalyzerRule.Id} - {string.Format(BuildAnalyzerRule.MessageFormat, MessageArgs)}";
_message ??= $"{(Equals(Location ?? ElementLocation.EmptyLocation, ElementLocation.EmptyLocation) ? string.Empty : (Location!.LocationString + ": "))}https://aka.ms/buildcheck/codes#{BuildAnalyzerRule.Id} - {string.Format(BuildAnalyzerRule.MessageFormat, MessageArgs)}";

private string? _message;
}
6 changes: 4 additions & 2 deletions src/Framework/BuildCheck/BuildCheckEventArgs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ internal override void CreateFromStream(BinaryReader reader, int version)
}
internal sealed class BuildCheckResultWarning : BuildWarningEventArgs
{
public BuildCheckResultWarning(IBuildCheckResult result)
public BuildCheckResultWarning(IBuildCheckResult result, string code)
: base(subcategory: null, code: code, file: null, lineNumber: 0, columnNumber: 0, endLineNumber: 0, endColumnNumber: 0, message: result.FormatMessage(), helpKeyword: null, senderName: null)
{
RawMessage = result.FormatMessage();
}
Expand All @@ -131,7 +132,8 @@ internal override void CreateFromStream(BinaryReader reader, int version)

internal sealed class BuildCheckResultError : BuildErrorEventArgs
{
public BuildCheckResultError(IBuildCheckResult result)
public BuildCheckResultError(IBuildCheckResult result, string code)
: base(subcategory: null, code: code, file: null, lineNumber: 0, columnNumber: 0, endLineNumber: 0, endColumnNumber: 0, message: result.FormatMessage(), helpKeyword: null, senderName: null)
{
RawMessage = result.FormatMessage();
}
Expand Down

0 comments on commit c2f9b76

Please sign in to comment.