Fix ShouldTreatWarningAsError in OOP TaskHost checking wrong collection

[Copilot]

Context

OutOfProcTaskHostNode.ShouldTreatWarningAsError() checks WarningsAsMessages instead of WarningsAsErrors when matching specific warning codes. This means <MSBuildWarningsAsErrors>CS0618</MSBuildWarningsAsErrors> silently fails (or NREs) when the task runs out-of-process via TaskHostFactory.

Introduced in v17.10.0 (PR #7309). The in-proc TaskHost.cs has the correct implementation; the OOP copy diverged.

// Bug: last operand checks wrong collection
return (WarningsAsErrors.Count == 0 && WarningAsErrorNotOverriden(warningCode))
    || WarningsAsMessages.Contains(warningCode);  // ← should be WarningsAsErrors

Scenario	Expected	Actual
OOP + specific codes (MY0001)	Error	NRE crash (MSB4018)
OOP + treat-all (-warnaserror)	Error	Warning (not promoted)

Changes Made

• src/MSBuild/OutOfProcTaskHostNode.cs: Fix the return line to check WarningsAsErrors.Contains(warningCode), gated behind ChangeWaves.Wave18_6
• documentation/wiki/ChangeWaves.md: Document the fix under 18.6
• src/Build.UnitTests/WarningsAsMessagesAndErrors_Tests.cs: 3 integration tests that route CustomLogAndReturnTask through TaskHostFactory (out-of-proc) with warning-as-error properties:
  • TreatWarningsAsErrorsWhenSpecified_TaskHostFactory — specific code in MSBuildWarningsAsErrors promotes warning to error
  • TreatAllWarningsAsErrors_TaskHostFactory — MSBuildTreatWarningsAsErrors=true promotes all warnings to errors
  • TreatWarningAsMessageOverridesTreatingItAsError_TaskHostFactory — MSBuildWarningsAsMessages overrides MSBuildWarningsAsErrors

Testing

3 new integration tests in WarningsAsMessagesAndErrors_Tests — all passing. Tests exercise the actual OOP task host path by routing tasks through TaskHostFactory, confirming warnings are correctly promoted to errors in out-of-proc scenarios. All 31 tests in the suite (28 existing + 3 new) pass.

Notes

Gated behind ChangeWave 18.6 per request, since the fix changes observable behavior (builds that previously succeeded with unpromoted warnings will now correctly fail).

Original prompt

---

This section details on the original issue you should resolve

<issue_title>ShouldTreatWarningAsError in OOP TaskHost checks wrong collection (NRE + silent failure)</issue_title>
<issue_description>## Summary

OutOfProcTaskHostNode.ShouldTreatWarningAsError() checks WarningsAsMessages instead of WarningsAsErrors when matching specific warning codes. This means <MSBuildWarningsAsErrors>CS0618</MSBuildWarningsAsErrors> silently fails to promote warnings to errors when the task runs out-of-process via TaskHostFactory.

Introduced in: v17.10.0 (PR #7309)
Affected file: src/MSBuild/OutOfProcTaskHostNode.cs

Semantics (confusing but intentional)

WarningsAsErrors = null           → feature disabled
WarningsAsErrors = {} (empty set) → ALL warnings become errors (universal)
WarningsAsErrors = {"CS0618"}     → only CS0618 becomes error (specific)

Empty set = "all" is the established convention — see LoggingService.cs:2010:

"If the list is empty or contains the code, the warning should be treated as an error"

Bug

// OutOfProcTaskHostNode.cs — BUGGY (main)
public bool ShouldTreatWarningAsError(string warningCode)
{
    if (WarningsAsErrors == null || WarningsAsMessages?.Contains(warningCode) == true)
        return false;

    return (WarningsAsErrors.Count == 0 && WarningAsErrorNotOverriden(warningCode))
        || WarningsAsMessages.Contains(warningCode);
//          ^^^^^^^^^^^^^^^^^ should be WarningsAsErrors
}

Compare the canonical correct implementation in LoggingService.cs:2011:

(WarningsAsErrors.Count == 0 && WarningAsErrorNotOverriden(warningEvent))
    || WarningsAsErrors.Contains(warningEvent.Code)
//     ^^^^^^^^^^^^^^^^ correct collection

Trace: specific codes (e.g. WarningsAsErrors = {"CS0618"}, task emits CS0618)

Step	Expression	Value	Note
1	WarningsAsErrors == null	false	set has 1 item
2	WarningsAsMessages?.Contains("CS0618")	false or null	CS0618 not in messages
3	→ don't return false, continue
4	WarningsAsErrors.Count == 0	false	Count is 1, not 0
5	→ first operand of || is false
6	WarningsAsMessages.Contains("CS0618")	NRE or false	Wrong collection!
7	→ returns false		BUG: should return true

Step 6 hits the wrong collection. If WarningsAsMessages is null → NRE crash. If non-null → always false here (step 2 already verified the code isn't in messages). Either way, specific-code matching never works.

Trace: treat-all (e.g. WarningsAsErrors = {}, task emits MY0001)

The empty-set path was also confirmed broken in testing (warning not promoted). The WarningsAsMessages.Contains() on the return line likely crashes or returns false before the Count == 0 short-circuit can help, depending on null state of WarningsAsMessages. Even if it doesn't NRE, the overall method is unreliable for OOP TaskHost scenarios.

Impact

Verified on .NET SDK 10.0.101 (public release):

Scenario	Expected	Actual (main)
In-proc + specific codes	✅ Error	✅ Error
OOP + specific codes (MY0001)	✅ Error	❌ NRE crash (MSB4018)
OOP + treat-all (-warnaserror)	✅ Error	❌ Warning (not promoted)

Both OOP paths are broken. The NRE crash (not a silent pass) is the more common scenario since most users don't set <MSBuildWarningsAsMessages>.

NRE stack trace (shipped MSBuild)

error MSB4018: The "EmitWarningTask" task failed unexpectedly.
System.NullReferenceException: Object reference not set to an instance of an object.
   at Microsoft.Build.CommandLine.OutOfProcTaskHostNode.ShouldTreatWarningAsError(String warningCode)

Repro

See attached repro.zip. Steps:

cd WarnTask && dotnet build && cd ..

# Control — in-proc promotes correctly (build FAILS):
dotnet msbuild TestInProc.proj -t:Build

# Bug — OOP TaskHost does NOT promote (build SUCCEEDS):
dotnet msbuild TestOOP.proj -t:Build

Fix

- return (WarningsAsErrors.Count == 0 && WarningAsErrorNotOverriden(warningCode))
-     || WarningsAsMessages.Contains(warningCode);
+ return (warningsAsErrors.Count == 0 && WarningAsErrorNotOverriden(warningCode))
+     || warningsAsErrors.Contains(warningCode);

repro.zip

</issue_description>

<agent_instructions>fix this with 18_6 changewave</agent_instructions>

Comments on the Issue (you are @copilot in this section)

• Fixes ShouldTreatWarningAsError in OOP TaskHost checks wrong collection (NRE + silent failure) #13438

---

⚡ Quickly spin up Copilot coding agent tasks from anywhere on your macOS or Windows machine with Raycast.

[dotnet-policy-service]

Hello @@copilot, I noticed that you’re changing an .swr file or any file under src/Package/MSBuild.VSSetup.. Please make sure to validate this change by an experimental VS insertion. This is accomplished by pushing to an exp/* branch, which requires write permissions to this repo.

[JanProvaznik]

@copilot

[Copilot]

Pull request overview

Fixes a regression in out-of-proc task execution (TaskHostFactory) where OutOfProcTaskHostNode.ShouldTreatWarningAsError() checked the wrong warning-code collection, causing specific MSBuildWarningsAsErrors codes to be ignored (and potentially triggering an NRE). The behavior change is gated behind ChangeWave 18.6 and is validated with new integration tests that run a real OOP task-host path.

Changes:

• Correct OutOfProcTaskHostNode.ShouldTreatWarningAsError() to match against WarningsAsErrors (ChangeWave 18.6 gated).
• Add OOP (TaskHostFactory) integration tests covering specific-code, treat-all, and “message overrides error” scenarios.
• Document the fix under ChangeWave 18.6.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/MSBuild/OutOfProcTaskHostNode.cs	Fixes warning-as-error matching logic for OOP task host behind ChangeWave 18.6.
src/Build.UnitTests/WarningsAsMessagesAndErrors_Tests.cs	Adds integration tests that execute a task via TaskHostFactory to validate OOP behavior.
documentation/wiki/ChangeWaves.md	Records the feature/fix as part of ChangeWave 18.6 documentation.