Skip to content

Fix chained item function empty result comparison in conditions#12901

Merged
JanProvaznik merged 4 commits into
dotnet:mainfrom
JanProvaznik:copilot/fix-item-function-comparison
Feb 2, 2026
Merged

Fix chained item function empty result comparison in conditions#12901
JanProvaznik merged 4 commits into
dotnet:mainfrom
JanProvaznik:copilot/fix-item-function-comparison

Conversation

@JanProvaznik
Copy link
Copy Markdown
Member

Fixes #12877

Context

Chained item functions incorrectly evaluate as non-empty in conditions when the final result is empty. For example:

<ItemGroup>
  <TestItem Include="Test1" Foo="Bar" />
</ItemGroup>
<Message Text="Success" 
  Condition="'@(TestItem->WithMetadataValue('Identity','Test1')->WithMetadataValue('Foo','Baz'))' == ''" />

The first transform matches Test1, but the second filters it out (Foo="Bar" != "Baz"). The final result is empty, yet the condition fails.

Changes Made

  • src/Build/Evaluation/Expander.cs

    • Moved BreakOnNotEmpty check in Transform method to execute after all chained transforms complete, not after each intermediate transform
    • Previously checked after each transform at line ~2024, causing early bailout on intermediate non-empty results
    • Now checks only the final result at line ~2032-2043
  • src/Build.UnitTests/Evaluation/Expander_Tests.cs

    • Added ChainedItemFunctionEmptyResultInCondition test verifying the fix

Testing

  • Unit test added to validate the specific scenario.
  • Full test suite and manual verification pending.

Notes

  • The bug only manifested when using BreakOnNotEmpty optimization (condition evaluation path), not in normal property expansion.

Copilot AI and others added 3 commits December 5, 2025 14:18
The issue is that when chaining item functions like:
@(TestItem->WithMetadataValue('Identity', 'Test1')->WithMetadataValue('Foo', 'Baz'))

The result incorrectly evaluates as non-empty when compared in a condition, even though the actual result is empty.

Initial hypothesis was that the BreakOnNotEmpty optimization in Transform() was breaking early after the first transform. However, testing revealed that the Transform() method is NOT being called at all for chained transforms with BreakOnNotEmpty option.

This suggests the bug is elsewhere - possibly in how ExpressionShredder parses chained transforms or in how they're evaluated separately.

Co-authored-by: JanProvaznik <25267098+JanProvaznik@users.noreply.github.com>
When chaining item functions like @(Item->Func1()->Func2()), and the final result is empty, the condition comparison with empty string was incorrectly evaluating to false.

The bug was in the Transform method in Expander.cs. The original code checked for BreakOnNotEmpty after EACH intermediate transform, causing it to break early when an intermediate result was non-empty, even if subsequent transforms would filter to an empty result.

The fix moves the BreakOnNotEmpty check to after ALL transforms in the chain complete, ensuring we only check the final result, not intermediate ones.

Added a unit test to verify the fix works correctly.

Co-authored-by: JanProvaznik <25267098+JanProvaznik@users.noreply.github.com>
Copilot AI review requested due to automatic review settings December 9, 2025 12:10
@JanProvaznik JanProvaznik self-assigned this Dec 9, 2025
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a bug (#12877) where chained item functions incorrectly evaluated as non-empty in conditions when the final result was actually empty. The issue occurred because the BreakOnNotEmpty optimization was checking intermediate results after each transform in the chain, rather than checking only the final result after all transforms completed.

Key changes:

  • Modified the Transform method in Expander.cs to delay the BreakOnNotEmpty check until after all chained transforms complete
  • Added a test case to verify chained item functions with empty results are correctly evaluated in conditions

Reviewed changes

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

File Description
src/Build/Evaluation/Expander.cs Moved BreakOnNotEmpty optimization check from inside the transform loop to after all transforms complete, ensuring correct evaluation of chained item functions
src/Build.UnitTests/Evaluation/Expander_Tests.cs Added test case ChainedItemFunctionEmptyResultInCondition to verify the bug fix with a scenario using chained WithMetadataValue functions

Comment thread src/Build/Evaluation/Expander.cs
@JanProvaznik JanProvaznik merged commit ba3bd3e into dotnet:main Feb 2, 2026
10 checks passed
Copilot AI added a commit that referenced this pull request Feb 3, 2026
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: JanProvaznik <25267098+JanProvaznik@users.noreply.github.com>
This was referenced May 17, 2026
github-actions Bot pushed a commit to IntelliTect/Multitool that referenced this pull request May 19, 2026
Updated [Microsoft.Build](https://github.com/dotnet/msbuild) from 18.4.0
to 18.6.3.

<details>
<summary>Release notes</summary>

_Sourced from [Microsoft.Build's
releases](https://github.com/dotnet/msbuild/releases)._

## 18.6.3

## What's Changed
* Improve cross-platform node discovery for reuse with NodeMode
filtering by @​Copilot in dotnet/msbuild#13256
* Updated common types XSD to remove errors from redefining `Include`
attributes by @​glektarssza in
dotnet/msbuild#13284
* Update VersionPrefix to 18.6.0 + insertion flow by @​MichalPavlik in
dotnet/msbuild#13296
* Log warnings for skipped STR resource keys instead of failing the
build by @​OvesN in dotnet/msbuild#13291
* Isolate MSBuildTaskHost from the rest of MSBuild Codebase by
@​DustinCampbell in dotnet/msbuild#13232
* Improve error messages when ToolTask overrides exit code 0 to -1 due
to logged errors by @​OvesN in
dotnet/msbuild#13303
* Migrate Exec task to TaskEnvironment API by @​Copilot in
dotnet/msbuild#13171
* Enhance crash telemetry with richer diagnostics and EndBuild hang
detection by @​YuliiaKovalova in
dotnet/msbuild#13304
* [main] Update dependencies from nuget/nuget.client by
@​dotnet-maestro[bot] in dotnet/msbuild#13309
* [IBuildEngine callbacks] Stage 2: RequestCores/ReleaseCores by
@​JanProvaznik in dotnet/msbuild#13306
* Only get command line args names on modern .NET by @​baronfel in
dotnet/msbuild#13314
* Detect and correct worker node over-provisioning by @​Copilot in
dotnet/msbuild#13220
* Add App Host Support for MSBuild by @​YuliiaKovalova in
dotnet/msbuild#13175
* [main] Update dependencies from dotnet/arcade by @​dotnet-maestro[bot]
in dotnet/msbuild#13311
* Fix ObjectDisposedException in BuildsWhileBuildIsRunningOnServer test
by @​Copilot in dotnet/msbuild#13316
* Add PoC of pipelines check skill by @​JanKrivanek in
dotnet/msbuild#13242
* Fix CodeSign.MissingSigningCert for xsd/Update-MSBuildXsds.ps1 by
@​JanProvaznik in dotnet/msbuild#13320
* Fix task host launch regressions from apphost support by
@​YuliiaKovalova in dotnet/msbuild#13325
* Enlighten GetFrameworkPath and GetFrameworkSdkPath. by @​AR-May in
dotnet/msbuild#13282
* Add VMR codeflow health check to pipelines skill by @​JanProvaznik in
dotnet/msbuild#13326
* [main] Update dependencies from dotnet/roslyn by @​dotnet-maestro[bot]
in dotnet/msbuild#13281
* Fix GenerateResource to track all ResXFileRef linked files for
incremental builds by @​OvesN in
dotnet/msbuild#13327
* Add escape hatch for not sharing assemblies from tools directory by
@​AR-May in dotnet/msbuild#13305
* Add merge-dependency-updates skill for bot PR triage by @​JanProvaznik
in dotnet/msbuild#13331
* Add diagnostic data to crash/hang telemetry and move null-Project
check after RetrieveFromCache by @​YuliiaKovalova in
dotnet/msbuild#13332
* Update remote-host-object.md with SDK .tlb shipping and IDispatch
example by @​YuliiaKovalova in
dotnet/msbuild#13324
* [main] Update dependencies from dotnet/arcade by @​dotnet-maestro[bot]
in dotnet/msbuild#13341
* improve task migration skill by @​JanProvaznik in
dotnet/msbuild#13234
* Fix telemetry PII concerns: sanitize exceptions, project paths, and
custom names by @​YuliiaKovalova in
dotnet/msbuild#13344
* Use .exe.config when loading "as full Framework" by @​rainersigwald in
dotnet/msbuild#13349
* Fix RequestCores/ReleaseCores fallback in OOP TaskHost: throw
NotImplementedException instead of logging error by @​JanProvaznik in
dotnet/msbuild#13345
* Fixed indentation for
_GetCopyToOutputDirectoryItemsFromTransitiveProjectReferences by
@​CEbbinghaus in dotnet/msbuild#13358
* Fix Unix SessionId in handshake to enable cross-terminal node reuse by
@​JakeRadMSFT in dotnet/msbuild#13354
* Revert "Migrate Exec task to TaskEnvironment API" by @​JanProvaznik in
dotnet/msbuild#13367
* Look for apphost when considering node reuse by @​rainersigwald in
dotnet/msbuild#13368
* Move lots of shared code to Microsoft.Build.Framework by
@​DustinCampbell in dotnet/msbuild#13364
* [main] Source code updates from dotnet/dotnet by @​dotnet-maestro[bot]
in dotnet/msbuild#13353
* Fix ScheduleTimeRecord.AccumulatedTime hang during solution close by
@​YuliiaKovalova in dotnet/msbuild#13375
* Update runtime package references to 10.0.3 by @​Copilot in
dotnet/msbuild#13376
* [main] Source code updates from dotnet/dotnet by @​dotnet-maestro[bot]
in dotnet/msbuild#13378
* Fix ProjectImports.zip regression from shared FileUtilities statics by
@​JanProvaznik in dotnet/msbuild#13382
* Enrich EndBuild hang diagnostics with logging service and submission
state by @​YuliiaKovalova in
dotnet/msbuild#13385
* Enhance path normalization: add handling for consecutive directory
separators by @​tommcdon in dotnet/msbuild#13369
* Move task environment drivers to Framework. by @​AR-May in
dotnet/msbuild#13380
* Update MicrosoftBuildVersion in analyzer template by
@​github-actions[bot] in dotnet/msbuild#13298
* Replace ProjectCacheService null Project crash with diagnostic
telemetry by @​YuliiaKovalova in
dotnet/msbuild#13396
* Add agentic workflow to auto-close PRs older than 180 days by
@​Copilot in dotnet/msbuild#13400
* Localized file check-in by OneLocBuild Task: Build definition ID 9434:
Build ID 13575337 by @​dotnet-bot in
dotnet/msbuild#13394
* Respect MSBUILDPRESERVETOOLTEMPFILES in ProcessExit cleanup by
@​DmitriyShepelev in dotnet/msbuild#13395
 ... (truncated)

## 18.5.4

## What's Changed
* remove dead code by @​SimaTian in
dotnet/msbuild#13125
* Update VersionPrefix to 18.5.0 + insertion flow by @​YuliiaKovalova in
dotnet/msbuild#13134
* add multithreaded task migration agent skill by @​JanProvaznik in
dotnet/msbuild#13131
* Update MicrosoftBuildVersion in analyzer template by
@​github-actions[bot] in dotnet/msbuild#13139
* Migrate VerifyFileHash task to TaskEnvironment API by @​Copilot in
dotnet/msbuild#13112
* Migrate GetFileHash tasks to TaskEnvironment API by @​Copilot in
dotnet/msbuild#13111
* Diagram of VS/SDK component interactions by @​rainersigwald in
dotnet/msbuild#13127
* Fix package validation telemetry assembly resolution warnings by
@​JanProvaznik in dotnet/msbuild#13144
* Adds validation to throw MSB4259 when property references contain
leading or trailing whitespace outside of conditions. by
@​huulinhnguyen-dev in dotnet/msbuild#13076
* Localized file check-in by OneLocBuild Task: Build definition ID 9434:
Build ID 13203963 by @​dotnet-bot in
dotnet/msbuild#13151
* Add MSBuild app host design by @​YuliiaKovalova in
dotnet/msbuild#12857
* Add Stabilize-Release.ps1 script for release process by
@​rainersigwald in dotnet/msbuild#13146
* Fix chained item function empty result comparison in conditions by
@​JanProvaznik in dotnet/msbuild#12901
* [main] Update dependencies from dotnet/roslyn by @​dotnet-maestro[bot]
in dotnet/msbuild#13162
* [main] Update dependencies from dotnet/arcade by @​dotnet-maestro[bot]
in dotnet/msbuild#13160
* Localized file check-in by OneLocBuild Task: Build definition ID 9434:
Build ID 13217622 by @​dotnet-bot in
dotnet/msbuild#13163
* Fix items logged as type name during -getitem argument by @​Copilot in
dotnet/msbuild#13166
* Respect NetCoreSdkRoot property for TaskHostParameters by
@​ViktorHofer in dotnet/msbuild#13176
* Remove MachineIndependent configuration by @​Copilot in
dotnet/msbuild#13180
* Remove redundant #nullable disable from 153 files by @​Copilot in
dotnet/msbuild#13157
* Revert #​13076 "Adds validation to throw MSB4259 when property
references contain leading or trailing whitespace outside of conditions.
by @​JanProvaznik in dotnet/msbuild#13184
* Convert MSBuild.sln to slnx format and upate refs by @​ViktorHofer in
dotnet/msbuild#13185
* Implement IMultiThreadableTask for Move task by @​Copilot in
dotnet/msbuild#13108
* Add hostservices translation support for clr 4 task host by
@​YuliiaKovalova in dotnet/msbuild#13154
* Handle null ProjectFile in InvalidProjectFileException by
@​ViktorHofer in dotnet/msbuild#13179
* Add $(LatestDotNetCoreForMSBuild) infrastructure for centralized
framework targeting by @​Copilot in
dotnet/msbuild#13189
* Fix TaskHost crash when task returns string[] with null elements by
@​JanProvaznik in dotnet/msbuild#13190
* Revert "Refactor Microsoft.IO usage" by @​YuliiaKovalova in
dotnet/msbuild#13194
* Allow null SdkResult from SdkResolver.Resolve by @​rainersigwald in
dotnet/msbuild#13197
* Skill to test changes using just-built MSBuild by @​rainersigwald in
dotnet/msbuild#13202
* Tell Copilot not to allow breaking changes by @​rainersigwald in
dotnet/msbuild#13200
* Make nologo switch accept boolean values to enable explicit logo
display control by @​Copilot in
dotnet/msbuild#12541
* Migrate Unzip task to use TaskEnvironment API by @​Copilot in
dotnet/msbuild#13109
* Migrate ZipDirectory task to TaskEnvironment API by @​Copilot in
dotnet/msbuild#13110
* Localized file check-in by OneLocBuild Task: Build definition ID 9434:
Build ID 13246767 by @​dotnet-bot in
dotnet/msbuild#13204
* Add .NET Standard compatibility warnings by @​ViktorHofer in
dotnet/msbuild#13187
* Make WriteCodeFragment task locale-independent for reproducible builds
by @​Copilot in dotnet/msbuild#13192
* Localized file check-in by OneLocBuild Task: Build definition ID 9434:
Build ID 13249478 by @​dotnet-bot in
dotnet/msbuild#13207
* Fix TerminalLogger IndexOutOfRangeException when replaying binlog with
fewer nodes by @​Copilot in dotnet/msbuild#12809
* Migrate DownloadFile task to use TaskEnvironment API by @​Copilot in
dotnet/msbuild#13113
* Add CI job for 2-stage build with -mt mode by @​Copilot in
dotnet/msbuild#13124
* Localize AbsolutePath validation messages by @​JanProvaznik in
dotnet/msbuild#13115
* Refactor FrameworkFileUtilities for better performance by @​AR-May in
dotnet/msbuild#13143
* Add agent instructions for MSBuild repository by @​YuliiaKovalova in
dotnet/msbuild#13198
* Add GetCanonicalForm to the AbsolutePath API by @​AR-May in
dotnet/msbuild#13088
* Shouldly 4.3.0 by @​rainersigwald in
dotnet/msbuild#13213
* Migrate WriteCodeFragment task to use TaskEnvironment API by @​Copilot
in dotnet/msbuild#13169
* Run the issue-labeler over pull requests using polling by @​Copilot in
dotnet/msbuild#13223
* [main] Update dependencies from dotnet/arcade by @​dotnet-maestro[bot]
in dotnet/msbuild#13225
 ... (truncated)

Commits viewable in [compare
view](dotnet/msbuild@v18.4.0...v18.6.3).
</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

When chaining item functions and comparing the result to empty string, the comparison will return false incorrectly.

5 participants