Core support for AbsolutePath/FileInfo/DirectoryInfo and ITaskItem<T> as task parameters#13971
Core support for AbsolutePath/FileInfo/DirectoryInfo and ITaskItem<T> as task parameters#13971baronfel wants to merge 20 commits into
Conversation
The TASKHOST define is no longer set by any project in the main codebase since the .NET 3.5 taskhost was separated into a frozen project. These guards were always evaluating to true. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The out-of-proc task host gathers all [Output] properties and serializes them via TaskParameter. The PR's new AbsolutePath/FileInfo/DirectoryInfo outputs crashed the node: AbsolutePath (a struct) is never null so it always serialized, and Convert.ChangeType threw InvalidCastException because it is not IConvertible; FileInfo/DirectoryInfo additionally hit Assumed.Unreachable in the constructor. Route FileInfo/DirectoryInfo (scalar and array) to ValueType/ValueTypeArray, and convert values on the write side using ValueTypeParser.ToString - the same canonical conversion the in-process engine uses in TaskExecutionHost.GetValueOutputs - so in-proc and out-of-proc produce identical strings. Harden ValueTypeParser.ToString for a default AbsolutePath (null Value). The legacy net35 task host is intentionally untouched. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Unwrap TargetInvocationException in CreateTaskItemOfT so that parse errors from value-type constructors surface as the original exception (typically InvalidProjectFileException) rather than being wrapped in TargetInvocationException, which was swallowed by the outer catch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
TaskItemData throws InvalidOperationException on all write operations (SetMetadata, RemoveMetadata, CopyMetadataTo). Switch the TaskItem<T>(T) constructor to use Utilities.TaskItem as the backing item, which supports full metadata mutation as expected by ITaskItem consumers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When constructing TaskItem<T> from an ITaskItem where T is FileInfo, DirectoryInfo, or AbsolutePath, prefer the FullPath metadata (which the MSBuild item system computes as an absolute rooted path) over ItemSpec, which may be a relative path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace Value?.Equals(other.Value) with EqualityComparer<T>.Default.Equals to avoid boxing for value types, correctly handle null reference types, and use the appropriate comparer for each type T. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The code already correctly handles ITaskItem<T>[] for both input (IsValidVectorInputParameter) and output (IsAssignableToITaskItem), but had no explicit test coverage. Add tests for all four methods covering scalar and array ITaskItem<T> to prevent future regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Treat arrays with ITaskItem-implementing element types as assignable outputs so TaskItem<T>[] follows the item-output path consistently with TaskExecutionHost behavior. Add unit tests covering TaskItem<int>[] assignability and output validity. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nfo, DirectoryInfo) in PR1 Value-type T (int, bool, etc.) support for ITaskItem<T> will be added in a subsequent PR. Also remove ITaskItem<int> tests from PR1 test files. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds core infrastructure to support strongly-typed MSBuild task parameters and outputs for path-like values, including a new AbsolutePath-based flow, FileInfo/DirectoryInfo binding, and generic task items (ITaskItem<T> / TaskItem<T>). This primarily touches the task parameter binding pipeline (TaskExecutionHost), parameter type validation/serialization (TaskParameter*), and adds corresponding unit tests + docs updates.
Changes:
- Introduces
ITaskItem<T>(Framework) andTaskItem<T>(Utilities) to enable typed item identities. - Adds
ValueTypeParserand switches task output serialization to a canonical string conversion path. - Extends
TaskExecutionHostbinding + unit tests to coverAbsolutePath,FileInfo/DirectoryInfo, and typed task items.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Utilities/TaskItem_T.cs | New TaskItem<T> struct implementing typed task items over an ITaskItem backing instance. |
| src/Utilities.UnitTests/TaskItem_Tests.cs | Unit tests for TaskItem<T> metadata behavior and parsing behavior. |
| src/Tasks/Microsoft.Build.Tasks.csproj | Minor ItemGroup structure tweak. |
| src/Shared/ValueTypeParser.cs | New shared parser/formatter for value-like task parameter conversions. |
| src/Shared/UnitTests/TaskParameter_Tests.cs | Adds serialization tests for AbsolutePath / FileInfo / DirectoryInfo. |
| src/Shared/TaskParameterTypeVerifier.cs | Extends supported parameter types to include typed items and path-related types. |
| src/Shared/TaskParameter.cs | Updates value-type output serialization to use ValueTypeParser and support FileInfo/DirectoryInfo. |
| src/Framework/Microsoft.Build.Framework.csproj | Links ValueTypeParser.cs into Framework build. |
| src/Framework/ITaskItem_T.cs | New public ITaskItem<T> interface. |
| src/Build/Microsoft.Build.csproj | Adds Build → Utilities project reference (to use TaskItem<T>). |
| src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs | Adds parameter binding + output gathering support for typed path parameters and typed task items. |
| src/Build.UnitTests/BackEnd/TaskParameterTypeVerifier_Tests.cs | New tests covering typed parameter validation scenarios. |
| src/Build.UnitTests/BackEnd/TaskExecutionHost_Tests.cs | Expanded tests for new parameter types and typed task items. |
| src/Build.UnitTests/BackEnd/TaskBuilderTestTask.cs | Adds test task properties/outputs for new supported parameter types. |
| documentation/wiki/Tasks.md | Updates docs for supported task parameter types. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Move TaskExecutionHost's duplicated path-like TaskItem<T>/ITaskItem<T> helper logic into TaskParameterTypeVerifier so type checks and typed item construction are centralized and reusable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| /// <summary> | ||
| /// A default <see cref="AbsolutePath"/> (null Value) must serialize cleanly. This is the | ||
| /// scenario that crashed the out-of-proc task host: a struct-typed output is never null, so | ||
| /// it always gets serialized, and the old code threw InvalidCastException because AbsolutePath | ||
| /// is not IConvertible. See https://github.com/dotnet/msbuild/pull/13016. | ||
| /// </summary> | ||
| [Fact] | ||
| public void DefaultAbsolutePathParameter() | ||
| { | ||
| TaskParameter t = new TaskParameter(default(AbsolutePath)); | ||
|
|
||
| Assert.Equal(TaskParameterType.ValueType, t.ParameterType); | ||
|
|
||
| ((ITranslatable)t).Translate(TranslationHelpers.GetWriteTranslator()); | ||
| TaskParameter t2 = TaskParameter.FactoryForDeserialization(TranslationHelpers.GetReadTranslator()); | ||
|
|
||
| // A default AbsolutePath has a null Value and serializes to the empty string. | ||
| Assert.Equal(string.Empty, t2.WrappedParameter); | ||
| Assert.Equal(TaskParameterType.ValueType, t2.ParameterType); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void AbsolutePathParameter() | ||
| { | ||
| string path = Path.Combine(Path.GetTempPath(), "TaskParameterTests_file.txt"); | ||
| AbsolutePath value = new AbsolutePath(path); | ||
| TaskParameter t = new TaskParameter(value); | ||
|
|
||
| Assert.Equal(TaskParameterType.ValueType, t.ParameterType); | ||
|
|
||
| ((ITranslatable)t).Translate(TranslationHelpers.GetWriteTranslator()); | ||
| TaskParameter t2 = TaskParameter.FactoryForDeserialization(TranslationHelpers.GetReadTranslator()); | ||
|
|
||
| // Path types are serialized using the same canonical conversion as the in-process engine. | ||
| Assert.Equal(value.Value, t2.WrappedParameter); | ||
| Assert.Equal(TaskParameterType.ValueType, t2.ParameterType); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void AbsolutePathArrayParameter() | ||
| { | ||
| AbsolutePath[] value = new AbsolutePath[] | ||
| { | ||
| new AbsolutePath(Path.Combine(Path.GetTempPath(), "a.txt")), | ||
| new AbsolutePath(Path.Combine(Path.GetTempPath(), "b.txt")), | ||
| }; | ||
| TaskParameter t = new TaskParameter(value); | ||
|
|
||
| Assert.Equal(TaskParameterType.ValueTypeArray, t.ParameterType); | ||
|
|
||
| ((ITranslatable)t).Translate(TranslationHelpers.GetWriteTranslator()); | ||
| TaskParameter t2 = TaskParameter.FactoryForDeserialization(TranslationHelpers.GetReadTranslator()); | ||
|
|
||
| string[] stringArray = Assert.IsType<string[]>(t2.WrappedParameter); | ||
| Assert.Equal(value[0].Value, stringArray[0]); | ||
| Assert.Equal(value[1].Value, stringArray[1]); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void FileInfoParameter() | ||
| { | ||
| FileInfo value = new FileInfo(Path.Combine(Path.GetTempPath(), "TaskParameterTests_file.txt")); | ||
| TaskParameter t = new TaskParameter(value); | ||
|
|
||
| Assert.Equal(TaskParameterType.ValueType, t.ParameterType); | ||
|
|
||
| ((ITranslatable)t).Translate(TranslationHelpers.GetWriteTranslator()); | ||
| TaskParameter t2 = TaskParameter.FactoryForDeserialization(TranslationHelpers.GetReadTranslator()); | ||
|
|
||
| // FileInfo is serialized as its full path, matching ValueTypeParser/the in-process engine. | ||
| Assert.Equal(value.FullName, t2.WrappedParameter); | ||
| Assert.Equal(TaskParameterType.ValueType, t2.ParameterType); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void DirectoryInfoParameter() | ||
| { | ||
| DirectoryInfo value = new DirectoryInfo(Path.Combine(Path.GetTempPath(), "TaskParameterTests_dir")); | ||
| TaskParameter t = new TaskParameter(value); | ||
|
|
||
| Assert.Equal(TaskParameterType.ValueType, t.ParameterType); | ||
|
|
||
| ((ITranslatable)t).Translate(TranslationHelpers.GetWriteTranslator()); | ||
| TaskParameter t2 = TaskParameter.FactoryForDeserialization(TranslationHelpers.GetReadTranslator()); | ||
|
|
||
| Assert.Equal(value.FullName, t2.WrappedParameter); | ||
| Assert.Equal(TaskParameterType.ValueType, t2.ParameterType); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void FileInfoArrayParameter() | ||
| { | ||
| FileInfo[] value = new FileInfo[] | ||
| { | ||
| new FileInfo(Path.Combine(Path.GetTempPath(), "a.txt")), | ||
| new FileInfo(Path.Combine(Path.GetTempPath(), "b.txt")), | ||
| }; | ||
| TaskParameter t = new TaskParameter(value); | ||
|
|
||
| Assert.Equal(TaskParameterType.ValueTypeArray, t.ParameterType); | ||
|
|
||
| ((ITranslatable)t).Translate(TranslationHelpers.GetWriteTranslator()); | ||
| TaskParameter t2 = TaskParameter.FactoryForDeserialization(TranslationHelpers.GetReadTranslator()); | ||
|
|
||
| string[] stringArray = Assert.IsType<string[]>(t2.WrappedParameter); | ||
| Assert.Equal(value[0].FullName, stringArray[0]); | ||
| Assert.Equal(value[1].FullName, stringArray[1]); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void DirectoryInfoArrayParameter() | ||
| { | ||
| DirectoryInfo[] value = new DirectoryInfo[] | ||
| { | ||
| new DirectoryInfo(Path.Combine(Path.GetTempPath(), "dirA")), | ||
| new DirectoryInfo(Path.Combine(Path.GetTempPath(), "dirB")), | ||
| }; | ||
| TaskParameter t = new TaskParameter(value); | ||
|
|
||
| Assert.Equal(TaskParameterType.ValueTypeArray, t.ParameterType); | ||
|
|
||
| ((ITranslatable)t).Translate(TranslationHelpers.GetWriteTranslator()); | ||
| TaskParameter t2 = TaskParameter.FactoryForDeserialization(TranslationHelpers.GetReadTranslator()); | ||
|
|
||
| string[] stringArray = Assert.IsType<string[]>(t2.WrappedParameter); | ||
| Assert.Equal(value[0].FullName, stringArray[0]); | ||
| Assert.Equal(value[1].FullName, stringArray[1]); | ||
| } |
There was a problem hiding this comment.
This set of tests just confirms that the new types can be sent from TaskHosts to the main worker nodes.
| /// <summary> | ||
| /// Tests for TaskParameterTypeVerifier class | ||
| /// </summary> | ||
| public class TaskParameterTypeVerifier_Tests |
There was a problem hiding this comment.
these tests seem kind of low-quality to me but could be a safety net?
|
|
||
| #endregion | ||
|
|
||
| #region AbsolutePath Params |
There was a problem hiding this comment.
These tests validate that actually using the new types as inputs/outputs results in the Test Task correctly receiving them and processing them.
Summary
Adds support for using AbsolutePath, System.IO.FileInfo, System.IO.DirectoryInfo, and ITaskItem (for path-like T) as MSBuild task input/output parameters, in addition to the existing string and ITaskItem types.
Changes
ITaskItem<T>: A new generic interface allowing typed access to item specs, with T restricted to AbsolutePath, FileInfo, or DirectoryInfo in this PRTaskItem<T>: Implementation of ITaskItem with mutable metadata backingStacked on
/cc @baronfel