Skip to content

Align ITaskItem<T> binding with ValueTypeParser and flag culture-sensitive conversions - #13974

Merged
baronfel merged 4 commits into
mainfrom
typed-params/value-type-support
Jul 16, 2026
Merged

Align ITaskItem<T> binding with ValueTypeParser and flag culture-sensitive conversions#13974
baronfel merged 4 commits into
mainfrom
typed-params/value-type-support

Conversation

@baronfel

@baronfel baronfel commented Jun 5, 2026

Copy link
Copy Markdown
Member

Summary

Expands ITaskItem<T> / TaskItem<T> engine binding beyond path types while keeping the accepted type set aligned with ValueTypeParser.

The binder now accepts:

  • Directly parsed types: string, bool, AbsolutePath, FileInfo, DirectoryInfo
  • Convert.ChangeType types: char, numeric primitives, decimal, and DateTime

Enums, nullable value types, Guid, TimeSpan, and custom structs remain unsupported.

Changes

  • Replaces broad IsValueType checks with an explicit ValueTypeParser-aligned allowlist.
  • Applies the same validation to scalar and array inputs and outputs for both ITaskItem<T> and concrete TaskItem<T>.
  • Adds binding coverage for string, MSBuild boolean syntax, and the full supported/unsupported type matrix.
  • Updates MSBuildTask0009 to match runtime support and recommend only directly parsed types.
  • Adds MSBuildTask0010 as an error when ITaskItem<T> uses a type parsed through Convert.ChangeType. These conversions use CultureInfo.InvariantCulture, which may not match the task's intended culture. Authors should bind as ITaskItem<string> and parse explicitly with the intended culture.

Dependency status

