From 51074bb6945e0db3a95e9080c515b99793acb75c Mon Sep 17 00:00:00 2001 From: VolPlita Date: Mon, 24 Aug 2026 12:31:14 +0200 Subject: [PATCH 1/2] Finalize TaskAnalyzer scope and severities Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../MultiThreadableTaskAnalyzerTests.cs | 222 ++++++++++++++++++ src/TaskAnalyzer.Tests/TestHelpers.cs | 93 ++++++-- .../TransitiveCallChainAnalyzerTests.cs | 68 ++++++ .../UnsupportedTaskItemTypeAnalyzerTests.cs | 50 +++- .../AnalyzerReleases.Unshipped.md | 10 +- src/TaskAnalyzer/DiagnosticDescriptors.cs | 4 +- .../MultiThreadableTaskAnalyzer.cs | 28 +-- src/TaskAnalyzer/README.md | 58 +++-- src/TaskAnalyzer/SharedAnalyzerHelpers.cs | 10 +- .../TransitiveCallChainAnalyzer.cs | 2 +- .../UnsupportedTaskItemTypeAnalyzer.cs | 7 + 11 files changed, 469 insertions(+), 83 deletions(-) diff --git a/src/TaskAnalyzer.Tests/MultiThreadableTaskAnalyzerTests.cs b/src/TaskAnalyzer.Tests/MultiThreadableTaskAnalyzerTests.cs index cc204fdcddb..3b2c543af06 100644 --- a/src/TaskAnalyzer.Tests/MultiThreadableTaskAnalyzerTests.cs +++ b/src/TaskAnalyzer.Tests/MultiThreadableTaskAnalyzerTests.cs @@ -3,6 +3,9 @@ using System.Linq; using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Testing; +using Microsoft.CodeAnalysis.Testing; using Shouldly; using Xunit; using static Microsoft.Build.TaskAuthoring.Analyzer.Tests.TestHelpers; @@ -1870,6 +1873,225 @@ public override bool Execute() // Scope option tests // ═══════════════════════════════════════════════════════════════════════ + [Fact] + public async Task Scope_Default_PlainTask_DoesNotGetScopedDiagnostics() + { + var diags = await GetDiagnosticsWithDefaultScopeAsync(""" + using System; + using System.IO; + using System.Reflection; + public class PlainTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + Console.WriteLine("test"); + var value = Environment.GetEnvironmentVariable("KEY"); + Assembly.Load("Test"); + return File.Exists("relative.txt"); + } + } + """); + + diags.Where(d => d.Id == DiagnosticIds.CriticalError).ShouldBeEmpty(); + diags.Where(d => d.Id == DiagnosticIds.TaskEnvironmentRequired).ShouldBeEmpty(); + diags.Where(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute).ShouldBeEmpty(); + diags.Where(d => d.Id == DiagnosticIds.PotentialIssue).ShouldBeEmpty(); + } + + [Fact] + public async Task Scope_Default_MultiThreadableTask_GetsAllScopedDiagnostics() + { + var diags = await GetDiagnosticsWithDefaultScopeAsync(""" + using System; + using System.IO; + using System.Reflection; + using Microsoft.Build.Framework; + public class MtTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + Console.WriteLine("test"); + var value = Environment.GetEnvironmentVariable("KEY"); + Assembly.Load("Test"); + return File.Exists("relative.txt"); + } + } + """); + + diags.Where(d => d.Id == DiagnosticIds.CriticalError).ShouldHaveSingleItem() + .Severity.ShouldBe(DiagnosticSeverity.Error); + diags.Where(d => d.Id == DiagnosticIds.TaskEnvironmentRequired).ShouldHaveSingleItem() + .Severity.ShouldBe(DiagnosticSeverity.Warning); + diags.Where(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute).ShouldHaveSingleItem() + .Severity.ShouldBe(DiagnosticSeverity.Warning); + diags.Where(d => d.Id == DiagnosticIds.PotentialIssue).ShouldHaveSingleItem() + .Severity.ShouldBe(DiagnosticSeverity.Warning); + } + + [Fact] + public async Task Scope_All_PlainTask_GetsAllScopedDiagnostics() + { + var diags = await GetDiagnosticsWithScopeAsync(""" + using System; + using System.IO; + using System.Reflection; + public class PlainTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + Console.WriteLine("test"); + var value = Environment.GetEnvironmentVariable("KEY"); + Assembly.Load("Test"); + return File.Exists("relative.txt"); + } + } + """, SharedAnalyzerHelpers.ScopeAll); + + diags.Where(d => d.Id == DiagnosticIds.CriticalError).ShouldHaveSingleItem() + .Severity.ShouldBe(DiagnosticSeverity.Error); + diags.Where(d => d.Id == DiagnosticIds.TaskEnvironmentRequired).ShouldHaveSingleItem() + .Severity.ShouldBe(DiagnosticSeverity.Warning); + diags.Where(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute).ShouldHaveSingleItem() + .Severity.ShouldBe(DiagnosticSeverity.Warning); + diags.Where(d => d.Id == DiagnosticIds.PotentialIssue).ShouldHaveSingleItem() + .Severity.ShouldBe(DiagnosticSeverity.Warning); + } + + [Fact] + public async Task Scope_GlobalConfig_All_AnalyzesPlainTask() + { + var test = new CSharpAnalyzerTest + { + TestCode = """ + using System; + public class PlainTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + var value = {|#0:Environment.GetEnvironmentVariable("KEY")|}; + return true; + } + } + """, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + }; + test.TestState.Sources.Add(("Stubs.cs", FrameworkStubs)); + test.TestState.AnalyzerConfigFiles.Add(("/.globalconfig", """ + is_global = true + msbuild_task_analyzer.scope = all + """)); + test.ExpectedDiagnostics.Add( + new DiagnosticResult(DiagnosticIds.TaskEnvironmentRequired, DiagnosticSeverity.Warning).WithLocation(0)); + + await test.RunAsync(); + } + + [Fact] + public async Task Scope_UnrecognizedValue_UsesDefault() + { + var diags = await GetDiagnosticsWithScopeAsync(""" + using System; + using System.Reflection; + public class PlainTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + Console.WriteLine("test"); + var value = Environment.GetEnvironmentVariable("KEY"); + Assembly.Load("Test"); + return true; + } + } + """, "unrecognized"); + + diags.Where(d => d.Id == DiagnosticIds.CriticalError).ShouldBeEmpty(); + diags.Where(d => d.Id == DiagnosticIds.TaskEnvironmentRequired).ShouldBeEmpty(); + diags.Where(d => d.Id == DiagnosticIds.PotentialIssue).ShouldBeEmpty(); + } + + [Fact] + public async Task Scope_Default_PlainTask_TreatWarningsAsErrors_DoesNotGetScopedDiagnostics() + { + var diags = await GetDiagnosticsWithDefaultScopeAsync(""" + using System; + using System.Reflection; + public class PlainTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + Console.WriteLine("test"); + Assembly.Load("Test"); + return true; + } + } + """, ReportDiagnostic.Error); + + diags.Where(d => d.Id == DiagnosticIds.CriticalError).ShouldBeEmpty(); + diags.Where(d => d.Id == DiagnosticIds.PotentialIssue).ShouldBeEmpty(); + } + + [Fact] + public async Task Scope_Default_MultiThreadableTask_TreatWarningsAsErrors_PromotesWarning() + { + var diags = await GetDiagnosticsWithDefaultScopeAsync(""" + using System.Reflection; + using Microsoft.Build.Framework; + public class MtTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + Assembly.Load("Test"); + return true; + } + } + """, ReportDiagnostic.Error); + + Diagnostic diagnostic = diags.Where(d => d.Id == DiagnosticIds.PotentialIssue).ShouldHaveSingleItem(); + diagnostic.Severity.ShouldBe(DiagnosticSeverity.Error); + diagnostic.IsWarningAsError.ShouldBeTrue(); + } + + [Fact] + public async Task Scope_Default_MultiThreadableAttribute_OptsTaskIn() + { + var diags = await GetDiagnosticsWithDefaultScopeAsync(""" + using System; + using Microsoft.Build.Framework; + [MSBuildMultiThreadableTask] + public class MtTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + var value = Environment.GetEnvironmentVariable("KEY"); + return true; + } + } + """); + + diags.Where(d => d.Id == DiagnosticIds.TaskEnvironmentRequired).ShouldHaveSingleItem(); + } + + [Fact] + public async Task Scope_Default_AnalyzedAttribute_OptsHelperIn() + { + var diags = await GetDiagnosticsWithDefaultScopeAsync(""" + using System; + using Microsoft.Build.Framework; + [MSBuildMultiThreadableTaskAnalyzed] + public class MtHelper + { + public void Execute() + { + var value = Environment.GetEnvironmentVariable("KEY"); + } + } + """); + + diags.Where(d => d.Id == DiagnosticIds.TaskEnvironmentRequired).ShouldHaveSingleItem(); + } + [Fact] public async Task Scope_MultithreadableOnly_PlainTask_NoDiagnostic() { diff --git a/src/TaskAnalyzer.Tests/TestHelpers.cs b/src/TaskAnalyzer.Tests/TestHelpers.cs index 73bbbb2dbf2..a5997c06d1f 100644 --- a/src/TaskAnalyzer.Tests/TestHelpers.cs +++ b/src/TaskAnalyzer.Tests/TestHelpers.cs @@ -138,34 +138,16 @@ public static string FullyQualifiedPath(string tail) => public static MetadataReference[] GetCoreReferences() => s_coreReferences; /// - /// Runs the MultiThreadableTaskAnalyzer on the given source code and returns analyzer diagnostics. - /// Source is combined with framework stubs automatically. + /// Runs the MultiThreadableTaskAnalyzer in explicit all-task migration mode. /// - public static async System.Threading.Tasks.Task> GetDiagnosticsAsync(string source) - { - var compilation = CreateCompilation(source); - var analyzer = new MultiThreadableTaskAnalyzer(); - var compilationWithAnalyzers = compilation.WithAnalyzers( - ImmutableArray.Create(analyzer)); - - var allDiags = await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync(); - return allDiags; - } + public static System.Threading.Tasks.Task> GetDiagnosticsAsync(string source) => + GetDiagnosticsWithScopeAsync(source, SharedAnalyzerHelpers.ScopeAll); /// - /// Runs BOTH the direct and transitive analyzers on the given source code. + /// Runs both the direct and transitive analyzers in explicit all-task migration mode. /// - public static async System.Threading.Tasks.Task> GetAllDiagnosticsAsync(string source) - { - var compilation = CreateCompilation(source); - var analyzers = ImmutableArray.Create( - new MultiThreadableTaskAnalyzer(), - new TransitiveCallChainAnalyzer()); - var compilationWithAnalyzers = compilation.WithAnalyzers(analyzers); - - var allDiags = await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync(); - return allDiags; - } + public static System.Threading.Tasks.Task> GetAllDiagnosticsAsync(string source) => + GetAllDiagnosticsWithScopeAsync(source, SharedAnalyzerHelpers.ScopeAll); /// /// Runs compiler diagnostics together with analyzers and suppressors and returns @@ -260,6 +242,69 @@ public static async System.Threading.Tasks.Task> GetD return await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync(); } + /// + /// Runs the MultiThreadableTaskAnalyzer without a scope option. + /// + public static async System.Threading.Tasks.Task> GetDiagnosticsWithDefaultScopeAsync(string source) + { + var compilation = CreateCompilation(source); + var analyzer = new MultiThreadableTaskAnalyzer(); + var compilationWithAnalyzers = compilation.WithAnalyzers( + ImmutableArray.Create(analyzer)); + return await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync(); + } + + /// + /// Runs the MultiThreadableTaskAnalyzer without a scope option and applies a general diagnostic action. + /// + public static async System.Threading.Tasks.Task> GetDiagnosticsWithDefaultScopeAsync( + string source, + ReportDiagnostic generalDiagnosticOption) + { + var compilation = CreateCompilation(source); + compilation = compilation.WithOptions( + ((CSharpCompilationOptions)compilation.Options).WithGeneralDiagnosticOption(generalDiagnosticOption)); + + var analyzer = new MultiThreadableTaskAnalyzer(); + var compilationWithAnalyzers = compilation.WithAnalyzers( + ImmutableArray.Create(analyzer)); + return await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync(); + } + + /// + /// Runs both the direct and transitive analyzers with a specific scope option. + /// + public static async System.Threading.Tasks.Task> GetAllDiagnosticsWithScopeAsync(string source, string scope) + { + var compilation = CreateCompilation(source); + var analyzers = ImmutableArray.Create( + new MultiThreadableTaskAnalyzer(), + new TransitiveCallChainAnalyzer()); + + var globalOptions = new Dictionary + { + { $"build_property.{SharedAnalyzerHelpers.ScopeOptionKey}", scope } + }; + var optionsProvider = new TestAnalyzerConfigOptionsProvider(globalOptions); + var options = new AnalyzerOptions(ImmutableArray.Empty, optionsProvider); + + var compilationWithAnalyzers = compilation.WithAnalyzers(analyzers, options); + return await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync(); + } + + /// + /// Runs both the direct and transitive analyzers without a scope option. + /// + public static async System.Threading.Tasks.Task> GetAllDiagnosticsWithDefaultScopeAsync(string source) + { + var compilation = CreateCompilation(source); + var analyzers = ImmutableArray.Create( + new MultiThreadableTaskAnalyzer(), + new TransitiveCallChainAnalyzer()); + var compilationWithAnalyzers = compilation.WithAnalyzers(analyzers); + return await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync(); + } + private static MetadataReference[] CreateCoreReferences() { // Reference the core runtime assemblies needed diff --git a/src/TaskAnalyzer.Tests/TransitiveCallChainAnalyzerTests.cs b/src/TaskAnalyzer.Tests/TransitiveCallChainAnalyzerTests.cs index 44e2e5a73bb..23ad3b18b14 100644 --- a/src/TaskAnalyzer.Tests/TransitiveCallChainAnalyzerTests.cs +++ b/src/TaskAnalyzer.Tests/TransitiveCallChainAnalyzerTests.cs @@ -244,4 +244,72 @@ public override bool Execute() msg.ShouldContain("A.Step1"); msg.ShouldContain("B.Step2"); } + + [Fact] + public async Task Scope_Default_PlainTask_DoesNotGetTransitiveDiagnostic() + { + var diags = await GetAllDiagnosticsWithDefaultScopeAsync(""" + using System; + public static class Helper + { + public static void Run() => Environment.GetEnvironmentVariable("KEY"); + } + public class PlainTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + Helper.Run(); + return true; + } + } + """); + + diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ShouldBeEmpty(); + } + + [Fact] + public async Task Scope_Default_MultiThreadableTask_GetsTransitiveDiagnostic() + { + var diags = await GetAllDiagnosticsWithDefaultScopeAsync(""" + using System; + using Microsoft.Build.Framework; + public static class Helper + { + public static void Run() => Environment.GetEnvironmentVariable("KEY"); + } + public class MtTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + Helper.Run(); + return true; + } + } + """); + + diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ShouldHaveSingleItem(); + } + + [Fact] + public async Task Scope_All_PlainTask_GetsTransitiveDiagnostic() + { + var diags = await GetAllDiagnosticsWithScopeAsync(""" + using System; + public static class Helper + { + public static void Run() => Environment.GetEnvironmentVariable("KEY"); + } + public class PlainTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + Helper.Run(); + return true; + } + } + """, SharedAnalyzerHelpers.ScopeAll); + + diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ShouldHaveSingleItem(); + } } \ No newline at end of file diff --git a/src/TaskAnalyzer.Tests/UnsupportedTaskItemTypeAnalyzerTests.cs b/src/TaskAnalyzer.Tests/UnsupportedTaskItemTypeAnalyzerTests.cs index f3b39a46004..3a7ee231bce 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.Error); diags[0].GetMessage().ShouldContain("Item"); diags[0].GetMessage().ShouldContain("Guid"); diags[0].GetMessage().ShouldContain("string, bool, AbsolutePath, FileInfo, DirectoryInfo"); @@ -225,6 +227,42 @@ public class MyTask : Microsoft.Build.Utilities.Task diags[0].GetMessage().ShouldContain("TimeSpan"); } + [Fact] + public async Task TypedTaskItemDiagnostics_AreIndependentOfMtScope() + { + var diags = await GetUnsupportedTaskItemTypeDiagnosticsAsync(""" + using System; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + 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.Error); + 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 4dcafa5e110..069f3a4cb31 100644 --- a/src/TaskAnalyzer/AnalyzerReleases.Unshipped.md +++ b/src/TaskAnalyzer/AnalyzerReleases.Unshipped.md @@ -7,9 +7,9 @@ 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) -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 +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 | Error | ITaskItem used with a type argument T that MSBuild cannot bind as a task parameter +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 diff --git a/src/TaskAnalyzer/DiagnosticDescriptors.cs b/src/TaskAnalyzer/DiagnosticDescriptors.cs index 396f6ae2c86..9e6aae24910 100644 --- a/src/TaskAnalyzer/DiagnosticDescriptors.cs +++ b/src/TaskAnalyzer/DiagnosticDescriptors.cs @@ -90,7 +90,7 @@ internal static class DiagnosticDescriptors title: "ITaskItem used with unsupported type argument", messageFormat: "Task property '{0}' uses ITaskItem<{1}> but MSBuild cannot automatically parse '{1}' from item metadata. Use one of the directly parsed types: {2}.", category: "MSBuild.TaskAuthoring", - defaultSeverity: DiagnosticSeverity.Warning, + defaultSeverity: DiagnosticSeverity.Error, isEnabledByDefault: true, description: "MSBuild can only bind ITaskItem properties when T is a supported type. Using an unsupported type will cause a runtime failure when MSBuild tries to bind the parameter."); @@ -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/MultiThreadableTaskAnalyzer.cs b/src/TaskAnalyzer/MultiThreadableTaskAnalyzer.cs index 26a0d711125..8e8f8adc6be 100644 --- a/src/TaskAnalyzer/MultiThreadableTaskAnalyzer.cs +++ b/src/TaskAnalyzer/MultiThreadableTaskAnalyzer.cs @@ -16,10 +16,9 @@ namespace Microsoft.Build.TaskAuthoring.Analyzer /// /// Roslyn analyzer that detects unsafe API usage in MSBuild task implementations. /// - /// Scope (controlled by .editorconfig option "msbuild_task_analyzer.scope"): - /// - "all" (default): All rules fire on ALL ITask implementations - /// - "multithreadable_only": MSBuildTask0002, 0003 fire only on IMultiThreadableTask or [MSBuildMultiThreadableTask] - /// (MSBuildTask0001 and MSBuildTask0004 always fire on all tasks regardless) + /// Scope (controlled by analyzer option "msbuild_task_analyzer.scope"): + /// - "multithreadable_only" (default): MSBuildTask0001-0004 report only for MT tasks and explicitly analyzed helpers + /// - "all": Enables MSBuildTask0001-0004 for all ITask implementations during migration /// /// Per review feedback from @rainersigwald: /// - Console.* promoted to MSBuildTask0001 (always wrong in tasks) @@ -29,8 +28,8 @@ namespace Microsoft.Build.TaskAuthoring.Analyzer public sealed class MultiThreadableTaskAnalyzer : DiagnosticAnalyzer { /// - /// The .editorconfig key controlling analysis scope. - /// Values: "all" (default) | "multithreadable_only" + /// The analyzer configuration key controlling analysis scope. + /// Values: "multithreadable_only" (default) | "all" /// internal const string ScopeOptionKey = SharedAnalyzerHelpers.ScopeOptionKey; internal const string ScopeAll = SharedAnalyzerHelpers.ScopeAll; @@ -55,7 +54,7 @@ private void OnCompilationStart(CompilationStartAnalysisContext compilationConte return; } - // Read scope option from .editorconfig: "all" (default) or "multithreadable_only" + // Read scope option: "multithreadable_only" (default) or "all" bool analyzeAllTasks = SharedAnalyzerHelpers.ReadAnalyzeAllTasksOption(compilationContext.Options.AnalyzerConfigOptionsProvider); var iMultiThreadableTaskType = compilationContext.Compilation.GetTypeByMetadataName(WellKnownTypeNames.IMultiThreadableTaskFullName); @@ -97,12 +96,12 @@ private void OnCompilationStart(CompilationStartAnalysisContext compilationConte // Helper classes with the attribute or tasks with [MSBuildMultiThreadableTask] are treated as IMultiThreadableTask bool analyzeAsMultiThreadable = isMultiThreadableTask || hasAnalyzedAttribute || hasMultiThreadableAttribute; - // When scope is "multithreadable_only", only analyze MSBuildTask0002/0003 for multithreadable tasks - bool reportEnvironmentRules = analyzeAllTasks || analyzeAsMultiThreadable; + // The default scope reports MSBuildTask0001-0004 only for MT tasks and explicitly analyzed helpers. + bool reportScopedRules = analyzeAllTasks || analyzeAsMultiThreadable; // Register operation-level analysis within this type symbolStartContext.RegisterOperationAction( - ctx => AnalyzeOperation(ctx, bannedApiLookup, filePathTypes, reportEnvironmentRules, + ctx => AnalyzeOperation(ctx, bannedApiLookup, filePathTypes, reportScopedRules, taskEnvironmentType, absolutePathType, iTaskItemType, consoleType), OperationKind.Invocation, OperationKind.ObjectCreation, @@ -117,7 +116,7 @@ private static void AnalyzeOperation( OperationAnalysisContext context, Dictionary bannedApiLookup, ImmutableHashSet filePathTypes, - bool reportEnvironmentRules, + bool reportScopedRules, INamedTypeSymbol? taskEnvironmentType, INamedTypeSymbol? absolutePathType, INamedTypeSymbol? iTaskItemType, @@ -165,8 +164,7 @@ private static void AnalyzeOperation( // Check banned API lookup (handles MSBuildTask0001, 0002, 0004) if (bannedApiLookup.TryGetValue(referencedSymbol, out var entry)) { - // MSBuildTask0002 (TaskEnvironment) is gated by scope setting - if (entry.Category == BannedApiDefinitions.ApiCategory.TaskEnvironment && !reportEnvironmentRules) + if (!reportScopedRules) { return; } @@ -180,7 +178,7 @@ private static void AnalyzeOperation( // Type-level Console ban: ANY member of System.Console is flagged. // This catches all Console methods/properties including ones added in newer .NET versions. - if (consoleType is not null) + if (reportScopedRules && consoleType is not null) { var containingType = referencedSymbol.ContainingType; if (containingType is not null && SymbolEqualityComparer.Default.Equals(containingType, consoleType)) @@ -198,7 +196,7 @@ private static void AnalyzeOperation( } // Check file path APIs (MSBuildTask0003) - gated by scope setting - if (reportEnvironmentRules && !arguments.IsDefaultOrEmpty) + if (reportScopedRules && !arguments.IsDefaultOrEmpty) { var method = referencedSymbol as IMethodSymbol; if (method is not null) diff --git a/src/TaskAnalyzer/README.md b/src/TaskAnalyzer/README.md index d4e3cc2f511..155ee823e90 100644 --- a/src/TaskAnalyzer/README.md +++ b/src/TaskAnalyzer/README.md @@ -14,16 +14,16 @@ This analyzer catches unsafe API usage at compile time and offers code fixes to | ID | Severity | Scope | Title | |---|---|---|---| -| **MSBuildTask0001** | Error | All `ITask` implementations | API is never safe in MSBuild tasks | -| **MSBuildTask0002** | Warning | All `ITask` implementations | API requires `TaskEnvironment` alternative | -| **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()` | -| **MSBuildTask0009** | Warning | All `ITask` implementations | `ITaskItem` used with unsupported type argument | -| **MSBuildTask0010** | Error | All `ITask` implementations | `ITaskItem` relies on culture-sensitive conversion | +| **MSBuildTask0001** | Error | MT tasks by default; all tasks in migration mode | API is never safe in MSBuild tasks | +| **MSBuildTask0002** | Warning | MT tasks by default; all tasks in migration mode | API requires `TaskEnvironment` alternative | +| **MSBuildTask0003** | Warning | MT tasks by default; all tasks in migration mode | File system API requires absolute path | +| **MSBuildTask0004** | Warning | MT tasks by default; all tasks in migration mode | API may cause issues in multithreaded tasks | +| **MSBuildTask0005** | Warning | MT tasks by default; all tasks in migration mode | Transitive unsafe API usage in task call chain | +| **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** | Error | All `ITask` implementations with typed task properties | `ITaskItem` used with unsupported type argument | +| **MSBuildTask0010** | Warning | All `ITask` implementations with typed task properties | `ITaskItem` relies on culture-sensitive conversion | | **MSBuildTask0011** | Info | Concrete `IMultiThreadableTask` implementations | Prefer constructor injection for `TaskEnvironment` | ### MSBuildTask0001 — Critical: No Safe Alternative @@ -230,19 +230,19 @@ For a reference-typed target (`FileInfo`/`DirectoryInfo`) the guard is a null-co ### MSBuildTask0009 — Unsupported `ITaskItem` Type Argument -When a task property is typed as `ITaskItem` or `ITaskItem[]` but `T` is not supported by MSBuild's task parameter binder, a **Warning** is emitted. Using an unsupported type will cause a runtime failure when MSBuild tries to bind the parameter. +When a task property is typed as `ITaskItem` or `ITaskItem[]` but `T` is not supported by MSBuild's task parameter binder, an **Error** is emitted. Using an unsupported type will cause a runtime failure when MSBuild tries to bind the parameter. **Directly parsed type arguments:** `string`, `bool`, `AbsolutePath`, `FileInfo`, `DirectoryInfo`. The binder also accepts `char`, numeric primitives, `decimal`, and `DateTime`, but MSBuildTask0010 rejects those types because they rely on `Convert.ChangeType`. ```csharp -// ⚠️ MSBuildTask0009: Task property 'Id' uses ITaskItem but 'Guid' is not supported +// ❌ MSBuildTask0009: Task property 'Id' uses ITaskItem but 'Guid' is not supported 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 Id { get; set; } // error + public ITaskItem[] Durations { get; set; } // error + public ITaskItem Count { get; set; } // MSBuildTask0010 warning } ``` @@ -252,13 +252,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 } ``` @@ -289,16 +289,25 @@ The engine prefers this constructor when it is present. A public parameterless c ## Analysis Scope -The analyzer determines what to check based on the type declaration: +The default `multithreadable_only` scope prevents MT-specific warnings from affecting regular tasks. It recognizes `IMultiThreadableTask`, `[MSBuildMultiThreadableTask]`, and `[MSBuildMultiThreadableTaskAnalyzed]` as MT opt-ins. | Type | Rules Applied | |---|---| -| Any class implementing `ITask` | MSBuildTask0001–MSBuildTask0005, MSBuildTask0009–MSBuildTask0010 | +| Regular class implementing `ITask` | MSBuildTask0009–MSBuildTask0010 | | Class with `[MSBuildMultiThreadableTask]` attribute applied directly | MSBuildTask0006–MSBuildTask0008 (in addition to MSBuildTask0001–0005) | | Concrete class implementing `IMultiThreadableTask` without the attribute | MSBuildTask0001–MSBuildTask0005 and MSBuildTask0009–MSBuildTask0011 | -| Helper class with `[MSBuildMultiThreadableTaskAnalyzed]` attribute | MSBuildTask0001–MSBuildTask0005 | +| Helper class with `[MSBuildMultiThreadableTaskAnalyzed]` attribute | MSBuildTask0001–MSBuildTask0004 | | Regular class (no task interface or attribute) | Not analyzed | +Create a `.globalconfig` file to analyze regular tasks for MSBuildTask0001–MSBuildTask0005 before MT migration: + +```ini +is_global = true +msbuild_task_analyzer.scope = all +``` + +Missing and unrecognized values use the safe `multithreadable_only` default. + MSBuildTask0006–MSBuildTask0008 apply only when the `[MSBuildMultiThreadableTask]` attribute is applied **directly** to the task class. The attribute is `Inherited = false`, so a task that merely derives from a base class implementing `IMultiThreadableTask` (or carrying the attribute) has not itself opted into multithreaded support and is not subject to these three rules. Input properties are collected from the task class **and its base classes**, so an `ITaskItem`/`string` input declared on a shared base task is still analyzed. The `[MSBuildMultiThreadableTaskAnalyzed]` attribute allows opting helper classes into **direct** analysis by the `MultiThreadableTaskAnalyzer` (MSBuildTask0001–0004). Without it, only classes implementing `ITask` receive per-line diagnostics and code fixes for those rules. The **transitive** analyzer (MSBuildTask0005) already discovers helpers via call graph analysis, but it reports only at the task entry point. Adding this attribute to a helper class gives you inline diagnostics and code fixes directly in the helper's source. @@ -307,10 +316,9 @@ 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. +- **MSBuildTask0001 and MSBuildTask0009** report as **Error** when their scope applies. +- **MSBuildTask0002–MSBuildTask0005 and MSBuildTask0010** report as **Warning** when their scope applies. +- **MSBuildTask0006–MSBuildTask0008 and MSBuildTask0011** report as **Info** — these are modernization suggestions, not correctness issues. ## Code Fixes diff --git a/src/TaskAnalyzer/SharedAnalyzerHelpers.cs b/src/TaskAnalyzer/SharedAnalyzerHelpers.cs index 3c8f9643cb1..ba954c35633 100644 --- a/src/TaskAnalyzer/SharedAnalyzerHelpers.cs +++ b/src/TaskAnalyzer/SharedAnalyzerHelpers.cs @@ -17,8 +17,8 @@ namespace Microsoft.Build.TaskAuthoring.Analyzer internal static class SharedAnalyzerHelpers { /// - /// The .editorconfig key controlling analysis scope. - /// Values: "all" (default) | "multithreadable_only" + /// The analyzer configuration key controlling analysis scope. + /// Values: "multithreadable_only" (default) | "all" /// internal const string ScopeOptionKey = "msbuild_task_analyzer.scope"; internal const string ScopeAll = "all"; @@ -26,17 +26,17 @@ internal static class SharedAnalyzerHelpers /// /// Reads the scope option from the analyzer config options provider. - /// Returns true if all tasks should be analyzed; false if only multithreadable tasks. + /// Returns true only when all-task migration analysis is explicitly enabled. /// internal static bool ReadAnalyzeAllTasksOption(AnalyzerConfigOptionsProvider optionsProvider) { if (optionsProvider.GlobalOptions.TryGetValue($"build_property.{ScopeOptionKey}", out var scopeValue) || optionsProvider.GlobalOptions.TryGetValue(ScopeOptionKey, out scopeValue)) { - return !string.Equals(scopeValue, ScopeMultiThreadableOnly, StringComparison.OrdinalIgnoreCase); + return string.Equals(scopeValue, ScopeAll, StringComparison.OrdinalIgnoreCase); } - return true; // default: analyze all tasks + return false; } /// /// Represents a resolved banned API entry for O(1) lookup during analysis. diff --git a/src/TaskAnalyzer/TransitiveCallChainAnalyzer.cs b/src/TaskAnalyzer/TransitiveCallChainAnalyzer.cs index 9dc9d26aa99..d5855578a9a 100644 --- a/src/TaskAnalyzer/TransitiveCallChainAnalyzer.cs +++ b/src/TaskAnalyzer/TransitiveCallChainAnalyzer.cs @@ -50,7 +50,7 @@ private void OnCompilationStart(CompilationStartAnalysisContext compilationConte return; } - // Read scope option from .editorconfig + // Read scope option bool analyzeAllTasks = SharedAnalyzerHelpers.ReadAnalyzeAllTasksOption(compilationContext.Options.AnalyzerConfigOptionsProvider); var iMultiThreadableTaskType = compilationContext.Compilation.GetTypeByMetadataName(WellKnownTypeNames.IMultiThreadableTaskFullName); 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( From 6da85eabcb15cf20d91c8528319f593bb00937eb Mon Sep 17 00:00:00 2001 From: VolPlita Date: Wed, 26 Aug 2026 01:47:18 +0200 Subject: [PATCH 2/2] Refine TaskAnalyzer diagnostic severities Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../MultiThreadableTaskAnalyzerTests.cs | 222 ------------------ .../PreferTypedParameterAnalyzerTests.cs | 6 +- src/TaskAnalyzer.Tests/TestHelpers.cs | 93 ++------ .../TransitiveCallChainAnalyzerTests.cs | 68 ------ .../UnsupportedTaskItemTypeAnalyzerTests.cs | 9 +- .../AnalyzerReleases.Unshipped.md | 2 +- src/TaskAnalyzer/DiagnosticDescriptors.cs | 8 +- .../MultiThreadableTaskAnalyzer.cs | 28 ++- src/TaskAnalyzer/README.md | 41 ++-- src/TaskAnalyzer/SharedAnalyzerHelpers.cs | 10 +- .../TransitiveCallChainAnalyzer.cs | 2 +- 11 files changed, 73 insertions(+), 416 deletions(-) diff --git a/src/TaskAnalyzer.Tests/MultiThreadableTaskAnalyzerTests.cs b/src/TaskAnalyzer.Tests/MultiThreadableTaskAnalyzerTests.cs index 3b2c543af06..cc204fdcddb 100644 --- a/src/TaskAnalyzer.Tests/MultiThreadableTaskAnalyzerTests.cs +++ b/src/TaskAnalyzer.Tests/MultiThreadableTaskAnalyzerTests.cs @@ -3,9 +3,6 @@ using System.Linq; using System.Threading.Tasks; -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CSharp.Testing; -using Microsoft.CodeAnalysis.Testing; using Shouldly; using Xunit; using static Microsoft.Build.TaskAuthoring.Analyzer.Tests.TestHelpers; @@ -1873,225 +1870,6 @@ public override bool Execute() // Scope option tests // ═══════════════════════════════════════════════════════════════════════ - [Fact] - public async Task Scope_Default_PlainTask_DoesNotGetScopedDiagnostics() - { - var diags = await GetDiagnosticsWithDefaultScopeAsync(""" - using System; - using System.IO; - using System.Reflection; - public class PlainTask : Microsoft.Build.Utilities.Task - { - public override bool Execute() - { - Console.WriteLine("test"); - var value = Environment.GetEnvironmentVariable("KEY"); - Assembly.Load("Test"); - return File.Exists("relative.txt"); - } - } - """); - - diags.Where(d => d.Id == DiagnosticIds.CriticalError).ShouldBeEmpty(); - diags.Where(d => d.Id == DiagnosticIds.TaskEnvironmentRequired).ShouldBeEmpty(); - diags.Where(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute).ShouldBeEmpty(); - diags.Where(d => d.Id == DiagnosticIds.PotentialIssue).ShouldBeEmpty(); - } - - [Fact] - public async Task Scope_Default_MultiThreadableTask_GetsAllScopedDiagnostics() - { - var diags = await GetDiagnosticsWithDefaultScopeAsync(""" - using System; - using System.IO; - using System.Reflection; - using Microsoft.Build.Framework; - public class MtTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask - { - public TaskEnvironment TaskEnvironment { get; set; } - public override bool Execute() - { - Console.WriteLine("test"); - var value = Environment.GetEnvironmentVariable("KEY"); - Assembly.Load("Test"); - return File.Exists("relative.txt"); - } - } - """); - - diags.Where(d => d.Id == DiagnosticIds.CriticalError).ShouldHaveSingleItem() - .Severity.ShouldBe(DiagnosticSeverity.Error); - diags.Where(d => d.Id == DiagnosticIds.TaskEnvironmentRequired).ShouldHaveSingleItem() - .Severity.ShouldBe(DiagnosticSeverity.Warning); - diags.Where(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute).ShouldHaveSingleItem() - .Severity.ShouldBe(DiagnosticSeverity.Warning); - diags.Where(d => d.Id == DiagnosticIds.PotentialIssue).ShouldHaveSingleItem() - .Severity.ShouldBe(DiagnosticSeverity.Warning); - } - - [Fact] - public async Task Scope_All_PlainTask_GetsAllScopedDiagnostics() - { - var diags = await GetDiagnosticsWithScopeAsync(""" - using System; - using System.IO; - using System.Reflection; - public class PlainTask : Microsoft.Build.Utilities.Task - { - public override bool Execute() - { - Console.WriteLine("test"); - var value = Environment.GetEnvironmentVariable("KEY"); - Assembly.Load("Test"); - return File.Exists("relative.txt"); - } - } - """, SharedAnalyzerHelpers.ScopeAll); - - diags.Where(d => d.Id == DiagnosticIds.CriticalError).ShouldHaveSingleItem() - .Severity.ShouldBe(DiagnosticSeverity.Error); - diags.Where(d => d.Id == DiagnosticIds.TaskEnvironmentRequired).ShouldHaveSingleItem() - .Severity.ShouldBe(DiagnosticSeverity.Warning); - diags.Where(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute).ShouldHaveSingleItem() - .Severity.ShouldBe(DiagnosticSeverity.Warning); - diags.Where(d => d.Id == DiagnosticIds.PotentialIssue).ShouldHaveSingleItem() - .Severity.ShouldBe(DiagnosticSeverity.Warning); - } - - [Fact] - public async Task Scope_GlobalConfig_All_AnalyzesPlainTask() - { - var test = new CSharpAnalyzerTest - { - TestCode = """ - using System; - public class PlainTask : Microsoft.Build.Utilities.Task - { - public override bool Execute() - { - var value = {|#0:Environment.GetEnvironmentVariable("KEY")|}; - return true; - } - } - """, - ReferenceAssemblies = ReferenceAssemblies.Net.Net80, - }; - test.TestState.Sources.Add(("Stubs.cs", FrameworkStubs)); - test.TestState.AnalyzerConfigFiles.Add(("/.globalconfig", """ - is_global = true - msbuild_task_analyzer.scope = all - """)); - test.ExpectedDiagnostics.Add( - new DiagnosticResult(DiagnosticIds.TaskEnvironmentRequired, DiagnosticSeverity.Warning).WithLocation(0)); - - await test.RunAsync(); - } - - [Fact] - public async Task Scope_UnrecognizedValue_UsesDefault() - { - var diags = await GetDiagnosticsWithScopeAsync(""" - using System; - using System.Reflection; - public class PlainTask : Microsoft.Build.Utilities.Task - { - public override bool Execute() - { - Console.WriteLine("test"); - var value = Environment.GetEnvironmentVariable("KEY"); - Assembly.Load("Test"); - return true; - } - } - """, "unrecognized"); - - diags.Where(d => d.Id == DiagnosticIds.CriticalError).ShouldBeEmpty(); - diags.Where(d => d.Id == DiagnosticIds.TaskEnvironmentRequired).ShouldBeEmpty(); - diags.Where(d => d.Id == DiagnosticIds.PotentialIssue).ShouldBeEmpty(); - } - - [Fact] - public async Task Scope_Default_PlainTask_TreatWarningsAsErrors_DoesNotGetScopedDiagnostics() - { - var diags = await GetDiagnosticsWithDefaultScopeAsync(""" - using System; - using System.Reflection; - public class PlainTask : Microsoft.Build.Utilities.Task - { - public override bool Execute() - { - Console.WriteLine("test"); - Assembly.Load("Test"); - return true; - } - } - """, ReportDiagnostic.Error); - - diags.Where(d => d.Id == DiagnosticIds.CriticalError).ShouldBeEmpty(); - diags.Where(d => d.Id == DiagnosticIds.PotentialIssue).ShouldBeEmpty(); - } - - [Fact] - public async Task Scope_Default_MultiThreadableTask_TreatWarningsAsErrors_PromotesWarning() - { - var diags = await GetDiagnosticsWithDefaultScopeAsync(""" - using System.Reflection; - using Microsoft.Build.Framework; - public class MtTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask - { - public TaskEnvironment TaskEnvironment { get; set; } - public override bool Execute() - { - Assembly.Load("Test"); - return true; - } - } - """, ReportDiagnostic.Error); - - Diagnostic diagnostic = diags.Where(d => d.Id == DiagnosticIds.PotentialIssue).ShouldHaveSingleItem(); - diagnostic.Severity.ShouldBe(DiagnosticSeverity.Error); - diagnostic.IsWarningAsError.ShouldBeTrue(); - } - - [Fact] - public async Task Scope_Default_MultiThreadableAttribute_OptsTaskIn() - { - var diags = await GetDiagnosticsWithDefaultScopeAsync(""" - using System; - using Microsoft.Build.Framework; - [MSBuildMultiThreadableTask] - public class MtTask : Microsoft.Build.Utilities.Task - { - public override bool Execute() - { - var value = Environment.GetEnvironmentVariable("KEY"); - return true; - } - } - """); - - diags.Where(d => d.Id == DiagnosticIds.TaskEnvironmentRequired).ShouldHaveSingleItem(); - } - - [Fact] - public async Task Scope_Default_AnalyzedAttribute_OptsHelperIn() - { - var diags = await GetDiagnosticsWithDefaultScopeAsync(""" - using System; - using Microsoft.Build.Framework; - [MSBuildMultiThreadableTaskAnalyzed] - public class MtHelper - { - public void Execute() - { - var value = Environment.GetEnvironmentVariable("KEY"); - } - } - """); - - diags.Where(d => d.Id == DiagnosticIds.TaskEnvironmentRequired).ShouldHaveSingleItem(); - } - [Fact] public async Task Scope_MultithreadableOnly_PlainTask_NoDiagnostic() { 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/TestHelpers.cs b/src/TaskAnalyzer.Tests/TestHelpers.cs index a5997c06d1f..73bbbb2dbf2 100644 --- a/src/TaskAnalyzer.Tests/TestHelpers.cs +++ b/src/TaskAnalyzer.Tests/TestHelpers.cs @@ -138,16 +138,34 @@ public static string FullyQualifiedPath(string tail) => public static MetadataReference[] GetCoreReferences() => s_coreReferences; /// - /// Runs the MultiThreadableTaskAnalyzer in explicit all-task migration mode. + /// Runs the MultiThreadableTaskAnalyzer on the given source code and returns analyzer diagnostics. + /// Source is combined with framework stubs automatically. /// - public static System.Threading.Tasks.Task> GetDiagnosticsAsync(string source) => - GetDiagnosticsWithScopeAsync(source, SharedAnalyzerHelpers.ScopeAll); + public static async System.Threading.Tasks.Task> GetDiagnosticsAsync(string source) + { + var compilation = CreateCompilation(source); + var analyzer = new MultiThreadableTaskAnalyzer(); + var compilationWithAnalyzers = compilation.WithAnalyzers( + ImmutableArray.Create(analyzer)); + + var allDiags = await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync(); + return allDiags; + } /// - /// Runs both the direct and transitive analyzers in explicit all-task migration mode. + /// Runs BOTH the direct and transitive analyzers on the given source code. /// - public static System.Threading.Tasks.Task> GetAllDiagnosticsAsync(string source) => - GetAllDiagnosticsWithScopeAsync(source, SharedAnalyzerHelpers.ScopeAll); + public static async System.Threading.Tasks.Task> GetAllDiagnosticsAsync(string source) + { + var compilation = CreateCompilation(source); + var analyzers = ImmutableArray.Create( + new MultiThreadableTaskAnalyzer(), + new TransitiveCallChainAnalyzer()); + var compilationWithAnalyzers = compilation.WithAnalyzers(analyzers); + + var allDiags = await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync(); + return allDiags; + } /// /// Runs compiler diagnostics together with analyzers and suppressors and returns @@ -242,69 +260,6 @@ public static async System.Threading.Tasks.Task> GetD return await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync(); } - /// - /// Runs the MultiThreadableTaskAnalyzer without a scope option. - /// - public static async System.Threading.Tasks.Task> GetDiagnosticsWithDefaultScopeAsync(string source) - { - var compilation = CreateCompilation(source); - var analyzer = new MultiThreadableTaskAnalyzer(); - var compilationWithAnalyzers = compilation.WithAnalyzers( - ImmutableArray.Create(analyzer)); - return await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync(); - } - - /// - /// Runs the MultiThreadableTaskAnalyzer without a scope option and applies a general diagnostic action. - /// - public static async System.Threading.Tasks.Task> GetDiagnosticsWithDefaultScopeAsync( - string source, - ReportDiagnostic generalDiagnosticOption) - { - var compilation = CreateCompilation(source); - compilation = compilation.WithOptions( - ((CSharpCompilationOptions)compilation.Options).WithGeneralDiagnosticOption(generalDiagnosticOption)); - - var analyzer = new MultiThreadableTaskAnalyzer(); - var compilationWithAnalyzers = compilation.WithAnalyzers( - ImmutableArray.Create(analyzer)); - return await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync(); - } - - /// - /// Runs both the direct and transitive analyzers with a specific scope option. - /// - public static async System.Threading.Tasks.Task> GetAllDiagnosticsWithScopeAsync(string source, string scope) - { - var compilation = CreateCompilation(source); - var analyzers = ImmutableArray.Create( - new MultiThreadableTaskAnalyzer(), - new TransitiveCallChainAnalyzer()); - - var globalOptions = new Dictionary - { - { $"build_property.{SharedAnalyzerHelpers.ScopeOptionKey}", scope } - }; - var optionsProvider = new TestAnalyzerConfigOptionsProvider(globalOptions); - var options = new AnalyzerOptions(ImmutableArray.Empty, optionsProvider); - - var compilationWithAnalyzers = compilation.WithAnalyzers(analyzers, options); - return await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync(); - } - - /// - /// Runs both the direct and transitive analyzers without a scope option. - /// - public static async System.Threading.Tasks.Task> GetAllDiagnosticsWithDefaultScopeAsync(string source) - { - var compilation = CreateCompilation(source); - var analyzers = ImmutableArray.Create( - new MultiThreadableTaskAnalyzer(), - new TransitiveCallChainAnalyzer()); - var compilationWithAnalyzers = compilation.WithAnalyzers(analyzers); - return await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync(); - } - private static MetadataReference[] CreateCoreReferences() { // Reference the core runtime assemblies needed diff --git a/src/TaskAnalyzer.Tests/TransitiveCallChainAnalyzerTests.cs b/src/TaskAnalyzer.Tests/TransitiveCallChainAnalyzerTests.cs index 23ad3b18b14..44e2e5a73bb 100644 --- a/src/TaskAnalyzer.Tests/TransitiveCallChainAnalyzerTests.cs +++ b/src/TaskAnalyzer.Tests/TransitiveCallChainAnalyzerTests.cs @@ -244,72 +244,4 @@ public override bool Execute() msg.ShouldContain("A.Step1"); msg.ShouldContain("B.Step2"); } - - [Fact] - public async Task Scope_Default_PlainTask_DoesNotGetTransitiveDiagnostic() - { - var diags = await GetAllDiagnosticsWithDefaultScopeAsync(""" - using System; - public static class Helper - { - public static void Run() => Environment.GetEnvironmentVariable("KEY"); - } - public class PlainTask : Microsoft.Build.Utilities.Task - { - public override bool Execute() - { - Helper.Run(); - return true; - } - } - """); - - diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ShouldBeEmpty(); - } - - [Fact] - public async Task Scope_Default_MultiThreadableTask_GetsTransitiveDiagnostic() - { - var diags = await GetAllDiagnosticsWithDefaultScopeAsync(""" - using System; - using Microsoft.Build.Framework; - public static class Helper - { - public static void Run() => Environment.GetEnvironmentVariable("KEY"); - } - public class MtTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask - { - public TaskEnvironment TaskEnvironment { get; set; } - public override bool Execute() - { - Helper.Run(); - return true; - } - } - """); - - diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ShouldHaveSingleItem(); - } - - [Fact] - public async Task Scope_All_PlainTask_GetsTransitiveDiagnostic() - { - var diags = await GetAllDiagnosticsWithScopeAsync(""" - using System; - public static class Helper - { - public static void Run() => Environment.GetEnvironmentVariable("KEY"); - } - public class PlainTask : Microsoft.Build.Utilities.Task - { - public override bool Execute() - { - Helper.Run(); - return true; - } - } - """, SharedAnalyzerHelpers.ScopeAll); - - diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ShouldHaveSingleItem(); - } } \ No newline at end of file diff --git a/src/TaskAnalyzer.Tests/UnsupportedTaskItemTypeAnalyzerTests.cs b/src/TaskAnalyzer.Tests/UnsupportedTaskItemTypeAnalyzerTests.cs index 3a7ee231bce..c006d540d02 100644 --- a/src/TaskAnalyzer.Tests/UnsupportedTaskItemTypeAnalyzerTests.cs +++ b/src/TaskAnalyzer.Tests/UnsupportedTaskItemTypeAnalyzerTests.cs @@ -202,7 +202,7 @@ public class MyTask : Microsoft.Build.Utilities.Task """); diags.ShouldContain(d => d.Id == DiagnosticIds.UnsupportedTaskItemType); - diags.ShouldHaveSingleItem().Severity.ShouldBe(Microsoft.CodeAnalysis.DiagnosticSeverity.Error); + 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"); @@ -228,14 +228,13 @@ public class MyTask : Microsoft.Build.Utilities.Task } [Fact] - public async Task TypedTaskItemDiagnostics_AreIndependentOfMtScope() + public async Task TypedTaskItemDiagnostics_AreIndependentOfMtOptIn() { var diags = await GetUnsupportedTaskItemTypeDiagnosticsAsync(""" using System; using Microsoft.Build.Framework; - public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + public class MyTask : Microsoft.Build.Utilities.Task { - public TaskEnvironment TaskEnvironment { get; set; } public ITaskItem Invalid { get; set; } = null!; public ITaskItem CultureSensitive { get; set; } = null!; public override bool Execute() => true; @@ -243,7 +242,7 @@ public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask """); diags.Where(d => d.Id == DiagnosticIds.UnsupportedTaskItemType).ShouldHaveSingleItem() - .Severity.ShouldBe(Microsoft.CodeAnalysis.DiagnosticSeverity.Error); + .Severity.ShouldBe(Microsoft.CodeAnalysis.DiagnosticSeverity.Warning); diags.Where(d => d.Id == DiagnosticIds.CultureSensitiveTaskItemType).ShouldHaveSingleItem() .Severity.ShouldBe(Microsoft.CodeAnalysis.DiagnosticSeverity.Warning); } diff --git a/src/TaskAnalyzer/AnalyzerReleases.Unshipped.md b/src/TaskAnalyzer/AnalyzerReleases.Unshipped.md index 069f3a4cb31..55f16851397 100644 --- a/src/TaskAnalyzer/AnalyzerReleases.Unshipped.md +++ b/src/TaskAnalyzer/AnalyzerReleases.Unshipped.md @@ -10,6 +10,6 @@ MSBuildTask0005 | MSBuild.TaskAuthoring | Warning | Transitive unsafe API usage 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 | Error | ITaskItem used with a type argument T that MSBuild cannot bind as a task parameter +MSBuildTask0009 | MSBuild.TaskAuthoring | Warning | ITaskItem used with a type argument T that MSBuild cannot bind as a task parameter 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 diff --git a/src/TaskAnalyzer/DiagnosticDescriptors.cs b/src/TaskAnalyzer/DiagnosticDescriptors.cs index 9e6aae24910..7c9907df94d 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."); @@ -90,7 +90,7 @@ internal static class DiagnosticDescriptors title: "ITaskItem used with unsupported type argument", messageFormat: "Task property '{0}' uses ITaskItem<{1}> but MSBuild cannot automatically parse '{1}' from item metadata. Use one of the directly parsed types: {2}.", category: "MSBuild.TaskAuthoring", - defaultSeverity: DiagnosticSeverity.Error, + defaultSeverity: DiagnosticSeverity.Warning, isEnabledByDefault: true, description: "MSBuild can only bind ITaskItem properties when T is a supported type. Using an unsupported type will cause a runtime failure when MSBuild tries to bind the parameter."); diff --git a/src/TaskAnalyzer/MultiThreadableTaskAnalyzer.cs b/src/TaskAnalyzer/MultiThreadableTaskAnalyzer.cs index 8e8f8adc6be..26a0d711125 100644 --- a/src/TaskAnalyzer/MultiThreadableTaskAnalyzer.cs +++ b/src/TaskAnalyzer/MultiThreadableTaskAnalyzer.cs @@ -16,9 +16,10 @@ namespace Microsoft.Build.TaskAuthoring.Analyzer /// /// Roslyn analyzer that detects unsafe API usage in MSBuild task implementations. /// - /// Scope (controlled by analyzer option "msbuild_task_analyzer.scope"): - /// - "multithreadable_only" (default): MSBuildTask0001-0004 report only for MT tasks and explicitly analyzed helpers - /// - "all": Enables MSBuildTask0001-0004 for all ITask implementations during migration + /// Scope (controlled by .editorconfig option "msbuild_task_analyzer.scope"): + /// - "all" (default): All rules fire on ALL ITask implementations + /// - "multithreadable_only": MSBuildTask0002, 0003 fire only on IMultiThreadableTask or [MSBuildMultiThreadableTask] + /// (MSBuildTask0001 and MSBuildTask0004 always fire on all tasks regardless) /// /// Per review feedback from @rainersigwald: /// - Console.* promoted to MSBuildTask0001 (always wrong in tasks) @@ -28,8 +29,8 @@ namespace Microsoft.Build.TaskAuthoring.Analyzer public sealed class MultiThreadableTaskAnalyzer : DiagnosticAnalyzer { /// - /// The analyzer configuration key controlling analysis scope. - /// Values: "multithreadable_only" (default) | "all" + /// The .editorconfig key controlling analysis scope. + /// Values: "all" (default) | "multithreadable_only" /// internal const string ScopeOptionKey = SharedAnalyzerHelpers.ScopeOptionKey; internal const string ScopeAll = SharedAnalyzerHelpers.ScopeAll; @@ -54,7 +55,7 @@ private void OnCompilationStart(CompilationStartAnalysisContext compilationConte return; } - // Read scope option: "multithreadable_only" (default) or "all" + // Read scope option from .editorconfig: "all" (default) or "multithreadable_only" bool analyzeAllTasks = SharedAnalyzerHelpers.ReadAnalyzeAllTasksOption(compilationContext.Options.AnalyzerConfigOptionsProvider); var iMultiThreadableTaskType = compilationContext.Compilation.GetTypeByMetadataName(WellKnownTypeNames.IMultiThreadableTaskFullName); @@ -96,12 +97,12 @@ private void OnCompilationStart(CompilationStartAnalysisContext compilationConte // Helper classes with the attribute or tasks with [MSBuildMultiThreadableTask] are treated as IMultiThreadableTask bool analyzeAsMultiThreadable = isMultiThreadableTask || hasAnalyzedAttribute || hasMultiThreadableAttribute; - // The default scope reports MSBuildTask0001-0004 only for MT tasks and explicitly analyzed helpers. - bool reportScopedRules = analyzeAllTasks || analyzeAsMultiThreadable; + // When scope is "multithreadable_only", only analyze MSBuildTask0002/0003 for multithreadable tasks + bool reportEnvironmentRules = analyzeAllTasks || analyzeAsMultiThreadable; // Register operation-level analysis within this type symbolStartContext.RegisterOperationAction( - ctx => AnalyzeOperation(ctx, bannedApiLookup, filePathTypes, reportScopedRules, + ctx => AnalyzeOperation(ctx, bannedApiLookup, filePathTypes, reportEnvironmentRules, taskEnvironmentType, absolutePathType, iTaskItemType, consoleType), OperationKind.Invocation, OperationKind.ObjectCreation, @@ -116,7 +117,7 @@ private static void AnalyzeOperation( OperationAnalysisContext context, Dictionary bannedApiLookup, ImmutableHashSet filePathTypes, - bool reportScopedRules, + bool reportEnvironmentRules, INamedTypeSymbol? taskEnvironmentType, INamedTypeSymbol? absolutePathType, INamedTypeSymbol? iTaskItemType, @@ -164,7 +165,8 @@ private static void AnalyzeOperation( // Check banned API lookup (handles MSBuildTask0001, 0002, 0004) if (bannedApiLookup.TryGetValue(referencedSymbol, out var entry)) { - if (!reportScopedRules) + // MSBuildTask0002 (TaskEnvironment) is gated by scope setting + if (entry.Category == BannedApiDefinitions.ApiCategory.TaskEnvironment && !reportEnvironmentRules) { return; } @@ -178,7 +180,7 @@ private static void AnalyzeOperation( // Type-level Console ban: ANY member of System.Console is flagged. // This catches all Console methods/properties including ones added in newer .NET versions. - if (reportScopedRules && consoleType is not null) + if (consoleType is not null) { var containingType = referencedSymbol.ContainingType; if (containingType is not null && SymbolEqualityComparer.Default.Equals(containingType, consoleType)) @@ -196,7 +198,7 @@ private static void AnalyzeOperation( } // Check file path APIs (MSBuildTask0003) - gated by scope setting - if (reportScopedRules && !arguments.IsDefaultOrEmpty) + if (reportEnvironmentRules && !arguments.IsDefaultOrEmpty) { var method = referencedSymbol as IMethodSymbol; if (method is not null) diff --git a/src/TaskAnalyzer/README.md b/src/TaskAnalyzer/README.md index 155ee823e90..10409fdb97d 100644 --- a/src/TaskAnalyzer/README.md +++ b/src/TaskAnalyzer/README.md @@ -14,16 +14,16 @@ This analyzer catches unsafe API usage at compile time and offers code fixes to | ID | Severity | Scope | Title | |---|---|---|---| -| **MSBuildTask0001** | Error | MT tasks by default; all tasks in migration mode | API is never safe in MSBuild tasks | -| **MSBuildTask0002** | Warning | MT tasks by default; all tasks in migration mode | API requires `TaskEnvironment` alternative | -| **MSBuildTask0003** | Warning | MT tasks by default; all tasks in migration mode | File system API requires absolute path | -| **MSBuildTask0004** | Warning | MT tasks by default; all tasks in migration mode | API may cause issues in multithreaded tasks | -| **MSBuildTask0005** | Warning | MT tasks by default; all tasks in migration mode | Transitive unsafe API usage in task call chain | +| **MSBuildTask0001** | Error | All `ITask` implementations | API is never safe in MSBuild tasks | +| **MSBuildTask0002** | Warning | All `ITask` implementations | API requires `TaskEnvironment` alternative | +| **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** | 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** | Error | All `ITask` implementations with typed task properties | `ITaskItem` used with unsupported type argument | -| **MSBuildTask0010** | Warning | All `ITask` implementations with typed task properties | `ITaskItem` relies on culture-sensitive conversion | +| **MSBuildTask0009** | Warning | All `ITask` implementations | `ITaskItem` used with unsupported type argument | +| **MSBuildTask0010** | Warning | All `ITask` implementations | `ITaskItem` relies on culture-sensitive conversion | | **MSBuildTask0011** | Info | Concrete `IMultiThreadableTask` implementations | Prefer constructor injection for `TaskEnvironment` | ### MSBuildTask0001 — Critical: No Safe Alternative @@ -230,18 +230,18 @@ For a reference-typed target (`FileInfo`/`DirectoryInfo`) the guard is a null-co ### MSBuildTask0009 — Unsupported `ITaskItem` Type Argument -When a task property is typed as `ITaskItem` or `ITaskItem[]` but `T` is not supported by MSBuild's task parameter binder, an **Error** is emitted. Using an unsupported type will cause a runtime failure when MSBuild tries to bind the parameter. +When a task property is typed as `ITaskItem` or `ITaskItem[]` but `T` is not supported by MSBuild's task parameter binder, a **Warning** is emitted. Using an unsupported type will cause a runtime failure when MSBuild tries to bind the parameter. **Directly parsed type arguments:** `string`, `bool`, `AbsolutePath`, `FileInfo`, `DirectoryInfo`. The binder also accepts `char`, numeric primitives, `decimal`, and `DateTime`, but MSBuildTask0010 rejects those types because they rely on `Convert.ChangeType`. ```csharp -// ❌ MSBuildTask0009: Task property 'Id' uses ITaskItem but 'Guid' is not supported +// ⚠️ MSBuildTask0009: Task property 'Id' uses ITaskItem but 'Guid' is not supported public class MyTask : Task { - public ITaskItem Id { get; set; } // error - public ITaskItem[] Durations { get; set; } // error + public ITaskItem Id { get; set; } // warning + public ITaskItem[] Durations { get; set; } // warning public ITaskItem Count { get; set; } // MSBuildTask0010 warning } ``` @@ -289,25 +289,16 @@ The engine prefers this constructor when it is present. A public parameterless c ## Analysis Scope -The default `multithreadable_only` scope prevents MT-specific warnings from affecting regular tasks. It recognizes `IMultiThreadableTask`, `[MSBuildMultiThreadableTask]`, and `[MSBuildMultiThreadableTaskAnalyzed]` as MT opt-ins. +The analyzer determines what to check based on the type declaration: | Type | Rules Applied | |---|---| -| Regular class implementing `ITask` | MSBuildTask0009–MSBuildTask0010 | +| Any class implementing `ITask` | MSBuildTask0001–MSBuildTask0005, MSBuildTask0009–MSBuildTask0010 | | Class with `[MSBuildMultiThreadableTask]` attribute applied directly | MSBuildTask0006–MSBuildTask0008 (in addition to MSBuildTask0001–0005) | | Concrete class implementing `IMultiThreadableTask` without the attribute | MSBuildTask0001–MSBuildTask0005 and MSBuildTask0009–MSBuildTask0011 | -| Helper class with `[MSBuildMultiThreadableTaskAnalyzed]` attribute | MSBuildTask0001–MSBuildTask0004 | +| Helper class with `[MSBuildMultiThreadableTaskAnalyzed]` attribute | MSBuildTask0001–MSBuildTask0005 | | Regular class (no task interface or attribute) | Not analyzed | -Create a `.globalconfig` file to analyze regular tasks for MSBuildTask0001–MSBuildTask0005 before MT migration: - -```ini -is_global = true -msbuild_task_analyzer.scope = all -``` - -Missing and unrecognized values use the safe `multithreadable_only` default. - MSBuildTask0006–MSBuildTask0008 apply only when the `[MSBuildMultiThreadableTask]` attribute is applied **directly** to the task class. The attribute is `Inherited = false`, so a task that merely derives from a base class implementing `IMultiThreadableTask` (or carrying the attribute) has not itself opted into multithreaded support and is not subject to these three rules. Input properties are collected from the task class **and its base classes**, so an `ITaskItem`/`string` input declared on a shared base task is still analyzed. The `[MSBuildMultiThreadableTaskAnalyzed]` attribute allows opting helper classes into **direct** analysis by the `MultiThreadableTaskAnalyzer` (MSBuildTask0001–0004). Without it, only classes implementing `ITask` receive per-line diagnostics and code fixes for those rules. The **transitive** analyzer (MSBuildTask0005) already discovers helpers via call graph analysis, but it reports only at the task entry point. Adding this attribute to a helper class gives you inline diagnostics and code fixes directly in the helper's source. @@ -316,8 +307,8 @@ The `[MSBuildMultiThreadableTaskAnalyzed]` attribute allows opting helper classe ### Severity Levels -- **MSBuildTask0001 and MSBuildTask0009** report as **Error** when their scope applies. -- **MSBuildTask0002–MSBuildTask0005 and MSBuildTask0010** report as **Warning** when their scope applies. +- **MSBuildTask0001** is always **Error** — these APIs are never safe in any MSBuild task. +- **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/SharedAnalyzerHelpers.cs b/src/TaskAnalyzer/SharedAnalyzerHelpers.cs index ba954c35633..3c8f9643cb1 100644 --- a/src/TaskAnalyzer/SharedAnalyzerHelpers.cs +++ b/src/TaskAnalyzer/SharedAnalyzerHelpers.cs @@ -17,8 +17,8 @@ namespace Microsoft.Build.TaskAuthoring.Analyzer internal static class SharedAnalyzerHelpers { /// - /// The analyzer configuration key controlling analysis scope. - /// Values: "multithreadable_only" (default) | "all" + /// The .editorconfig key controlling analysis scope. + /// Values: "all" (default) | "multithreadable_only" /// internal const string ScopeOptionKey = "msbuild_task_analyzer.scope"; internal const string ScopeAll = "all"; @@ -26,17 +26,17 @@ internal static class SharedAnalyzerHelpers /// /// Reads the scope option from the analyzer config options provider. - /// Returns true only when all-task migration analysis is explicitly enabled. + /// Returns true if all tasks should be analyzed; false if only multithreadable tasks. /// internal static bool ReadAnalyzeAllTasksOption(AnalyzerConfigOptionsProvider optionsProvider) { if (optionsProvider.GlobalOptions.TryGetValue($"build_property.{ScopeOptionKey}", out var scopeValue) || optionsProvider.GlobalOptions.TryGetValue(ScopeOptionKey, out scopeValue)) { - return string.Equals(scopeValue, ScopeAll, StringComparison.OrdinalIgnoreCase); + return !string.Equals(scopeValue, ScopeMultiThreadableOnly, StringComparison.OrdinalIgnoreCase); } - return false; + return true; // default: analyze all tasks } /// /// Represents a resolved banned API entry for O(1) lookup during analysis. diff --git a/src/TaskAnalyzer/TransitiveCallChainAnalyzer.cs b/src/TaskAnalyzer/TransitiveCallChainAnalyzer.cs index d5855578a9a..9dc9d26aa99 100644 --- a/src/TaskAnalyzer/TransitiveCallChainAnalyzer.cs +++ b/src/TaskAnalyzer/TransitiveCallChainAnalyzer.cs @@ -50,7 +50,7 @@ private void OnCompilationStart(CompilationStartAnalysisContext compilationConte return; } - // Read scope option + // Read scope option from .editorconfig bool analyzeAllTasks = SharedAnalyzerHelpers.ReadAnalyzeAllTasksOption(compilationContext.Options.AnalyzerConfigOptionsProvider); var iMultiThreadableTaskType = compilationContext.Compilation.GetTypeByMetadataName(WellKnownTypeNames.IMultiThreadableTaskFullName);