diff --git a/src/TaskAnalyzer.Tests/PreferTypedParameterAnalyzerTests.cs b/src/TaskAnalyzer.Tests/PreferTypedParameterAnalyzerTests.cs index a75853187e8..823e7993600 100644 --- a/src/TaskAnalyzer.Tests/PreferTypedParameterAnalyzerTests.cs +++ b/src/TaskAnalyzer.Tests/PreferTypedParameterAnalyzerTests.cs @@ -39,7 +39,7 @@ public override bool Execute() diags.Length.ShouldBe(1); diags[0].GetMessage().ShouldContain("InputPath"); diags[0].GetMessage().ShouldContain("AbsolutePath"); - diags[0].Severity.ShouldBe(Microsoft.CodeAnalysis.DiagnosticSeverity.Warning); + diags[0].Severity.ShouldBe(Microsoft.CodeAnalysis.DiagnosticSeverity.Info); } [Fact] @@ -66,7 +66,7 @@ public override bool Execute() diags.ShouldNotContain(d => d.Id == DiagnosticIds.PreferTypedPathParameter); diags[0].GetMessage().ShouldContain("InputPath"); diags[0].GetMessage().ShouldContain("AbsolutePath"); - diags[0].Severity.ShouldBe(Microsoft.CodeAnalysis.DiagnosticSeverity.Warning); + diags[0].Severity.ShouldBe(Microsoft.CodeAnalysis.DiagnosticSeverity.Info); } [Fact] @@ -373,7 +373,7 @@ public override bool Execute() diags.Length.ShouldBe(1); diags[0].GetMessage().ShouldContain("int"); diags[0].GetMessage().ShouldContain("Item"); - diags[0].Severity.ShouldBe(Microsoft.CodeAnalysis.DiagnosticSeverity.Warning); + diags[0].Severity.ShouldBe(Microsoft.CodeAnalysis.DiagnosticSeverity.Info); } [Fact] diff --git a/src/TaskAnalyzer.Tests/UnsupportedTaskItemTypeAnalyzerTests.cs b/src/TaskAnalyzer.Tests/UnsupportedTaskItemTypeAnalyzerTests.cs index f3b39a46004..c006d540d02 100644 --- a/src/TaskAnalyzer.Tests/UnsupportedTaskItemTypeAnalyzerTests.cs +++ b/src/TaskAnalyzer.Tests/UnsupportedTaskItemTypeAnalyzerTests.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Linq; using System.Threading.Tasks; using Microsoft.CodeAnalysis; using Shouldly; @@ -45,7 +46,7 @@ public class MyTask : Microsoft.Build.Utilities.Task [InlineData("double")] [InlineData("decimal")] [InlineData("System.DateTime")] - public async Task ConvertChangeTypeType_ProducesError(string typeName) + public async Task ConvertChangeTypeType_ProducesWarning(string typeName) { var diags = await GetUnsupportedTaskItemTypeDiagnosticsAsync($$""" using Microsoft.Build.Framework; @@ -59,7 +60,7 @@ public class MyTask : Microsoft.Build.Utilities.Task diags.ShouldNotContain(d => d.Id == DiagnosticIds.UnsupportedTaskItemType); Diagnostic diagnostic = diags.ShouldHaveSingleItem(); diagnostic.Id.ShouldBe(DiagnosticIds.CultureSensitiveTaskItemType); - diagnostic.Severity.ShouldBe(Microsoft.CodeAnalysis.DiagnosticSeverity.Error); + diagnostic.Severity.ShouldBe(Microsoft.CodeAnalysis.DiagnosticSeverity.Warning); diagnostic.GetMessage().ShouldContain("Convert.ChangeType"); diagnostic.GetMessage().ShouldContain("CultureInfo.InvariantCulture"); } @@ -116,7 +117,7 @@ public class MyTask : Microsoft.Build.Utilities.Task // ═══════════════════════════════════════════════════════════════════════ [Fact] - public async Task ConvertChangeTypeArray_ProducesError() + public async Task ConvertChangeTypeArray_ProducesWarning() { var diags = await GetUnsupportedTaskItemTypeDiagnosticsAsync(""" using Microsoft.Build.Framework; @@ -129,7 +130,7 @@ public class MyTask : Microsoft.Build.Utilities.Task Diagnostic diagnostic = diags.ShouldHaveSingleItem(); diagnostic.Id.ShouldBe(DiagnosticIds.CultureSensitiveTaskItemType); - diagnostic.Severity.ShouldBe(Microsoft.CodeAnalysis.DiagnosticSeverity.Error); + diagnostic.Severity.ShouldBe(Microsoft.CodeAnalysis.DiagnosticSeverity.Warning); } [Fact] @@ -149,7 +150,7 @@ public class MyTask : Microsoft.Build.Utilities.Task } [Fact] - public async Task ConvertChangeTypeOutputProperty_ProducesError() + public async Task ConvertChangeTypeOutputProperty_ProducesWarning() { var diags = await GetUnsupportedTaskItemTypeDiagnosticsAsync(""" using Microsoft.Build.Framework; @@ -163,7 +164,7 @@ public class MyTask : Microsoft.Build.Utilities.Task Diagnostic diagnostic = diags.ShouldHaveSingleItem(); diagnostic.Id.ShouldBe(DiagnosticIds.CultureSensitiveTaskItemType); - diagnostic.Severity.ShouldBe(Microsoft.CodeAnalysis.DiagnosticSeverity.Error); + diagnostic.Severity.ShouldBe(Microsoft.CodeAnalysis.DiagnosticSeverity.Warning); } [Fact] @@ -201,6 +202,7 @@ public class MyTask : Microsoft.Build.Utilities.Task """); diags.ShouldContain(d => d.Id == DiagnosticIds.UnsupportedTaskItemType); + diags.ShouldHaveSingleItem().Severity.ShouldBe(Microsoft.CodeAnalysis.DiagnosticSeverity.Warning); diags[0].GetMessage().ShouldContain("Item"); diags[0].GetMessage().ShouldContain("Guid"); diags[0].GetMessage().ShouldContain("string, bool, AbsolutePath, FileInfo, DirectoryInfo"); @@ -225,6 +227,41 @@ public class MyTask : Microsoft.Build.Utilities.Task diags[0].GetMessage().ShouldContain("TimeSpan"); } + [Fact] + public async Task TypedTaskItemDiagnostics_AreIndependentOfMtOptIn() + { + var diags = await GetUnsupportedTaskItemTypeDiagnosticsAsync(""" + using System; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task + { + public ITaskItem Invalid { get; set; } = null!; + public ITaskItem CultureSensitive { get; set; } = null!; + public override bool Execute() => true; + } + """); + + diags.Where(d => d.Id == DiagnosticIds.UnsupportedTaskItemType).ShouldHaveSingleItem() + .Severity.ShouldBe(Microsoft.CodeAnalysis.DiagnosticSeverity.Warning); + diags.Where(d => d.Id == DiagnosticIds.CultureSensitiveTaskItemType).ShouldHaveSingleItem() + .Severity.ShouldBe(Microsoft.CodeAnalysis.DiagnosticSeverity.Warning); + } + + [Fact] + public async Task GenericTaskItemTypeParameter_NoDiagnostic() + { + var diags = await GetUnsupportedTaskItemTypeDiagnosticsAsync(""" + using Microsoft.Build.Framework; + public class GenericTask : Microsoft.Build.Utilities.Task + { + public ITaskItem Item { get; set; } = null!; + public override bool Execute() => true; + } + """); + + diags.ShouldBeEmpty(); + } + [Fact] public async Task Enum_ProducesDiagnostic() { diff --git a/src/TaskAnalyzer/AnalyzerReleases.Unshipped.md b/src/TaskAnalyzer/AnalyzerReleases.Unshipped.md index 9a9ac6e70b5..1a3d3ba0137 100644 --- a/src/TaskAnalyzer/AnalyzerReleases.Unshipped.md +++ b/src/TaskAnalyzer/AnalyzerReleases.Unshipped.md @@ -7,11 +7,11 @@ MSBuildTask0002 | MSBuild.TaskAuthoring | Warning | APIs that should use TaskEnv MSBuildTask0003 | MSBuild.TaskAuthoring | Warning | File APIs that need absolute paths MSBuildTask0004 | MSBuild.TaskAuthoring | Warning | APIs that may cause issues in multithreaded task execution MSBuildTask0005 | MSBuild.TaskAuthoring | Warning | Transitive unsafe API usage detected in task call chain -MSBuildTask0006 | MSBuild.TaskAuthoring | Warning | Prefer typed path parameter (AbsolutePath/FileInfo/DirectoryInfo) over string (code fix available) -MSBuildTask0007 | MSBuild.TaskAuthoring | Warning | Prefer ITaskItem over manual ItemSpec parsing (code fix available) -MSBuildTask0008 | MSBuild.TaskAuthoring | Warning | Initialize a relative default path in Execute() so TaskEnvironment can root it when the property is retyped (code fix available) +MSBuildTask0006 | MSBuild.TaskAuthoring | Info | Prefer typed path parameter (AbsolutePath/FileInfo/DirectoryInfo) over string (code fix available) +MSBuildTask0007 | MSBuild.TaskAuthoring | Info | Prefer ITaskItem over manual ItemSpec parsing (code fix available) +MSBuildTask0008 | MSBuild.TaskAuthoring | Info | Initialize a relative default path in Execute() so TaskEnvironment can root it when the property is retyped (code fix available) MSBuildTask0009 | MSBuild.TaskAuthoring | Warning | ITaskItem used with a type argument T that MSBuild cannot bind as a task parameter -MSBuildTask0010 | MSBuild.TaskAuthoring | Error | ITaskItem used with a type argument T that MSBuild parses through Convert.ChangeType +MSBuildTask0010 | MSBuild.TaskAuthoring | Warning | ITaskItem used with a type argument T that MSBuild parses through Convert.ChangeType MSBuildTask0011 | MSBuild.TaskAuthoring | Info | Prefer constructor injection for TaskEnvironment MSBuildTask0012 | MSBuild.TaskAuthoring | Warning | TaskEnvironment property is never assigned by MSBuild because the task does not implement IMultiThreadableTask MSBuildTask0013 | MSBuild.TaskAuthoring | Info | Task declares IMultiThreadableTask but is not marked with [MSBuildMultiThreadableTask] (disabled by default) diff --git a/src/TaskAnalyzer/DiagnosticDescriptors.cs b/src/TaskAnalyzer/DiagnosticDescriptors.cs index b21f762241e..2698e65c45a 100644 --- a/src/TaskAnalyzer/DiagnosticDescriptors.cs +++ b/src/TaskAnalyzer/DiagnosticDescriptors.cs @@ -63,7 +63,7 @@ internal static class DiagnosticDescriptors title: "Prefer typed path parameter over manual path construction", messageFormat: "Consider changing task property '{0}' from '{1}' to '{2}' instead of converting inside the task body", category: "MSBuild.TaskAuthoring", - defaultSeverity: DiagnosticSeverity.Warning, + defaultSeverity: DiagnosticSeverity.Info, isEnabledByDefault: true, description: "MSBuild can bind AbsolutePath, FileInfo, and DirectoryInfo task parameters automatically for tasks that opt into multithreaded support. Using these types avoids manual path construction in the task body."); @@ -72,7 +72,7 @@ internal static class DiagnosticDescriptors title: "Prefer ITaskItem over manual ItemSpec parsing", messageFormat: "Consider changing task property '{0}' from '{1}' to 'ITaskItem<{2}>{3}' instead of parsing ItemSpec manually", category: "MSBuild.TaskAuthoring", - defaultSeverity: DiagnosticSeverity.Warning, + defaultSeverity: DiagnosticSeverity.Info, isEnabledByDefault: true, description: "MSBuild can bind ITaskItem task parameters that provide a strongly-typed Value property parsed from ItemSpec for tasks that opt into multithreaded support. Using ITaskItem avoids manual parsing in the task body."); @@ -81,7 +81,7 @@ internal static class DiagnosticDescriptors title: "Initialize relative default path in Execute()", messageFormat: "Task property '{0}' has a relative default path; initialize it in Execute() so it can be rooted through TaskEnvironment when the property is changed to '{1}'", category: "MSBuild.TaskAuthoring", - defaultSeverity: DiagnosticSeverity.Warning, + defaultSeverity: DiagnosticSeverity.Info, isEnabledByDefault: true, description: "A relative default path cannot be rooted in a property initializer because the MSBuild engine only assigns TaskEnvironment after the task is constructed. Move the default into Execute(), where TaskEnvironment.GetAbsolutePath can resolve it, guarding the assignment so a value bound from the project is not overwritten."); @@ -99,7 +99,7 @@ internal static class DiagnosticDescriptors title: "ITaskItem type argument relies on culture-sensitive conversion", messageFormat: "Task property '{0}' uses ITaskItem<{1}>, which MSBuild parses through Convert.ChangeType using CultureInfo.InvariantCulture. Use ITaskItem and parse explicitly with a chosen culture.", category: "MSBuild.TaskAuthoring", - defaultSeverity: DiagnosticSeverity.Error, + defaultSeverity: DiagnosticSeverity.Warning, isEnabledByDefault: true, description: "ITaskItem type arguments parsed through Convert.ChangeType use CultureInfo.InvariantCulture. Bind the item as a string and parse it explicitly with the intended culture."); diff --git a/src/TaskAnalyzer/README.md b/src/TaskAnalyzer/README.md index 4fb8589df72..9102156d562 100644 --- a/src/TaskAnalyzer/README.md +++ b/src/TaskAnalyzer/README.md @@ -19,11 +19,11 @@ This analyzer catches unsafe API usage at compile time and offers code fixes to | **MSBuildTask0003** | Warning | All `ITask` implementations | File system API requires absolute path | | **MSBuildTask0004** | Warning | All `ITask` implementations | API may cause issues in multithreaded tasks | | **MSBuildTask0005** | Warning | All `ITask` implementations | Transitive unsafe API usage in task call chain | -| **MSBuildTask0006** | Warning | Tasks with `[MSBuildMultiThreadableTask]` applied directly | Prefer typed path parameter over string | -| **MSBuildTask0007** | Warning | Tasks with `[MSBuildMultiThreadableTask]` applied directly | Prefer `ITaskItem` over manual ItemSpec parsing | -| **MSBuildTask0008** | Warning | Tasks with `[MSBuildMultiThreadableTask]` applied directly | Initialize a relative-default path property in `Execute()` | +| **MSBuildTask0006** | Info | Tasks with `[MSBuildMultiThreadableTask]` applied directly | Prefer typed path parameter over string | +| **MSBuildTask0007** | Info | Tasks with `[MSBuildMultiThreadableTask]` applied directly | Prefer `ITaskItem` over manual ItemSpec parsing | +| **MSBuildTask0008** | Info | Tasks with `[MSBuildMultiThreadableTask]` applied directly | Initialize a relative-default path property in `Execute()` | | **MSBuildTask0009** | Warning | All `ITask` implementations | `ITaskItem` used with unsupported type argument | -| **MSBuildTask0010** | Error | All `ITask` implementations | `ITaskItem` relies on culture-sensitive conversion | +| **MSBuildTask0010** | Warning | All `ITask` implementations | `ITaskItem` relies on culture-sensitive conversion | | **MSBuildTask0011** | Info | Concrete `IMultiThreadableTask` implementations | Prefer constructor injection for `TaskEnvironment` | | **MSBuildTask0012** | Warning | Concrete tasks with `[MSBuildMultiThreadableTask]` applied directly | MSBuild never assigns the `TaskEnvironment` property | | **MSBuildTask0013** | Info (off by default) | Concrete tasks declaring `IMultiThreadableTask` in their own base list | Missing `[MSBuildMultiThreadableTask]`, so the task still runs out-of-proc | @@ -245,7 +245,7 @@ public class MyTask : Task { public ITaskItem Id { get; set; } // warning public ITaskItem[] Durations { get; set; } // warning - public ITaskItem Count { get; set; } // MSBuildTask0010 error + public ITaskItem Count { get; set; } // MSBuildTask0010 warning } ``` @@ -255,13 +255,13 @@ No code fix is offered for MSBuildTask0009 — the resolution depends on the int ### MSBuildTask0010 — Culture-Sensitive `ITaskItem` Conversion -MSBuild binds `ITaskItem` for `char`, numeric primitives, `decimal`, and `DateTime` through `Convert.ChangeType` using `CultureInfo.InvariantCulture`. Because this implicit conversion may not match the task's intended culture, the analyzer reports an **Error** whenever one of these types is used. +MSBuild binds `ITaskItem` for `char`, numeric primitives, `decimal`, and `DateTime` through `Convert.ChangeType` using `CultureInfo.InvariantCulture`. Because this implicit conversion may not match the task's intended culture, the analyzer reports a **Warning** whenever one of these types is used. ```csharp public class MyTask : Task { - public ITaskItem Count { get; set; } // error - public ITaskItem[] Dates { get; set; } // error + public ITaskItem Count { get; set; } // warning + public ITaskItem[] Dates { get; set; } // warning } ``` @@ -414,12 +414,8 @@ The `[MSBuildMultiThreadableTaskAnalyzed]` attribute allows opting helper classe ### Severity Levels - **MSBuildTask0001** is always **Error** — these APIs are never safe in any MSBuild task. -- **MSBuildTask0010** is always **Error** — task item conversions must not rely on `Convert.ChangeType`. -- **MSBuildTask0002–MSBuildTask0009** report as **Warning**, with MSBuildTask0006–MSBuildTask0008 limited to tasks directly marked with `[MSBuildMultiThreadableTask]`. -- **MSBuildTask0011** reports as **Info** — it is a modernization suggestion rather than a correctness issue. -- **MSBuildTask0012** reports as **Warning** — the `TaskEnvironment` property is silently inert, which is a correctness issue. -- **MSBuildTask0013** is **disabled by default** — running out-of-proc is a performance characteristic, and the shape it reports is a valid intermediate migration state. -- **MSBuildTask0014** reports as **Warning** — the attribute is inert, and the task the author meant to mark is usually still running out-of-proc. +- **MSBuildTask0002–MSBuildTask0005, MSBuildTask0009, and MSBuildTask0010** report as **Warning**. +- **MSBuildTask0006–MSBuildTask0008 and MSBuildTask0011** report as **Info** — these are modernization suggestions, not correctness issues. ## Code Fixes diff --git a/src/TaskAnalyzer/UnsupportedTaskItemTypeAnalyzer.cs b/src/TaskAnalyzer/UnsupportedTaskItemTypeAnalyzer.cs index b5bde275f21..f215016290f 100644 --- a/src/TaskAnalyzer/UnsupportedTaskItemTypeAnalyzer.cs +++ b/src/TaskAnalyzer/UnsupportedTaskItemTypeAnalyzer.cs @@ -95,6 +95,13 @@ property.ContainingType is not null && ITypeSymbol typeArg = namedPropertyType.TypeArguments[0]; + // A generic task can be constructed with a supported type. Its open type + // parameter does not provide enough information for a binding diagnostic. + if (typeArg.TypeKind == TypeKind.TypeParameter) + { + continue; + } + if (SupportedTaskItemTypes.IsConvertChangeTypeTaskItemType(typeArg.SpecialType)) { symbolContext.ReportDiagnostic(Diagnostic.Create(