The preceding PRs (#13971, #13972, and #13973) are merged, and this PR is rebased directly on main.

/cc @baronfel

@baronfel baronfel changed the title Expand ITaskItem<T> binding to support any value type T Expand ITaskItem<T> binding to support arbitrary value types (int, bool, etc.) Jun 5, 2026
@baronfel
baronfel force-pushed the typed-params/unsupported-type-analyzer branch from 066020c to a06f9e6 Compare June 5, 2026 21:12
@baronfel
baronfel force-pushed the typed-params/value-type-support branch from 676b7c9 to 1460831 Compare June 5, 2026 21:13
@baronfel
baronfel force-pushed the typed-params/unsupported-type-analyzer branch from a06f9e6 to 7531acb Compare June 5, 2026 21:22
@baronfel
baronfel force-pushed the typed-params/value-type-support branch from 1460831 to 2e0738d Compare June 5, 2026 21:27
@baronfel
baronfel requested a review from a team June 8, 2026 15:00
@baronfel

baronfel commented Jun 11, 2026

Copy link
Copy Markdown
Member Author

Note: we don't want this to be any value type - consult w/ @rainersigwald on the allowed subset (likely bool, char (??), enum, etc) and consider loosening at a later date.

@baronfel
baronfel force-pushed the typed-params/unsupported-type-analyzer branch from 7531acb to 92f144c Compare July 1, 2026 17:32
@baronfel
baronfel force-pushed the typed-params/value-type-support branch from 2e0738d to dffb8c8 Compare July 1, 2026 17:32
Comment thread src/Framework/TaskItemTypeDetector.cs Outdated
Comment thread src/Framework/TaskItemTypeDetector.cs Outdated
@baronfel
baronfel force-pushed the typed-params/unsupported-type-analyzer branch from 92f144c to 0cf9297 Compare July 13, 2026 22:06
baronfel added a commit that referenced this pull request Jul 14, 2026
…em<T> type arguments (#13973)

## Summary

Adds **MSBuildTask0009 (UnsupportedTaskItemType)**: a Roslyn analyzer
that fires when a task property uses `ITaskItem<T>` where `T` is not
currently supported by MSBuild's task parameter binder. This catches
generic type arguments that would fail at runtime.

The analyzer currently recognizes `AbsolutePath`, `FileInfo`, and
`DirectoryInfo`. Primitive and other value types will be added when the
engine-side binding support lands.

## Stacked on
- Requires: #13972 (MSBuildTask0006/0007 migration analyzers)
- Next: #13974 (value type support)

/cc @baronfel

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Base automatically changed from typed-params/unsupported-type-analyzer to main July 14, 2026 18:31
Extends the engine to accept ITaskItem<T> where T is any value type
(int, bool, long, etc.) in addition to path-like types. Uses
ValueTypeParser for parsing and formatting. Also adds corresponding
unit tests for ITaskItem<int> binding.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@baronfel
baronfel force-pushed the typed-params/value-type-support branch from dffb8c8 to 9e37fa5 Compare July 14, 2026 18:45
Limit typed task item binding to ValueTypeParser-supported types and keep runtime and analyzer validation consistent. Add MSBuildTask0010 as an error for types parsed through Convert.ChangeType so task authors choose culture-explicit parsing instead.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: f684d8fd-b7da-4512-81ad-ca364f87414b
@baronfel baronfel changed the title Expand ITaskItem<T> binding to support arbitrary value types (int, bool, etc.) Align ITaskItem<T> binding with ValueTypeParser and flag culture-sensitive conversions Jul 14, 2026
@baronfel
baronfel requested a review from OvesN July 14, 2026 22:00
@baronfel baronfel added User Experience Area: Engine Issues impacting the core execution of targets and tasks. Area: Analyzer labels Jul 14, 2026
@baronfel
baronfel marked this pull request as ready for review July 15, 2026 14:22
Copilot AI review requested due to automatic review settings July 15, 2026 14:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 expands MSBuild’s engine-side binding support for ITaskItem<T> / TaskItem<T> to a broader, ValueTypeParser-aligned set of T, and updates the task-authoring analyzers to (1) reflect the new supported set and (2) flag Convert.ChangeType-based bindings as an error (MSBuildTask0010) to steer authors toward explicit parsing.

Changes:

  • Align runtime type support for ITaskItem<T> / TaskItem<T> with ValueTypeParser via explicit allowlists and consistent validation across scalar/array inputs & outputs.
  • Extend the Roslyn analyzer to recognize the expanded support matrix (MSBuildTask0009) and add MSBuildTask0010 for Convert.ChangeType-based T.
  • Add/expand unit tests for analyzer diagnostics and engine binding (including scalar/array input and output coverage).

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/TaskAnalyzer/UnsupportedTaskItemTypeAnalyzer.cs Adds MSBuildTask0010 reporting and updates supported diagnostics for ITaskItem<T> type arguments.
src/TaskAnalyzer/SupportedTaskItemTypes.cs Centralizes analyzer-side allowlists (directly-parsed vs Convert.ChangeType special types).
src/TaskAnalyzer/README.md Documents the new diagnostic (MSBuildTask0010) and updates supported type guidance.
src/TaskAnalyzer/DiagnosticIds.cs Adds the MSBuildTask0010 diagnostic ID constant.
src/TaskAnalyzer/DiagnosticDescriptors.cs Introduces the MSBuildTask0010 descriptor and refines MSBuildTask0009 messaging.
src/TaskAnalyzer/AnalyzerReleases.Unshipped.md Records MSBuildTask0010 as a new, unshipped analyzer rule.
src/TaskAnalyzer.Tests/UnsupportedTaskItemTypeAnalyzerTests.cs Adds test coverage for MSBuildTask0010 and updates MSBuildTask0009 expectations.
src/Shared/TaskParameterTypeVerifier.cs Updates verifier helpers to use supported-type detection for ITaskItem<T> / TaskItem<T>.
src/Framework/TaskItemTypeDetector.cs Implements the runtime supported-type allowlist and generic TaskItem<T> detection helpers.
src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs Expands ITaskItem<T> / TaskItem<T> parameter handling beyond path-like types.
src/Build.UnitTests/BackEnd/TaskParameterTypeVerifier_Tests.cs Adds matrix tests for supported/unsupported T for both ITaskItem<T> and TaskItem<T>.
src/Build.UnitTests/BackEnd/TaskExecutionHost_Tests.cs Adds binding coverage for TaskItem<T> scalar/array inputs and outputs.
src/Build.UnitTests/BackEnd/TaskBuilderTestTask.cs Extends the test task with new ITaskItem<int> input/output properties used by host tests.

Comment thread src/Build.UnitTests/BackEnd/TaskExecutionHost_Tests.cs
Comment thread src/TaskAnalyzer/DiagnosticDescriptors.cs Outdated
Comment thread src/TaskAnalyzer/README.md
State that Convert.ChangeType binding uses CultureInfo.InvariantCulture and clarify that the string binding test verifies the item value rather than CLR reference identity.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: f684d8fd-b7da-4512-81ad-ca364f87414b

@OvesN OvesN left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, just one comment

Comment thread src/Shared/TaskParameterTypeVerifier.cs Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: f684d8fd-b7da-4512-81ad-ca364f87414b
@baronfel
baronfel enabled auto-merge (squash) July 16, 2026 17:15
@baronfel
baronfel merged commit 566cbd8 into main Jul 16, 2026
10 checks passed
@baronfel
baronfel deleted the typed-params/value-type-support branch July 16, 2026 19:53
baronfel pushed a commit that referenced this pull request Jul 20, 2026
## Summary

- add `MSBuildTask0011` at Info severity for concrete
`IMultiThreadableTask` implementations that do not expose a public
single-`TaskEnvironment` constructor
- cover valid, invalid, abstract, and inherited-constructor cases
- document the recommended constructor-injection pattern in the analyzer
README and thread-safe task specification

Replaces #14400, which GitHub automatically closed when its stacked base
branch was deleted after #14315 merged. The branch is now rebased
directly onto `main`. The rule moved from the originally proposed ID
0010 to 0011 because `main` assigned `MSBuildTask0010` in #13974.

## Compatibility

The task-authoring analyzer is currently non-shipping/opt-in, and this
diagnostic defaults to Info severity, so it does not break builds using
warnings-as-errors. No ChangeWave is needed.

## Performance

Measured with the BenchmarkDotNet harness from #14387 in a disposable
integration checkout, using the default job on an Apple M4 Max with .NET
10.0.8 / SDK 10.0.300. `MSBuildTask0009` is the closest existing
symbol-action analyzer.

| Diagnostics | MSBuildTask0009 | MSBuildTask0011 | 0009 allocated |
0011 allocated |
|---:|---:|---:|---:|---:|
| 1 | 358.4 us | 357.9 us | 352.02 KB | 352.26 KB |
| 10 | 563.5 us | 588.8 us | 693.73 KB | 683.04 KB |
| 100 | 2.731 ms | 2.835 ms | 3,570.28 KB | 3,584.23 KB |

The initial no-reference path measured 48.08 us / 32.41 KB.
Short-circuiting after the mandatory `IMultiThreadableTask` lookup
reduced that to 35.52 us / 30.56 KB, close to `MSBuildTask0009` at 31.46
us / 30.56 KB. Diagnostic time, allocations, and 1/10/100 scaling are
comparable, so no further complexity is justified.

## Validation

- `./build.sh -v quiet`
- `dotnet test --project
src/TaskAnalyzer.Tests/TaskAnalyzer.Tests.csproj -c Release`
- #14387 BenchmarkDotNet no-op and exact-count 1/10/100 diagnostic
scenarios

Fixes #14378
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Analyzer Area: Engine Issues impacting the core execution of targets and tasks. User Experience

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants