-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add a new buildcheck that encourages MSBuild logic authors to dispose of unused Private Items from their Targets #12887
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
… of unused Items in their Targets.
There was a problem hiding this 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 introduces BC0303 - ItemDisposalCheck, a new BuildCheck analyzer that detects memory waste from undisposed private item lists in MSBuild targets. Private items (prefixed with _) are conventionally used for internal calculations within targets but persist in memory for the entire build unless explicitly removed, which can waste memory in large builds. This analyzer encourages build authors to clean up these temporary items using Remove operations.
Key changes:
- Implements static analysis of target XML to detect private items without matching Remove operations
- Uses MSBuild's ExpressionShredder API for robust parsing of Outputs/Returns attributes
- Includes comprehensive test suite with 31 test cases covering positive, negative, and edge cases
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/Build/BuildCheck/Checks/ItemDisposalCheck.cs |
Core implementation of BC0303 analyzer with target analysis logic and ExpressionShredder-based parsing |
src/BuildCheck.UnitTests/ItemDisposalCheck_Tests.cs |
Comprehensive test suite with 31 tests covering all scenarios |
src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs |
Registration of new ItemDisposalCheck in built-in checks array |
src/Build/Resources/Strings.resx |
Added BC0303 title and message format resource strings |
src/Build/Resources/xlf/*.xlf |
Localization entries for 12 languages (marked as state="new") |
documentation/specs/BuildCheck/ItemDisposalCheck.md |
Detailed specification including motivation, detection logic, and examples |
documentation/specs/BuildCheck/Codes.md |
Documentation entry for BC0303 with code examples and resolution guidance |
.claude/agents/msbuild-buildcheck-creator.md |
Agent documentation for creating BuildCheck analyzers (meta-documentation) |
|
As long as this results in a compile error to force them to dispose of such wasted memory when finished with it. That way they are forced to fix their code with no way to disable this error. |
|
@AraHaan it definitely will not be an error in the initial rollout - but like all MSBuild diagnostics it will be able to be treated as an error via |
Add BC0303 BuildCheck: Detect undisposed private item lists in targets
Summary
Adds a new BuildCheck analyzer (BC0303) that detects memory waste from undisposed private item lists in MSBuild targets.
Problem
MSBuild targets often create temporary "private" item lists (conventionally prefixed with
_) for internal calculations. These items persist in memory for the duration of the build unless explicitly removed, which can waste memory in large builds.Example of problematic code:
Solution
This PR introduces BC0303 - ItemDisposalCheck, a new built-in BuildCheck analyzer that:
_) created within targets viaIncludeRemoveoperationOutputsorReturnsattributes (part of the target's public contract)Technical Details
Implementation
src/Build/BuildCheck/Checks/ItemDisposalCheck.csExpressionShredder.GetReferencedItemNamesAndMetadata()for robust parsing of Outputs/Returns expressionsWhy ExpressionShredder instead of Regex?
Initially considered regex-based parsing, but switched to MSBuild's built-in
ExpressionShredderto properly handle:@(_Items->'%(Identity);%(FullPath)')@(_Sources);@(_Resources);@(_Content)@(_Files, ';')@(_Duplicates->Distinct())Documentation
documentation/specs/BuildCheck/Codes.mddocumentation/specs/BuildCheck/ItemDisposalCheck.mdTesting
Added 31 comprehensive test cases in
src/BuildCheck.UnitTests/ItemDisposalCheck_Tests.cs:Positive tests (should fire):
Negative tests (should NOT fire):
_prefix)OutputsorReturnsEdge cases:
All 31 tests pass ✅
Related Work
This analyzer was created using the msbuild-buildcheck-creator agent, following established patterns in the BuildCheck infrastructure.