diff --git a/src/ThreadSafeTaskAnalyzer.Tests/MultiThreadableTaskAnalyzerTests.cs b/src/ThreadSafeTaskAnalyzer.Tests/MultiThreadableTaskAnalyzerTests.cs new file mode 100644 index 00000000000..6b7b6f0a3ad --- /dev/null +++ b/src/ThreadSafeTaskAnalyzer.Tests/MultiThreadableTaskAnalyzerTests.cs @@ -0,0 +1,1989 @@ +// 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 Shouldly; +using Xunit; +using static Microsoft.Build.TaskAuthoring.Analyzer.Tests.TestHelpers; + +namespace Microsoft.Build.TaskAuthoring.Analyzer.Tests; + +/// +/// Tests for covering all 4 diagnostic rules. +/// Uses manual compilation to avoid fragile message argument matching. +/// +public class MultiThreadableTaskAnalyzerTests +{ + // ═══════════════════════════════════════════════════════════════════════ + // MSBuildTask0001: Critical errors + // ═══════════════════════════════════════════════════════════════════════ + + [Fact] + public async Task ConsoleWriteLine_InAnyTask_ProducesError() + { + var diags = await GetDiagnosticsAsync(""" + using System; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + Console.WriteLine("hello"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.CriticalError); + diags.Length.ShouldBe(1); + } + + [Fact] + public async Task ConsoleWrite_MultipleOverloads_AllDetected() + { + var diags = await GetDiagnosticsAsync(""" + using System; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + Console.Write("a"); + Console.Write(42); + Console.Write(true); + Console.Write('c'); + return true; + } + } + """); + + diags.Where(d => d.Id == DiagnosticIds.CriticalError).Count().ShouldBe(4); + } + + [Fact] + public async Task ConsoleOut_PropertyAccess_ProducesError() + { + var diags = await GetDiagnosticsAsync(""" + using System; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + var writer = Console.Out; + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.CriticalError); + } + + [Fact] + public async Task EnvironmentExit_ProducesError() + { + var diags = await GetDiagnosticsAsync(""" + using System; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + Environment.Exit(1); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.CriticalError); + diags.Length.ShouldBe(1); + } + + [Fact] + public async Task EnvironmentFailFast_ProducesError() + { + var diags = await GetDiagnosticsAsync(""" + using System; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + Environment.FailFast("crash"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.CriticalError); + } + + [Fact] + public async Task ThreadPoolSetMinMaxThreads_ProducesError() + { + var diags = await GetDiagnosticsAsync(""" + using System.Threading; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + ThreadPool.SetMinThreads(4, 4); + ThreadPool.SetMaxThreads(16, 16); + return true; + } + } + """); + + diags.Where(d => d.Id == DiagnosticIds.CriticalError).Count().ShouldBe(2); + } + + [Fact] + public async Task CultureInfoDefaults_ProducesError() + { + var diags = await GetDiagnosticsAsync(""" + using System.Globalization; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + CultureInfo.DefaultThreadCurrentCulture = CultureInfo.InvariantCulture; + CultureInfo.DefaultThreadCurrentUICulture = CultureInfo.InvariantCulture; + return true; + } + } + """); + + diags.Where(d => d.Id == DiagnosticIds.CriticalError).Count().ShouldBe(2); + } + + [Fact] + public async Task ConsoleReadLine_ProducesError() + { + var diags = await GetDiagnosticsAsync(""" + using System; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + var input = Console.ReadLine(); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.CriticalError); + } + + [Fact] + public async Task ProcessKill_InAnyTask_ProducesError() + { + var diags = await GetDiagnosticsAsync(""" + using System.Diagnostics; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + var p = Process.GetCurrentProcess(); + p.Kill(); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.CriticalError); + } + + [Fact] + public async Task MSBuildTask0001_FiresForRegularTask() + { + var diags = await GetDiagnosticsAsync(""" + using System; + public class RegularTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + Console.WriteLine("hello"); + Environment.Exit(1); + return true; + } + } + """); + + diags.Where(d => d.Id == DiagnosticIds.CriticalError).Count().ShouldBe(2); + } + + // ═══════════════════════════════════════════════════════════════════════ + // MSBuildTask0002: TaskEnvironment required (only for IMultiThreadableTask) + // ═══════════════════════════════════════════════════════════════════════ + + [Fact] + public async Task EnvironmentGetEnvVar_InMultiThreadableTask_ProducesWarning() + { + var diags = await GetDiagnosticsAsync(""" + using System; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + var val = Environment.GetEnvironmentVariable("PATH"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.TaskEnvironmentRequired); + diags.Length.ShouldBe(1); + } + + [Fact] + public async Task EnvironmentSetEnvVar_InMultiThreadableTask_ProducesWarning() + { + var diags = await GetDiagnosticsAsync(""" + using System; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + Environment.SetEnvironmentVariable("KEY", "VALUE"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.TaskEnvironmentRequired); + } + + [Fact] + public async Task EnvironmentCurrentDirectory_InMultiThreadableTask_ProducesWarning() + { + var diags = await GetDiagnosticsAsync(""" + using System; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + var dir = Environment.CurrentDirectory; + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.TaskEnvironmentRequired); + } + + [Fact] + public async Task PathGetFullPath_InMultiThreadableTask_ProducesWarning() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + var p = Path.GetFullPath("relative"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.TaskEnvironmentRequired); + } + + [Fact] + public async Task ProcessStart_InMultiThreadableTask_ProducesWarning() + { + var diags = await GetDiagnosticsAsync(""" + using System.Diagnostics; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + Process.Start("cmd.exe"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.TaskEnvironmentRequired); + } + + [Fact] + public async Task ProcessStartInfo_Constructor_ProducesWarning() + { + var diags = await GetDiagnosticsAsync(""" + using System.Diagnostics; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + var psi = new ProcessStartInfo("cmd.exe"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.TaskEnvironmentRequired); + } + + [Fact] + public async Task EnvironmentGetEnvVar_InRegularTask_ProducesDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + var val = Environment.GetEnvironmentVariable("PATH"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.TaskEnvironmentRequired); + } + + [Fact] + public async Task ExpandEnvironmentVariables_InMultiThreadableTask_ProducesWarning() + { + var diags = await GetDiagnosticsAsync(""" + using System; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + var result = Environment.ExpandEnvironmentVariables("%PATH%"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.TaskEnvironmentRequired); + } + + // ═══════════════════════════════════════════════════════════════════════ + // MSBuildTask0003: File path requires absolute (only for IMultiThreadableTask) + // ═══════════════════════════════════════════════════════════════════════ + + [Fact] + public async Task FileExists_WithStringArg_ProducesWarning() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + File.Exists("foo.txt"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + [Fact] + public async Task FileReadAllText_WithStringArg_ProducesWarning() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + var content = File.ReadAllText("file.txt"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + [Fact] + public async Task DirectoryExists_WithStringArg_ProducesWarning() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + Directory.Exists("mydir"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + [Fact] + public async Task NewFileInfo_WithStringArg_ProducesWarning() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + var fi = new FileInfo("file.txt"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + [Fact] + public async Task NewStreamReader_WithStringArg_ProducesWarning() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + using var sr = new StreamReader("file.txt"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + // ═══════════════════════════════════════════════════════════════════════ + // MSBuildTask0003 Safe Patterns: No diagnostic expected + // ═══════════════════════════════════════════════════════════════════════ + + [Fact] + public async Task FileExists_WithGetAbsolutePath_NoDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + File.Exists(TaskEnvironment.GetAbsolutePath("foo.txt")); + return true; + } + } + """); + + diags.ShouldNotContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + [Fact] + public async Task FileExists_WithAbsolutePathVariable_NoDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + AbsolutePath p = TaskEnvironment.GetAbsolutePath("foo.txt"); + File.Exists(p); + return true; + } + } + """); + + diags.ShouldNotContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + [Fact] + public async Task FileDelete_WithNullableAbsolutePathVariable_NoDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + AbsolutePath? filePath = TaskEnvironment.GetAbsolutePath("foo.txt"); + if (filePath.HasValue) + { + File.Delete(filePath.Value); + } + return true; + } + } + """); + + diags.ShouldNotContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + [Fact] + public async Task FileExists_WithGetMetadataFullPath_NoDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public ITaskItem[] Items { get; set; } + public override bool Execute() + { + foreach (var item in Items) + { + File.Exists(item.GetMetadata("FullPath")); + } + return true; + } + } + """); + + diags.ShouldNotContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + [Fact] + public async Task FileExists_WithFullNameProperty_NoDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + var fi = new FileInfo(TaskEnvironment.GetAbsolutePath("path.txt")); + File.Exists(fi.FullName); + return true; + } + } + """); + + diags.ShouldNotContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + // ═══════════════════════════════════════════════════════════════════════ + // Safe wrapper recognition: Path.GetDirectoryName, Path.Combine, Path.GetFullPath + // ═══════════════════════════════════════════════════════════════════════ + + [Fact] + public async Task DirectoryCreate_WithGetDirectoryNameOfAbsolutePath_NoDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + AbsolutePath filePath = TaskEnvironment.GetAbsolutePath("file.txt"); + string dir = Path.GetDirectoryName(filePath); + Directory.CreateDirectory(dir); + return true; + } + } + """); + + diags.ShouldNotContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + [Fact] + public async Task DirectoryCreate_WithGetDirectoryNameOfString_ProducesDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + string dir = Path.GetDirectoryName("relative/file.txt"); + Directory.CreateDirectory(dir); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + [Fact] + public async Task FileInfo_WithPathCombineSafeBase_NoDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + AbsolutePath baseDir = TaskEnvironment.GetAbsolutePath("dir"); + string combined = Path.Combine(baseDir, "sub", "file.txt"); + var fi = new FileInfo(combined); + return true; + } + } + """); + + diags.ShouldNotContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + [Fact] + public async Task FileInfo_WithPathCombineUnsafeBase_ProducesDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + string combined = Path.Combine("relative", "file.txt"); + var fi = new FileInfo(combined); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + [Fact] + public async Task FileInfo_WithGetFullPathOfAbsolutePath_NoDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + AbsolutePath basePath = TaskEnvironment.GetAbsolutePath("dir"); + string fullPath = Path.GetFullPath(basePath); + var fi = new FileInfo(fullPath); + return true; + } + } + """); + + diags.ShouldNotContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + [Fact] + public async Task FileInfo_WithGetFullPathOfRelative_ProducesDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + string fullPath = Path.GetFullPath("relative.txt"); + var fi = new FileInfo(fullPath); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + [Fact] + public async Task FileWriteAllText_WithNestedPathCombineGetDirectoryName_NoDiagnostic() + { + // Simulates WriteLinesToFile pattern: Path.Combine(Path.GetDirectoryName(AbsolutePath), random) + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + AbsolutePath filePath = TaskEnvironment.GetAbsolutePath("file.txt"); + string dir = Path.GetDirectoryName(filePath); + string tempPath = Path.Combine(dir, Path.GetRandomFileName() + "~"); + File.WriteAllText(tempPath, "contents"); + return true; + } + } + """); + + diags.ShouldNotContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + [Fact] + public async Task FileInfo_WithPathCombineOfFullName_NoDiagnostic() + { + // Simulates DownloadFile pattern: Path.Combine(DirectoryInfo.FullName, filename) + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + AbsolutePath dirPath = TaskEnvironment.GetAbsolutePath("dir"); + DirectoryInfo di = new DirectoryInfo(dirPath); + string combined = Path.Combine(di.FullName, "file.txt"); + var fi = new FileInfo(combined); + return true; + } + } + """); + + diags.ShouldNotContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + [Fact] + public async Task FileInfo_WithGetFullPathOfPathCombineSafe_NoDiagnostic() + { + // Simulates Unzip pattern: Path.GetFullPath(Path.Combine(DirectoryInfo.FullName, entry)) + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + AbsolutePath dirPath = TaskEnvironment.GetAbsolutePath("dir"); + DirectoryInfo di = new DirectoryInfo(dirPath); + string fullPath = Path.GetFullPath(Path.Combine(di.FullName, "sub/entry")); + var fi = new FileInfo(fullPath); + return true; + } + } + """); + + diags.ShouldNotContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + [Fact] + public async Task FileApi_InRegularTask_ProducesDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + File.Exists("foo.txt"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + [Fact] + public async Task DirectoryInfoFullName_SafePattern_NoDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + var di = new DirectoryInfo(TaskEnvironment.GetAbsolutePath("mydir")); + Directory.Exists(di.FullName); + return true; + } + } + """); + + diags.ShouldNotContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + // ═══════════════════════════════════════════════════════════════════════ + // MSBuildTask0004: Potential issues (review required) + // ═══════════════════════════════════════════════════════════════════════ + + [Fact] + public async Task AssemblyLoadFrom_ProducesWarning() + { + var diags = await GetDiagnosticsAsync(""" + using System.Reflection; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + Assembly.LoadFrom("mylib.dll"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.PotentialIssue); + } + + [Fact] + public async Task AssemblyLoad_ByteArray_ProducesWarning() + { + var diags = await GetDiagnosticsAsync(""" + using System.Reflection; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + Assembly.Load(new byte[0]); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.PotentialIssue); + } + + // ═══════════════════════════════════════════════════════════════════════ + // Non-task classes: No diagnostics + // ═══════════════════════════════════════════════════════════════════════ + + [Fact] + public async Task NonTaskClass_NoDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System; + using System.IO; + public class NotATask + { + public void DoStuff() + { + Console.WriteLine("hello"); + File.Exists("foo.txt"); + Environment.Exit(1); + var psi = new System.Diagnostics.ProcessStartInfo("cmd.exe"); + } + } + """); + + diags.ShouldBeEmpty(); + } + + // ═══════════════════════════════════════════════════════════════════════ + // Edge Cases + // ═══════════════════════════════════════════════════════════════════════ + + [Fact] + public async Task Lambda_InsideTask_DetectsUnsafeApi() + { + var diags = await GetDiagnosticsAsync(""" + using System; + using System.Collections.Generic; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + var items = new List { "a", "b" }; + items.ForEach(x => Console.WriteLine(x)); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.CriticalError); + } + + [Fact] + public async Task DerivedTask_InheritsITask_DetectsUnsafeApi() + { + var diags = await GetDiagnosticsAsync(""" + using System; + public class BaseTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() => true; + } + public class DerivedTask : BaseTask + { + public void DoWork() + { + Console.WriteLine("derived"); + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.CriticalError); + } + + [Fact] + public async Task GenericTask_DetectsUnsafeApi() + { + var diags = await GetDiagnosticsAsync(""" + using System; + using Microsoft.Build.Framework; + public class GenericTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + Console.WriteLine(typeof(T)); + var val = Environment.GetEnvironmentVariable("X"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.CriticalError); + diags.ShouldContain(d => d.Id == DiagnosticIds.TaskEnvironmentRequired); + } + + [Fact] + public async Task PropertyGetter_WithUnsafeApi_DetectsIt() + { + var diags = await GetDiagnosticsAsync(""" + using System; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string Dir => Environment.CurrentDirectory; + public override bool Execute() => true; + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.TaskEnvironmentRequired); + } + + [Fact] + public async Task MethodReference_AsDelegate_DetectsIt() + { + var diags = await GetDiagnosticsAsync(""" + using System; + using System.Collections.Generic; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + var items = new List { "a", "b" }; + items.ForEach(Console.WriteLine); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.CriticalError); + } + + [Fact] + public async Task MultipleUnsafeApis_AllDetected() + { + var diags = await GetDiagnosticsAsync(""" + using System; + using System.IO; + using System.Reflection; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + Console.WriteLine("hello"); + var val = Environment.GetEnvironmentVariable("X"); + File.Exists("foo.txt"); + Assembly.LoadFrom("lib.dll"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.CriticalError); + diags.ShouldContain(d => d.Id == DiagnosticIds.TaskEnvironmentRequired); + diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + diags.ShouldContain(d => d.Id == DiagnosticIds.PotentialIssue); + diags.Length.ShouldBe(4); + } + + [Fact] + public async Task CorrectlyMigratedTask_NoDiagnostics() + { + var diags = await GetDiagnosticsAsync(""" + using System; + using System.IO; + using Microsoft.Build.Framework; + public class CorrectTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public ITaskItem[] Items { get; set; } + public override bool Execute() + { + var dir = TaskEnvironment.ProjectDirectory; + var env = TaskEnvironment.GetEnvironmentVariable("X"); + TaskEnvironment.SetEnvironmentVariable("X", "val"); + AbsolutePath abs = TaskEnvironment.GetAbsolutePath("file.txt"); + File.Exists(abs); + File.ReadAllText(TaskEnvironment.GetAbsolutePath("other.txt")); + + foreach (var item in Items) + { + File.Exists(item.GetMetadata("FullPath")); + } + return true; + } + } + """); + + diags.ShouldBeEmpty(); + } + + [Fact] + public async Task FullyQualifiedConsole_DetectsIt() + { + var diags = await GetDiagnosticsAsync(""" + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + System.Console.WriteLine("hello"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.CriticalError); + } + + [Fact] + public async Task TryCatchFinally_DetectsAllUnsafeApis() + { + var diags = await GetDiagnosticsAsync(""" + using System; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + try { Console.WriteLine("try"); } + catch { Console.WriteLine("catch"); } + finally { Console.WriteLine("finally"); } + return true; + } + } + """); + + diags.Where(d => d.Id == DiagnosticIds.CriticalError).Count().ShouldBe(3); + } + + [Fact] + public async Task AsyncMethod_InsideTask_DetectsUnsafeApi() + { + var diags = await GetDiagnosticsAsync(""" + using System; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + ExecuteAsync().Wait(); + return true; + } + private async System.Threading.Tasks.Task ExecuteAsync() + { + await System.Threading.Tasks.Task.Delay(1); + Console.WriteLine("async"); + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.CriticalError); + } + + [Fact] + public async Task StringInterpolation_WithConsole_DetectsIt() + { + var diags = await GetDiagnosticsAsync(""" + using System; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + var name = "world"; + Console.WriteLine($"Hello {name}"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.CriticalError); + } + + [Fact] + public async Task EmptyTask_NoDiagnostics() + { + var diags = await GetDiagnosticsAsync(""" + public class EmptyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() => true; + } + """); + + diags.ShouldBeEmpty(); + } + + [Fact] + public async Task GetMetadata_NonFullPath_StillTriggersWarning() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public ITaskItem[] Items { get; set; } + public override bool Execute() + { + foreach (var item in Items) + { + File.Exists(item.GetMetadata("Identity")); + } + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + [Fact] + public async Task NestedClass_NotTask_NoFalsePositive() + { + var diags = await GetDiagnosticsAsync(""" + using System; + public class OuterTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + new Inner().DoWork(); + return true; + } + private class Inner + { + public void DoWork() { Console.WriteLine("nested"); } + } + } + """); + + // Inner class is not an ITask - should NOT get diagnostics + diags.ShouldBeEmpty(); + } + + [Fact] + public async Task MSBuildTask0002_FiredForRegularTask() + { + var diags = await GetDiagnosticsAsync(""" + using System; + using System.IO; + using System.Diagnostics; + public class RegularTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + Environment.GetEnvironmentVariable("X"); + Path.GetFullPath("foo"); + Process.Start("cmd.exe"); + var psi = new ProcessStartInfo("cmd.exe"); + return true; + } + } + """); + + // MSBuildTask0002 now fires for all ITask implementations + diags.ShouldContain(d => d.Id == DiagnosticIds.TaskEnvironmentRequired); + } + + [Fact] + public async Task MSBuildTask0003_FiredForRegularTask() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + public class RegularTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + File.Exists("foo.txt"); + new FileInfo("bar.txt"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + [Fact] + public async Task XDocumentSave_WithStringPath_ProducesDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System.Xml.Linq; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + var doc = new XDocument(); + doc.Save("output.xml"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + [Fact] + public async Task XmlReaderCreate_WithStringPath_ProducesDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System.Xml; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + using var reader = XmlReader.Create("input.xml"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + [Fact] + public async Task ZipFileOpenRead_WithStringPath_ProducesDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO.Compression; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + using var archive = ZipFile.OpenRead("archive.zip"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + // ═══════════════════════════════════════════════════════════════════════ + // Iteration 9-13: New APIs and features + // ═══════════════════════════════════════════════════════════════════════ + + [Fact] + public async Task DirectorySetCurrentDirectory_ProducesCriticalError() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + Directory.SetCurrentDirectory("/tmp"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.CriticalError); + } + + [Fact] + public async Task DirectoryGetCurrentDirectory_InMultiThreadable_ProducesWarning() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + var dir = Directory.GetCurrentDirectory(); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.TaskEnvironmentRequired); + } + + [Fact] + public async Task PathGetTempPath_InMultiThreadable_ProducesWarning() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + var tmp = Path.GetTempPath(); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.TaskEnvironmentRequired); + } + + [Fact] + public async Task PathGetTempFileName_InMultiThreadable_ProducesWarning() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + var f = Path.GetTempFileName(); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.TaskEnvironmentRequired); + } + + [Fact] + public async Task EnvironmentGetFolderPath_InMultiThreadable_ProducesWarning() + { + var diags = await GetDiagnosticsAsync(""" + using System; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + var p = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.TaskEnvironmentRequired); + } + + [Fact] + public async Task ConsoleSetOut_TypeLevelBan_ProducesError() + { + var diags = await GetDiagnosticsAsync(""" + using System; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + Console.ResetColor(); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.CriticalError); + } + + [Fact] + public async Task ConsoleForegroundColor_TypeLevelBan_ProducesError() + { + var diags = await GetDiagnosticsAsync(""" + using System; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + Console.ForegroundColor = ConsoleColor.Red; + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.CriticalError); + } + + [Fact] + public async Task ConsoleTitle_TypeLevelBan_ProducesError() + { + var diags = await GetDiagnosticsAsync(""" + using System; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + var t = Console.Title; + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.CriticalError); + } + + [Fact] + public async Task ProcessStartWithPSI_InMultiThreadable_ProducesWarning() + { + var diags = await GetDiagnosticsAsync(""" + using System.Diagnostics; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + var psi = new ProcessStartInfo("cmd"); + Process.Start(psi); + return true; + } + } + """); + + // Should flag both: new ProcessStartInfo and Process.Start(ProcessStartInfo) + diags.Where(d => d.Id == DiagnosticIds.TaskEnvironmentRequired).Count().ShouldBeGreaterThanOrEqualTo(2); + } + + // ═══════════════════════════════════════════════════════════════════════ + // [MSBuildMultiThreadableTaskAnalyzed] attribute on helper classes + // ═══════════════════════════════════════════════════════════════════════ + + [Fact] + public async Task HelperClass_WithAttribute_AnalyzedLikeMultiThreadableTask() + { + var diags = await GetDiagnosticsAsync(""" + using System; + using System.IO; + using Microsoft.Build.Framework; + + [MSBuildMultiThreadableTaskAnalyzed] + public class FileHelper + { + public void ProcessFile(string path) + { + File.Exists(path); + var env = Environment.GetEnvironmentVariable("PATH"); + } + } + """); + + // Should detect File.Exists with unwrapped path (MSBuildTask0003) + diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + // Should detect Environment.GetEnvironmentVariable (MSBuildTask0002) + diags.ShouldContain(d => d.Id == DiagnosticIds.TaskEnvironmentRequired); + } + + [Fact] + public async Task HelperClass_WithoutAttribute_NotAnalyzed() + { + var diags = await GetDiagnosticsAsync(""" + using System; + using System.IO; + + public class RegularHelper + { + public void ProcessFile(string path) + { + File.Exists(path); + var env = Environment.GetEnvironmentVariable("PATH"); + Console.WriteLine("hello"); + } + } + """); + + diags.ShouldBeEmpty(); + } + + [Fact] + public async Task HelperClass_WithAttribute_ConsoleDetected() + { + var diags = await GetDiagnosticsAsync(""" + using System; + using Microsoft.Build.Framework; + + [MSBuildMultiThreadableTaskAnalyzed] + public class LogHelper + { + public void Log(string message) + { + Console.WriteLine(message); + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.CriticalError); + } + + [Fact] + public async Task HelperClass_WithAttribute_SafePatterns_NoDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + + [MSBuildMultiThreadableTaskAnalyzed] + public class SafeHelper + { + public void Process(TaskEnvironment env, ITaskItem item) + { + AbsolutePath abs = env.GetAbsolutePath("file.txt"); + File.Exists(abs); + File.ReadAllText(item.GetMetadata("FullPath")); + } + } + """); + + diags.Where(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute).ShouldBeEmpty(); + } + + // ═══════════════════════════════════════════════════════════════════════ + // Multi-path parameter correctness (File.Copy has 2 path args) + // ═══════════════════════════════════════════════════════════════════════ + + [Fact] + public async Task FileCopy_SecondArgUnwrapped_ProducesDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + AbsolutePath src = TaskEnvironment.GetAbsolutePath("src.txt"); + File.Copy(src, "dest.txt"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + [Fact] + public async Task FileCopy_BothArgsWrapped_NoDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + AbsolutePath src = TaskEnvironment.GetAbsolutePath("src.txt"); + AbsolutePath dst = TaskEnvironment.GetAbsolutePath("dst.txt"); + File.Copy(src, dst); + return true; + } + } + """); + + diags.Where(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute).ShouldBeEmpty(); + } + + [Fact] + public async Task FileCopy_FirstArgUnwrapped_ProducesDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + AbsolutePath dst = TaskEnvironment.GetAbsolutePath("dst.txt"); + File.Copy("src.txt", dst); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + // ═══════════════════════════════════════════════════════════════════════ + // Edge cases: LINQ lambdas, nested types, string literals + // ═══════════════════════════════════════════════════════════════════════ + + [Fact] + public async Task Lambda_FileExistsInsideLambda_ProducesDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System; + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + Func check = path => File.Exists(path); + return check("test.txt"); + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + [Fact] + public async Task NestedClass_InsideTask_NotAnalyzedSeparately() + { + // Nested class within a task should NOT independently trigger analysis + // (it's not a task itself and doesn't have the attribute) + var diags = await GetDiagnosticsAsync(""" + using System; + using System.IO; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + var helper = new Helper(); + return true; + } + + private class Helper + { + public void DoWork() + { + File.Exists("foo.txt"); + Console.WriteLine("nested"); + } + } + } + """); + + // The outer task has Console.* in its scope but NOT in nested class + // Nested class operations are NOT in the outer type's symbol scope + diags.Where(d => d.Id == DiagnosticIds.CriticalError).ShouldBeEmpty(); + } + + [Fact] + public async Task StringInterpolation_ConsoleWithInterpolation_ProducesDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + int x = 42; + Console.WriteLine($"value = {x}"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.CriticalError); + } + + [Fact] + public async Task StaticMethod_InTask_DetectsUnsafeApis() + { + var diags = await GetDiagnosticsAsync(""" + using System; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + LogMessage("hello"); + return true; + } + + private static void LogMessage(string msg) + { + Console.WriteLine(msg); + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.CriticalError); + } + + [Fact] + public async Task PropertyGetter_WithBannedApi_ProducesDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string CurrentDir => Environment.CurrentDirectory; + public override bool Execute() => true; + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.TaskEnvironmentRequired); + } + + [Fact] + public async Task AsyncHelper_WithBannedApi_ProducesDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System; + using System.IO; + using System.Threading.Tasks; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + ProcessAsync().Wait(); + return true; + } + + private async Task ProcessAsync() + { + File.Exists("file.txt"); + await Task.Delay(1); + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + [Fact] + public async Task ConditionalAccess_WithBannedApi_ProducesDiagnostic() + { + var diags = await GetDiagnosticsAsync(""" + using System; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string Path { get; set; } + public override bool Execute() + { + var dir = System.IO.Path.GetFullPath(Path ?? "."); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.TaskEnvironmentRequired); + } + + [Fact] + public async Task FileApi_WithConstantPath_StillProducesDiagnostic() + { + // Even constant paths need absolutization - the task might be invoked from different directories + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + private const string LogFile = "build.log"; + public override bool Execute() + { + File.WriteAllText(LogFile, "done"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + [Fact] + public async Task MultipleViolationsInSingleMethod_AllDetected() + { + var diags = await GetDiagnosticsAsync(""" + using System; + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + Console.WriteLine("a"); + var env = Environment.GetEnvironmentVariable("X"); + File.Exists("foo"); + Console.ReadLine(); + return true; + } + } + """); + + diags.Where(d => d.Id == DiagnosticIds.CriticalError).Count().ShouldBeGreaterThanOrEqualTo(2); + diags.ShouldContain(d => d.Id == DiagnosticIds.TaskEnvironmentRequired); + diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + } + + // ═══════════════════════════════════════════════════════════════════════ + // [MSBuildMultiThreadableTask] attribute on task classes + // ═══════════════════════════════════════════════════════════════════════ + + [Fact] + public async Task Task_WithMultiThreadableAttribute_AnalyzedForAllRules() + { + // A task with [MSBuildMultiThreadableTask] but NOT IMultiThreadableTask + // should still get MSBuildTask0002 and MSBuildTask0003 + var diags = await GetDiagnosticsAsync(""" + using System; + using System.IO; + using Microsoft.Build.Framework; + + [MSBuildMultiThreadableTask] + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + File.Exists("foo.txt"); + var env = Environment.GetEnvironmentVariable("PATH"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + diags.ShouldContain(d => d.Id == DiagnosticIds.TaskEnvironmentRequired); + } + + [Fact] + public async Task Task_WithoutMultiThreadableAttribute_GetsAllRules() + { + // All rules now fire on all ITask implementations + var diags = await GetDiagnosticsAsync(""" + using System; + using System.IO; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + File.Exists("foo.txt"); + var env = Environment.GetEnvironmentVariable("PATH"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute); + diags.ShouldContain(d => d.Id == DiagnosticIds.TaskEnvironmentRequired); + } + + [Fact] + public async Task UsingStaticConsole_DetectedByTypeLevelBan() + { + var diags = await GetDiagnosticsAsync(""" + using static System.Console; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + WriteLine("hello from using static"); + return true; + } + } + """); + + diags.ShouldContain(d => d.Id == DiagnosticIds.CriticalError); + } + + [Fact] + public async Task FileWriteAllText_NonPathStringParam_NoDiagnosticForContents() + { + // File.WriteAllText(string path, string contents) - "contents" should NOT be flagged + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + AbsolutePath p = TaskEnvironment.GetAbsolutePath("file.txt"); + File.WriteAllText(p, "contents here"); + return true; + } + } + """); + + diags.Where(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute).ShouldBeEmpty(); + } + + [Fact] + public async Task FileAppendAllText_PathUnwrapped_FlagsPath() + { + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + File.AppendAllText("file.txt", "contents"); + return true; + } + } + """); + + // Should flag the path parameter but NOT the contents parameter + diags.Where(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute).Count().ShouldBe(1); + } + + [Fact] + public async Task FileWriteAllText_NamedArguments_HandledCorrectly() + { + // Named arguments can change the order of arguments in source vs parameters + var diags = await GetDiagnosticsAsync(""" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + // Named argument puts "contents" first in source, but "path" is still the path param + File.WriteAllText(contents: "some text", path: "file.txt"); + return true; + } + } + """); + + // Should still flag the path parameter + diags.Where(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute).Count().ShouldBe(1); + } +} diff --git a/src/ThreadSafeTaskAnalyzer.Tests/MultiThreadableTaskCodeFixProviderTests.cs b/src/ThreadSafeTaskAnalyzer.Tests/MultiThreadableTaskCodeFixProviderTests.cs new file mode 100644 index 00000000000..7425b201373 --- /dev/null +++ b/src/ThreadSafeTaskAnalyzer.Tests/MultiThreadableTaskCodeFixProviderTests.cs @@ -0,0 +1,246 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.CSharp.Testing; +using Microsoft.CodeAnalysis.Testing; +using Xunit; +using static Microsoft.Build.TaskAuthoring.Analyzer.Tests.TestHelpers; + +namespace Microsoft.Build.TaskAuthoring.Analyzer.Tests; + +/// +/// Tests for . +/// Uses CSharpCodeFixTest for verifying code transformations. +/// Arguments are provided with nullable annotations matching .NET 8+ BCL. +/// +public class MultiThreadableTaskCodeFixProviderTests +{ + private static CSharpCodeFixTest CreateFixTest( + string testCode, string fixedCode, params DiagnosticResult[] expected) + { + var test = new CSharpCodeFixTest + { + TestCode = testCode, + FixedCode = fixedCode, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + }; + test.TestState.Sources.Add(("Stubs.cs", FrameworkStubs)); + test.FixedState.Sources.Add(("Stubs.cs", FrameworkStubs)); + test.ExpectedDiagnostics.AddRange(expected); + return test; + } + + private static DiagnosticResult Diag(string id) => + CSharpAnalyzerVerifier.Diagnostic(id); + + [Fact] + public async Task Fix_GetEnvironmentVariable() + { + await CreateFixTest( + testCode: """ + using System; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + var val = {|#0:Environment.GetEnvironmentVariable("PATH")|}; + return true; + } + } + """, + fixedCode: """ + using System; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + var val = TaskEnvironment.GetEnvironmentVariable("PATH"); + return true; + } + } + """, + Diag(DiagnosticIds.TaskEnvironmentRequired).WithLocation(0) + .WithArguments("Environment.GetEnvironmentVariable(string)", "use TaskEnvironment.GetEnvironmentVariable instead") + ).RunAsync(); + } + + [Fact] + public async Task Fix_SetEnvironmentVariable() + { + await CreateFixTest( + testCode: """ + using System; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + {|#0:Environment.SetEnvironmentVariable("KEY", "VALUE")|}; + return true; + } + } + """, + fixedCode: """ + using System; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + TaskEnvironment.SetEnvironmentVariable("KEY", "VALUE"); + return true; + } + } + """, + Diag(DiagnosticIds.TaskEnvironmentRequired).WithLocation(0) + .WithArguments("Environment.SetEnvironmentVariable(string, string?)", "use TaskEnvironment.SetEnvironmentVariable instead") + ).RunAsync(); + } + + [Fact] + public async Task Fix_PathGetFullPath() + { + await CreateFixTest( + testCode: """ + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + var p = {|#0:Path.GetFullPath("relative")|}; + return true; + } + } + """, + fixedCode: """ + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + var p = TaskEnvironment.GetAbsolutePath("relative"); + return true; + } + } + """, + Diag(DiagnosticIds.TaskEnvironmentRequired).WithLocation(0) + .WithArguments("Path.GetFullPath(string)", "use TaskEnvironment.GetAbsolutePath instead") + ).RunAsync(); + } + + [Fact] + public async Task Fix_EnvironmentCurrentDirectory() + { + await CreateFixTest( + testCode: """ + using System; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + var dir = {|#0:Environment.CurrentDirectory|}; + return true; + } + } + """, + fixedCode: """ + using System; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + var dir = TaskEnvironment.ProjectDirectory; + return true; + } + } + """, + Diag(DiagnosticIds.TaskEnvironmentRequired).WithLocation(0) + .WithArguments("Environment.CurrentDirectory", "use TaskEnvironment.ProjectDirectory instead") + ).RunAsync(); + } + + [Fact] + public async Task Fix_FileExists_WrapsWithGetAbsolutePath() + { + await CreateFixTest( + testCode: """ + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + {|#0:File.Exists("foo.txt")|}; + return true; + } + } + """, + fixedCode: """ + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + File.Exists(TaskEnvironment.GetAbsolutePath("foo.txt")); + return true; + } + } + """, + Diag(DiagnosticIds.FilePathRequiresAbsolute).WithLocation(0) + .WithArguments("File.Exists(string?)", "wrap path argument with TaskEnvironment.GetAbsolutePath()") + ).RunAsync(); + } + + [Fact] + public async Task Fix_NewFileInfo_WrapsWithGetAbsolutePath() + { + await CreateFixTest( + testCode: """ + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + var fi = {|#0:new FileInfo("file.txt")|}; + return true; + } + } + """, + fixedCode: """ + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + var fi = new FileInfo(TaskEnvironment.GetAbsolutePath("file.txt")); + return true; + } + } + """, + Diag(DiagnosticIds.FilePathRequiresAbsolute).WithLocation(0) + .WithArguments("new FileInfo(...)", "wrap path argument with TaskEnvironment.GetAbsolutePath()") + ).RunAsync(); + } +} diff --git a/src/ThreadSafeTaskAnalyzer.Tests/TestHelpers.cs b/src/ThreadSafeTaskAnalyzer.Tests/TestHelpers.cs new file mode 100644 index 00000000000..a92fdf9cbdc --- /dev/null +++ b/src/ThreadSafeTaskAnalyzer.Tests/TestHelpers.cs @@ -0,0 +1,194 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Immutable; +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.Build.TaskAuthoring.Analyzer.Tests; + +/// +/// Helpers for setting up analyzer tests with MSBuild Framework stubs. +/// Uses manual compilation + analyzer invocation rather than CSharpAnalyzerTest +/// to avoid strict message argument comparison issues with nullable annotations. +/// +internal static class TestHelpers +{ + /// + /// Minimal stubs for ITask, IMultiThreadableTask, TaskEnvironment, AbsolutePath, and ITaskItem. + /// + public const string FrameworkStubs = """ + namespace Microsoft.Build.Framework + { + public interface IBuildEngine { } + + public interface ITask + { + IBuildEngine BuildEngine { get; set; } + bool Execute(); + } + + public interface IMultiThreadableTask : ITask + { + TaskEnvironment TaskEnvironment { get; set; } + } + + public class TaskEnvironment + { + public string ProjectDirectory { get; } + public string GetEnvironmentVariable(string name) => null; + public void SetEnvironmentVariable(string name, string value) { } + public System.Collections.IDictionary GetEnvironmentVariables() => null; + public AbsolutePath GetAbsolutePath(string path) => default; + public System.Diagnostics.ProcessStartInfo GetProcessStartInfo() => null; + } + + public struct AbsolutePath + { + public string Value { get; } + public string OriginalValue { get; } + public static implicit operator string(AbsolutePath p) => p.Value; + } + + public interface ITaskItem + { + string ItemSpec { get; set; } + string GetMetadata(string metadataName); + } + + public class TaskItem : ITaskItem + { + public string ItemSpec { get; set; } + public string GetMetadata(string metadataName) => null; + public string GetMetadataValue(string metadataName) => null; + } + + [System.AttributeUsage(System.AttributeTargets.Class)] + public class MSBuildMultiThreadableTaskAnalyzedAttribute : System.Attribute { } + + [System.AttributeUsage(System.AttributeTargets.Class)] + public class MSBuildMultiThreadableTaskAttribute : System.Attribute { } + } + + namespace Microsoft.Build.Utilities + { + public abstract class Task : Microsoft.Build.Framework.ITask + { + public Microsoft.Build.Framework.IBuildEngine BuildEngine { get; set; } + public abstract bool Execute(); + } + + public abstract class ToolTask : Task + { + protected abstract string ToolName { get; } + protected abstract string GenerateFullPathToTool(); + } + } + """; + + private static readonly MetadataReference[] s_coreReferences = CreateCoreReferences(); + + /// + /// Returns the core runtime references used by test compilations. + /// + 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. + /// + 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 on the given source code. + /// + 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; + } + + /// + /// Creates a compilation with the given source code and framework stubs. + /// + public static CSharpCompilation CreateCompilation(string source) + { + var syntaxTrees = new[] + { + CSharpSyntaxTree.ParseText(source, path: "Test.cs"), + CSharpSyntaxTree.ParseText(FrameworkStubs, path: "Stubs.cs"), + }; + + return CSharpCompilation.Create( + "TestAssembly", + syntaxTrees, + s_coreReferences, + new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary) + .WithNullableContextOptions(NullableContextOptions.Enable)); + } + + private static MetadataReference[] CreateCoreReferences() + { + // Reference the core runtime assemblies needed + var assemblies = new[] + { + typeof(object).Assembly, // System.Runtime / mscorlib + typeof(System.Console).Assembly, // System.Console + typeof(System.IO.File).Assembly, // System.IO.FileSystem + typeof(System.IO.FileInfo).Assembly, // System.IO.FileSystem + typeof(System.IO.StreamReader).Assembly, // System.IO + typeof(System.IO.FileStream).Assembly, // System.IO.FileSystem + typeof(System.Diagnostics.Process).Assembly, // System.Diagnostics.Process + typeof(System.Diagnostics.ProcessStartInfo).Assembly, + typeof(System.Reflection.Assembly).Assembly, // System.Reflection + typeof(System.Threading.ThreadPool).Assembly, // System.Threading.ThreadPool + typeof(System.Globalization.CultureInfo).Assembly, // System.Globalization + typeof(System.Collections.IDictionary).Assembly, // System.Collections + typeof(System.Collections.Generic.List<>).Assembly, + typeof(System.Linq.Enumerable).Assembly, // System.Linq + typeof(System.Threading.Tasks.Task).Assembly, // System.Threading.Tasks + typeof(System.Runtime.InteropServices.GuidAttribute).Assembly, // System.Runtime + typeof(System.Xml.Linq.XDocument).Assembly, // System.Xml.Linq + typeof(System.Xml.XmlReader).Assembly, // System.Xml.ReaderWriter + typeof(System.IO.Compression.ZipFile).Assembly, // System.IO.Compression.ZipFile + typeof(System.IO.Compression.ZipArchive).Assembly, // System.IO.Compression + }; + + var locations = assemblies + .Select(a => a.Location) + .Distinct() + .ToList(); + + // Ensure System.Runtime is included (needed for Emit on .NET 10+) + var runtimeDir = System.IO.Path.GetDirectoryName(typeof(object).Assembly.Location); + if (runtimeDir is not null) + { + var systemRuntime = System.IO.Path.Combine(runtimeDir, "System.Runtime.dll"); + if (System.IO.File.Exists(systemRuntime) && !locations.Contains(systemRuntime)) + { + locations.Add(systemRuntime); + } + } + + return locations + .Select(loc => (MetadataReference)MetadataReference.CreateFromFile(loc)) + .ToArray(); + } +} diff --git a/src/ThreadSafeTaskAnalyzer.Tests/ThreadSafeTaskAnalyzer.Tests.csproj b/src/ThreadSafeTaskAnalyzer.Tests/ThreadSafeTaskAnalyzer.Tests.csproj new file mode 100644 index 00000000000..079ba80fbc9 --- /dev/null +++ b/src/ThreadSafeTaskAnalyzer.Tests/ThreadSafeTaskAnalyzer.Tests.csproj @@ -0,0 +1,38 @@ + + + + net10.0 + 13.0 + enable + false + true + false + + true + + false + false + false + + + + + + + + + all + + + + + + + + + + + + + + diff --git a/src/ThreadSafeTaskAnalyzer.Tests/TransitiveCallChainAnalyzerTests.cs b/src/ThreadSafeTaskAnalyzer.Tests/TransitiveCallChainAnalyzerTests.cs new file mode 100644 index 00000000000..92e10c781ec --- /dev/null +++ b/src/ThreadSafeTaskAnalyzer.Tests/TransitiveCallChainAnalyzerTests.cs @@ -0,0 +1,1161 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Immutable; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; +using Shouldly; +using Xunit; +using static Microsoft.Build.TaskAuthoring.Analyzer.Tests.TestHelpers; + +namespace Microsoft.Build.TaskAuthoring.Analyzer.Tests; + +/// +/// Tests for — verifies that unsafe API usage +/// reachable through helper method calls is detected and reported with call chains. +/// +public class TransitiveCallChainAnalyzerTests +{ + [Fact] + public async Task HelperCallingConsole_TransitivelyFromTask_ProducesDiagnostic() + { + var diags = await GetAllDiagnosticsAsync(""" + using System; + public class Helper + { + public static void Log(string msg) { Console.WriteLine(msg); } + } + + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + Helper.Log("hello"); + return true; + } + } + """); + + var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ToArray(); + transitive.ShouldNotBeEmpty(); + transitive[0].GetMessage().ShouldContain("Console.WriteLine"); + transitive[0].GetMessage().ShouldContain("Helper.Log"); + } + + [Fact] + public async Task TwoLevelChain_HelperCallingHelperCallingBannedApi() + { + var diags = await GetAllDiagnosticsAsync(""" + using System; + public class InnerHelper + { + public static void DoExit() { Environment.Exit(1); } + } + public class OuterHelper + { + public static void Process() { InnerHelper.DoExit(); } + } + + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + OuterHelper.Process(); + return true; + } + } + """); + + var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ToArray(); + transitive.ShouldNotBeEmpty(); + var msg = transitive[0].GetMessage(); + msg.ShouldContain("Environment.Exit"); + // Chain should show: MyTask.Execute → OuterHelper.Process → InnerHelper.DoExit → Environment.Exit + msg.ShouldContain("OuterHelper.Process"); + msg.ShouldContain("InnerHelper.DoExit"); + } + + [Fact] + public async Task HelperCallingFileExists_WithoutAbsolutePath_ProducesDiagnostic() + { + var diags = await GetAllDiagnosticsAsync(""" + using System.IO; + public class FileHelper + { + public static bool CheckFile(string path) { return File.Exists(path); } + } + + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + FileHelper.CheckFile("test.txt"); + return true; + } + } + """); + + var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ToArray(); + transitive.ShouldNotBeEmpty(); + transitive[0].GetMessage().ShouldContain("File.Exists"); + } + + [Fact] + public async Task HelperCallingEnvironmentGetVar_ProducesDiagnostic() + { + var diags = await GetAllDiagnosticsAsync(""" + using System; + public class ConfigHelper + { + public static string GetConfig(string key) + { + return Environment.GetEnvironmentVariable(key); + } + } + + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + var val = ConfigHelper.GetConfig("MY_VAR"); + return true; + } + } + """); + + var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ToArray(); + transitive.ShouldNotBeEmpty(); + transitive[0].GetMessage().ShouldContain("GetEnvironmentVariable"); + } + + [Fact] + public async Task DirectCallInTask_NotReportedAsTransitive() + { + // Direct calls within the task should only produce direct diagnostics, not transitive + var diags = await GetAllDiagnosticsAsync(""" + using System; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + Console.WriteLine("direct"); + return true; + } + } + """); + + var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall); + transitive.ShouldBeEmpty(); + + var direct = diags.Where(d => d.Id == DiagnosticIds.CriticalError); + direct.ShouldNotBeEmpty(); + } + + [Fact] + public async Task SafeHelper_NoTransitiveDiagnostic() + { + var diags = await GetAllDiagnosticsAsync(""" + public class SafeHelper + { + public static int Add(int a, int b) => a + b; + } + + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + var result = SafeHelper.Add(1, 2); + return true; + } + } + """); + + var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall); + transitive.ShouldBeEmpty(); + } + + [Fact] + public async Task RecursiveCallChain_DoesNotStackOverflow() + { + var diags = await GetAllDiagnosticsAsync(""" + using System; + public class RecursiveHelper + { + public static void A() { B(); } + public static void B() { A(); Console.WriteLine("recurse"); } + } + + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + RecursiveHelper.A(); + return true; + } + } + """); + + // Should still detect the violation without infinite loop + var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ToArray(); + transitive.ShouldNotBeEmpty(); + transitive[0].GetMessage().ShouldContain("Console.WriteLine"); + } + + [Fact] + public async Task InstanceMethodHelper_TransitivelyDetected() + { + var diags = await GetAllDiagnosticsAsync(""" + using System; + public class Logger + { + public void Write(string msg) { Console.Write(msg); } + } + + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + var logger = new Logger(); + logger.Write("hello"); + return true; + } + } + """); + + var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ToArray(); + transitive.ShouldNotBeEmpty(); + transitive[0].GetMessage().ShouldContain("Console.Write"); + } + + [Fact] + public async Task MultipleViolationsInChain_AllReported() + { + var diags = await GetAllDiagnosticsAsync(""" + using System; + using System.IO; + public class UnsafeHelper + { + public static void DoStuff() + { + Console.WriteLine("log"); + Environment.Exit(1); + File.Exists("test.txt"); + } + } + + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + UnsafeHelper.DoStuff(); + return true; + } + } + """); + + var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ToArray(); + transitive.Length.ShouldBeGreaterThanOrEqualTo(3); + } + + [Fact] + public async Task ChainMessageFormat_ContainsArrowSeparatedMethods() + { + var diags = await GetAllDiagnosticsAsync(""" + using System; + public class A + { + public static void Step1() { B.Step2(); } + } + public class B + { + public static void Step2() { Environment.Exit(1); } + } + + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + A.Step1(); + return true; + } + } + """); + + var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ToArray(); + transitive.ShouldNotBeEmpty(); + var msg = transitive[0].GetMessage(); + // Should contain arrow-separated chain + msg.ShouldContain("→"); + msg.ShouldContain("A.Step1"); + msg.ShouldContain("B.Step2"); + } + + #region Cross-Assembly IL Analysis Tests + + /// + /// Creates a two-compilation setup: + /// 1. A "library" assembly compiled from librarySource + /// 2. A "task" assembly that references the library + /// Runs the transitive analyzer on the task assembly. + /// + private static async Task> GetCrossAssemblyDiagnosticsAsync( + string librarySource, string taskSource, string libraryAssemblyName = "TestLibrary") + { + // Step 1: Compile the library assembly + var libSyntaxTree = CSharpSyntaxTree.ParseText(librarySource, path: "Library.cs"); + var stubSyntaxTree = CSharpSyntaxTree.ParseText(FrameworkStubs, path: "Stubs.cs"); + + var coreRefs = GetCoreReferences(); + + var libCompilation = CSharpCompilation.Create( + libraryAssemblyName, + [libSyntaxTree, stubSyntaxTree], + coreRefs, + new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary) + .WithNullableContextOptions(NullableContextOptions.Enable)); + + // Emit to in-memory stream + using var libStream = new System.IO.MemoryStream(); + var emitResult = libCompilation.Emit(libStream); + emitResult.Success.ShouldBeTrue( + $"Library compilation failed: {string.Join(", ", emitResult.Diagnostics.Where(d => d.Severity == DiagnosticSeverity.Error))}"); + + libStream.Position = 0; + + // Write to temp file so PEReader can read it + var tempPath = System.IO.Path.Combine( + System.IO.Path.GetTempPath(), + $"{libraryAssemblyName}_{System.Guid.NewGuid():N}.dll"); + try + { + System.IO.File.WriteAllBytes(tempPath, libStream.ToArray()); + + // Step 2: Compile the task assembly referencing the library + var libReference = MetadataReference.CreateFromFile(tempPath); + var taskSyntaxTree = CSharpSyntaxTree.ParseText(taskSource, path: "Task.cs"); + var taskStubTree = CSharpSyntaxTree.ParseText(FrameworkStubs, path: "TaskStubs.cs"); + + var taskCompilation = CSharpCompilation.Create( + "TaskAssembly", + [taskSyntaxTree, taskStubTree], + coreRefs.Append(libReference).ToArray(), + new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary) + .WithNullableContextOptions(NullableContextOptions.Enable)); + + // Run only the transitive analyzer + var analyzer = new TransitiveCallChainAnalyzer(); + var compilationWithAnalyzers = taskCompilation.WithAnalyzers( + ImmutableArray.Create(analyzer)); + + return await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync(); + } + finally + { + try { System.IO.File.Delete(tempPath); } catch { } + } + } + + [Fact] + public async Task CrossAssembly_LibraryCallingConsole_DetectedViaIL() + { + var librarySource = """ + using System; + namespace MyLib + { + public static class LibHelper + { + public static void UnsafeLog(string msg) + { + Console.WriteLine(msg); + } + } + } + """; + + var taskSource = """ + using MyLib; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + LibHelper.UnsafeLog("hello"); + return true; + } + } + """; + + var diags = await GetCrossAssemblyDiagnosticsAsync(librarySource, taskSource); + + var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ToArray(); + transitive.ShouldNotBeEmpty("Should detect Console.WriteLine in referenced library"); + transitive[0].GetMessage().ShouldContain("Console"); + } + + [Fact] + public async Task CrossAssembly_LibraryCallingPathGetFullPath_DetectedViaIL() + { + var librarySource = """ + using System.IO; + namespace MyLib + { + public static class PathHelper + { + public static string ResolvePath(string relativePath) + { + return Path.GetFullPath(relativePath); + } + } + } + """; + + var taskSource = """ + using MyLib; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + PathHelper.ResolvePath("test.txt"); + return true; + } + } + """; + + var diags = await GetCrossAssemblyDiagnosticsAsync(librarySource, taskSource); + + var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ToArray(); + transitive.ShouldNotBeEmpty("Should detect Path.GetFullPath in referenced library"); + transitive[0].GetMessage().ShouldContain("GetFullPath"); + } + + [Fact] + public async Task CrossAssembly_SafeType_NotFlagged() + { + // Simulate AbsolutePath calling Path.GetFullPath internally — should NOT be flagged + var librarySource = """ + using System.IO; + namespace Microsoft.Build.Framework + { + public struct AbsolutePath + { + public string Value { get; } + public AbsolutePath(string path) { Value = path; } + public static implicit operator string(AbsolutePath p) => p.Value; + + public AbsolutePath GetCanonicalForm() + { + // Internally calls Path.GetFullPath — this should be suppressed + return new AbsolutePath(Path.GetFullPath(Value)); + } + } + + public class TaskEnvironment + { + public string ProjectDirectory { get; } + public string GetEnvironmentVariable(string name) => null; + public void SetEnvironmentVariable(string name, string value) { } + public System.Collections.IDictionary GetEnvironmentVariables() => null; + public AbsolutePath GetAbsolutePath(string path) => default; + public System.Diagnostics.ProcessStartInfo GetProcessStartInfo() => null; + } + + public interface IBuildEngine { } + public interface ITask + { + IBuildEngine BuildEngine { get; set; } + bool Execute(); + } + public interface IMultiThreadableTask : ITask + { + TaskEnvironment TaskEnvironment { get; set; } + } + public interface ITaskItem + { + string ItemSpec { get; set; } + string GetMetadata(string metadataName); + } + } + namespace Microsoft.Build.Utilities + { + public abstract class Task : Microsoft.Build.Framework.ITask + { + public Microsoft.Build.Framework.IBuildEngine BuildEngine { get; set; } + public abstract bool Execute(); + } + } + """; + + var taskSource = """ + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + var ap = new AbsolutePath("C:\\test"); + var canonical = ap.GetCanonicalForm(); + return true; + } + } + """; + + // For safe-type tests, compile without the default stubs + var libTree = CSharpSyntaxTree.ParseText(librarySource, path: "Lib.cs"); + var coreRefs = GetCoreReferences(); + + var libCompilation = CSharpCompilation.Create( + "FrameworkLib", + [libTree], + coreRefs, + new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary) + .WithNullableContextOptions(NullableContextOptions.Enable)); + + using var libStream = new System.IO.MemoryStream(); + var emitResult = libCompilation.Emit(libStream); + emitResult.Success.ShouldBeTrue( + $"Lib failed: {string.Join(", ", emitResult.Diagnostics.Where(d => d.Severity == DiagnosticSeverity.Error))}"); + + var tempPath = System.IO.Path.Combine( + System.IO.Path.GetTempPath(), + $"FrameworkLib_{System.Guid.NewGuid():N}.dll"); + try + { + System.IO.File.WriteAllBytes(tempPath, libStream.ToArray()); + var libRef = MetadataReference.CreateFromFile(tempPath); + + var taskTree = CSharpSyntaxTree.ParseText(taskSource, path: "Task.cs"); + var taskCompilation = CSharpCompilation.Create( + "TaskAssembly", + [taskTree], + coreRefs.Append(libRef).ToArray(), + new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary) + .WithNullableContextOptions(NullableContextOptions.Enable)); + + var analyzer = new TransitiveCallChainAnalyzer(); + var compilationWithAnalyzers = taskCompilation.WithAnalyzers( + ImmutableArray.Create(analyzer)); + var diags = await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync(); + + var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ToArray(); + // Path.GetFullPath inside AbsolutePath.GetCanonicalForm should be suppressed + var getFullPathViolations = transitive.Where(d => d.GetMessage().Contains("GetFullPath")).ToArray(); + getFullPathViolations.ShouldBeEmpty( + "Path.GetFullPath inside AbsolutePath (safe type) should not be flagged"); + } + finally + { + try { System.IO.File.Delete(tempPath); } catch { } + } + } + + [Fact] + public async Task CrossAssembly_DeepCallChain_DetectedViaIL() + { + var librarySource = """ + using System; + using System.IO; + namespace MyLib + { + public static class Level1 + { + public static void Start() { Level2.Middle(); } + } + public static class Level2 + { + public static void Middle() { Level3.End(); } + } + public static class Level3 + { + public static void End() { Environment.Exit(1); } + } + } + """; + + var taskSource = """ + using MyLib; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + Level1.Start(); + return true; + } + } + """; + + var diags = await GetCrossAssemblyDiagnosticsAsync(librarySource, taskSource); + + var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ToArray(); + transitive.ShouldNotBeEmpty("Should detect Environment.Exit through 3-level deep chain in library"); + var msg = transitive[0].GetMessage(); + msg.ShouldContain("Exit"); + msg.ShouldContain("→"); + } + + [Fact] + public async Task CrossAssembly_BclBoundary_NotTraversed() + { + // A library calling string.Format which internally calls... lots of stuff. + // We should NOT traverse into System.Private.CoreLib. + var librarySource = """ + namespace MyLib + { + public static class SafeHelper + { + public static string Format(string template, object arg) + { + return string.Format(template, arg); + } + } + } + """; + + var taskSource = """ + using MyLib; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + SafeHelper.Format("hello {0}", "world"); + return true; + } + } + """; + + var diags = await GetCrossAssemblyDiagnosticsAsync(librarySource, taskSource); + + var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ToArray(); + // string.Format is NOT a banned API, and we should not traverse INTO System.* assemblies + transitive.ShouldBeEmpty("BCL internals should not be traversed"); + } + + [Fact] + public async Task CrossAssembly_MixedSafeAndUnsafe_OnlyUnsafeFlagged() + { + var librarySource = """ + using System; + using System.IO; + namespace MyLib + { + public static class MixedHelper + { + public static string SafeMethod(string a, string b) + { + return a + b; // totally safe + } + + public static void UnsafeMethod() + { + Environment.Exit(42); + } + } + } + """; + + var taskSource = """ + using MyLib; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + var result = MixedHelper.SafeMethod("a", "b"); + MixedHelper.UnsafeMethod(); + return true; + } + } + """; + + var diags = await GetCrossAssemblyDiagnosticsAsync(librarySource, taskSource); + + var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ToArray(); + // Only UnsafeMethod → Environment.Exit should be flagged + transitive.ShouldNotBeEmpty(); + transitive.All(d => d.GetMessage().Contains("UnsafeMethod")).ShouldBeTrue( + "Only the unsafe method chain should be flagged"); + } + + [Fact] + public async Task CrossAssembly_PropertyGetter_CallingUnsafeApi_Detected() + { + var librarySource = """ + using System; + namespace MyLib + { + public class ConfigReader + { + public string CurrentDir + { + get { return Environment.CurrentDirectory; } + } + } + } + """; + + var taskSource = """ + using MyLib; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + var reader = new ConfigReader(); + var dir = reader.CurrentDir; + return true; + } + } + """; + + var diags = await GetCrossAssemblyDiagnosticsAsync(librarySource, taskSource); + var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ToArray(); + transitive.ShouldNotBeEmpty("Should detect Environment.CurrentDirectory in property getter via IL"); + } + + [Fact] + public async Task CrossAssembly_ConstructorCallingUnsafeApi_Detected() + { + var librarySource = """ + using System; + namespace MyLib + { + public class FileLogger + { + public FileLogger(string logPath) + { + // Call a banned API directly + Environment.Exit(99); + } + } + } + """; + + var taskSource = """ + using MyLib; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + var logger = new FileLogger("log.txt"); + return true; + } + } + """; + + var diags = await GetCrossAssemblyDiagnosticsAsync(librarySource, taskSource); + var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ToArray(); + transitive.ShouldNotBeEmpty("Should detect Environment.Exit in constructor via IL"); + } + + [Fact] + public async Task CrossAssembly_InstanceMethodChain_DetectedViaIL() + { + // Test that a library method calling Path.GetFullPath is detected + var librarySource = """ + namespace MyLib + { + public class Processor + { + public string Process(string input) + { + return System.IO.Path.GetFullPath(input); + } + } + } + """; + + var taskSource = """ + using MyLib; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + var p = new Processor(); + p.Process("test"); + return true; + } + } + """; + + var diags = await GetCrossAssemblyDiagnosticsAsync(librarySource, taskSource); + var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ToArray(); + transitive.ShouldNotBeEmpty("Should detect Path.GetFullPath in library method"); + } + + [Fact] + public async Task CrossAssembly_NoUnsafeCode_NoDiagnostics() + { + // Library that has no unsafe code should produce no violations + var librarySource = """ + namespace MyLib + { + public static class MathHelper + { + public static int Add(int a, int b) => a + b; + public static int Multiply(int a, int b) => a * b; + } + } + """; + + var taskSource = """ + using MyLib; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + int result = MathHelper.Add(1, 2); + return result > 0; + } + } + """; + + var diags = await GetCrossAssemblyDiagnosticsAsync(librarySource, taskSource); + var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ToArray(); + transitive.ShouldBeEmpty("Pure math helper should not produce any violations"); + } + + #endregion + + #region Multi-Assembly (nested package hierarchy) Tests + + /// + /// Compiles a chain of library assemblies (each referencing the previous), then + /// compiles a task assembly referencing the first library. Runs the transitive analyzer. + /// Libraries are ordered bottom-up: libraries[0] has no deps, libraries[1] refs libraries[0], etc. + /// The task references only the last library. + /// + private static async Task> GetMultiAssemblyDiagnosticsAsync( + (string source, string assemblyName)[] libraries, + string taskSource) + { + var coreRefs = GetCoreReferences(); + var stubTree = CSharpSyntaxTree.ParseText(FrameworkStubs, path: "Stubs.cs"); + var tempFiles = new System.Collections.Generic.List(); + var libReferences = new System.Collections.Generic.List(); + + try + { + // Compile libraries in order — each one can reference all previously compiled ones + foreach (var (source, assemblyName) in libraries) + { + var syntaxTree = CSharpSyntaxTree.ParseText(source, path: $"{assemblyName}.cs"); + var refs = coreRefs.Concat(libReferences).Append(MetadataReference.CreateFromFile(typeof(object).Assembly.Location)); + var allRefs = coreRefs.Concat(libReferences).ToArray(); + + var compilation = CSharpCompilation.Create( + assemblyName, + [syntaxTree, stubTree], + allRefs, + new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary) + .WithNullableContextOptions(NullableContextOptions.Enable)); + + using var stream = new System.IO.MemoryStream(); + var emitResult = compilation.Emit(stream); + emitResult.Success.ShouldBeTrue( + $"Library '{assemblyName}' compilation failed: {string.Join(", ", emitResult.Diagnostics.Where(d => d.Severity == DiagnosticSeverity.Error))}"); + + var tempPath = System.IO.Path.Combine( + System.IO.Path.GetTempPath(), + $"{assemblyName}_{System.Guid.NewGuid():N}.dll"); + System.IO.File.WriteAllBytes(tempPath, stream.ToArray()); + tempFiles.Add(tempPath); + libReferences.Add(MetadataReference.CreateFromFile(tempPath)); + } + + // Compile task assembly referencing all libraries + var taskSyntaxTree = CSharpSyntaxTree.ParseText(taskSource, path: "Task.cs"); + var taskStubTree = CSharpSyntaxTree.ParseText(FrameworkStubs, path: "TaskStubs.cs"); + + var taskCompilation = CSharpCompilation.Create( + "TaskAssembly", + [taskSyntaxTree, taskStubTree], + coreRefs.Concat(libReferences).ToArray(), + new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary) + .WithNullableContextOptions(NullableContextOptions.Enable)); + + var analyzer = new TransitiveCallChainAnalyzer(); + var compilationWithAnalyzers = taskCompilation.WithAnalyzers( + ImmutableArray.Create(analyzer)); + + return await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync(); + } + finally + { + foreach (var f in tempFiles) + { + try { System.IO.File.Delete(f); } catch { } + } + } + } + + [Fact] + public async Task MultiAssembly_TwoLibraries_FileOpenDetected() + { + // LibBase: has File.Open + // LibMiddle: calls LibBase + // Task: calls LibMiddle + // Expected: Task → LibMiddle.Process → LibBase.ReadData → File.Open detected + + var libraries = new (string source, string assemblyName)[] + { + (""" + using System.IO; + namespace LibBase + { + public static class DataReader + { + public static string ReadData(string path) + { + using var fs = File.OpenRead(path); + using var sr = new StreamReader(fs); + return sr.ReadToEnd(); + } + } + } + """, "LibBase"), + (""" + namespace LibMiddle + { + public static class Processor + { + public static string Process(string filePath) + { + return LibBase.DataReader.ReadData(filePath); + } + } + } + """, "LibMiddle"), + }; + + var taskSource = """ + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + LibMiddle.Processor.Process("data.json"); + return true; + } + } + """; + + var diags = await GetMultiAssemblyDiagnosticsAsync(libraries, taskSource); + var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ToArray(); + transitive.ShouldNotBeEmpty("Should detect File.OpenRead through 2-library chain"); + var msg = transitive[0].GetMessage(); + msg.ShouldContain("→"); + } + + [Fact] + public async Task MultiAssembly_ThreeLibraries_EnvironmentExitDetected() + { + // LibC: calls Environment.Exit + // LibB: calls LibC + // LibA: calls LibB + // Task: calls LibA + + var libraries = new (string source, string assemblyName)[] + { + (""" + using System; + namespace LibC + { + public static class Terminator + { + public static void Terminate() { Environment.Exit(1); } + } + } + """, "LibC"), + (""" + namespace LibB + { + public static class Gateway + { + public static void Check(bool fail) + { + if (fail) LibC.Terminator.Terminate(); + } + } + } + """, "LibB"), + (""" + namespace LibA + { + public static class Facade + { + public static void Run() + { + LibB.Gateway.Check(false); + } + } + } + """, "LibA"), + }; + + var taskSource = """ + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + LibA.Facade.Run(); + return true; + } + } + """; + + var diags = await GetMultiAssemblyDiagnosticsAsync(libraries, taskSource); + var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ToArray(); + transitive.ShouldNotBeEmpty("Should detect Environment.Exit through 3-library chain"); + var msg = transitive[0].GetMessage(); + msg.ShouldContain("Exit"); + } + + [Fact] + public async Task MultiAssembly_XDocumentSave_DetectedThroughLibrary() + { + // Simulates the SDK pattern: Task → LockFileCache → LockFileUtilities (File.Open) + var libraries = new (string source, string assemblyName)[] + { + (""" + using System.Xml.Linq; + namespace ConfigLib + { + public static class ConfigWriter + { + public static void Write(XDocument doc, string outputPath) + { + doc.Save(outputPath); + } + } + } + """, "ConfigLib"), + }; + + var taskSource = """ + using System.Xml.Linq; + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + var doc = new XDocument(); + ConfigLib.ConfigWriter.Write(doc, "output.xml"); + return true; + } + } + """; + + var diags = await GetMultiAssemblyDiagnosticsAsync(libraries, taskSource); + var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ToArray(); + transitive.ShouldNotBeEmpty("Should detect XDocument.Save through library wrapper"); + } + + [Fact] + public async Task MultiAssembly_MixedCleanAndDirty_OnlyDirtyFlagged() + { + var libraries = new (string source, string assemblyName)[] + { + (""" + using System; + using System.IO; + namespace MixedLib + { + public static class SafeHelper + { + public static int Add(int a, int b) => a + b; + } + public static class UnsafeHelper + { + public static string LoadFile(string path) => File.ReadAllText(path); + } + } + """, "MixedLib"), + }; + + var taskSource = """ + public class MyTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + var sum = MixedLib.SafeHelper.Add(1, 2); + var text = MixedLib.UnsafeHelper.LoadFile("data.txt"); + return true; + } + } + """; + + var diags = await GetMultiAssemblyDiagnosticsAsync(libraries, taskSource); + var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ToArray(); + transitive.ShouldNotBeEmpty("Should detect File.ReadAllText through UnsafeHelper"); + // All violations should trace through UnsafeHelper, not through SafeHelper.Add + foreach (var d in transitive) + { + d.GetMessage().ShouldContain("UnsafeHelper"); + } + // SafeHelper.Add should not appear as a standalone chain element + transitive.Where(d => d.GetMessage().Contains("SafeHelper.Add")).ShouldBeEmpty(); + } + + /// + /// Tests the pattern where a source-level helper class (in the same compilation as the task) + /// calls an external library method that performs unsafe file I/O. + /// This matches the SDK pattern: Task → LockFileCache (source) → NuGet.ProjectModel (external) → File.Open + /// + [Fact] + public async Task SourceHelper_CallingExternalLib_DetectedViaIL() + { + // External library (simulating NuGet.ProjectModel) + var libraries = new (string source, string assemblyName)[] + { + (""" + using System; + using System.IO; + namespace NuGetShim + { + public static class LockFileUtilities + { + public static string GetLockFile(string path, object logger) + { + using var s = File.Open(path, FileMode.Open, FileAccess.Read, FileShare.Read); + return path; + } + } + } + """, "NuGet.ProjectModel"), + }; + + // Task source includes BOTH the task AND a source-level helper + var taskSource = """ + namespace TestNs + { + internal class LockFileCache + { + public string GetLockFile(string path) + { + return LoadLockFile(path); + } + private string LoadLockFile(string path) + { + return NuGetShim.LockFileUtilities.GetLockFile(path, null); + } + } + + public class CheckTask : Microsoft.Build.Utilities.Task + { + public override bool Execute() + { + var cache = new LockFileCache(); + cache.GetLockFile("test.json"); + return true; + } + } + } + """; + + var diags = await GetMultiAssemblyDiagnosticsAsync(libraries, taskSource); + var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ToArray(); + transitive.ShouldNotBeEmpty("Should detect File.Open through source helper → external NuGet lib"); + // The chain should include both the source helper and the external library method + var msg = string.Join("; ", transitive.Select(d => d.GetMessage())); + msg.ShouldContain("LockFileCache"); + msg.ShouldContain("LockFileUtilities"); + } + + #endregion +} diff --git a/src/ThreadSafeTaskAnalyzer.Tests/WriteAllTextDetailedTest.cs b/src/ThreadSafeTaskAnalyzer.Tests/WriteAllTextDetailedTest.cs new file mode 100644 index 00000000000..69a00f8fa4d --- /dev/null +++ b/src/ThreadSafeTaskAnalyzer.Tests/WriteAllTextDetailedTest.cs @@ -0,0 +1,43 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Threading.Tasks; +using Shouldly; +using Xunit; +using static Microsoft.Build.TaskAuthoring.Analyzer.Tests.TestHelpers; +using System.Linq; + +namespace Microsoft.Build.TaskAuthoring.Analyzer.Tests +{ + public class WriteAllTextDetailedTest + { + [Fact] + public async Task File_WriteAllText_ChecksDiagnosticCount() + { + var diags = await GetDiagnosticsAsync(@" + using System.IO; + using Microsoft.Build.Framework; + public class MyTask : Microsoft.Build.Utilities.Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public override bool Execute() + { + File.WriteAllText(""file.txt"", ""contents""); + return true; + } + } + "); + + var pathDiags = diags.Where(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute).ToArray(); + + // Debug: print all diagnostics + System.Console.WriteLine($"Total diagnostics: {diags.Length}"); + foreach (var diag in diags) + { + System.Console.WriteLine($" [{diag.Id}] {diag.GetMessage()} at {diag.Location}"); + } + + pathDiags.Length.ShouldBe(1, $"Expected exactly 1 diagnostic but got {pathDiags.Length}: {string.Join(", ", pathDiags.Select(d => d.GetMessage()))}"); + } + } +} diff --git a/src/ThreadSafeTaskAnalyzer/AnalyzerReleases.Shipped.md b/src/ThreadSafeTaskAnalyzer/AnalyzerReleases.Shipped.md new file mode 100644 index 00000000000..f50bb1fe210 --- /dev/null +++ b/src/ThreadSafeTaskAnalyzer/AnalyzerReleases.Shipped.md @@ -0,0 +1,2 @@ +; Shipped analyzer releases +; https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md diff --git a/src/ThreadSafeTaskAnalyzer/AnalyzerReleases.Unshipped.md b/src/ThreadSafeTaskAnalyzer/AnalyzerReleases.Unshipped.md new file mode 100644 index 00000000000..d58299f2d54 --- /dev/null +++ b/src/ThreadSafeTaskAnalyzer/AnalyzerReleases.Unshipped.md @@ -0,0 +1,9 @@ +### New Rules + +Rule ID | Category | Severity | Notes +--------|----------|----------|------- +MSBuildTask0001 | MSBuild.TaskAuthoring | Error | APIs that must not be used in any MSBuild task (Environment.Exit, Console.*, etc.) +MSBuildTask0002 | MSBuild.TaskAuthoring | Warning | APIs that should use TaskEnvironment alternatives in IMultiThreadableTask +MSBuildTask0003 | MSBuild.TaskAuthoring | Warning | File APIs that need absolute paths in IMultiThreadableTask +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 diff --git a/src/ThreadSafeTaskAnalyzer/BannedApiDefinitions.cs b/src/ThreadSafeTaskAnalyzer/BannedApiDefinitions.cs new file mode 100644 index 00000000000..c12047dd775 --- /dev/null +++ b/src/ThreadSafeTaskAnalyzer/BannedApiDefinitions.cs @@ -0,0 +1,182 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Immutable; + +namespace Microsoft.Build.TaskAuthoring.Analyzer +{ + /// + /// Defines the banned API entries with their symbol documentation IDs, category, and diagnostic message. + /// File path APIs (MSBuildTask0003) are handled separately via pattern matching, not symbol matching. + /// + internal static class BannedApiDefinitions + { + internal enum ApiCategory + { + /// MSBuildTask0001: Critical errors - no safe alternative. + CriticalError, + /// MSBuildTask0002: Requires TaskEnvironment replacement. + TaskEnvironment, + /// MSBuildTask0004: Potential issue - review required. + PotentialIssue, + } + + internal readonly struct BannedApi + { + public string DeclarationId { get; } + public ApiCategory Category { get; } + public string Message { get; } + + public BannedApi(string declarationId, ApiCategory category, string message) + { + DeclarationId = declarationId; + Category = category; + Message = message; + } + } + + private static readonly ImmutableArray s_all = CreateAll(); + + public static ImmutableArray GetAll() => s_all; + + private static ImmutableArray CreateAll() + { + return ImmutableArray.Create( + // ══════════════════════════════════════════════════════════════ + // MSBuildTask0001: Critical errors - no safe alternative + // Console.* is handled at the TYPE level in the analyzer. + // ══════════════════════════════════════════════════════════════ + + // Process termination + new BannedApi("M:System.Environment.Exit(System.Int32)", + ApiCategory.CriticalError, "terminates entire process; return false or throw instead"), + new BannedApi("M:System.Environment.FailFast(System.String)", + ApiCategory.CriticalError, "terminates entire process; return false or throw instead"), + new BannedApi("M:System.Environment.FailFast(System.String,System.Exception)", + ApiCategory.CriticalError, "terminates entire process; return false or throw instead"), + new BannedApi("M:System.Environment.FailFast(System.String,System.Exception,System.String)", + ApiCategory.CriticalError, "terminates entire process; return false or throw instead"), + + // Process kill + new BannedApi("M:System.Diagnostics.Process.Kill", + ApiCategory.CriticalError, "may terminate the MSBuild host process"), + new BannedApi("M:System.Diagnostics.Process.Kill(System.Boolean)", + ApiCategory.CriticalError, "may terminate the MSBuild host process"), + + // ThreadPool - process-wide settings + new BannedApi("M:System.Threading.ThreadPool.SetMinThreads(System.Int32,System.Int32)", + ApiCategory.CriticalError, "modifies process-wide thread pool settings"), + new BannedApi("M:System.Threading.ThreadPool.SetMaxThreads(System.Int32,System.Int32)", + ApiCategory.CriticalError, "modifies process-wide thread pool settings"), + + // CultureInfo defaults - affect all new threads + new BannedApi("P:System.Globalization.CultureInfo.DefaultThreadCurrentCulture", + ApiCategory.CriticalError, "affects culture of all new threads in process"), + new BannedApi("P:System.Globalization.CultureInfo.DefaultThreadCurrentUICulture", + ApiCategory.CriticalError, "affects UI culture of all new threads in process"), + + // Directory.SetCurrentDirectory - affects all threads + new BannedApi("M:System.IO.Directory.SetCurrentDirectory(System.String)", + ApiCategory.CriticalError, "modifies process-wide working directory; use TaskEnvironment.ProjectDirectory instead"), + + // ══════════════════════════════════════════════════════════════ + // MSBuildTask0002: TaskEnvironment required + // ══════════════════════════════════════════════════════════════ + + // Environment.CurrentDirectory + new BannedApi("P:System.Environment.CurrentDirectory", + ApiCategory.TaskEnvironment, "use TaskEnvironment.ProjectDirectory instead"), + + // Directory.GetCurrentDirectory + new BannedApi("M:System.IO.Directory.GetCurrentDirectory", + ApiCategory.TaskEnvironment, "use TaskEnvironment.ProjectDirectory instead"), + + // Environment variable access + new BannedApi("M:System.Environment.GetEnvironmentVariable(System.String)", + ApiCategory.TaskEnvironment, "use TaskEnvironment.GetEnvironmentVariable instead"), + new BannedApi("M:System.Environment.GetEnvironmentVariable(System.String,System.EnvironmentVariableTarget)", + ApiCategory.TaskEnvironment, "use TaskEnvironment.GetEnvironmentVariable instead"), + new BannedApi("M:System.Environment.GetEnvironmentVariables", + ApiCategory.TaskEnvironment, "use TaskEnvironment.GetEnvironmentVariables instead"), + new BannedApi("M:System.Environment.SetEnvironmentVariable(System.String,System.String)", + ApiCategory.TaskEnvironment, "use TaskEnvironment.SetEnvironmentVariable instead"), + new BannedApi("M:System.Environment.SetEnvironmentVariable(System.String,System.String,System.EnvironmentVariableTarget)", + ApiCategory.TaskEnvironment, "drop target parameter; use TaskEnvironment.SetEnvironmentVariable instead"), + new BannedApi("M:System.Environment.ExpandEnvironmentVariables(System.String)", + ApiCategory.TaskEnvironment, "use TaskEnvironment.GetEnvironmentVariable for individual variables instead"), + + // Environment.GetFolderPath - uses process-wide state + new BannedApi("M:System.Environment.GetFolderPath(System.Environment.SpecialFolder)", + ApiCategory.TaskEnvironment, "may be affected by environment variable overrides; use TaskEnvironment.GetEnvironmentVariable instead"), + new BannedApi("M:System.Environment.GetFolderPath(System.Environment.SpecialFolder,System.Environment.SpecialFolderOption)", + ApiCategory.TaskEnvironment, "may be affected by environment variable overrides; use TaskEnvironment.GetEnvironmentVariable instead"), + + // Path.GetFullPath + new BannedApi("M:System.IO.Path.GetFullPath(System.String)", + ApiCategory.TaskEnvironment, "use TaskEnvironment.GetAbsolutePath instead"), + new BannedApi("M:System.IO.Path.GetFullPath(System.String,System.String)", + ApiCategory.TaskEnvironment, "use TaskEnvironment.GetAbsolutePath instead"), + + // Path.GetTempPath / GetTempFileName - depend on environment variables + new BannedApi("M:System.IO.Path.GetTempPath", + ApiCategory.TaskEnvironment, "depends on TMP/TEMP environment variables; use TaskEnvironment.GetEnvironmentVariable(\"TMP\") instead"), + new BannedApi("M:System.IO.Path.GetTempFileName", + ApiCategory.TaskEnvironment, "depends on TMP/TEMP environment variables; use TaskEnvironment.GetEnvironmentVariable(\"TMP\") instead"), + + // Process.Start - use TaskEnvironment.GetProcessStartInfo + new BannedApi("M:System.Diagnostics.Process.Start(System.String)", + ApiCategory.TaskEnvironment, "use TaskEnvironment.GetProcessStartInfo instead"), + new BannedApi("M:System.Diagnostics.Process.Start(System.String,System.String)", + ApiCategory.TaskEnvironment, "use TaskEnvironment.GetProcessStartInfo instead"), + new BannedApi("M:System.Diagnostics.Process.Start(System.String,System.Collections.Generic.IEnumerable{System.String})", + ApiCategory.TaskEnvironment, "use TaskEnvironment.GetProcessStartInfo instead"), + new BannedApi("M:System.Diagnostics.Process.Start(System.Diagnostics.ProcessStartInfo)", + ApiCategory.TaskEnvironment, "use TaskEnvironment.GetProcessStartInfo instead"), + new BannedApi("M:System.Diagnostics.Process.Start(System.String,System.String,System.String,System.Security.SecureString,System.String)", + ApiCategory.TaskEnvironment, "use TaskEnvironment.GetProcessStartInfo instead"), + + // ProcessStartInfo constructors + new BannedApi("M:System.Diagnostics.ProcessStartInfo.#ctor", + ApiCategory.TaskEnvironment, "use TaskEnvironment.GetProcessStartInfo() instead"), + new BannedApi("M:System.Diagnostics.ProcessStartInfo.#ctor(System.String)", + ApiCategory.TaskEnvironment, "use TaskEnvironment.GetProcessStartInfo() instead"), + new BannedApi("M:System.Diagnostics.ProcessStartInfo.#ctor(System.String,System.String)", + ApiCategory.TaskEnvironment, "use TaskEnvironment.GetProcessStartInfo() instead"), + new BannedApi("M:System.Diagnostics.ProcessStartInfo.#ctor(System.String,System.Collections.Generic.IEnumerable{System.String})", + ApiCategory.TaskEnvironment, "use TaskEnvironment.GetProcessStartInfo() instead"), + + // ══════════════════════════════════════════════════════════════ + // MSBuildTask0004: Potential issues - review required + // ══════════════════════════════════════════════════════════════ + + // Assembly loading - may cause version conflicts + new BannedApi("M:System.Reflection.Assembly.LoadFrom(System.String)", + ApiCategory.PotentialIssue, "may cause version conflicts in shared task host"), + new BannedApi("M:System.Reflection.Assembly.LoadFile(System.String)", + ApiCategory.PotentialIssue, "may cause version conflicts in shared task host"), + new BannedApi("M:System.Reflection.Assembly.Load(System.String)", + ApiCategory.PotentialIssue, "may cause version conflicts in shared task host"), + new BannedApi("M:System.Reflection.Assembly.Load(System.Byte[])", + ApiCategory.PotentialIssue, "may cause version conflicts in shared task host"), + new BannedApi("M:System.Reflection.Assembly.Load(System.Byte[],System.Byte[])", + ApiCategory.PotentialIssue, "may cause version conflicts in shared task host"), + new BannedApi("M:System.Reflection.Assembly.LoadWithPartialName(System.String)", + ApiCategory.PotentialIssue, "obsolete and may cause version conflicts"), + new BannedApi("M:System.Activator.CreateInstanceFrom(System.String,System.String)", + ApiCategory.PotentialIssue, "may cause version conflicts in shared task host"), + new BannedApi("M:System.Activator.CreateInstance(System.String,System.String)", + ApiCategory.PotentialIssue, "may cause version conflicts in shared task host"), + + // AppDomain + new BannedApi("M:System.AppDomain.Load(System.String)", + ApiCategory.PotentialIssue, "may cause version conflicts in shared task host"), + new BannedApi("M:System.AppDomain.Load(System.Byte[])", + ApiCategory.PotentialIssue, "may cause version conflicts in shared task host"), + new BannedApi("M:System.AppDomain.CreateInstanceFrom(System.String,System.String)", + ApiCategory.PotentialIssue, "may cause version conflicts in shared task host"), + new BannedApi("M:System.AppDomain.CreateInstance(System.String,System.String)", + ApiCategory.PotentialIssue, "may cause version conflicts in shared task host") + ); + } + } +} diff --git a/src/ThreadSafeTaskAnalyzer/DiagnosticDescriptors.cs b/src/ThreadSafeTaskAnalyzer/DiagnosticDescriptors.cs new file mode 100644 index 00000000000..1de5221e89e --- /dev/null +++ b/src/ThreadSafeTaskAnalyzer/DiagnosticDescriptors.cs @@ -0,0 +1,67 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; + +namespace Microsoft.Build.TaskAuthoring.Analyzer +{ + /// + /// Diagnostic descriptors for the thread-safe task analyzer. + /// + internal static class DiagnosticDescriptors + { + public static readonly DiagnosticDescriptor CriticalError = new( + id: DiagnosticIds.CriticalError, + title: "API is never safe in MSBuild task implementations", + messageFormat: "'{0}' must not be used in MSBuild tasks: {1}", + category: "MSBuild.TaskAuthoring", + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: "This API has no safe alternative in MSBuild tasks. It affects the entire process or interferes with build infrastructure."); + + public static readonly DiagnosticDescriptor TaskEnvironmentRequired = new( + id: DiagnosticIds.TaskEnvironmentRequired, + title: "API requires TaskEnvironment alternative in MSBuild tasks", + messageFormat: "'{0}' should use TaskEnvironment alternative: {1}", + category: "MSBuild.TaskAuthoring", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "This API accesses process-global state. Use the corresponding TaskEnvironment method instead."); + + public static readonly DiagnosticDescriptor FilePathRequiresAbsolute = new( + id: DiagnosticIds.FilePathRequiresAbsolute, + title: "File system API requires absolute path in MSBuild tasks", + messageFormat: "'{0}' may resolve relative paths against the process working directory: {1}", + category: "MSBuild.TaskAuthoring", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "File system APIs must receive absolute paths. Use TaskEnvironment.GetAbsolutePath() to convert relative paths."); + + public static readonly DiagnosticDescriptor PotentialIssue = new( + id: DiagnosticIds.PotentialIssue, + title: "API may cause issues in multithreaded MSBuild tasks", + messageFormat: "'{0}' may cause issues in multithreaded tasks: {1}", + category: "MSBuild.TaskAuthoring", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "This API may cause threading issues or version conflicts. Review usage carefully."); + + public static readonly DiagnosticDescriptor TransitiveUnsafeCall = new( + id: DiagnosticIds.TransitiveUnsafeCall, + title: "Transitive unsafe API usage in task call chain", + messageFormat: "'{0}' transitively calls unsafe API '{1}' via: {2}", + category: "MSBuild.TaskAuthoring", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "A method called from this task transitively uses an API that is unsafe in multithreaded task execution. Review the call chain and migrate the callee.", + customTags: WellKnownDiagnosticTags.CompilationEnd); + + public static ImmutableArray All { get; } = ImmutableArray.Create( + CriticalError, + TaskEnvironmentRequired, + FilePathRequiresAbsolute, + PotentialIssue, + TransitiveUnsafeCall); + } +} diff --git a/src/ThreadSafeTaskAnalyzer/DiagnosticIds.cs b/src/ThreadSafeTaskAnalyzer/DiagnosticIds.cs new file mode 100644 index 00000000000..03c40e0e054 --- /dev/null +++ b/src/ThreadSafeTaskAnalyzer/DiagnosticIds.cs @@ -0,0 +1,27 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.Build.TaskAuthoring.Analyzer +{ + /// + /// Diagnostic IDs for the thread-safe task analyzer. + /// Using a distinct namespace from MSBuild build-time diagnostics (MSBxxxx). + /// + public static class DiagnosticIds + { + /// Critical APIs with no safe alternative (Environment.Exit, Console.*, ThreadPool). + public const string CriticalError = "MSBuildTask0001"; + + /// APIs requiring TaskEnvironment alternatives. + public const string TaskEnvironmentRequired = "MSBuildTask0002"; + + /// File APIs requiring absolute paths. + public const string FilePathRequiresAbsolute = "MSBuildTask0003"; + + /// Potentially problematic APIs (Assembly.Load*, Activator.CreateInstance*). + public const string PotentialIssue = "MSBuildTask0004"; + + /// Transitive unsafe API usage detected in task call chain. + public const string TransitiveUnsafeCall = "MSBuildTask0005"; + } +} diff --git a/src/ThreadSafeTaskAnalyzer/ILCallGraphExtender.cs b/src/ThreadSafeTaskAnalyzer/ILCallGraphExtender.cs new file mode 100644 index 00000000000..5c0ca47b94e --- /dev/null +++ b/src/ThreadSafeTaskAnalyzer/ILCallGraphExtender.cs @@ -0,0 +1,1011 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.IO; +using System.Reflection; +using System.Reflection.Metadata; +using System.Reflection.Metadata.Ecma335; +using System.Reflection.PortableExecutable; +using Microsoft.CodeAnalysis; + +namespace Microsoft.Build.TaskAuthoring.Analyzer +{ + /// + /// Extends the call graph into referenced assemblies by reading IL from PE files. + /// When the transitive analyzer's BFS encounters a method with no source code + /// (i.e., defined in a referenced assembly), this class reads the method's IL + /// to discover outgoing call edges and banned API violations. + /// + internal sealed class ILCallGraphExtender : IDisposable + { + /// + /// Types whose internal method implementations are considered safe. + /// Violations found inside these types are suppressed because they handle + /// paths safely internally (e.g., AbsolutePath.GetCanonicalForm calls Path.GetFullPath + /// on an already-absolute path). + /// + private static readonly HashSet SafeTypeMetadataNames = new HashSet(StringComparer.Ordinal) + { + "Microsoft.Build.Framework.AbsolutePath", + "Microsoft.Build.Framework.TaskEnvironment", + "Microsoft.Build.BackEnd.TaskExecutionHost.MultiThreadedTaskEnvironmentDriver", + "Microsoft.Build.BackEnd.TaskExecutionHost.MultiProcessTaskEnvironmentDriver", + }; + + /// + /// Assembly name prefixes for BCL assemblies where we stop IL traversal. + /// + private static readonly string[] BclPrefixes = + { + "System.", + "System,", + "mscorlib", + "netstandard", + "Microsoft.CSharp", + "Microsoft.VisualBasic", + "Microsoft.Win32", + "WindowsBase", + }; + + private readonly Compilation _compilation; + + /// + /// Cache of PEReader instances keyed by assembly identity. + /// + private readonly ConcurrentDictionary _peReaderCache + = new ConcurrentDictionary(StringComparer.OrdinalIgnoreCase); + + /// + /// Set of methods already analyzed via IL to avoid redundant work. + /// Keyed by a string representation: "TypeMetadataName.MethodName(ParamTypes)". + /// + private readonly ConcurrentDictionary _analyzedMethods + = new ConcurrentDictionary(StringComparer.Ordinal); + + /// + /// Maps assembly name to its IAssemblySymbol for symbol resolution. + /// + private readonly Dictionary _assemblySymbolMap; + + public ILCallGraphExtender(Compilation compilation) + { + _compilation = compilation; + _assemblySymbolMap = BuildAssemblySymbolMap(compilation); + } + + /// + /// Determines whether a method is in a referenced assembly (no source code). + /// + public static bool IsExternalMethod(IMethodSymbol method) + { + return method.DeclaringSyntaxReferences.IsEmpty; + } + + /// + /// Determines whether the containing type of a method is on the safe-type list. + /// + public static bool IsInSafeType(IMethodSymbol method) + { + var containingType = method.ContainingType; + if (containingType is null) + { + return false; + } + + string metadataName = GetTypeMetadataName(containingType); + return SafeTypeMetadataNames.Contains(metadataName); + } + + /// + /// Determines whether a method is in a BCL assembly where we should stop traversal. + /// + public static bool IsInBclAssembly(IMethodSymbol method) + { + var assembly = method.ContainingAssembly; + if (assembly is null) + { + return true; // treat unknown as BCL to be safe + } + + return IsAssemblyBcl(assembly.Identity.Name); + } + + /// + /// Analyzes a method's IL to discover outgoing call edges. + /// Returns a list of (callee IMethodSymbol, isViolation, violationInfo) tuples. + /// + public List GetCallTargets(IMethodSymbol method) + { + var results = new List(); + + var methodKey = GetMethodKey(method); + if (!_analyzedMethods.TryAdd(methodKey, true)) + { + return results; // already analyzed + } + + var containingType = method.ContainingType; + if (containingType is null) + { + return results; + } + + var assembly = containingType.ContainingAssembly; + if (assembly is null) + { + return results; + } + + var peEntry = GetOrLoadPEReader(assembly); + if (peEntry is null) + { + return results; + } + + var reader = peEntry.MetadataReader; + + // Find the MethodDefinition for this IMethodSymbol + var methodDef = FindMethodDefinition(reader, method); + if (methodDef is null) + { + return results; + } + + // Read IL body + var methodBody = GetMethodBody(peEntry.PEReader, reader, methodDef.Value); + if (methodBody is null) + { + return results; + } + + // Parse IL opcodes to find call targets + ParseILForCalls(peEntry, reader, methodBody, assembly, results); + + return results; + } + + private void ParseILForCalls( + PEReaderEntry peEntry, + MetadataReader reader, + MethodBodyBlock methodBody, + IAssemblySymbol containingAssembly, + List results) + { + var ilBytes = methodBody.GetILBytes(); + if (ilBytes is null || ilBytes.Length == 0) + { + return; + } + + int offset = 0; + while (offset < ilBytes.Length) + { + int opcodeStart = offset; + ushort opcodeValue; + + byte first = ilBytes[offset++]; + if (first == 0xFE && offset < ilBytes.Length) + { + byte second = ilBytes[offset++]; + opcodeValue = (ushort)(0xFE00 | second); + } + else + { + opcodeValue = first; + } + + // Determine operand size based on opcode + int operandSize = GetOperandSize(opcodeValue, ilBytes, offset); + bool isCall = opcodeValue == 0x28 // call + || opcodeValue == 0x6F // callvirt + || opcodeValue == 0x73 // newobj + || opcodeValue == 0xFE06 // ldftn + || opcodeValue == 0xFE07; // ldvirtftn + + if (isCall && operandSize == 4 && offset + 4 <= ilBytes.Length) + { + int token = ilBytes[offset] + | (ilBytes[offset + 1] << 8) + | (ilBytes[offset + 2] << 16) + | (ilBytes[offset + 3] << 24); + + try + { + var handle = MetadataTokens.EntityHandle(token); + var resolved = ResolveCallTarget(reader, handle, containingAssembly); + if (resolved is not null) + { + results.Add(resolved); + } + } + catch + { + // Invalid token — skip + } + } + + offset += operandSize; + + // Safety: prevent infinite loops from bad IL + if (offset <= opcodeStart) + { + break; + } + } + } + + /// + /// Gets the operand size for an IL opcode. + /// Uses ECMA-335 Partition III opcode definitions. + /// + private static int GetOperandSize(ushort opcode, byte[] ilBytes, int offset) + { + // Switch table (InlineSwitch) — variable length + if (opcode == 0x45) + { + if (offset + 4 > ilBytes.Length) + { + return 0; + } + + int count = ilBytes[offset] + | (ilBytes[offset + 1] << 8) + | (ilBytes[offset + 2] << 16) + | (ilBytes[offset + 3] << 24); + return 4 + count * 4; + } + + // Single-byte opcodes + if (opcode <= 0xFF) + { + return GetSingleByteOperandSize((byte)opcode); + } + + // Two-byte opcodes (0xFE prefix) + return GetFEPrefixOperandSize((byte)(opcode & 0xFF)); + } + + /// + /// Resolves a metadata token from an IL call instruction to an IMethodSymbol. + /// + private ILCallTarget? ResolveCallTarget( + MetadataReader reader, + EntityHandle handle, + IAssemblySymbol containingAssembly) + { + string? typeName = null; + string? methodName = null; + string? assemblyName = null; + int paramCount = -1; // -1 means unknown + + switch (handle.Kind) + { + case HandleKind.MethodDefinition: + { + var methodDef = reader.GetMethodDefinition((MethodDefinitionHandle)handle); + methodName = reader.GetString(methodDef.Name); + var declaringType = reader.GetTypeDefinition(methodDef.GetDeclaringType()); + typeName = GetFullTypeName(reader, declaringType); + assemblyName = containingAssembly.Identity.Name; + // Count parameters from PE metadata + paramCount = CountMethodDefParams(reader, methodDef); + break; + } + + case HandleKind.MemberReference: + { + var memberRef = reader.GetMemberReference((MemberReferenceHandle)handle); + methodName = reader.GetString(memberRef.Name); + (typeName, assemblyName) = ResolveMemberRefParent(reader, memberRef.Parent, containingAssembly); + // Extract parameter count from MemberRef signature blob + paramCount = GetParamCountFromSignature(reader, memberRef.Signature); + break; + } + + case HandleKind.MethodSpecification: + { + var methodSpec = reader.GetMethodSpecification((MethodSpecificationHandle)handle); + // Recurse into the underlying method + return ResolveCallTarget(reader, methodSpec.Method, containingAssembly); + } + + default: + return null; + } + + if (typeName is null || methodName is null) + { + return null; + } + + // Resolve to IMethodSymbol via the compilation + var resolvedSymbol = ResolveToMethodSymbol(typeName, methodName, assemblyName, paramCount); + if (resolvedSymbol is null) + { + return null; + } + + return new ILCallTarget(resolvedSymbol); + } + + /// + /// Counts the parameters of a MethodDefinition, excluding the return type pseudo-parameter. + /// + private static int CountMethodDefParams(MetadataReader reader, MethodDefinition methodDef) + { + // Read param count from the method signature blob to avoid the return-type + // pseudo-parameter issue in GetParameters() + return GetParamCountFromSignature(reader, methodDef.Signature); + } + + /// + /// Extracts the parameter count from a method signature blob (ECMA-335 II.23.2.1/II.23.2.2). + /// Format: CallingConvention GenParamCount? ParamCount RetType Param* + /// + private static int GetParamCountFromSignature(MetadataReader reader, BlobHandle signatureHandle) + { + try + { + var blobReader = reader.GetBlobReader(signatureHandle); + byte callingConvention = blobReader.ReadByte(); + // Check for generic method — if so, skip GenParamCount + if ((callingConvention & 0x10) != 0) // IMAGE_CEE_CS_CALLCONV_GENERIC + { + blobReader.ReadCompressedInteger(); // GenParamCount + } + return blobReader.ReadCompressedInteger(); // ParamCount + } + catch + { + return -1; + } + } + + private (string? typeName, string? assemblyName) ResolveMemberRefParent( + MetadataReader reader, + EntityHandle parent, + IAssemblySymbol containingAssembly) + { + switch (parent.Kind) + { + case HandleKind.TypeReference: + { + var typeRef = reader.GetTypeReference((TypeReferenceHandle)parent); + string ns = reader.GetString(typeRef.Namespace); + string name = reader.GetString(typeRef.Name); + string typeName = string.IsNullOrEmpty(ns) ? name : $"{ns}.{name}"; + + // Resolve the assembly from ResolutionScope + string? asmName = ResolveAssemblyFromScope(reader, typeRef.ResolutionScope, containingAssembly); + return (typeName, asmName); + } + + case HandleKind.TypeDefinition: + { + var typeDef = reader.GetTypeDefinition((TypeDefinitionHandle)parent); + string typeName = GetFullTypeName(reader, typeDef); + return (typeName, containingAssembly.Identity.Name); + } + + case HandleKind.TypeSpecification: + { + // Generic type instantiation — try to resolve the underlying type + var typeSpec = reader.GetTypeSpecification((TypeSpecificationHandle)parent); + // For simplicity, try to decode the blob to get the base type + // This is complex; skip for now + return (null, null); + } + + default: + return (null, null); + } + } + + private string? ResolveAssemblyFromScope( + MetadataReader reader, + EntityHandle scope, + IAssemblySymbol containingAssembly) + { + switch (scope.Kind) + { + case HandleKind.AssemblyReference: + { + var asmRef = reader.GetAssemblyReference((AssemblyReferenceHandle)scope); + return reader.GetString(asmRef.Name); + } + + case HandleKind.ModuleDefinition: + return containingAssembly.Identity.Name; + + case HandleKind.TypeReference: + { + // Nested type — resolve parent + var parentRef = reader.GetTypeReference((TypeReferenceHandle)scope); + return ResolveAssemblyFromScope(reader, parentRef.ResolutionScope, containingAssembly); + } + + default: + return containingAssembly.Identity.Name; + } + } + + /// + /// Resolves a type+method name to an IMethodSymbol via the Compilation. + /// + private IMethodSymbol? ResolveToMethodSymbol(string typeName, string methodName, string? assemblyName, int paramCount = -1) + { + // First try to find the type in the compilation + var type = _compilation.GetTypeByMetadataName(typeName); + + // If not found and we have an assembly name, try assembly-specific lookup + if (type is null && assemblyName is not null) + { + if (_assemblySymbolMap.TryGetValue(assemblyName, out var asmSymbol)) + { + type = asmSymbol.GetTypeByMetadataName(typeName); + } + } + + if (type is null) + { + return null; + } + + // Find the method by name, using parameter count for disambiguation when available + IMethodSymbol? firstMatch = null; + foreach (var member in type.GetMembers(methodName)) + { + if (member is IMethodSymbol method) + { + if (paramCount >= 0 && method.Parameters.Length == paramCount) + { + return method.OriginalDefinition; // exact param count match + } + + firstMatch ??= method; + } + } + + // If no exact param match, return first match (fallback) + if (firstMatch is not null) + { + return firstMatch.OriginalDefinition; + } + + // Check for constructors + if (methodName == ".ctor" || methodName == ".cctor") + { + // Prefer constructors with path-like string parameters + IMethodSymbol? bestMatch = null; + foreach (var ctor in type.Constructors) + { + if (bestMatch is null) + { + bestMatch = ctor; + } + + // Check if this constructor has a string parameter with a path-like name + foreach (var param in ctor.Parameters) + { + if (param.Type.SpecialType == SpecialType.System_String && + IsPathParameterName(param.Name)) + { + return ctor.OriginalDefinition; + } + } + } + + return bestMatch?.OriginalDefinition; + } + + // Check for property getters/setters + foreach (var member in type.GetMembers()) + { + if (member is IPropertySymbol prop) + { + if (prop.GetMethod?.Name == methodName) + { + return prop.GetMethod.OriginalDefinition; + } + + if (prop.SetMethod?.Name == methodName) + { + return prop.SetMethod.OriginalDefinition; + } + } + } + + return null; + } + + #region PE Reader Management + + private PEReaderEntry? GetOrLoadPEReader(IAssemblySymbol assembly) + { + string name = assembly.Identity.Name; + + if (IsAssemblyBcl(name)) + { + return null; + } + + return _peReaderCache.GetOrAdd(name, _ => LoadPEReader(assembly)); + } + + private PEReaderEntry? LoadPEReader(IAssemblySymbol assembly) + { + // Find the PortableExecutableReference for this assembly + foreach (var reference in _compilation.References) + { + if (reference is PortableExecutableReference peRef) + { + var refSymbol = _compilation.GetAssemblyOrModuleSymbol(peRef); + if (refSymbol is IAssemblySymbol asmSymbol && + string.Equals(asmSymbol.Identity.Name, assembly.Identity.Name, StringComparison.OrdinalIgnoreCase)) + { + string? filePath = peRef.FilePath; + if (filePath is null) + { + continue; + } + + try + { + // Use FileStream to read PE data — this is necessary for IL analysis + // and is the only way to get method bodies from referenced assemblies. +#pragma warning disable RS1035 // File IO is required here to read PE metadata for cross-assembly IL analysis + var stream = new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.Read); +#pragma warning restore RS1035 + var peReader = new PEReader(stream); + if (peReader.HasMetadata) + { + var metadataReader = peReader.GetMetadataReader(); + return new PEReaderEntry(peReader, metadataReader, stream); + } + + peReader.Dispose(); + stream.Dispose(); + } + catch + { + // Silently skip assemblies we can't read + } + } + } + } + + return null; + } + + #endregion + + #region MethodDefinition Lookup + + /// + /// Finds the MethodDefinition in the PE metadata that corresponds to the given IMethodSymbol. + /// + private MethodDefinitionHandle? FindMethodDefinition(MetadataReader reader, IMethodSymbol method) + { + var containingType = method.ContainingType; + if (containingType is null) + { + return null; + } + + string typeMetadataName = GetTypeMetadataName(containingType); + + // Search all type definitions for a matching type + foreach (var typeDefHandle in reader.TypeDefinitions) + { + var typeDef = reader.GetTypeDefinition(typeDefHandle); + string fullName = GetFullTypeName(reader, typeDef); + + if (!string.Equals(fullName, typeMetadataName, StringComparison.Ordinal)) + { + continue; + } + + // Found the type — now find the method + foreach (var methodDefHandle in typeDef.GetMethods()) + { + var methodDef = reader.GetMethodDefinition(methodDefHandle); + string name = reader.GetString(methodDef.Name); + + if (string.Equals(name, method.Name, StringComparison.Ordinal) || + (method.MethodKind == MethodKind.Constructor && name == ".ctor") || + (method.MethodKind == MethodKind.StaticConstructor && name == ".cctor")) + { + // Match parameter count for basic overload disambiguation + var paramHandles = methodDef.GetParameters(); + int paramCount = 0; + foreach (var _ in paramHandles) + { + paramCount++; + } + + if (paramCount == method.Parameters.Length || + paramCount == method.Parameters.Length + 1) // +1 for return type pseudo-param + { + return methodDefHandle; + } + } + } + + // If we found the type but no exact method match, try without param count check + foreach (var methodDefHandle in typeDef.GetMethods()) + { + var methodDef = reader.GetMethodDefinition(methodDefHandle); + string name = reader.GetString(methodDef.Name); + + if (string.Equals(name, method.Name, StringComparison.Ordinal) || + (method.MethodKind == MethodKind.Constructor && name == ".ctor")) + { + return methodDefHandle; + } + } + } + + return null; + } + + private static MethodBodyBlock? GetMethodBody(PEReader peReader, MetadataReader reader, MethodDefinitionHandle methodDef) + { + var def = reader.GetMethodDefinition(methodDef); + if (def.RelativeVirtualAddress == 0) + { + return null; // abstract/extern/interface method + } + + try + { + return peReader.GetMethodBody(def.RelativeVirtualAddress); + } + catch + { + return null; + } + } + + #endregion + + #region Helpers + + private static bool IsPathParameterName(string paramName) + { + return paramName.IndexOf("path", StringComparison.OrdinalIgnoreCase) >= 0 + || paramName.IndexOf("file", StringComparison.OrdinalIgnoreCase) >= 0 + || paramName.IndexOf("dir", StringComparison.OrdinalIgnoreCase) >= 0 + || paramName.IndexOf("folder", StringComparison.OrdinalIgnoreCase) >= 0 + || paramName.IndexOf("source", StringComparison.OrdinalIgnoreCase) >= 0 + || paramName.IndexOf("dest", StringComparison.OrdinalIgnoreCase) >= 0 + || paramName.IndexOf("uri", StringComparison.OrdinalIgnoreCase) >= 0 + || paramName.IndexOf("url", StringComparison.OrdinalIgnoreCase) >= 0; + } + + private static string GetFullTypeName(MetadataReader reader, TypeDefinition typeDef) + { + string name = reader.GetString(typeDef.Name); + string ns = reader.GetString(typeDef.Namespace); + + // Check for nested type + if (typeDef.IsNested) + { + var declaringTypeHandle = typeDef.GetDeclaringType(); + var declaringType = reader.GetTypeDefinition(declaringTypeHandle); + string parentName = GetFullTypeName(reader, declaringType); + return $"{parentName}+{name}"; + } + + return string.IsNullOrEmpty(ns) ? name : $"{ns}.{name}"; + } + + private static string GetTypeMetadataName(INamedTypeSymbol type) + { + var parts = new List(); + var current = type; + + while (current is not null) + { + parts.Add(current.MetadataName); + if (current.ContainingType is not null) + { + current = current.ContainingType; + } + else + { + break; + } + } + + // Build namespace prefix + var ns = type.ContainingNamespace; + var nsParts = new List(); + while (ns is not null && !ns.IsGlobalNamespace) + { + nsParts.Add(ns.Name); + ns = ns.ContainingNamespace; + } + + nsParts.Reverse(); + parts.Reverse(); + + string nsPrefix = nsParts.Count > 0 ? string.Join(".", nsParts) + "." : ""; + return nsPrefix + string.Join("+", parts); + } + + private static string GetMethodKey(IMethodSymbol method) + { + return $"{GetTypeMetadataName(method.ContainingType)}.{method.Name}({method.Parameters.Length})"; + } + + private static bool IsAssemblyBcl(string assemblyName) + { + foreach (var prefix in BclPrefixes) + { + if (assemblyName.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) + { + return true; + } + } + + // Also skip runtime assemblies + if (assemblyName.Equals("System", StringComparison.OrdinalIgnoreCase)) + { + return true; + } + + return false; + } + + private static Dictionary BuildAssemblySymbolMap(Compilation compilation) + { + var map = new Dictionary(StringComparer.OrdinalIgnoreCase); + + foreach (var reference in compilation.References) + { + var symbol = compilation.GetAssemblyOrModuleSymbol(reference); + if (symbol is IAssemblySymbol asm) + { + map[asm.Identity.Name] = asm; + } + } + + return map; + } + + /// + /// Returns the operand size for a single-byte opcode (excluding 0xFE prefix). + /// Based on ECMA-335 Partition III, Table III.1. + /// + private static int GetSingleByteOperandSize(byte opcode) + { + // Organized by operand type for clarity and correctness + switch (opcode) + { + // InlineNone (0 bytes): no operand + case 0x00: // nop + case 0x01: // break + case 0x02: case 0x03: case 0x04: case 0x05: // ldarg.0-3 + case 0x06: case 0x07: case 0x08: case 0x09: // ldloc.0-3 + case 0x0A: case 0x0B: case 0x0C: case 0x0D: // stloc.0-3 + case 0x14: // ldnull + case 0x15: case 0x16: case 0x17: case 0x18: // ldc.i4.m1, ldc.i4.0-2 + case 0x19: case 0x1A: case 0x1B: case 0x1C: // ldc.i4.3-6 + case 0x1D: case 0x1E: // ldc.i4.7, ldc.i4.8 + case 0x25: // dup + case 0x26: // pop + case 0x2A: // ret + case 0x46: case 0x47: case 0x48: case 0x49: // ldind.i1..u2 + case 0x4A: case 0x4B: case 0x4C: case 0x4D: // ldind.u2..r4 + case 0x4E: case 0x4F: case 0x50: // ldind.r8, ldind.ref, stind.ref + case 0x51: case 0x52: case 0x53: case 0x54: // stind.i1..r4 + case 0x55: case 0x56: // stind.r8, stind.i (0x55=stind.r8, 0x56=?) + case 0x57: // (unused placeholder, safe as 0) + case 0x58: case 0x59: case 0x5A: case 0x5B: // add, sub, mul, div + case 0x5C: case 0x5D: case 0x5E: case 0x5F: // div.un, rem, rem.un, and + case 0x60: case 0x61: case 0x62: case 0x63: // or, xor, shl, shr + case 0x64: case 0x65: case 0x66: case 0x67: // shr.un, neg, not, conv.i1 + case 0x68: case 0x69: case 0x6A: case 0x6B: // conv.i2, conv.i4, conv.i8, conv.r4 + case 0x6C: case 0x6D: case 0x6E: // conv.r8, conv.u4, conv.u8 + case 0x76: case 0x77: case 0x78: // conv.r.un, (unused), (unused) + case 0x82: case 0x83: case 0x84: case 0x85: // conv.ovf.i1..u2 + case 0x86: case 0x87: case 0x88: case 0x89: // conv.ovf.u2..i8 + case 0x8A: case 0x8B: // conv.ovf.u8, (unused) + case 0x8E: // ldlen + case 0x90: case 0x91: case 0x92: case 0x93: // ldelem.i1..u1 + case 0x94: case 0x95: case 0x96: case 0x97: // ldelem.u2..r4 + case 0x98: case 0x99: case 0x9A: // ldelem.r8, ldelem.i, ldelem.ref + case 0x9B: case 0x9C: case 0x9D: case 0x9E: // stelem.i, stelem.i1..i4 + case 0x9F: case 0xA0: case 0xA1: case 0xA2: // stelem.i8..ref + case 0xB3: case 0xB4: case 0xB5: case 0xB6: // conv.ovf.i1.un..u2.un + case 0xB7: case 0xB8: case 0xB9: case 0xBA: // conv.ovf.i4.un..u8.un + case 0xC3: // ckfinite + case 0xD1: case 0xD2: case 0xD3: // conv.u2, conv.u1, conv.i + case 0xDC: // endfinally + case 0xE0: // (prefix — treated as 0 operand) + return 0; + + // ShortInlineVar (1 byte) + case 0x0E: // ldarg.s + case 0x0F: // ldarga.s + case 0x10: // starg.s + case 0x11: // ldloc.s + case 0x12: // ldloca.s + case 0x13: // stloc.s + // ShortInlineI (1 byte) + case 0x1F: // ldc.i4.s + // ShortInlineBrTarget (1 byte) + case 0x2B: // br.s + case 0x2C: case 0x2D: case 0x2E: case 0x2F: // brfalse.s..bge.s + case 0x30: case 0x31: case 0x32: case 0x33: // bgt.s..bne.un.s + case 0x34: case 0x35: case 0x36: case 0x37: // bge.un.s..blt.un.s + case 0xDD: // leave.s + return 1; + + // InlineI (4 bytes) + case 0x20: // ldc.i4 + return 4; + + // InlineI8 (8 bytes) + case 0x21: // ldc.i8 + return 8; + + // ShortInlineR (4 bytes) + case 0x22: // ldc.r4 + return 4; + + // InlineR (8 bytes) + case 0x23: // ldc.r8 + return 8; + + // InlineMethod (4 bytes) — call, callvirt, newobj handled specially before this + case 0x27: // jmp + case 0x28: // call + case 0x29: // calli + case 0x6F: // callvirt + case 0x73: // newobj + return 4; + + // InlineBrTarget (4 bytes) + case 0x38: // br + case 0x39: case 0x3A: case 0x3B: case 0x3C: // brfalse..bge + case 0x3D: case 0x3E: case 0x3F: case 0x40: // bgt..bne.un + case 0x41: case 0x42: case 0x43: case 0x44: // bge.un..blt.un + case 0xDB: // leave + return 4; + + // InlineType / InlineField / InlineString / InlineTok (all 4 bytes) + case 0x70: // cpobj + case 0x71: // ldobj + case 0x72: // ldstr + case 0x74: // castclass + case 0x75: // isinst + case 0x79: // unbox + case 0x7B: // ldfld + case 0x7C: // ldflda + case 0x7D: // stfld + case 0x7E: // ldsfld + case 0x7F: // ldsflda + case 0x80: // stsfld + case 0x81: // stobj + case 0x8C: // box + case 0x8D: // newarr + case 0x8F: // ldelema + case 0xA3: // ldelem + case 0xA4: // stelem + case 0xA5: // unbox.any + case 0xC2: // refanyval + case 0xC6: // mkrefany + case 0xD0: // ldtoken + return 4; + + // InlineSwitch — handled in GetOperandSize before reaching here + case 0x45: + return 0; // should not reach here + + default: + return 0; + } + } + + /// + /// Returns the operand size for opcodes with 0xFE prefix. + /// Based on ECMA-335 Partition III, Table III.2. + /// + private static int GetFEPrefixOperandSize(byte opcode2) + { + switch (opcode2) + { + // InlineNone + case 0x00: // arglist + case 0x01: // ceq + case 0x02: // cgt + case 0x03: // cgt.un + case 0x04: // clt + case 0x05: // clt.un + case 0x0F: // localloc + case 0x11: // endfilter + case 0x12: // volatile. (prefix) + case 0x13: // tail. (prefix) + case 0x16: // readonly. (prefix) + case 0x17: // cpblk + case 0x18: // initblk + case 0x1A: // rethrow + case 0x1D: // refanytype + return 0; + + // InlineMethod (4 bytes) + case 0x06: // ldftn + case 0x07: // ldvirtftn + return 4; + + // InlineType (4 bytes) + case 0x14: // initobj + case 0x15: // constrained. (prefix) + case 0x1C: // sizeof + return 4; + + // InlineVar (2 bytes) + case 0x09: // ldarg + case 0x0A: // ldarga + case 0x0B: // starg + case 0x0C: // ldloc + case 0x0D: // ldloca + case 0x0E: // stloc + return 2; + + default: + return 0; + } + } + + #endregion + + public void Dispose() + { + foreach (var entry in _peReaderCache.Values) + { + entry?.Dispose(); + } + + _peReaderCache.Clear(); + } + + private sealed class PEReaderEntry : IDisposable + { + public PEReader PEReader { get; } + public MetadataReader MetadataReader { get; } + private readonly FileStream _stream; + + public PEReaderEntry(PEReader peReader, MetadataReader metadataReader, FileStream stream) + { + PEReader = peReader; + MetadataReader = metadataReader; + _stream = stream; + } + + public void Dispose() + { + PEReader.Dispose(); + _stream.Dispose(); + } + } + } + + /// + /// Represents a call target discovered via IL analysis. + /// + internal sealed class ILCallTarget + { + public IMethodSymbol Method { get; } + + public ILCallTarget(IMethodSymbol method) + { + Method = method; + } + } +} diff --git a/src/ThreadSafeTaskAnalyzer/MultiThreadableTaskAnalyzer.cs b/src/ThreadSafeTaskAnalyzer/MultiThreadableTaskAnalyzer.cs new file mode 100644 index 00000000000..a76b0511bdc --- /dev/null +++ b/src/ThreadSafeTaskAnalyzer/MultiThreadableTaskAnalyzer.cs @@ -0,0 +1,592 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +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) + /// + /// Per review feedback from @rainersigwald: + /// - Console.* promoted to MSBuildTask0001 (always wrong in tasks) + /// - Helper classes can opt in via [MSBuildMultiThreadableTaskAnalyzed] attribute + /// + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public sealed class MultiThreadableTaskAnalyzer : DiagnosticAnalyzer + { + /// + /// The .editorconfig key controlling analysis scope. + /// Values: "all" (default) | "multithreadable_only" + /// + internal const string ScopeOptionKey = "msbuild_task_analyzer.scope"; + internal const string ScopeAll = "all"; + internal const string ScopeMultiThreadableOnly = "multithreadable_only"; + // Well-known type names + private const string ITaskFullName = "Microsoft.Build.Framework.ITask"; + private const string IMultiThreadableTaskFullName = "Microsoft.Build.Framework.IMultiThreadableTask"; + private const string TaskEnvironmentFullName = "Microsoft.Build.Framework.TaskEnvironment"; + private const string AbsolutePathFullName = "Microsoft.Build.Framework.AbsolutePath"; + private const string ITaskItemFullName = "Microsoft.Build.Framework.ITaskItem"; + private const string AnalyzedAttributeFullName = "Microsoft.Build.Framework.MSBuildMultiThreadableTaskAnalyzedAttribute"; + private const string MultiThreadableTaskAttributeFullName = "Microsoft.Build.Framework.MSBuildMultiThreadableTaskAttribute"; + + public override ImmutableArray SupportedDiagnostics => DiagnosticDescriptors.All; + + public override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.RegisterCompilationStartAction(OnCompilationStart); + } + + private void OnCompilationStart(CompilationStartAnalysisContext compilationContext) + { + // Resolve well-known types + var iTaskType = compilationContext.Compilation.GetTypeByMetadataName(ITaskFullName); + if (iTaskType is null) + { + // No ITask in compilation - nothing to analyze + return; + } + + // Read scope option from .editorconfig: "all" (default) or "multithreadable_only" + bool analyzeAllTasks = true; + if (compilationContext.Options.AnalyzerConfigOptionsProvider + .GlobalOptions.TryGetValue($"build_property.{ScopeOptionKey}", out var scopeValue) || + compilationContext.Options.AnalyzerConfigOptionsProvider + .GlobalOptions.TryGetValue(ScopeOptionKey, out scopeValue)) + { + analyzeAllTasks = !string.Equals(scopeValue, ScopeMultiThreadableOnly, StringComparison.OrdinalIgnoreCase); + } + + var iMultiThreadableTaskType = compilationContext.Compilation.GetTypeByMetadataName(IMultiThreadableTaskFullName); + var taskEnvironmentType = compilationContext.Compilation.GetTypeByMetadataName(TaskEnvironmentFullName); + var absolutePathType = compilationContext.Compilation.GetTypeByMetadataName(AbsolutePathFullName); + var iTaskItemType = compilationContext.Compilation.GetTypeByMetadataName(ITaskItemFullName); + var consoleType = compilationContext.Compilation.GetTypeByMetadataName("System.Console"); + var analyzedAttributeType = compilationContext.Compilation.GetTypeByMetadataName(AnalyzedAttributeFullName); + var multiThreadableTaskAttributeType = compilationContext.Compilation.GetTypeByMetadataName(MultiThreadableTaskAttributeFullName); + + // Build symbol lookup for banned APIs + var bannedApiLookup = BuildBannedApiLookup(compilationContext.Compilation); + + // Build set of file-path types for MSBuildTask0003 + var filePathTypes = ResolveFilePathTypes(compilationContext.Compilation); + + // Use RegisterSymbolStartAction for efficient per-type scoping + compilationContext.RegisterSymbolStartAction(symbolStartContext => + { + var namedType = (INamedTypeSymbol)symbolStartContext.Symbol; + + // Determine what kind of task this is + bool isTask = ImplementsInterface(namedType, iTaskType); + bool isMultiThreadableTask = iMultiThreadableTaskType is not null && ImplementsInterface(namedType, iMultiThreadableTaskType); + + // Helper classes can opt-in via [MSBuildMultiThreadableTaskAnalyzed] attribute + bool hasAnalyzedAttribute = analyzedAttributeType is not null && + namedType.GetAttributes().Any(a => SymbolEqualityComparer.Default.Equals(a.AttributeClass, analyzedAttributeType)); + + // Tasks marked with [MSBuildMultiThreadableTask] should be analyzed as multithreadable + bool hasMultiThreadableAttribute = multiThreadableTaskAttributeType is not null && + namedType.GetAttributes().Any(a => SymbolEqualityComparer.Default.Equals(a.AttributeClass, multiThreadableTaskAttributeType)); + + if (!isTask && !hasAnalyzedAttribute) + { + return; + } + + // 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; + + // Register operation-level analysis within this type + symbolStartContext.RegisterOperationAction( + ctx => AnalyzeOperation(ctx, bannedApiLookup, filePathTypes, reportEnvironmentRules, + taskEnvironmentType, absolutePathType, iTaskItemType, consoleType), + OperationKind.Invocation, + OperationKind.ObjectCreation, + OperationKind.PropertyReference, + OperationKind.FieldReference, + OperationKind.MethodReference, + OperationKind.EventReference); + }, SymbolKind.NamedType); + } + + private static void AnalyzeOperation( + OperationAnalysisContext context, + Dictionary bannedApiLookup, + ImmutableHashSet filePathTypes, + bool reportEnvironmentRules, + INamedTypeSymbol? taskEnvironmentType, + INamedTypeSymbol? absolutePathType, + INamedTypeSymbol? iTaskItemType, + INamedTypeSymbol? consoleType) + { + ISymbol? referencedSymbol = null; + ImmutableArray arguments = default; + bool isConstructor = false; + + switch (context.Operation) + { + case IInvocationOperation invocation: + referencedSymbol = invocation.TargetMethod; + arguments = invocation.Arguments; + break; + + case IObjectCreationOperation creation: + referencedSymbol = creation.Constructor; + arguments = creation.Arguments; + isConstructor = true; + break; + + case IPropertyReferenceOperation propRef: + referencedSymbol = propRef.Property; + break; + + case IFieldReferenceOperation fieldRef: + referencedSymbol = fieldRef.Field; + break; + + case IMethodReferenceOperation methodRef: + referencedSymbol = methodRef.Method; + break; + + case IEventReferenceOperation eventRef: + referencedSymbol = eventRef.Event; + break; + } + + if (referencedSymbol is null) + { + return; + } + + // 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) + { + return; + } + + var descriptor = GetDescriptor(entry.Category); + var displayName = referencedSymbol.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat); + context.ReportDiagnostic(Diagnostic.Create(descriptor, context.Operation.Syntax.GetLocation(), + displayName, entry.Message)); + return; + } + + // 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) + { + var containingType = referencedSymbol.ContainingType; + if (containingType is not null && SymbolEqualityComparer.Default.Equals(containingType, consoleType)) + { + var displayName = referencedSymbol.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat); + string message = referencedSymbol.Name.StartsWith("Read", StringComparison.Ordinal) + ? "may cause deadlocks in automated builds" + : "interferes with build logging; use Log.LogMessage instead"; + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.CriticalError, + context.Operation.Syntax.GetLocation(), + displayName, message)); + return; + } + } + + // Check file path APIs (MSBuildTask0003) - gated by scope setting + if (reportEnvironmentRules && !arguments.IsDefaultOrEmpty) + { + var method = referencedSymbol as IMethodSymbol; + if (method is not null) + { + var containingType = method.ContainingType; + if (containingType is not null && filePathTypes.Contains(containingType)) + { + if (HasUnwrappedPathArgument(arguments, taskEnvironmentType, absolutePathType, iTaskItemType)) + { + string displayName = isConstructor + ? $"new {containingType.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat)}(...)" + : referencedSymbol.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat); + + string hint = "wrap path argument with TaskEnvironment.GetAbsolutePath()"; + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.FilePathRequiresAbsolute, + context.Operation.Syntax.GetLocation(), + displayName, hint)); + } + } + } + } + } + + /// + /// Checks whether ANY path-typed string parameter of a file API call is NOT wrapped with a safe pattern. + /// Only checks parameters whose names suggest they are file paths (e.g., "path", "fileName", "sourceFileName"). + /// Non-path string parameters (e.g., "contents", "searchPattern") are skipped. + /// + private static bool HasUnwrappedPathArgument( + ImmutableArray arguments, + INamedTypeSymbol? taskEnvironmentType, + INamedTypeSymbol? absolutePathType, + INamedTypeSymbol? iTaskItemType) + { + // Check each argument, using its associated parameter to determine if it's a path + for (int i = 0; i < arguments.Length; i++) + { + var arg = arguments[i]; + var param = arg.Parameter; + if (param is null || param.Type.SpecialType != SpecialType.System_String) + { + continue; + } + + // Only check parameters that are likely paths based on their name + if (!IsPathParameterName(param.Name)) + { + continue; + } + + var argValue = arg.Value; + + if (!IsWrappedSafely(argValue, taskEnvironmentType, absolutePathType, iTaskItemType)) + { + return true; + } + } + + return false; + } + + /// + /// Determines if a parameter name suggests it represents a file system path. + /// + private static bool IsPathParameterName(string paramName) + { + // Common path parameter names in System.IO, System.Xml, and other BCL APIs + return paramName.IndexOf("path", StringComparison.OrdinalIgnoreCase) >= 0 + || paramName.IndexOf("file", StringComparison.OrdinalIgnoreCase) >= 0 + || paramName.IndexOf("dir", StringComparison.OrdinalIgnoreCase) >= 0 + || paramName.IndexOf("folder", StringComparison.OrdinalIgnoreCase) >= 0 + || paramName.IndexOf("source", StringComparison.OrdinalIgnoreCase) >= 0 + || paramName.IndexOf("dest", StringComparison.OrdinalIgnoreCase) >= 0 + || paramName.IndexOf("uri", StringComparison.OrdinalIgnoreCase) >= 0 + || paramName.IndexOf("url", StringComparison.OrdinalIgnoreCase) >= 0; + } + + /// + /// Recursively checks if an operation represents a safely-wrapped path. + /// + private static bool IsWrappedSafely( + IOperation operation, + INamedTypeSymbol? taskEnvironmentType, + INamedTypeSymbol? absolutePathType, + INamedTypeSymbol? iTaskItemType) + { + // Unwrap conversions (implicit AbsolutePath -> string, etc.) + while (operation is IConversionOperation conversion) + { + // Check if converting from AbsolutePath or Nullable + if (absolutePathType is not null && + IsAbsolutePathType(conversion.Operand.Type, absolutePathType)) + { + return true; + } + + operation = conversion.Operand; + } + + // Null literals and default values are safe — they don't represent relative paths + if (operation is ILiteralOperation lit && lit.ConstantValue.HasValue && lit.ConstantValue.Value is null) + { + return true; + } + + if (operation is IDefaultValueOperation) + { + return true; + } + + // Check: TaskEnvironment.GetAbsolutePath(...) + if (operation is IInvocationOperation invocation) + { + if (invocation.TargetMethod.Name == "GetAbsolutePath" && + taskEnvironmentType is not null && + SymbolEqualityComparer.Default.Equals(invocation.TargetMethod.ContainingType, taskEnvironmentType)) + { + return true; + } + + // Check: ITaskItem.GetMetadata("FullPath") / GetMetadataValue("FullPath") + if ((invocation.TargetMethod.Name == "GetMetadata" || invocation.TargetMethod.Name == "GetMetadataValue") && iTaskItemType is not null) + { + var receiverType = invocation.TargetMethod.ContainingType; + if (receiverType is not null && (SymbolEqualityComparer.Default.Equals(receiverType, iTaskItemType) || ImplementsInterface(receiverType, iTaskItemType))) + { + // Check if the argument is the literal string "FullPath" + if (invocation.Arguments.Length > 0 && + invocation.Arguments[0].Value is ILiteralOperation literal && + literal.ConstantValue.HasValue && + literal.ConstantValue.Value is string metadataName && + string.Equals(metadataName, "FullPath", StringComparison.OrdinalIgnoreCase)) + { + return true; + } + } + } + + // Check: Path.GetDirectoryName(safe) — directory of an absolute path is absolute + if (invocation.TargetMethod.Name == "GetDirectoryName" && + invocation.TargetMethod.ContainingType?.ToDisplayString() == "System.IO.Path" && + invocation.Arguments.Length >= 1 && + IsWrappedSafely(invocation.Arguments[0].Value, taskEnvironmentType, absolutePathType, iTaskItemType)) + { + return true; + } + + // Check: Path.Combine(safe, ...) — result is absolute when first arg is absolute + if (invocation.TargetMethod.Name == "Combine" && + invocation.TargetMethod.ContainingType?.ToDisplayString() == "System.IO.Path" && + invocation.Arguments.Length >= 2 && + IsWrappedSafely(invocation.Arguments[0].Value, taskEnvironmentType, absolutePathType, iTaskItemType)) + { + return true; + } + + // Check: Path.GetFullPath(safe) — safe only when input is already absolute. + // If input is relative, GetFullPath resolves against CWD (wrong in MT). + // The GetFullPath call itself is still flagged by MSBuildTask0002 regardless. + if (invocation.TargetMethod.Name == "GetFullPath" && + invocation.TargetMethod.ContainingType?.ToDisplayString() == "System.IO.Path" && + invocation.Arguments.Length >= 1 && + IsWrappedSafely(invocation.Arguments[0].Value, taskEnvironmentType, absolutePathType, iTaskItemType)) + { + return true; + } + } + + // Check: .FullName property (FileSystemInfo.FullName - base class of FileInfo/DirectoryInfo) + if (operation is IPropertyReferenceOperation propRef) + { + if (propRef.Property.Name == "FullName") + { + var containingTypeName = propRef.Property.ContainingType?.ToDisplayString(); + // FullName is declared on FileSystemInfo (base of FileInfo, DirectoryInfo) + if (containingTypeName is "System.IO.FileSystemInfo" or "System.IO.FileInfo" or "System.IO.DirectoryInfo") + { + return true; + } + } + } + + // Check: argument type is AbsolutePath or Nullable + if (absolutePathType is not null && + operation.Type is not null && + IsAbsolutePathType(operation.Type, absolutePathType)) + { + return true; + } + + // Trace through local variable assignments: if `string dir = Path.GetDirectoryName(abs);` + // then `dir` is safe because its initializer is safe. + if (operation is ILocalReferenceOperation localRef) + { + var local = localRef.Local; + if (local.DeclaringSyntaxReferences.Length == 1) + { + var syntax = local.DeclaringSyntaxReferences[0].GetSyntax(); + var semanticModel = operation.SemanticModel; + if (semanticModel is not null) + { + var declOp = semanticModel.GetOperation(syntax); + if (declOp is IVariableDeclaratorOperation declarator && + declarator.Initializer?.Value is IOperation initValue) + { + return IsWrappedSafely(initValue, taskEnvironmentType, absolutePathType, iTaskItemType); + } + } + } + } + + return false; + } + + /// + /// Checks if a type is AbsolutePath or Nullable<AbsolutePath>. + /// + private static bool IsAbsolutePathType(ITypeSymbol? type, INamedTypeSymbol absolutePathType) + { + if (type is null) + { + return false; + } + + // Direct match: AbsolutePath + if (SymbolEqualityComparer.Default.Equals(type, absolutePathType)) + { + return true; + } + + // Nullable — the type is Nullable where T is AbsolutePath + if (type is INamedTypeSymbol namedType && + namedType.OriginalDefinition.SpecialType == SpecialType.System_Nullable_T && + namedType.TypeArguments.Length == 1 && + SymbolEqualityComparer.Default.Equals(namedType.TypeArguments[0], absolutePathType)) + { + return true; + } + + return false; + } + + /// + /// Builds a dictionary mapping resolved ISymbols to their banned API entry for O(1) lookup. + /// + private static Dictionary BuildBannedApiLookup(Compilation compilation) + { + var result = new Dictionary(SymbolEqualityComparer.Default); + var definitions = BannedApiDefinitions.GetAll(); + + foreach (var def in definitions) + { + ImmutableArray symbols; + try + { + var resolved = DocumentationCommentId.GetSymbolsForDeclarationId(def.DeclarationId, compilation); + symbols = resolved.IsDefault ? ImmutableArray.Empty : resolved; + } + catch + { + continue; + } + + foreach (var symbol in symbols) + { + if (!result.ContainsKey(symbol)) + { + result[symbol] = new BannedApiEntry(def.Category, def.Message); + } + } + } + + return result; + } + + /// + /// Resolves the set of types whose methods take path parameters that need absolutization. + /// + private static ImmutableHashSet ResolveFilePathTypes(Compilation compilation) + { + var typeNames = new[] + { + // Core System.IO file/directory types + "System.IO.File", + "System.IO.Directory", + "System.IO.FileInfo", + "System.IO.DirectoryInfo", + "System.IO.FileStream", + "System.IO.StreamReader", + "System.IO.StreamWriter", + "System.IO.FileSystemWatcher", + "System.IO.BinaryReader", + "System.IO.BinaryWriter", + + // XML types that accept file paths in Load/Save/Create + "System.Xml.Linq.XDocument", + "System.Xml.Linq.XElement", + "System.Xml.XmlDocument", + "System.Xml.XmlReader", + "System.Xml.XmlWriter", + "System.Xml.XmlTextReader", + "System.Xml.XmlTextWriter", + "System.Xml.Xsl.XslCompiledTransform", + "System.Xml.Schema.XmlSchema", + + // Compression types that accept file paths + "System.IO.Compression.ZipFile", + + // Memory-mapped files + "System.IO.MemoryMappedFiles.MemoryMappedFile", + + // Security / certificates + "System.Security.Cryptography.X509Certificates.X509Certificate", + "System.Security.Cryptography.X509Certificates.X509Certificate2", + + // Diagnostics + "System.Diagnostics.FileVersionInfo", + "System.Diagnostics.TextWriterTraceListener", + + // Resources + "System.Resources.ResourceReader", + "System.Resources.ResourceWriter", + + // Assembly loading (supplements the banned-API list for path-based overloads) + "System.Runtime.Loader.AssemblyLoadContext", + }; + + var builder = ImmutableHashSet.CreateBuilder(SymbolEqualityComparer.Default); + foreach (var name in typeNames) + { + var type = compilation.GetTypeByMetadataName(name); + if (type is not null) + { + builder.Add(type); + } + } + + return builder.ToImmutable(); + } + + private static DiagnosticDescriptor GetDescriptor(BannedApiDefinitions.ApiCategory category) + { + return category switch + { + BannedApiDefinitions.ApiCategory.CriticalError => DiagnosticDescriptors.CriticalError, + BannedApiDefinitions.ApiCategory.TaskEnvironment => DiagnosticDescriptors.TaskEnvironmentRequired, + BannedApiDefinitions.ApiCategory.PotentialIssue => DiagnosticDescriptors.PotentialIssue, + _ => DiagnosticDescriptors.TaskEnvironmentRequired, + }; + } + + private static bool ImplementsInterface(INamedTypeSymbol type, INamedTypeSymbol interfaceType) + { + foreach (var iface in type.AllInterfaces) + { + if (SymbolEqualityComparer.Default.Equals(iface, interfaceType)) + { + return true; + } + } + + return false; + } + + private readonly struct BannedApiEntry + { + public BannedApiDefinitions.ApiCategory Category { get; } + public string Message { get; } + + public BannedApiEntry(BannedApiDefinitions.ApiCategory category, string message) + { + Category = category; + Message = message; + } + } + } +} diff --git a/src/ThreadSafeTaskAnalyzer/MultiThreadableTaskCodeFixProvider.cs b/src/ThreadSafeTaskAnalyzer/MultiThreadableTaskCodeFixProvider.cs new file mode 100644 index 00000000000..fe90106c623 --- /dev/null +++ b/src/ThreadSafeTaskAnalyzer/MultiThreadableTaskCodeFixProvider.cs @@ -0,0 +1,305 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Immutable; +using System.Composition; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Editing; + +namespace Microsoft.Build.TaskAuthoring.Analyzer +{ + /// + /// Code fixer for the thread-safe task analyzer. + /// Fixes: + /// - MSBuildTask0002: Replaces banned APIs with TaskEnvironment equivalents + /// - MSBuildTask0003: Wraps path arguments with TaskEnvironment.GetAbsolutePath() + /// + [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(MultiThreadableTaskCodeFixProvider)), Shared] + public sealed class MultiThreadableTaskCodeFixProvider : CodeFixProvider + { + public override ImmutableArray FixableDiagnosticIds => + ImmutableArray.Create(DiagnosticIds.TaskEnvironmentRequired, DiagnosticIds.FilePathRequiresAbsolute); + + public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + if (root is null) + { + return; + } + + foreach (var diagnostic in context.Diagnostics) + { + var node = root.FindNode(diagnostic.Location.SourceSpan); + + if (diagnostic.Id == DiagnosticIds.FilePathRequiresAbsolute) + { + RegisterFilePathFix(context, node, diagnostic); + } + else if (diagnostic.Id == DiagnosticIds.TaskEnvironmentRequired) + { + RegisterTaskEnvironmentFix(context, node, diagnostic); + } + } + } + + private static void RegisterFilePathFix(CodeFixContext context, SyntaxNode node, Diagnostic diagnostic) + { + // Find the invocation or object creation expression + var invocation = FindContainingCall(node); + if (invocation is null) + { + return; + } + + ArgumentListSyntax? argumentList = invocation switch + { + InvocationExpressionSyntax inv => inv.ArgumentList, + ObjectCreationExpressionSyntax obj => obj.ArgumentList, + ImplicitObjectCreationExpressionSyntax impl => impl.ArgumentList, + _ => null, + }; + + if (argumentList is null || argumentList.Arguments.Count == 0) + { + return; + } + + // Find the first argument that is NOT already wrapped with TaskEnvironment.GetAbsolutePath() + ArgumentSyntax? targetArg = null; + foreach (var arg in argumentList.Arguments) + { + if (!IsAlreadyWrapped(arg.Expression)) + { + targetArg = arg; + break; + } + } + + if (targetArg is null) + { + return; + } + + context.RegisterCodeFix( + CodeAction.Create( + title: "Wrap with TaskEnvironment.GetAbsolutePath()", + createChangedDocument: ct => WrapArgumentWithGetAbsolutePathAsync(context.Document, targetArg, ct), + equivalenceKey: "WrapWithGetAbsolutePath"), + diagnostic); + } + + /// + /// Checks whether an argument expression is already wrapped in TaskEnvironment.GetAbsolutePath(). + /// + private static bool IsAlreadyWrapped(ExpressionSyntax expression) + { + if (expression is InvocationExpressionSyntax inv && + inv.Expression is MemberAccessExpressionSyntax ma && + ma.Name.Identifier.Text == "GetAbsolutePath") + { + var receiverName = GetSimpleTypeName(ma.Expression); + return receiverName == "TaskEnvironment"; + } + + return false; + } + + private static void RegisterTaskEnvironmentFix(CodeFixContext context, SyntaxNode node, Diagnostic diagnostic) + { + // Try to determine which API replacement to offer + var invocation = node as InvocationExpressionSyntax ?? node.AncestorsAndSelf().OfType().FirstOrDefault(); + var memberAccess = node as MemberAccessExpressionSyntax ?? node.AncestorsAndSelf().OfType().FirstOrDefault(); + + if (invocation is not null && invocation.Expression is MemberAccessExpressionSyntax invMemberAccess) + { + var targetTypeName = GetSimpleTypeName(invMemberAccess.Expression); + var methodName = invMemberAccess.Name.Identifier.Text; + + if (targetTypeName == "Environment") + { + switch (methodName) + { + case "GetEnvironmentVariable": + RegisterSimpleReplacement(context, diagnostic, invocation, + "TaskEnvironment", "GetEnvironmentVariable", + "Use TaskEnvironment.GetEnvironmentVariable()"); + return; + + case "SetEnvironmentVariable" when invocation.ArgumentList.Arguments.Count == 2: + RegisterSimpleReplacement(context, diagnostic, invocation, + "TaskEnvironment", "SetEnvironmentVariable", + "Use TaskEnvironment.SetEnvironmentVariable()"); + return; + + case "GetEnvironmentVariables": + RegisterSimpleReplacement(context, diagnostic, invocation, + "TaskEnvironment", "GetEnvironmentVariables", + "Use TaskEnvironment.GetEnvironmentVariables()"); + return; + } + } + else if (targetTypeName == "Path" && methodName == "GetFullPath") + { + // Only offer fix for single-argument overload + if (invocation.ArgumentList.Arguments.Count == 1) + { + RegisterSimpleReplacement(context, diagnostic, invocation, + "TaskEnvironment", "GetAbsolutePath", + "Use TaskEnvironment.GetAbsolutePath()"); + } + return; + } + else if (targetTypeName == "Directory" && methodName == "GetCurrentDirectory") + { + // Directory.GetCurrentDirectory() → TaskEnvironment.ProjectDirectory + context.RegisterCodeFix( + CodeAction.Create( + title: "Use TaskEnvironment.ProjectDirectory", + createChangedDocument: ct => ReplaceInvocationWithPropertyAsync( + context.Document, invocation, "TaskEnvironment", "ProjectDirectory", ct), + equivalenceKey: "UseProjectDirectory"), + diagnostic); + return; + } + } + + // Handle Environment.CurrentDirectory (property access, not invocation) + if (memberAccess is not null) + { + var targetTypeName = GetSimpleTypeName(memberAccess.Expression); + var memberName = memberAccess.Name.Identifier.Text; + + if (targetTypeName == "Environment" && memberName == "CurrentDirectory") + { + context.RegisterCodeFix( + CodeAction.Create( + title: "Use TaskEnvironment.ProjectDirectory", + createChangedDocument: ct => ReplacePropertyAccessAsync( + context.Document, memberAccess, "TaskEnvironment", "ProjectDirectory", ct), + equivalenceKey: "UseProjectDirectory"), + diagnostic); + } + } + } + + private static void RegisterSimpleReplacement( + CodeFixContext context, Diagnostic diagnostic, + InvocationExpressionSyntax invocation, + string newTypeName, string newMethodName, string title) + { + context.RegisterCodeFix( + CodeAction.Create( + title: title, + createChangedDocument: ct => ReplaceInvocationTargetAsync( + context.Document, invocation, newTypeName, newMethodName, ct), + equivalenceKey: title), + diagnostic); + } + + private static async Task WrapArgumentWithGetAbsolutePathAsync( + Document document, ArgumentSyntax argument, CancellationToken ct) + { + var editor = await DocumentEditor.CreateAsync(document, ct).ConfigureAwait(false); + + var wrappedExpr = SyntaxFactory.InvocationExpression( + SyntaxFactory.MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + SyntaxFactory.IdentifierName("TaskEnvironment"), + SyntaxFactory.IdentifierName("GetAbsolutePath")), + SyntaxFactory.ArgumentList( + SyntaxFactory.SingletonSeparatedList( + SyntaxFactory.Argument(argument.Expression)))); + + var newArgument = argument.WithExpression(wrappedExpr); + editor.ReplaceNode(argument, newArgument); + + return editor.GetChangedDocument(); + } + + private static async Task ReplaceInvocationTargetAsync( + Document document, InvocationExpressionSyntax invocation, + string newTypeName, string newMethodName, CancellationToken ct) + { + var editor = await DocumentEditor.CreateAsync(document, ct).ConfigureAwait(false); + + var newMemberAccess = SyntaxFactory.MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + SyntaxFactory.IdentifierName(newTypeName), + SyntaxFactory.IdentifierName(newMethodName)); + + var newInvocation = invocation.WithExpression(newMemberAccess); + editor.ReplaceNode(invocation, newInvocation); + + return editor.GetChangedDocument(); + } + + private static async Task ReplacePropertyAccessAsync( + Document document, MemberAccessExpressionSyntax memberAccess, + string newTypeName, string newPropertyName, CancellationToken ct) + { + var editor = await DocumentEditor.CreateAsync(document, ct).ConfigureAwait(false); + + var newExpression = SyntaxFactory.MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + SyntaxFactory.IdentifierName(newTypeName), + SyntaxFactory.IdentifierName(newPropertyName)); + + editor.ReplaceNode(memberAccess, newExpression); + + return editor.GetChangedDocument(); + } + + /// + /// Replaces an invocation (e.g. Directory.GetCurrentDirectory()) with a property access (e.g. TaskEnvironment.ProjectDirectory). + /// + private static async Task ReplaceInvocationWithPropertyAsync( + Document document, InvocationExpressionSyntax invocation, + string newTypeName, string newPropertyName, CancellationToken ct) + { + var editor = await DocumentEditor.CreateAsync(document, ct).ConfigureAwait(false); + + var newExpression = SyntaxFactory.MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + SyntaxFactory.IdentifierName(newTypeName), + SyntaxFactory.IdentifierName(newPropertyName)); + + editor.ReplaceNode(invocation, newExpression); + + return editor.GetChangedDocument(); + } + + /// + /// Extracts the simple type name from an expression (handles both simple and qualified names). + /// + private static string? GetSimpleTypeName(ExpressionSyntax expression) + { + return expression switch + { + IdentifierNameSyntax id => id.Identifier.Text, + MemberAccessExpressionSyntax ma => ma.Name.Identifier.Text, + _ => null, + }; + } + + /// + /// Finds the containing invocation or object creation from a diagnostic node. + /// + private static SyntaxNode? FindContainingCall(SyntaxNode node) + { + return node.AncestorsAndSelf().FirstOrDefault(n => + n is InvocationExpressionSyntax || + n is ObjectCreationExpressionSyntax || + n is ImplicitObjectCreationExpressionSyntax); + } + } +} diff --git a/src/ThreadSafeTaskAnalyzer/README.md b/src/ThreadSafeTaskAnalyzer/README.md new file mode 100644 index 00000000000..ef57b6d6bf8 --- /dev/null +++ b/src/ThreadSafeTaskAnalyzer/README.md @@ -0,0 +1,252 @@ +# MSBuild Thread-Safe Task Analyzer + +A Roslyn analyzer that detects unsafe API usage in MSBuild task implementations. It guides task authors toward thread-safe patterns required for MSBuild's multithreaded task execution mode, where multiple tasks may run concurrently in the same process. + +## Background + +MSBuild is introducing multithreaded task execution via `IMultiThreadableTask`. Tasks opting into this mode share a process and can no longer safely use process-global state like environment variables, the current directory, or `Console` output. The `TaskEnvironment` abstraction provides per-task isolated access to these resources. + +This analyzer catches unsafe API usage at compile time and offers code fixes to migrate to the safe `TaskEnvironment` alternatives. + +## Diagnostic Rules + +| ID | Severity | Scope | Title | +|---|---|---|---| +| **MSBuildTask0001** | Error | All `ITask` implementations | API is never safe in MSBuild tasks | +| **MSBuildTask0002** | Warning | `IMultiThreadableTask` only | API requires `TaskEnvironment` alternative | +| **MSBuildTask0003** | Warning | `IMultiThreadableTask` only | File system API requires absolute path | +| **MSBuildTask0004** | Warning | All `ITask` implementations | API may cause issues in multithreaded tasks | + +### MSBuildTask0001 — Critical: No Safe Alternative + +These APIs affect the entire process or interfere with build infrastructure. They are **errors** and should never appear in any MSBuild task. + +| API | Why it's banned | +|---|---| +| `Console.*` (all members) | Interferes with build logging; use `Log.LogMessage` | +| `Console.ReadLine`, `Console.Read` | May cause deadlocks in automated builds | +| `Environment.Exit`, `Environment.FailFast` | Terminates the entire MSBuild process | +| `Process.Kill` | May terminate the MSBuild host process | +| `ThreadPool.SetMinThreads`, `SetMaxThreads` | Modifies process-wide thread pool settings | +| `CultureInfo.DefaultThreadCurrentCulture` | Affects culture of all new threads in process | +| `CultureInfo.DefaultThreadCurrentUICulture` | Affects UI culture of all new threads | +| `Directory.SetCurrentDirectory` | Modifies process-wide working directory | + +> **Note:** `Console.*` is detected at the **type level** — every member of `System.Console` is flagged, including properties like `Console.Out`, `Console.Error`, `Console.BufferWidth`, and any members added in future .NET versions. This also catches `using static System.Console` patterns. + +### MSBuildTask0002 — Use TaskEnvironment Alternative + +These APIs access process-global state that varies per task in multithreaded mode. Only reported for `IMultiThreadableTask` implementations. + +| Banned API | Replacement | +|---|---| +| `Environment.CurrentDirectory` | `TaskEnvironment.ProjectDirectory` | +| `Directory.GetCurrentDirectory()` | `TaskEnvironment.ProjectDirectory` | +| `Environment.GetEnvironmentVariable()` | `TaskEnvironment.GetEnvironmentVariable()` | +| `Environment.SetEnvironmentVariable()` | `TaskEnvironment.SetEnvironmentVariable()` | +| `Environment.GetEnvironmentVariables()` | `TaskEnvironment.GetEnvironmentVariables()` | +| `Environment.ExpandEnvironmentVariables()` | Use `TaskEnvironment.GetEnvironmentVariable()` per variable | +| `Environment.GetFolderPath()` | Use `TaskEnvironment.GetEnvironmentVariable()` | +| `Path.GetFullPath()` | `TaskEnvironment.GetAbsolutePath()` | +| `Path.GetTempPath()` | `TaskEnvironment.GetEnvironmentVariable("TMP")` | +| `Path.GetTempFileName()` | `TaskEnvironment.GetEnvironmentVariable("TMP")` | +| `Process.Start()` (all overloads) | `TaskEnvironment.GetProcessStartInfo()` | +| `new ProcessStartInfo()` (all overloads) | `TaskEnvironment.GetProcessStartInfo()` | + +### MSBuildTask0003 — File Paths Must Be Absolute + +File system APIs that accept a path parameter will resolve relative paths against the process working directory — which is shared and unpredictable in multithreaded mode. Only reported for `IMultiThreadableTask` implementations. + +**Monitored types:** `File`, `Directory`, `FileInfo`, `DirectoryInfo`, `FileStream`, `StreamReader`, `StreamWriter`, `FileSystemWatcher` + +The analyzer inspects parameter names to determine which arguments are paths (e.g., `path`, `fileName`, `sourceFileName`, `destFileName`) and skips non-path string parameters (e.g., `contents`, `searchPattern`). Named arguments are handled correctly. + +**Recognized safe patterns** (suppress the diagnostic): + +```csharp +// 1. TaskEnvironment.GetAbsolutePath() +File.Exists(TaskEnvironment.GetAbsolutePath(relativePath)) + +// 2. Implicit conversion from AbsolutePath +AbsolutePath abs = TaskEnvironment.GetAbsolutePath(relativePath); +File.Exists(abs) // implicit conversion to string + +// 3. FileInfo.FullName / DirectoryInfo.FullName +new FileInfo(somePath).FullName // already absolute + +// 4. ITaskItem.GetMetadata("FullPath") +File.Exists(item.GetMetadata("FullPath")) +File.Exists(item.GetMetadataValue("FullPath")) + +// 5. Argument already typed as AbsolutePath +void Helper(AbsolutePath p) => File.Exists(p); +``` + +### MSBuildTask0004 — Potential Issue (Review Required) + +These APIs may cause version conflicts or other issues in a shared task host. Reported as warnings in all `ITask` implementations. + +| API | Concern | +|---|---| +| `Assembly.Load`, `LoadFrom`, `LoadFile` | May cause version conflicts in shared task host | +| `Assembly.LoadWithPartialName` | Obsolete and may cause version conflicts | +| `Activator.CreateInstance(string, string)` | May cause version conflicts | +| `Activator.CreateInstanceFrom` | May cause version conflicts | +| `AppDomain.Load`, `CreateInstance`, `CreateInstanceFrom` | May cause version conflicts | + +## Analysis Scope + +The analyzer determines what to check based on the type declaration: + +| Type | Rules Applied | +|---|---| +| Any class implementing `ITask` | MSBuildTask0001 + MSBuildTask0004 | +| Class implementing `IMultiThreadableTask` | All four rules | +| Class with `[MSBuildMultiThreadableTask]` attribute | All four rules | +| Helper class with `[MSBuildMultiThreadableTaskAnalyzed]` attribute | All four rules | +| Regular class (no task interface or attribute) | Not analyzed | + +The `[MSBuildMultiThreadableTaskAnalyzed]` attribute allows opting in helper classes that are called from multithreadable tasks but don't implement `ITask` themselves. + +## Code Fixes + +The analyzer ships with a code fix provider that offers automatic replacements: + +| Diagnostic | Fix | +|---|---| +| MSBuildTask0002: `Environment.GetEnvironmentVariable(x)` | → `TaskEnvironment.GetEnvironmentVariable(x)` | +| MSBuildTask0002: `Environment.SetEnvironmentVariable(x, y)` | → `TaskEnvironment.SetEnvironmentVariable(x, y)` | +| MSBuildTask0002: `Environment.GetEnvironmentVariables()` | → `TaskEnvironment.GetEnvironmentVariables()` | +| MSBuildTask0002: `Path.GetFullPath(x)` | → `TaskEnvironment.GetAbsolutePath(x)` | +| MSBuildTask0002: `Environment.CurrentDirectory` | → `TaskEnvironment.ProjectDirectory` | +| MSBuildTask0002: `Directory.GetCurrentDirectory()` | → `TaskEnvironment.ProjectDirectory` | +| MSBuildTask0003: `File.Exists(relativePath)` | → `File.Exists(TaskEnvironment.GetAbsolutePath(relativePath))` | + +The MSBuildTask0003 fixer intelligently finds the first **unwrapped** path argument rather than blindly wrapping the first argument — so for `File.Copy(safePath, unsafePath)` it correctly wraps the second argument. + +## Installation + +### Project Reference (development) + +Reference the analyzer project directly: + +```xml + + + +``` + +### NuGet Package (future) + +When packaged as a NuGet analyzer, add it as a package reference: + +```xml + + + +``` + +## Example + +**Before** (unsafe): + +```csharp +public class CopyFiles : Task, IMultiThreadableTask +{ + public TaskEnvironment TaskEnvironment { get; set; } + public string Source { get; set; } + public string Destination { get; set; } + + public override bool Execute() + { + Console.WriteLine("Copying files..."); // ❌ MSBuildTask0001 + var tmp = Path.GetTempPath(); // ❌ MSBuildTask0002 + var envVar = Environment.GetEnvironmentVariable("MY_VAR"); // ❌ MSBuildTask0002 + File.Copy(Source, Destination); // ❌ MSBuildTask0003 (×2) + return true; + } +} +``` + +**After** (safe): + +```csharp +public class CopyFiles : Task, IMultiThreadableTask +{ + public TaskEnvironment TaskEnvironment { get; set; } + public string Source { get; set; } + public string Destination { get; set; } + + public override bool Execute() + { + Log.LogMessage("Copying files..."); // ✅ Use build logging + var tmp = TaskEnvironment.GetEnvironmentVariable("TMP"); // ✅ Per-task env + var envVar = TaskEnvironment.GetEnvironmentVariable("MY_VAR"); // ✅ Per-task env + File.Copy( // ✅ Absolute paths + TaskEnvironment.GetAbsolutePath(Source), + TaskEnvironment.GetAbsolutePath(Destination)); + return true; + } +} +``` + +## Demo Project + +The `demo/` directory contains 58 task and helper classes exercising every edge case. Build it to see the analyzer in action: + +``` +cd src/ThreadSafeTaskAnalyzer/demo +dotnet build +``` + +The demo produces ~150 unique diagnostics across all four rules with zero false positives, covering: + +- Every `Console` member (type-level detection) +- Process termination / kill +- All environment variable patterns +- File APIs with single and multiple path parameters +- All five safe-pattern recognitions (GetAbsolutePath, AbsolutePath conversion, FullName, GetMetadata("FullPath"), typed AbsolutePath) +- Helper class opt-in with `[MSBuildMultiThreadableTaskAnalyzed]` +- `[MSBuildMultiThreadableTask]` attribute on task classes +- Lambda, async, static method, property getter, string interpolation, conditional access, and constant edge cases +- Named arguments (`File.WriteAllText(contents: "text", path: "file.txt")`) +- Non-path string parameters correctly skipped (`File.WriteAllText(safePath, "contents")` — no false positive on `"contents"`) + +## Tests + +86 tests covering all rules, safe patterns, edge cases, and code fixes: + +``` +cd src/ThreadSafeTaskAnalyzer.Tests +dotnet test +``` + +## Architecture + +| File | Purpose | +|---|---| +| `MultiThreadableTaskAnalyzer.cs` | Core analyzer — `RegisterSymbolStartAction` scopes per type, `RegisterOperationAction` checks each API call | +| `MultiThreadableTaskCodeFixProvider.cs` | Code fixes for MSBuildTask0002 and MSBuildTask0003 | +| `BannedApiDefinitions.cs` | ~50 banned API entries resolved via `DocumentationCommentId` for O(1) symbol lookup | +| `DiagnosticDescriptors.cs` | Four diagnostic descriptors in category `MSBuild.TaskAuthoring` | +| `DiagnosticIds.cs` | Public constants: `MSBuildTask0001`–`MSBuildTask0004` | + +### Performance + +- **O(1) banned API lookup** via `Dictionary` with `SymbolEqualityComparer` +- **Per-type scoping** via `RegisterSymbolStartAction` — operations outside task classes are never analyzed +- **No LINQ on hot paths** — `ImplementsInterface` uses explicit loop +- **Static `AnalyzeOperation`** — no instance state captured +- **Cached banned API definitions** — built once per compilation via `static readonly` field +- **Type-level Console ban** — avoids enumerating 30+ `Console` method overloads + +## Related + +- [Multithreaded Task Execution Spec](https://github.com/dotnet/msbuild/pull/12583) +- [Analyzer Implementation PR](https://github.com/dotnet/msbuild/pull/12143) +- [IMultiThreadableTask Interface](../Framework/IMultiThreadableTask.cs) +- [TaskEnvironment Class](../Framework/TaskEnvironment.cs) +- [Migration Skill Guide](../../.github/skills/multithreaded-task-migration/SKILL.md) diff --git a/src/ThreadSafeTaskAnalyzer/ThreadSafeTaskAnalyzer.csproj b/src/ThreadSafeTaskAnalyzer/ThreadSafeTaskAnalyzer.csproj new file mode 100644 index 00000000000..e4fef8d465f --- /dev/null +++ b/src/ThreadSafeTaskAnalyzer/ThreadSafeTaskAnalyzer.csproj @@ -0,0 +1,37 @@ + + + + netstandard2.0 + 12.0 + enable + Microsoft.Build.TaskAuthoring.Analyzer + Microsoft.Build.TaskAuthoring.Analyzer + true + + false + false + + false + + + + + $(NoWarn);NU5128;RS1038 + + + + + + + + + + + + + + + + + + diff --git a/src/ThreadSafeTaskAnalyzer/TransitiveCallChainAnalyzer.cs b/src/ThreadSafeTaskAnalyzer/TransitiveCallChainAnalyzer.cs new file mode 100644 index 00000000000..2acc938b53c --- /dev/null +++ b/src/ThreadSafeTaskAnalyzer/TransitiveCallChainAnalyzer.cs @@ -0,0 +1,910 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using System.Threading; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace Microsoft.Build.TaskAuthoring.Analyzer +{ + /// + /// Roslyn analyzer that performs transitive call graph analysis to detect unsafe API usage + /// reachable from MSBuild task implementations. + /// + /// Unlike which only checks direct API calls within + /// a task class, this analyzer builds a compilation-wide call graph and traces method calls + /// transitively to find unsafe APIs called by helper methods, utility classes, etc. + /// + /// Reports MSBuildTask0005 with the full call chain for traceability. + /// + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public sealed class TransitiveCallChainAnalyzer : DiagnosticAnalyzer + { + private const string ITaskFullName = "Microsoft.Build.Framework.ITask"; + private const string TaskEnvironmentFullName = "Microsoft.Build.Framework.TaskEnvironment"; + private const string AbsolutePathFullName = "Microsoft.Build.Framework.AbsolutePath"; + private const string ITaskItemFullName = "Microsoft.Build.Framework.ITaskItem"; + + /// + /// Maximum call chain depth to prevent infinite traversal in recursive call graphs. + /// + private const int MaxCallChainDepth = 20; + + public override ImmutableArray SupportedDiagnostics => + ImmutableArray.Create(DiagnosticDescriptors.TransitiveUnsafeCall); + + public override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.RegisterCompilationStartAction(OnCompilationStart); + } + + private void OnCompilationStart(CompilationStartAnalysisContext compilationContext) + { + var iTaskType = compilationContext.Compilation.GetTypeByMetadataName(ITaskFullName); + if (iTaskType is null) + { + return; + } + + // Read scope option from .editorconfig + bool analyzeAllTasks = true; + if (compilationContext.Options.AnalyzerConfigOptionsProvider + .GlobalOptions.TryGetValue($"build_property.{MultiThreadableTaskAnalyzer.ScopeOptionKey}", out var scopeValue) || + compilationContext.Options.AnalyzerConfigOptionsProvider + .GlobalOptions.TryGetValue(MultiThreadableTaskAnalyzer.ScopeOptionKey, out scopeValue)) + { + analyzeAllTasks = !string.Equals(scopeValue, MultiThreadableTaskAnalyzer.ScopeMultiThreadableOnly, StringComparison.OrdinalIgnoreCase); + } + + var iMultiThreadableTaskType = analyzeAllTasks ? null : compilationContext.Compilation.GetTypeByMetadataName("Microsoft.Build.Framework.IMultiThreadableTask"); + var multiThreadableTaskAttributeType = analyzeAllTasks ? null : compilationContext.Compilation.GetTypeByMetadataName("Microsoft.Build.Framework.MSBuildMultiThreadableTaskAttribute"); + var analyzedAttributeType = analyzeAllTasks ? null : compilationContext.Compilation.GetTypeByMetadataName("Microsoft.Build.Framework.MSBuildMultiThreadableTaskAnalyzedAttribute"); + + var taskEnvironmentType = compilationContext.Compilation.GetTypeByMetadataName(TaskEnvironmentFullName); + var absolutePathType = compilationContext.Compilation.GetTypeByMetadataName(AbsolutePathFullName); + var iTaskItemType = compilationContext.Compilation.GetTypeByMetadataName(ITaskItemFullName); + var consoleType = compilationContext.Compilation.GetTypeByMetadataName("System.Console"); + + var bannedApiLookup = BuildBannedApiLookup(compilationContext.Compilation); + var filePathTypes = ResolveFilePathTypes(compilationContext.Compilation); + + // Thread-safe collections for building the graph across concurrent operation callbacks + var callGraph = new ConcurrentDictionary>(SymbolEqualityComparer.Default); + var directViolations = new ConcurrentDictionary>(SymbolEqualityComparer.Default); + + // Phase 1: Scan ALL operations in the compilation to build call graph + record violations + compilationContext.RegisterOperationAction(opCtx => + { + ScanOperation(opCtx, callGraph, directViolations, bannedApiLookup, filePathTypes, + taskEnvironmentType, absolutePathType, iTaskItemType, consoleType, iTaskType); + }, + OperationKind.Invocation, + OperationKind.ObjectCreation, + OperationKind.PropertyReference, + OperationKind.FieldReference); + + // Phase 2: At compilation end, compute transitive closure from task methods + // Phase 1.5 (IL-based cross-assembly analysis) is integrated into BFS + compilationContext.RegisterCompilationEndAction(endCtx => + { + AnalyzeTransitiveViolations(endCtx, callGraph, directViolations, iTaskType, + bannedApiLookup, filePathTypes, taskEnvironmentType, absolutePathType, iTaskItemType, consoleType, + analyzeAllTasks, iMultiThreadableTaskType, multiThreadableTaskAttributeType, analyzedAttributeType); + }); + } + + /// + /// Phase 1: For each operation in the compilation, record call graph edges and direct violations. + /// + private static void ScanOperation( + OperationAnalysisContext context, + ConcurrentDictionary> callGraph, + ConcurrentDictionary> directViolations, + Dictionary bannedApiLookup, + ImmutableHashSet filePathTypes, + INamedTypeSymbol? taskEnvironmentType, + INamedTypeSymbol? absolutePathType, + INamedTypeSymbol? iTaskItemType, + INamedTypeSymbol? consoleType, + INamedTypeSymbol iTaskType) + { + var containingSymbol = context.ContainingSymbol; + if (containingSymbol is not IMethodSymbol containingMethod) + { + return; + } + + // Normalize to OriginalDefinition for generic methods + var callerKey = containingMethod.OriginalDefinition; + + // Check if this method is inside a task type + var containingType = containingMethod.ContainingType; + bool isInsideTask = containingType is not null && ImplementsInterface(containingType, iTaskType); + + ISymbol? referencedSymbol = null; + ImmutableArray arguments = default; + + switch (context.Operation) + { + case IInvocationOperation invocation: + referencedSymbol = invocation.TargetMethod; + arguments = invocation.Arguments; + break; + + case IObjectCreationOperation creation: + referencedSymbol = creation.Constructor; + arguments = creation.Arguments; + break; + + case IPropertyReferenceOperation propRef: + referencedSymbol = propRef.Property; + break; + + case IFieldReferenceOperation fieldRef: + referencedSymbol = fieldRef.Field; + break; + } + + if (referencedSymbol is null) + { + return; + } + + // ALWAYS record call graph edges (even for task methods — needed for BFS traversal) + if (referencedSymbol is IMethodSymbol calleeMethod) + { + var calleeKey = calleeMethod.OriginalDefinition; + callGraph.GetOrAdd(callerKey, _ => new ConcurrentBag()).Add(calleeKey); + } + else if (referencedSymbol is IPropertySymbol property) + { + // Record edges to property getter and setter methods + if (property.GetMethod is not null) + { + callGraph.GetOrAdd(callerKey, _ => new ConcurrentBag()).Add(property.GetMethod.OriginalDefinition); + } + + if (property.SetMethod is not null) + { + callGraph.GetOrAdd(callerKey, _ => new ConcurrentBag()).Add(property.SetMethod.OriginalDefinition); + } + } + + // Only record violations for NON-task methods + // Task methods get direct analysis from MultiThreadableTaskAnalyzer + if (isInsideTask) + { + return; + } + + // Check if this is a banned API call → record as a direct violation + if (bannedApiLookup.TryGetValue(referencedSymbol, out var entry)) + { + var displayName = referencedSymbol.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat); + var violation = new ViolationInfo(entry.Category.ToString(), displayName, entry.Message); + directViolations.GetOrAdd(callerKey, _ => new ConcurrentBag()).Add(violation); + return; + } + + // Check Console type-level ban + if (consoleType is not null) + { + var memberContainingType = referencedSymbol.ContainingType; + if (memberContainingType is not null && SymbolEqualityComparer.Default.Equals(memberContainingType, consoleType)) + { + var displayName = referencedSymbol.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat); + string message = referencedSymbol.Name.StartsWith("Read", StringComparison.Ordinal) + ? "may cause deadlocks in automated builds" + : "interferes with build logging; use Log.LogMessage instead"; + var violation = new ViolationInfo("CriticalError", displayName, message); + directViolations.GetOrAdd(callerKey, _ => new ConcurrentBag()).Add(violation); + return; + } + } + + // Check file path APIs + if (!arguments.IsDefaultOrEmpty && referencedSymbol is IMethodSymbol method) + { + var methodContainingType = method.ContainingType; + if (methodContainingType is not null && filePathTypes.Contains(methodContainingType)) + { + if (HasUnwrappedPathArgument(arguments, taskEnvironmentType, absolutePathType, iTaskItemType)) + { + var displayName = referencedSymbol.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat); + var violation = new ViolationInfo("FilePathRequiresAbsolute", displayName, + "may resolve relative paths against the process working directory"); + directViolations.GetOrAdd(callerKey, _ => new ConcurrentBag()).Add(violation); + } + } + } + } + + /// + /// Phase 2: For each task type, BFS the call graph from its methods to find transitive violations. + /// Phase 1.5 (IL analysis) is integrated: when BFS hits an external method, we read its IL + /// to discover call edges and violations in referenced assemblies. + /// + private static void AnalyzeTransitiveViolations( + CompilationAnalysisContext context, + ConcurrentDictionary> callGraph, + ConcurrentDictionary> directViolations, + INamedTypeSymbol iTaskType, + Dictionary bannedApiLookup, + ImmutableHashSet filePathTypes, + INamedTypeSymbol? taskEnvironmentType, + INamedTypeSymbol? absolutePathType, + INamedTypeSymbol? iTaskItemType, + INamedTypeSymbol? consoleType, + bool analyzeAllTasks, + INamedTypeSymbol? iMultiThreadableTaskType, + INamedTypeSymbol? multiThreadableTaskAttributeType, + INamedTypeSymbol? analyzedAttributeType) + { + // Find all task types in the compilation + var taskTypes = new List(); + FindTaskTypes(context.Compilation.GlobalNamespace, iTaskType, taskTypes); + + if (taskTypes.Count == 0) + { + return; + } + + // When scope is "multithreadable_only", filter to only multithreadable tasks + if (!analyzeAllTasks) + { + taskTypes = taskTypes.Where(t => + (iMultiThreadableTaskType is not null && t.AllInterfaces.Any(i => SymbolEqualityComparer.Default.Equals(i, iMultiThreadableTaskType))) || + (multiThreadableTaskAttributeType is not null && t.GetAttributes().Any(a => SymbolEqualityComparer.Default.Equals(a.AttributeClass, multiThreadableTaskAttributeType))) || + (analyzedAttributeType is not null && t.GetAttributes().Any(a => SymbolEqualityComparer.Default.Equals(a.AttributeClass, analyzedAttributeType))) + ).ToList(); + + if (taskTypes.Count == 0) + { + return; + } + } + + // Phase 1.5: Create IL extender for cross-assembly analysis + using var ilExtender = new ILCallGraphExtender(context.Compilation); + + foreach (var taskType in taskTypes) + { + foreach (var member in taskType.GetMembers()) + { + if (member is not IMethodSymbol method || method.IsImplicitlyDeclared) + { + continue; + } + + // BFS from this method through the call graph + var visited = new HashSet(SymbolEqualityComparer.Default); + var queue = new Queue<(ISymbol current, List chain)>(); + + // Seed with methods called directly from this task method + var methodKey = method.OriginalDefinition; + if (callGraph.TryGetValue(methodKey, out var directCallees)) + { + // Snapshot ConcurrentBag to avoid thread-local enumeration issues + foreach (var callee in directCallees.ToArray()) + { + if (visited.Add(callee)) + { + var chain = new List(4) + { + FormatMethodShort(method), + FormatSymbolShort(callee), + }; + queue.Enqueue((callee, chain)); + } + } + } + + // Track reported violations to avoid duplicates + var reportedViolations = new HashSet(StringComparer.Ordinal); + + while (queue.Count > 0) + { + var (current, chain) = queue.Dequeue(); + + // Check if this method has direct violations (from source scan) + if (directViolations.TryGetValue(current, out var violations)) + { + foreach (var v in violations) + { + ReportTransitiveViolation(context, method, v, chain, reportedViolations); + } + } + + if (chain.Count >= MaxCallChainDepth) + { + continue; + } + + // Try source-level call graph first + bool hasSourceEdges = callGraph.TryGetValue(current, out var callees); + + if (hasSourceEdges) + { + // Snapshot ConcurrentBag to avoid thread-local enumeration issues + foreach (var callee in callees.ToArray()) + { + if (visited.Add(callee)) + { + var newChain = new List(chain) { FormatSymbolShort(callee) }; + queue.Enqueue((callee, newChain)); + } + } + } + + // Phase 1.5: If no source edges and method is external, analyze via IL + if (!hasSourceEdges && current is IMethodSymbol currentMethod && ILCallGraphExtender.IsExternalMethod(currentMethod)) + { + // Skip BCL assemblies + if (ILCallGraphExtender.IsInBclAssembly(currentMethod)) + { + continue; + } + + // Skip methods in safe types (their internals are trusted) + if (ILCallGraphExtender.IsInSafeType(currentMethod)) + { + continue; + } + + // Read IL to discover call targets + var ilTargets = ilExtender.GetCallTargets(currentMethod); + + foreach (var target in ilTargets) + { + var calleeKey = target.Method.OriginalDefinition; + + // Check if the IL-discovered callee is itself a banned API + CheckILDiscoveredViolation( + calleeKey, chain, bannedApiLookup, filePathTypes, consoleType, + context, method, reportedViolations, directViolations); + + // Add to BFS queue for further traversal + if (visited.Add(calleeKey)) + { + var newChain = new List(chain) { FormatSymbolShort(calleeKey) }; + queue.Enqueue((calleeKey, newChain)); + } + } + } + } + } + } + } + + /// + /// Reports a transitive violation with deduplication. + /// + private static void ReportTransitiveViolation( + CompilationAnalysisContext context, + IMethodSymbol taskMethod, + ViolationInfo violation, + List chain, + HashSet reportedViolations) + { + var chainWithApi = new List(chain) { violation.ApiDisplayName }; + var chainStr = string.Join(" → ", chainWithApi); + + var dedupeKey = $"{violation.ApiDisplayName}|{chainStr}"; + if (!reportedViolations.Add(dedupeKey)) + { + return; + } + + var location = taskMethod.Locations.Length > 0 ? taskMethod.Locations[0] : Location.None; + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.TransitiveUnsafeCall, + location, + FormatMethodFull(taskMethod), + violation.ApiDisplayName, + chainStr)); + } + + /// + /// Checks if an IL-discovered callee is a banned API and records the violation. + /// This is needed because IL-discovered calls don't go through Phase 1 source scanning. + /// + private static void CheckILDiscoveredViolation( + ISymbol calleeSymbol, + List currentChain, + Dictionary bannedApiLookup, + ImmutableHashSet filePathTypes, + INamedTypeSymbol? consoleType, + CompilationAnalysisContext context, + IMethodSymbol taskMethod, + HashSet reportedViolations, + ConcurrentDictionary> directViolations) + { + // Check if the callee itself is a banned API + BannedApiEntry entry; + bool found = bannedApiLookup.TryGetValue(calleeSymbol, out entry); + + // For property accessors (get_X/set_X), also check the associated property symbol + if (!found && calleeSymbol is IMethodSymbol methodSym && methodSym.AssociatedSymbol is IPropertySymbol prop) + { + found = bannedApiLookup.TryGetValue(prop, out entry); + } + + if (found) + { + var displayName = calleeSymbol.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat); + var violation = new ViolationInfo(entry.Category.ToString(), displayName, entry.Message); + + // Record in directViolations so it's found when BFS reaches this node + directViolations.GetOrAdd(calleeSymbol, _ => new ConcurrentBag()).Add(violation); + + // Also report immediately with current chain + var chainWithApi = new List(currentChain) { displayName }; + var chainStr = string.Join(" → ", chainWithApi); + var dedupeKey = $"{displayName}|{chainStr}"; + if (reportedViolations.Add(dedupeKey)) + { + var location = taskMethod.Locations.Length > 0 ? taskMethod.Locations[0] : Location.None; + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.TransitiveUnsafeCall, + location, + FormatMethodFull(taskMethod), + displayName, + chainStr)); + } + return; + } + + // Check Console type-level ban + if (consoleType is not null && calleeSymbol.ContainingType is not null) + { + if (SymbolEqualityComparer.Default.Equals(calleeSymbol.ContainingType, consoleType)) + { + var displayName = calleeSymbol.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat); + string message = calleeSymbol.Name.StartsWith("Read", StringComparison.Ordinal) + ? "may cause deadlocks in automated builds" + : "interferes with build logging; use Log.LogMessage instead"; + var violation = new ViolationInfo("CriticalError", displayName, message); + directViolations.GetOrAdd(calleeSymbol, _ => new ConcurrentBag()).Add(violation); + return; + } + } + + // Check file-path APIs (constructors/methods on File, FileStream, etc. with path params) + if (calleeSymbol is IMethodSymbol calledMethod && + calledMethod.ContainingType is not null && + filePathTypes.Contains(calledMethod.ContainingType)) + { + // Check ALL overloads of this method name for path-like parameters, + // not just the single overload that was resolved from IL (which may be wrong + // due to overload resolution limitations — we match by name, not signature). + bool hasPathParam = false; + foreach (var member in calledMethod.ContainingType.GetMembers(calledMethod.Name)) + { + if (member is IMethodSymbol overload) + { + foreach (var param in overload.Parameters) + { + if (param.Type.SpecialType == SpecialType.System_String && IsPathParameterName(param.Name)) + { + hasPathParam = true; + break; + } + } + + if (hasPathParam) + { + break; + } + } + } + + // Also check constructors if the resolved method is a constructor + if (!hasPathParam && calledMethod.MethodKind == MethodKind.Constructor) + { + foreach (var ctor in calledMethod.ContainingType.Constructors) + { + foreach (var param in ctor.Parameters) + { + if (param.Type.SpecialType == SpecialType.System_String && IsPathParameterName(param.Name)) + { + hasPathParam = true; + break; + } + } + + if (hasPathParam) + { + break; + } + } + } + + if (hasPathParam) + { + var displayName = calleeSymbol.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat); + var violation = new ViolationInfo("FilePathRequiresAbsolute", displayName, + "may resolve relative paths against the process working directory"); + directViolations.GetOrAdd(calleeSymbol, _ => new ConcurrentBag()).Add(violation); + } + } + } + + /// + /// Recursively finds all types implementing ITask in the namespace tree. + /// + private static void FindTaskTypes(INamespaceSymbol ns, INamedTypeSymbol iTaskType, List result) + { + foreach (var member in ns.GetMembers()) + { + if (member is INamespaceSymbol childNs) + { + FindTaskTypes(childNs, iTaskType, result); + } + else if (member is INamedTypeSymbol type) + { + if (!type.IsAbstract && ImplementsInterface(type, iTaskType)) + { + result.Add(type); + } + + // Check nested types + foreach (var nested in type.GetTypeMembers()) + { + if (!nested.IsAbstract && ImplementsInterface(nested, iTaskType)) + { + result.Add(nested); + } + } + } + } + } + + private static string FormatMethodShort(IMethodSymbol method) + { + return $"{method.ContainingType?.Name}.{method.Name}"; + } + + private static string FormatMethodFull(IMethodSymbol method) + { + return $"{method.ContainingType?.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat)}.{method.Name}"; + } + + private static string FormatSymbolShort(ISymbol symbol) + { + if (symbol is IMethodSymbol m) + { + return $"{m.ContainingType?.Name}.{m.Name}"; + } + + return symbol.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat); + } + + #region Shared helpers (duplicated from MultiThreadableTaskAnalyzer for independence) + + private static bool HasUnwrappedPathArgument( + ImmutableArray arguments, + INamedTypeSymbol? taskEnvironmentType, + INamedTypeSymbol? absolutePathType, + INamedTypeSymbol? iTaskItemType) + { + for (int i = 0; i < arguments.Length; i++) + { + var arg = arguments[i]; + var param = arg.Parameter; + if (param is null || param.Type.SpecialType != SpecialType.System_String) + { + continue; + } + + if (!IsPathParameterName(param.Name)) + { + continue; + } + + if (!IsWrappedSafely(arg.Value, taskEnvironmentType, absolutePathType, iTaskItemType)) + { + return true; + } + } + + return false; + } + + private static bool IsPathParameterName(string paramName) + { + return paramName.IndexOf("path", StringComparison.OrdinalIgnoreCase) >= 0 + || paramName.IndexOf("file", StringComparison.OrdinalIgnoreCase) >= 0 + || paramName.IndexOf("dir", StringComparison.OrdinalIgnoreCase) >= 0 + || paramName.IndexOf("folder", StringComparison.OrdinalIgnoreCase) >= 0 + || paramName.IndexOf("source", StringComparison.OrdinalIgnoreCase) >= 0 + || paramName.IndexOf("dest", StringComparison.OrdinalIgnoreCase) >= 0; + } + + private static bool IsWrappedSafely( + IOperation operation, + INamedTypeSymbol? taskEnvironmentType, + INamedTypeSymbol? absolutePathType, + INamedTypeSymbol? iTaskItemType) + { + while (operation is IConversionOperation conversion) + { + if (absolutePathType is not null && + IsAbsolutePathType(conversion.Operand.Type, absolutePathType)) + { + return true; + } + + operation = conversion.Operand; + } + + // Null literals and default values are safe + if (operation is ILiteralOperation lit && lit.ConstantValue.HasValue && lit.ConstantValue.Value is null) + { + return true; + } + + if (operation is IDefaultValueOperation) + { + return true; + } + + if (operation is IInvocationOperation invocation) + { + if (invocation.TargetMethod.Name == "GetAbsolutePath" && + taskEnvironmentType is not null && + SymbolEqualityComparer.Default.Equals(invocation.TargetMethod.ContainingType, taskEnvironmentType)) + { + return true; + } + + if ((invocation.TargetMethod.Name == "GetMetadata" || invocation.TargetMethod.Name == "GetMetadataValue") && iTaskItemType is not null) + { + var receiverType = invocation.TargetMethod.ContainingType; + if (receiverType is not null && (SymbolEqualityComparer.Default.Equals(receiverType, iTaskItemType) || ImplementsInterface(receiverType, iTaskItemType))) + { + if (invocation.Arguments.Length > 0 && + invocation.Arguments[0].Value is ILiteralOperation literal && + literal.ConstantValue.HasValue && + literal.ConstantValue.Value is string metadataName && + string.Equals(metadataName, "FullPath", StringComparison.OrdinalIgnoreCase)) + { + return true; + } + } + } + + // Path.GetDirectoryName(safe) — directory of an absolute path is absolute + if (invocation.TargetMethod.Name == "GetDirectoryName" && + invocation.TargetMethod.ContainingType?.ToDisplayString() == "System.IO.Path" && + invocation.Arguments.Length >= 1 && + IsWrappedSafely(invocation.Arguments[0].Value, taskEnvironmentType, absolutePathType, iTaskItemType)) + { + return true; + } + + // Path.Combine(safe, ...) — result is absolute when first arg is absolute + if (invocation.TargetMethod.Name == "Combine" && + invocation.TargetMethod.ContainingType?.ToDisplayString() == "System.IO.Path" && + invocation.Arguments.Length >= 2 && + IsWrappedSafely(invocation.Arguments[0].Value, taskEnvironmentType, absolutePathType, iTaskItemType)) + { + return true; + } + + // Path.GetFullPath(safe) — safe only when input is already absolute + if (invocation.TargetMethod.Name == "GetFullPath" && + invocation.TargetMethod.ContainingType?.ToDisplayString() == "System.IO.Path" && + invocation.Arguments.Length >= 1 && + IsWrappedSafely(invocation.Arguments[0].Value, taskEnvironmentType, absolutePathType, iTaskItemType)) + { + return true; + } + } + + if (operation is IPropertyReferenceOperation propRef && propRef.Property.Name == "FullName") + { + var containingTypeName = propRef.Property.ContainingType?.ToDisplayString(); + if (containingTypeName is "System.IO.FileSystemInfo" or "System.IO.FileInfo" or "System.IO.DirectoryInfo") + { + return true; + } + } + + if (absolutePathType is not null && + operation.Type is not null && + IsAbsolutePathType(operation.Type, absolutePathType)) + { + return true; + } + + // Trace through local variable assignments + if (operation is ILocalReferenceOperation localRef) + { + var local = localRef.Local; + if (local.DeclaringSyntaxReferences.Length == 1) + { + var syntax = local.DeclaringSyntaxReferences[0].GetSyntax(); + var semanticModel = operation.SemanticModel; + if (semanticModel is not null) + { + var declOp = semanticModel.GetOperation(syntax); + if (declOp is IVariableDeclaratorOperation declarator && + declarator.Initializer?.Value is IOperation initValue) + { + return IsWrappedSafely(initValue, taskEnvironmentType, absolutePathType, iTaskItemType); + } + } + } + } + + return false; + } + + /// + /// Checks if a type is AbsolutePath or Nullable<AbsolutePath>. + /// + private static bool IsAbsolutePathType(ITypeSymbol? type, INamedTypeSymbol absolutePathType) + { + if (type is null) + { + return false; + } + + if (SymbolEqualityComparer.Default.Equals(type, absolutePathType)) + { + return true; + } + + if (type is INamedTypeSymbol namedType && + namedType.OriginalDefinition.SpecialType == SpecialType.System_Nullable_T && + namedType.TypeArguments.Length == 1 && + SymbolEqualityComparer.Default.Equals(namedType.TypeArguments[0], absolutePathType)) + { + return true; + } + + return false; + } + + private static Dictionary BuildBannedApiLookup(Compilation compilation) + { + var result = new Dictionary(SymbolEqualityComparer.Default); + var definitions = BannedApiDefinitions.GetAll(); + + foreach (var def in definitions) + { + ImmutableArray symbols; + try + { + var resolved = DocumentationCommentId.GetSymbolsForDeclarationId(def.DeclarationId, compilation); + symbols = resolved.IsDefault ? ImmutableArray.Empty : resolved; + } + catch + { + continue; + } + + foreach (var symbol in symbols) + { + if (!result.ContainsKey(symbol)) + { + result[symbol] = new BannedApiEntry(def.Category, def.Message); + } + } + } + + return result; + } + + private static ImmutableHashSet ResolveFilePathTypes(Compilation compilation) + { + var typeNames = new[] + { + // Core System.IO file/directory types + "System.IO.File", + "System.IO.Directory", + "System.IO.FileInfo", + "System.IO.DirectoryInfo", + "System.IO.FileStream", + "System.IO.StreamReader", + "System.IO.StreamWriter", + "System.IO.FileSystemWatcher", + "System.IO.BinaryReader", + "System.IO.BinaryWriter", + + // XML types that accept file paths in Load/Save/Create + "System.Xml.Linq.XDocument", + "System.Xml.Linq.XElement", + "System.Xml.XmlDocument", + "System.Xml.XmlReader", + "System.Xml.XmlWriter", + "System.Xml.XmlTextReader", + "System.Xml.XmlTextWriter", + "System.Xml.Xsl.XslCompiledTransform", + "System.Xml.Schema.XmlSchema", + + // Compression types that accept file paths + "System.IO.Compression.ZipFile", + + // Memory-mapped files + "System.IO.MemoryMappedFiles.MemoryMappedFile", + + // Security / certificates + "System.Security.Cryptography.X509Certificates.X509Certificate", + "System.Security.Cryptography.X509Certificates.X509Certificate2", + + // Diagnostics + "System.Diagnostics.FileVersionInfo", + "System.Diagnostics.TextWriterTraceListener", + + // Resources + "System.Resources.ResourceReader", + "System.Resources.ResourceWriter", + + // Assembly loading (supplements the banned-API list for path-based overloads) + "System.Runtime.Loader.AssemblyLoadContext", + }; + + var builder = ImmutableHashSet.CreateBuilder(SymbolEqualityComparer.Default); + foreach (var name in typeNames) + { + var type = compilation.GetTypeByMetadataName(name); + if (type is not null) + { + builder.Add(type); + } + } + + return builder.ToImmutable(); + } + + private static bool ImplementsInterface(INamedTypeSymbol type, INamedTypeSymbol interfaceType) + { + foreach (var iface in type.AllInterfaces) + { + if (SymbolEqualityComparer.Default.Equals(iface, interfaceType)) + { + return true; + } + } + + return false; + } + + #endregion + + internal readonly struct BannedApiEntry + { + public BannedApiDefinitions.ApiCategory Category { get; } + public string Message { get; } + + public BannedApiEntry(BannedApiDefinitions.ApiCategory category, string message) + { + Category = category; + Message = message; + } + } + + internal readonly struct ViolationInfo + { + public string Category { get; } + public string ApiDisplayName { get; } + public string Message { get; } + + public ViolationInfo(string category, string apiDisplayName, string message) + { + Category = category; + ApiDisplayName = apiDisplayName; + Message = message; + } + } + } +} diff --git a/src/ThreadSafeTaskAnalyzer/demo/AdvancedEdgeCases.cs b/src/ThreadSafeTaskAnalyzer/demo/AdvancedEdgeCases.cs new file mode 100644 index 00000000000..709779048a5 --- /dev/null +++ b/src/ThreadSafeTaskAnalyzer/demo/AdvancedEdgeCases.cs @@ -0,0 +1,523 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// ═══════════════════════════════════════════════════════════════════════════════ +// ADVANCED EDGE CASES - Part 1: Partial Classes, Generics, Async, Lambdas +// ═══════════════════════════════════════════════════════════════════════════════ + +#nullable disable + +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.IO; +using System.Linq; +using System.Reflection; +using Microsoft.Build.Framework; +using Microsoft.Build.Utilities; +using Task = Microsoft.Build.Utilities.Task; + +namespace AnalyzerDemo +{ + // ───────────────────────────────────────────────────────────────────────── + // TASK 23: Partial class - Part A (see AdvancedEdgeCases2.cs for Part B) + // Both parts should be analyzed since the combined type implements IMultiThreadableTask + // ───────────────────────────────────────────────────────────────────────── + public partial class PartialTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string InputFile { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0001: Console in partial class Part A + Console.WriteLine("Part A"); + + // Call helper in Part B + DoPartBWork(); + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 24: Generic task class + // Should be analyzed since it implements IMultiThreadableTask + // ───────────────────────────────────────────────────────────────────────── + public class GenericTask : Task, IMultiThreadableTask where T : class + { + public TaskEnvironment TaskEnvironment { get; set; } + public T Data { get; set; } + public string OutputPath { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0003: File API with non-absolute path + File.WriteAllText(OutputPath, Data?.ToString()); + + // ❌ MSBuildTask0002: Environment.CurrentDirectory + string cwd = Environment.CurrentDirectory; + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 25: Lambda and delegate patterns + // Banned APIs used inside lambdas within task class should be caught + // ───────────────────────────────────────────────────────────────────────── + public class LambdaTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public ITaskItem[] Items { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0003: File.Exists inside lambda + var exists = Items.Select(i => File.Exists(i.ItemSpec)).ToArray(); + + // ❌ MSBuildTask0001: Console inside lambda + Items.ToList().ForEach(i => Console.WriteLine(i.ItemSpec)); + + // ❌ MSBuildTask0002: Environment in lambda + Func getVar = () => Environment.GetEnvironmentVariable("HOME"); + + // ❌ MSBuildTask0003: File.ReadAllText in local function + string ReadFile(string path) => File.ReadAllText(path); + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 26: LINQ with file operations + // ───────────────────────────────────────────────────────────────────────── + public class LinqFileTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public ITaskItem[] SourceFiles { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0003: File.Exists in LINQ Where + var validFiles = SourceFiles + .Where(f => File.Exists(f.ItemSpec)) + .ToArray(); + + // ❌ MSBuildTask0003: File.ReadAllText in LINQ Select + var contents = SourceFiles + .Select(f => File.ReadAllText(f.ItemSpec)) + .ToArray(); + + // ❌ MSBuildTask0003: Directory.GetFiles in LINQ SelectMany + var allFiles = SourceFiles + .SelectMany(f => Directory.GetFiles(f.ItemSpec)) + .ToArray(); + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 27: Property getters/setters with unsafe APIs + // ───────────────────────────────────────────────────────────────────────── + public class PropertyUnsafeTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + + // ❌ MSBuildTask0002: Environment.CurrentDirectory in property getter + public string WorkDir => Environment.CurrentDirectory; + + // ❌ MSBuildTask0002: Environment.GetEnvironmentVariable in property getter + public string HomePath => Environment.GetEnvironmentVariable("HOME"); + + public override bool Execute() + { + string wd = WorkDir; + string home = HomePath; + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 28: Nested class implementing ITask + // The nested class should be analyzed independently + // ───────────────────────────────────────────────────────────────────────── + public class OuterTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + + public override bool Execute() => true; // ✅ No issues + + // This nested class is also a task - should be analyzed + public class InnerTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string Path { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0003: File API in nested task + File.Exists(Path); + + // ❌ MSBuildTask0001: Console in nested task + Console.WriteLine("inner task"); + + return true; + } + } + + // This nested class is NOT a task - should NOT be analyzed + public class HelperNotTask + { + public void DoWork() + { + // ✅ No diagnostics - not a task + Console.WriteLine("not a task"); + File.Exists("whatever"); + } + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 29: Explicit interface implementation + // ───────────────────────────────────────────────────────────────────────── + public class ExplicitInterfaceTask : ITask, IMultiThreadableTask + { + public IBuildEngine BuildEngine { get; set; } + public ITaskHost HostObject { get; set; } + public TaskEnvironment TaskEnvironment { get; set; } + + bool ITask.Execute() + { + // ❌ MSBuildTask0001: Console in explicit impl + Console.WriteLine("explicit"); + + // ❌ MSBuildTask0003: File API + File.Exists("test.txt"); + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 30: Conditional compilation + // Diagnostics should fire in both branches + // ───────────────────────────────────────────────────────────────────────── + public class ConditionalCompilationTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string InputFile { get; set; } + + public override bool Execute() + { +#if DEBUG + // ❌ MSBuildTask0001: Console even in DEBUG + Console.WriteLine("debug mode"); +#else + // ❌ MSBuildTask0001: Console in release too + Console.WriteLine("release mode"); +#endif + + // ❌ MSBuildTask0003: File API + File.Exists(InputFile); + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 31: DirectoryInfo.FullName safe pattern + // ───────────────────────────────────────────────────────────────────────── + public class DirectoryInfoFullNameTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string Dir { get; set; } + + public override bool Execute() + { + AbsolutePath absDir = TaskEnvironment.GetAbsolutePath(Dir); + var di = new DirectoryInfo(absDir); + + // ✅ Safe: DirectoryInfo.FullName (declared on FileSystemInfo) + Directory.Exists(di.FullName); + + // ✅ Safe: FileInfo.FullName + var fi = new FileInfo(absDir); + File.Exists(fi.FullName); + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 32: String interpolation with Environment variables + // ───────────────────────────────────────────────────────────────────────── + public class StringInterpolationTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0002: Environment.GetEnvironmentVariable in interpolation + string msg = $"Home is {Environment.GetEnvironmentVariable("HOME")}"; + + // ❌ MSBuildTask0002: Environment.CurrentDirectory in interpolation + Log.LogMessage($"CWD: {Environment.CurrentDirectory}"); + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 33: Task with async pattern (Task-returning Execute override) + // Note: MSBuild ITask.Execute() returns bool, but tasks may have async helpers + // ───────────────────────────────────────────────────────────────────────── + public class AsyncHelperTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string InputFile { get; set; } + + public override bool Execute() + { + return DoWorkAsync().GetAwaiter().GetResult(); + } + + private async System.Threading.Tasks.Task DoWorkAsync() + { + // ❌ MSBuildTask0003: File API in async method + if (File.Exists(InputFile)) + { + // ❌ MSBuildTask0003: StreamReader in async method + using var reader = new StreamReader(InputFile); + string content = await reader.ReadToEndAsync(); + } + + // ❌ MSBuildTask0001: Console in async method + Console.WriteLine("async work done"); + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 34: Event handler patterns + // ───────────────────────────────────────────────────────────────────────── + public class EventHandlerTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string WatchDir { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0003: FileSystemWatcher path + using var watcher = new FileSystemWatcher(WatchDir); + + // Event handler with banned API + watcher.Changed += (s, e) => + { + // ❌ MSBuildTask0001: Console in event handler + Console.WriteLine($"Changed: {e.FullPath}"); + }; + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 35: ITaskItem.GetMetadataValue("FullPath") safe pattern + // ───────────────────────────────────────────────────────────────────────── + public class GetMetadataValueTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + + [Required] + public ITaskItem[] SourceFiles { get; set; } + + public override bool Execute() + { + foreach (ITaskItem item in SourceFiles) + { + // ✅ GetMetadata("FullPath") is safe - just like GetMetadata + string fullPath = item.GetMetadata("FullPath"); + + // But this assignment loses the safe origin (no data flow tracking): + // ❌ MSBuildTask0003: known limitation - variable lost safe origin + File.Exists(fullPath); + } + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 36: Method group / delegate reference + // ───────────────────────────────────────────────────────────────────────── + public class MethodGroupTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public ITaskItem[] Items { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0003: File.Exists used as method group + var results = Array.ConvertAll( + Array.ConvertAll(Items, i => i.ItemSpec), + File.Exists); + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 37: Multiple IMultiThreadableTask derivation chains + // Base class has TaskEnvironment, derived should still be analyzed + // ───────────────────────────────────────────────────────────────────────── + public abstract class BaseMultiThreadableTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + + protected void LogSafe(string message) => Log.LogMessage(message); + } + + public class DerivedMultiThreadableTask : BaseMultiThreadableTask + { + public string FilePath { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0003: derived class still analyzed + File.ReadAllText(FilePath); + + // ❌ MSBuildTask0001: Console in derived class + Console.WriteLine("derived"); + + // ✅ Safe: using base class helper + LogSafe("All good"); + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 38: Static method with file operations (inside task class) + // Static methods in task class should be analyzed + // ───────────────────────────────────────────────────────────────────────── + public class StaticMethodTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string InputFile { get; set; } + + public override bool Execute() + { + string content = ReadFileUnsafely(InputFile); + return content != null; + } + + // Static method - still within task class scope + private static string ReadFileUnsafely(string path) + { + // ❌ MSBuildTask0003: static method in task class + return File.ReadAllText(path); + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 39: Multiple unsafe APIs on same line + // ───────────────────────────────────────────────────────────────────────── + public class MultipleOnSameLineTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string Dir { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0003 + MSBuildTask0003: Two file ops on same expression + File.Copy(Dir, Path.Combine(Dir, "backup")); + + // ❌ MSBuildTask0002: Environment.GetEnvironmentVariable nested in another call + Log.LogMessage(Environment.GetEnvironmentVariable("BUILD_ID")); + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 40: Ternary / conditional expression with unsafe APIs + // ───────────────────────────────────────────────────────────────────────── + public class TernaryTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string InputFile { get; set; } + public bool UseDefault { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0003: File.Exists in condition + string content = File.Exists(InputFile) ? File.ReadAllText(InputFile) : ""; + + // ❌ MSBuildTask0002: Environment.GetEnvironmentVariable in ternary + string val = UseDefault ? "default" : Environment.GetEnvironmentVariable("X"); + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 41: try/catch/finally with unsafe APIs in each block + // ───────────────────────────────────────────────────────────────────────── + public class TryCatchFinallyTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string InputFile { get; set; } + + public override bool Execute() + { + try + { + // ❌ MSBuildTask0003: in try block + File.ReadAllText(InputFile); + } + catch (Exception) + { + // ❌ MSBuildTask0001: Console in catch block + Console.Error.WriteLine("error occurred"); + } + finally + { + // ❌ MSBuildTask0003: in finally block + File.Delete(InputFile); + } + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 42: Switch expression and switch statement with unsafe APIs + // ───────────────────────────────────────────────────────────────────────── + public class SwitchTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string InputFile { get; set; } + public string Mode { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0003: File API in switch arms + switch (Mode) + { + case "read": + File.ReadAllText(InputFile); + break; + case "write": + File.WriteAllText(InputFile, "data"); + break; + case "delete": + File.Delete(InputFile); + break; + } + + return true; + } + } +} diff --git a/src/ThreadSafeTaskAnalyzer/demo/AdvancedEdgeCases2.cs b/src/ThreadSafeTaskAnalyzer/demo/AdvancedEdgeCases2.cs new file mode 100644 index 00000000000..f6369839cb8 --- /dev/null +++ b/src/ThreadSafeTaskAnalyzer/demo/AdvancedEdgeCases2.cs @@ -0,0 +1,280 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// ═══════════════════════════════════════════════════════════════════════════════ +// ADVANCED EDGE CASES - Part 2: Partial class Part B, Cross-file analysis +// ═══════════════════════════════════════════════════════════════════════════════ + +#nullable disable + +using System; +using System.Collections.Generic; +using System.IO; +using Microsoft.Build.Framework; +using Microsoft.Build.Utilities; +using Task = Microsoft.Build.Utilities.Task; + +namespace AnalyzerDemo +{ + // ───────────────────────────────────────────────────────────────────────── + // TASK 23 (Part B): Partial class - second file + // This part of the partial class should also be analyzed because + // the combined type implements IMultiThreadableTask (declared in Part A) + // ───────────────────────────────────────────────────────────────────────── + public partial class PartialTask + { + private void DoPartBWork() + { + // ❌ MSBuildTask0003: File API in second file of partial class + File.ReadAllText(InputFile); + + // ❌ MSBuildTask0002: Environment.GetEnvironmentVariable in Part B + string val = Environment.GetEnvironmentVariable("TEST_VAR"); + + // ❌ MSBuildTask0001: Console in Part B + Console.WriteLine("Part B"); + } + } + + // ───────────────────────────────────────────────────────────────────────── + // Non-task helper class - should NOT be analyzed + // (unless [MSBuildMultiThreadableTaskAnalyzed] opt-in is added - future) + // ───────────────────────────────────────────────────────────────────────── + public static class UnsafeHelper + { + // ✅ No diagnostics - not a task class + public static string ReadFile(string path) + { + Console.WriteLine("helper reading"); // No diagnostic + return File.ReadAllText(path); // No diagnostic + } + + public static string GetEnvVar(string name) + { + return Environment.GetEnvironmentVariable(name); // No diagnostic + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 43: Task that calls the unsafe helper + // The call TO the helper is fine, but the helper's internal usage + // is not tracked (no cross-method data flow). This is a known limitation. + // ───────────────────────────────────────────────────────────────────────── + public class CallsHelperTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string InputFile { get; set; } + + public override bool Execute() + { + // ✅ No diagnostic on this line - the analyzer doesn't track + // into external method bodies. Known limitation. + string content = UnsafeHelper.ReadFile(InputFile); + + // ❌ MSBuildTask0003: Direct file API usage is still caught + File.Exists(InputFile); + + return content != null; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 44: Task with IDisposable pattern and using statements + // ───────────────────────────────────────────────────────────────────────── + public class DisposablePatternTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string InputFile { get; set; } + public string OutputFile { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0003: using statement with FileStream + using (var input = new FileStream(InputFile, FileMode.Open)) + using (var output = new FileStream(OutputFile, FileMode.Create)) + { + input.CopyTo(output); + } + + // ❌ MSBuildTask0003: using declaration with StreamReader + using var reader = new StreamReader(InputFile); + string line = reader.ReadLine(); + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 45: Object initializer and collection initializer patterns + // ───────────────────────────────────────────────────────────────────────── + public class ObjectInitializerTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0002: ProcessStartInfo in object initializer + var psi = new System.Diagnostics.ProcessStartInfo + { + FileName = "tool.exe", + UseShellExecute = false, + }; + + // ❌ MSBuildTask0002: Environment variable in initializer value + var config = new Dictionary + { + ["home"] = Environment.GetEnvironmentVariable("HOME"), + ["path"] = Environment.GetEnvironmentVariable("PATH"), + }; + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 46: Null-conditional and null-coalescing with unsafe APIs + // ───────────────────────────────────────────────────────────────────────── + public class NullConditionalTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string InputFile { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0002: Environment.GetEnvironmentVariable in null coalescing + string home = Environment.GetEnvironmentVariable("HOME") ?? "/tmp"; + + // ❌ MSBuildTask0003: File operations with null-conditional + string content = File.Exists(InputFile) ? File.ReadAllText(InputFile) : null; + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 47: Pattern matching with unsafe APIs + // ───────────────────────────────────────────────────────────────────────── + public class PatternMatchTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public ITaskItem Input { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0003: File API in pattern matching expression + if (File.Exists(Input.ItemSpec) is true) + { + // ❌ MSBuildTask0003 + var attrs = File.GetAttributes(Input.ItemSpec); + if (attrs is FileAttributes.Directory) + { + // ❌ MSBuildTask0003 + Directory.Delete(Input.ItemSpec, true); + } + } + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 48: Correctly migrated complex task - ZERO diagnostics expected + // Demonstrates proper migration patterns for all scenarios + // ───────────────────────────────────────────────────────────────────────── + public class CorrectlyMigratedComplexTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public ITaskItem[] SourceFiles { get; set; } + public ITaskItem DestinationFolder { get; set; } + + public override bool Execute() + { + bool success = true; + + // ✅ Using TaskEnvironment for environment + string projDir = TaskEnvironment.ProjectDirectory; + string buildId = TaskEnvironment.GetEnvironmentVariable("BUILD_ID"); + + // ✅ Using GetAbsolutePath for destination + AbsolutePath destDir = TaskEnvironment.GetAbsolutePath(DestinationFolder.ItemSpec); + Directory.CreateDirectory(destDir); + + foreach (ITaskItem item in SourceFiles) + { + try + { + // ✅ Using GetAbsolutePath for source + AbsolutePath source = TaskEnvironment.GetAbsolutePath(item.ItemSpec); + + // ✅ GetMetadata("FullPath") is safe + string fullPath = item.GetMetadata("FullPath"); + + // ✅ AbsolutePath combination + AbsolutePath dest = TaskEnvironment.GetAbsolutePath( + Path.Combine(destDir, Path.GetFileName(source))); + + if (File.Exists(source)) + { + File.Copy(source, dest, overwrite: true); + Log.LogMessage($"Copied {item.ItemSpec} to {dest}"); + } + } + catch (Exception ex) + { + Log.LogError($"Failed to process {item.ItemSpec}: {ex.Message}"); + success = false; + } + } + + // ✅ Using TaskEnvironment for process start + var psi = TaskEnvironment.GetProcessStartInfo(); + psi.FileName = "post-process.exe"; + + return success; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 49: Task with field initialization containing unsafe APIs + // ───────────────────────────────────────────────────────────────────────── + public class FieldInitTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + + // ❌ MSBuildTask0002: Field initializer + private string _defaultDir = Environment.CurrentDirectory; + + // ❌ MSBuildTask0002: Field initializer + private string _home = Environment.GetEnvironmentVariable("HOME"); + + public override bool Execute() + { + Log.LogMessage($"Dir: {_defaultDir}, Home: {_home}"); + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 50: Task with constructor containing unsafe APIs + // ───────────────────────────────────────────────────────────────────────── + public class ConstructorUnsafeTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + private readonly string _cachedDir; + + public ConstructorUnsafeTask() + { + // ❌ MSBuildTask0002: Environment.CurrentDirectory in constructor + _cachedDir = Environment.CurrentDirectory; + + // ❌ MSBuildTask0001: Console in constructor + Console.WriteLine("task created"); + } + + public override bool Execute() + { + Log.LogMessage(_cachedDir); + return true; + } + } +} diff --git a/src/ThreadSafeTaskAnalyzer/demo/AllEdgeCases.cs b/src/ThreadSafeTaskAnalyzer/demo/AllEdgeCases.cs new file mode 100644 index 00000000000..da8584f5d7b --- /dev/null +++ b/src/ThreadSafeTaskAnalyzer/demo/AllEdgeCases.cs @@ -0,0 +1,568 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// ═══════════════════════════════════════════════════════════════════════════════ +// DEMO 1: IMultiThreadableTask with ALL categories of problematic APIs +// Should produce diagnostics for MSBuildTask0001, 0002, 0003, 0004 +// ═══════════════════════════════════════════════════════════════════════════════ + +#nullable disable + +using System; +using System.Diagnostics; +using System.Globalization; +using System.IO; +using System.Reflection; +using System.Threading; +using Microsoft.Build.Framework; +using Microsoft.Build.Utilities; + +namespace AnalyzerDemo +{ + // ───────────────────────────────────────────────────────────────────────── + // TASK 1: Kitchen sink of all problematic patterns + // Expected: 2 errors (0001), 3 warnings (0002), 7+ warnings (0003), + // 1 warning (0004) + // ───────────────────────────────────────────────────────────────────────── + public class KitchenSinkTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string InputFile { get; set; } + public string OutputDir { get; set; } + + public override bool Execute() + { + // ── MSBuildTask0001: Critical errors ────────────────────────────── + Console.WriteLine("hello"); // MSBuildTask0001: Console + Environment.Exit(1); // MSBuildTask0001: Exit + + // ── MSBuildTask0002: TaskEnvironment required ───────────────────── + string cwd = Environment.CurrentDirectory; // MSBuildTask0002 + string val = Environment.GetEnvironmentVariable("X"); // MSBuildTask0002 + string fp = Path.GetFullPath(InputFile); // MSBuildTask0002 + + // ── MSBuildTask0003: File path needs absolute ───────────────────── + File.Exists(InputFile); // MSBuildTask0003 + Directory.Exists(OutputDir); // MSBuildTask0003 + Directory.CreateDirectory(OutputDir); // MSBuildTask0003 + File.ReadAllText(InputFile); // MSBuildTask0003 + var fi = new FileInfo(InputFile); // MSBuildTask0003 + var di = new DirectoryInfo(OutputDir); // MSBuildTask0003 + using var sr = new StreamReader(InputFile); // MSBuildTask0003 + + // ── MSBuildTask0004: Potential issues ───────────────────────────── + Assembly.LoadFrom("plugin.dll"); // MSBuildTask0004 + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 2: Correctly migrated task - should produce ZERO diagnostics + // ───────────────────────────────────────────────────────────────────────── + public class CorrectlyMigratedTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string InputFile { get; set; } + public string OutputDir { get; set; } + + public override bool Execute() + { + // ✅ Using TaskEnvironment for environment + string projDir = TaskEnvironment.ProjectDirectory; + string val = TaskEnvironment.GetEnvironmentVariable("X"); + TaskEnvironment.SetEnvironmentVariable("Y", "value"); + + // ✅ Using GetAbsolutePath for file operations + AbsolutePath absInput = TaskEnvironment.GetAbsolutePath(InputFile); + AbsolutePath absOutput = TaskEnvironment.GetAbsolutePath(OutputDir); + + File.Exists(absInput); // ✅ AbsolutePath implicit conversion + Directory.Exists(absOutput); // ✅ AbsolutePath implicit conversion + Directory.CreateDirectory(absOutput); + File.ReadAllText(absInput); + var fi = new FileInfo(absInput); // ✅ AbsolutePath conversion + var di = new DirectoryInfo(absOutput); + + // ✅ Using TaskEnvironment for process start + ProcessStartInfo psi = TaskEnvironment.GetProcessStartInfo(); + psi.FileName = "tool.exe"; + + // ✅ Logging instead of Console + Log.LogMessage("Processing file"); + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 3: Regular ITask (NOT IMultiThreadableTask) + // Should still get MSBuildTask0001 and MSBuildTask0004 but NOT 0002/0003 + // ───────────────────────────────────────────────────────────────────────── + public class RegularTask : Task + { + public string InputFile { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0001: Console.* always wrong in tasks + Console.WriteLine("hello from regular task"); + + // ❌ MSBuildTask0001: Environment.Exit always wrong + Environment.Exit(42); + + // ✅ No warning: Environment.GetEnvironmentVariable OK in regular task + string val = Environment.GetEnvironmentVariable("PATH"); + + // ✅ No warning: File.Exists OK in regular task (not multithreaded) + File.Exists(InputFile); + + // ❌ MSBuildTask0004: Assembly.LoadFrom still flagged + Assembly.LoadFrom("plugin.dll"); + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 4: Safe patterns - ITaskItem.GetMetadata("FullPath") + // Should produce ZERO diagnostics + // ───────────────────────────────────────────────────────────────────────── + public class MetadataFullPathTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + + [Required] + public ITaskItem[] SourceFiles { get; set; } + + public override bool Execute() + { + foreach (ITaskItem item in SourceFiles) + { + // ✅ GetMetadata("FullPath") is safe - MSBuild resolves it + string fullPath = item.GetMetadata("FullPath"); + File.Exists(fullPath); // ✅ No warning - comes from GetMetadata("FullPath") + // Note: this is still flagged because we check the argument + // at the call site, not data flow. This is a known limitation. + + // ✅ Using TaskEnvironment.GetAbsolutePath for ItemSpec + AbsolutePath abs = TaskEnvironment.GetAbsolutePath(item.ItemSpec); + File.ReadAllText(abs); // ✅ No warning + } + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 5: FileInfo.FullName is safe + // ───────────────────────────────────────────────────────────────────────── + public class FileInfoFullNameTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string InputFile { get; set; } + + public override bool Execute() + { + AbsolutePath abs = TaskEnvironment.GetAbsolutePath(InputFile); + var fi = new FileInfo(abs); + + // ✅ Using FileInfo.FullName is safe + File.ReadAllText(fi.FullName); + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 6: Multiple Console overloads + // All Console methods should be flagged as MSBuildTask0001 + // ───────────────────────────────────────────────────────────────────────── + public class ConsoleOverloadsTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + + public override bool Execute() + { + Console.Write("a"); // MSBuildTask0001 + Console.Write(42); // MSBuildTask0001 + Console.Write(true); // MSBuildTask0001 + Console.Write('x'); // MSBuildTask0001 + Console.Write("{0}", "a"); // MSBuildTask0001 + Console.WriteLine(); // MSBuildTask0001 + Console.WriteLine("b"); // MSBuildTask0001 + Console.WriteLine(42); // MSBuildTask0001 + Console.WriteLine("{0}", "b"); // MSBuildTask0001 + Console.ReadLine(); // MSBuildTask0001 + var _ = Console.In; // MSBuildTask0001 + var __ = Console.Out; // MSBuildTask0001 + var ___ = Console.Error; // MSBuildTask0001 + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 7: ProcessStartInfo and Process.Start + // ───────────────────────────────────────────────────────────────────────── + public class ProcessStartTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0002: ProcessStartInfo constructors + var psi1 = new ProcessStartInfo(); // MSBuildTask0002 + var psi2 = new ProcessStartInfo("cmd.exe"); // MSBuildTask0002 + var psi3 = new ProcessStartInfo("cmd", "/c"); // MSBuildTask0002 + + // ❌ MSBuildTask0002: Process.Start overloads + Process.Start("notepad.exe"); // MSBuildTask0002 + Process.Start("cmd.exe", "/c dir"); // MSBuildTask0002 + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 8: ThreadPool and CultureInfo + // ───────────────────────────────────────────────────────────────────────── + public class ThreadPoolCultureTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0001: ThreadPool settings + ThreadPool.SetMinThreads(4, 4); // MSBuildTask0001 + ThreadPool.SetMaxThreads(16, 16); // MSBuildTask0001 + + // ❌ MSBuildTask0001: CultureInfo defaults + CultureInfo.DefaultThreadCurrentCulture = CultureInfo.InvariantCulture; // MSBuildTask0001 + CultureInfo.DefaultThreadCurrentUICulture = CultureInfo.InvariantCulture; // MSBuildTask0001 + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 9: Environment.FailFast overloads + // ───────────────────────────────────────────────────────────────────────── + public class EnvironmentFailFastTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + + public override bool Execute() + { + if (false) + { + Environment.FailFast("fatal"); // MSBuildTask0001 + Environment.FailFast("fatal", new Exception()); // MSBuildTask0001 + } + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 10: Environment variable edge cases + // ───────────────────────────────────────────────────────────────────────── + public class EnvVarEdgeCasesTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0002: 3-parameter SetEnvironmentVariable + Environment.SetEnvironmentVariable("X", "Y", EnvironmentVariableTarget.Process); // MSBuildTask0002 + + // ❌ MSBuildTask0002: ExpandEnvironmentVariables + string expanded = Environment.ExpandEnvironmentVariables("%PATH%"); // MSBuildTask0002 + + // ❌ MSBuildTask0002: GetEnvironmentVariables (no parameter) + var envVars = Environment.GetEnvironmentVariables(); // MSBuildTask0002 + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 11: File API edge cases - StreamWriter, multiple parameters + // ───────────────────────────────────────────────────────────────────────── + public class FileApiEdgeCasesTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string Path1 { get; set; } + public string Path2 { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0003: StreamWriter constructor + using var sw = new StreamWriter(Path1); // MSBuildTask0003 + + // ❌ MSBuildTask0003: FileStream constructor + using var fs = new FileStream(Path1, FileMode.Open); // MSBuildTask0003 + + // ❌ MSBuildTask0003: File.Copy (both paths unwrapped) + File.Copy(Path1, Path2); // MSBuildTask0003 (first param) + + // ❌ MSBuildTask0003: File.Move + File.Move(Path1, Path2); // MSBuildTask0003 (first param) + + // ❌ MSBuildTask0003: File.WriteAllText + File.WriteAllText(Path1, "content"); // MSBuildTask0003 + + // ❌ MSBuildTask0003: Directory.GetFiles + Directory.GetFiles(Path1); // MSBuildTask0003 + + // ❌ MSBuildTask0003: Directory.EnumerateFiles + Directory.EnumerateFiles(Path1); // MSBuildTask0003 + + // ❌ MSBuildTask0003: Directory.Delete + Directory.Delete(Path1); // MSBuildTask0003 + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 12: Assembly loading edge cases + // ───────────────────────────────────────────────────────────────────────── + public class AssemblyLoadEdgeCasesTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0004: Assembly.Load overloads + Assembly.Load("MyAssembly"); // MSBuildTask0004 + Assembly.Load(new byte[] { }); // MSBuildTask0004 + Assembly.LoadFrom("MyAssembly.dll"); // MSBuildTask0004 + Assembly.LoadFile("MyAssembly.dll"); // MSBuildTask0004 + + // ❌ MSBuildTask0004: Activator + Activator.CreateInstance("asm", "type"); // MSBuildTask0004 + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 13: Mixed safe and unsafe in same method + // Tests that the analyzer correctly differentiates + // ───────────────────────────────────────────────────────────────────────── + public class MixedSafeUnsafeTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string InputFile { get; set; } + + public override bool Execute() + { + AbsolutePath absPath = TaskEnvironment.GetAbsolutePath(InputFile); + + // ✅ Safe: using AbsolutePath + if (File.Exists(absPath)) + { + string content = File.ReadAllText(absPath); + + // ❌ MSBuildTask0003: Raw string passed to File.WriteAllText + File.WriteAllText(InputFile + ".bak", content); + } + + // ✅ Safe: TaskEnvironment property + string projDir = TaskEnvironment.ProjectDirectory; + + // ❌ MSBuildTask0002: Environment.CurrentDirectory + string cwd = Environment.CurrentDirectory; + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 14: Path.GetFullPath with two parameters + // ───────────────────────────────────────────────────────────────────────── + public class PathGetFullPathTwoParamsTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string InputFile { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0002: Path.GetFullPath(string, string) overload + string full = Path.GetFullPath(InputFile, "C:\\base"); // MSBuildTask0002 + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 15: Process.Kill edge case + // ───────────────────────────────────────────────────────────────────────── + public class ProcessKillTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + + public override bool Execute() + { + var p = Process.GetCurrentProcess(); + p.Kill(); // MSBuildTask0001 + p.Kill(true); // MSBuildTask0001 + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 16: Helper method that delegates file operations + // Tests that analysis works within methods called from Execute + // ───────────────────────────────────────────────────────────────────────── + public class HelperMethodTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string InputFile { get; set; } + + public override bool Execute() + { + ProcessFile(InputFile); + return true; + } + + private void ProcessFile(string path) + { + // ❌ MSBuildTask0003: Helper method still flagged (it's in the task class) + File.ReadAllText(path); + + // ❌ MSBuildTask0001: Console in helper method too + Console.WriteLine($"Processed: {path}"); + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 17: Non-task class should NOT be analyzed + // ───────────────────────────────────────────────────────────────────────── + public class NotATask + { + public void DoWork() + { + // ✅ No diagnostics - this is not a task + Console.WriteLine("This is fine"); + File.Exists("anything"); + Environment.CurrentDirectory = "whatever"; + Environment.Exit(0); + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 18: Task inheriting from another task + // ───────────────────────────────────────────────────────────────────────── + public class DerivedTask : CorrectlyMigratedTask + { + public override bool Execute() + { + // ❌ This task inherits IMultiThreadableTask + File.Exists("relative/path.txt"); // MSBuildTask0003 + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 19: Task using ToolTask (still implements ITask) + // ───────────────────────────────────────────────────────────────────────── + public class ToolTaskDerivedTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string SomePath { get; set; } + public IBuildEngine BuildEngine { get; set; } + public ITaskHost HostObject { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0003 + Directory.CreateDirectory(SomePath); + + // ❌ MSBuildTask0001 + Console.Error.WriteLine("error!"); + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 20: Batch processing with exception handling pattern + // Tests that analysis works correctly with try/catch + // ───────────────────────────────────────────────────────────────────────── + public class BatchProcessingTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public ITaskItem[] SourceFiles { get; set; } + public ITaskItem[] DestinationFiles { get; set; } + + public override bool Execute() + { + bool success = true; + + for (int i = 0; i < SourceFiles.Length; i++) + { + string sourceSpec = SourceFiles[i].ItemSpec; + string destSpec = DestinationFiles[i].ItemSpec; + + try + { + AbsolutePath source = TaskEnvironment.GetAbsolutePath(sourceSpec); + AbsolutePath dest = TaskEnvironment.GetAbsolutePath(destSpec); + + File.Copy(source, dest); // ✅ Safe - AbsolutePath + } + catch (Exception ex) + { + Log.LogError($"Failed to copy {sourceSpec}: {ex.Message}"); + success = false; + } + } + + return success; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 21: Chained method calls + // ───────────────────────────────────────────────────────────────────────── + public class ChainedCallTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string Dir { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0003: Path.Combine result is not absolute + string combined = Path.Combine(Dir, "subdir", "file.txt"); + File.Exists(combined); + + // ✅ Safe: GetAbsolutePath of combined path + File.Exists(TaskEnvironment.GetAbsolutePath(combined)); + + return true; + } + } + + // ───────────────────────────────────────────────────────────────────────── + // TASK 22: Fully qualified API names (System.IO.File instead of File) + // ───────────────────────────────────────────────────────────────────────── + public class FullyQualifiedTask : Task, IMultiThreadableTask + { + public TaskEnvironment TaskEnvironment { get; set; } + public string InputFile { get; set; } + + public override bool Execute() + { + // ❌ Should still be caught even with fully qualified names + System.IO.File.Exists(InputFile); // MSBuildTask0003 + System.IO.Directory.CreateDirectory(InputFile); // MSBuildTask0003 + System.Environment.Exit(1); // MSBuildTask0001 + System.Console.WriteLine("bad"); // MSBuildTask0001 + + return true; + } + } +} diff --git a/src/ThreadSafeTaskAnalyzer/demo/AnalyzerDemo.csproj b/src/ThreadSafeTaskAnalyzer/demo/AnalyzerDemo.csproj new file mode 100644 index 00000000000..746d7789e92 --- /dev/null +++ b/src/ThreadSafeTaskAnalyzer/demo/AnalyzerDemo.csproj @@ -0,0 +1,28 @@ + + + + net10.0 + latest + enable + Library + false + + false + + false + + + + + + + + + + + + + + diff --git a/src/ThreadSafeTaskAnalyzer/demo/HelperClassAndNewApis.cs b/src/ThreadSafeTaskAnalyzer/demo/HelperClassAndNewApis.cs new file mode 100644 index 00000000000..73dcb330842 --- /dev/null +++ b/src/ThreadSafeTaskAnalyzer/demo/HelperClassAndNewApis.cs @@ -0,0 +1,208 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// ═══════════════════════════════════════════════════════════════════════════════ +// DEMO: Helper class opt-in with [MSBuildMultiThreadableTaskAnalyzed], +// new banned APIs, and type-level Console detection +// ═══════════════════════════════════════════════════════════════════════════════ + +#nullable disable + +using System; +using System.IO; +using Microsoft.Build.Framework; +using Microsoft.Build.Utilities; +using Task = Microsoft.Build.Utilities.Task; + +namespace Microsoft.Build.Framework +{ + [AttributeUsage(AttributeTargets.Class)] + public class MSBuildMultiThreadableTaskAnalyzedAttribute : Attribute { } +} + +// ───────────────────────────────────────────────────────────────────────── +// HELPER CLASS 1: Opted-in helper class - should be analyzed +// ───────────────────────────────────────────────────────────────────────── +[MSBuildMultiThreadableTaskAnalyzed] +public class FileProcessingHelper +{ + // ❌ MSBuildTask0003: File.Exists with unwrapped path + public bool FileExists(string path) => File.Exists(path); + + // ❌ MSBuildTask0002: Environment.GetEnvironmentVariable + public string GetEnv(string name) => Environment.GetEnvironmentVariable(name); + + // ❌ MSBuildTask0001: Console.WriteLine + public void Log(string msg) => Console.WriteLine(msg); + + // ✅ Safe: uses AbsolutePath + public bool FileExistsSafe(AbsolutePath path) => File.Exists(path); + + // ✅ Safe: uses ITaskItem.GetMetadata("FullPath") + public bool FileExistsMeta(ITaskItem item) => File.Exists(item.GetMetadata("FullPath")); +} + +// ───────────────────────────────────────────────────────────────────────── +// HELPER CLASS 2: NOT opted-in - should NOT be analyzed +// ───────────────────────────────────────────────────────────────────────── +public class RegularHelperClass +{ + // ✅ Not analyzed (no attribute, not a task) + public bool FileExists(string path) => File.Exists(path); + public string GetEnv(string name) => Environment.GetEnvironmentVariable(name); + public void Log(string msg) => Console.WriteLine(msg); +} + +// ───────────────────────────────────────────────────────────────────────── +// TASK 51: Directory.SetCurrentDirectory (CriticalError) +// ───────────────────────────────────────────────────────────────────────── +public class SetCurrentDirTask : Task +{ + public override bool Execute() + { + // ❌ MSBuildTask0001: Directory.SetCurrentDirectory is process-wide + Directory.SetCurrentDirectory("/tmp"); + return true; + } +} + +// ───────────────────────────────────────────────────────────────────────── +// TASK 52: Directory.GetCurrentDirectory (TaskEnvironment) +// ───────────────────────────────────────────────────────────────────────── +public class GetCurrentDirTask : Task, IMultiThreadableTask +{ + public TaskEnvironment TaskEnvironment { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0002: Directory.GetCurrentDirectory + var dir = Directory.GetCurrentDirectory(); + return true; + } +} + +// ───────────────────────────────────────────────────────────────────────── +// TASK 53: Path.GetTempPath and Path.GetTempFileName +// ───────────────────────────────────────────────────────────────────────── +public class TempPathTask : Task, IMultiThreadableTask +{ + public TaskEnvironment TaskEnvironment { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0002: Path.GetTempPath depends on TMP/TEMP env vars + var tmp = Path.GetTempPath(); + + // ❌ MSBuildTask0002: Path.GetTempFileName depends on TMP/TEMP env vars + var tempFile = Path.GetTempFileName(); + + return true; + } +} + +// ───────────────────────────────────────────────────────────────────────── +// TASK 54: Environment.GetFolderPath +// ───────────────────────────────────────────────────────────────────────── +public class FolderPathTask : Task, IMultiThreadableTask +{ + public TaskEnvironment TaskEnvironment { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0002: Environment.GetFolderPath + var home = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); + + // ❌ MSBuildTask0002: Environment.GetFolderPath with option + var appData = Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData, Environment.SpecialFolderOption.Create); + + return true; + } +} + +// ───────────────────────────────────────────────────────────────────────── +// TASK 55: Type-level Console ban - catches exotic Console members +// ───────────────────────────────────────────────────────────────────────── +public class ExoticConsoleTask : Task +{ + public override bool Execute() + { + // ❌ MSBuildTask0001: Console.ForegroundColor + Console.ForegroundColor = ConsoleColor.Red; + + // ❌ MSBuildTask0001: Console.ResetColor + Console.ResetColor(); + + // ❌ MSBuildTask0001: Console.Beep + Console.Beep(); + + // ❌ MSBuildTask0001: Console.Clear + Console.Clear(); + + // ❌ MSBuildTask0001: Console.BufferWidth + var w = Console.BufferWidth; + + return true; + } +} + +// ───────────────────────────────────────────────────────────────────────── +// TASK 56: Process.Start(ProcessStartInfo) overload +// ───────────────────────────────────────────────────────────────────────── +public class ProcessStartPsiTask : Task, IMultiThreadableTask +{ + public TaskEnvironment TaskEnvironment { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0002: new ProcessStartInfo + var psi = new System.Diagnostics.ProcessStartInfo("dotnet"); + + // ❌ MSBuildTask0002: Process.Start(ProcessStartInfo) + System.Diagnostics.Process.Start(psi); + + return true; + } +} + +// ───────────────────────────────────────────────────────────────────────── +// TASK 57: FileSystemWatcher constructor (file path type) +// ───────────────────────────────────────────────────────────────────────── +public class FileWatcherTask : Task, IMultiThreadableTask +{ + public TaskEnvironment TaskEnvironment { get; set; } + public string WatchDir { get; set; } + + public override bool Execute() + { + // ❌ MSBuildTask0003: FileSystemWatcher with unwrapped path + using var watcher = new FileSystemWatcher(WatchDir); + + // ✅ Safe: with GetAbsolutePath + using var watcher2 = new FileSystemWatcher(TaskEnvironment.GetAbsolutePath(WatchDir)); + + return true; + } +} + +// ───────────────────────────────────────────────────────────────────────── +// TASK 58: Non-path string params NOT flagged (File.WriteAllText fix) +// ───────────────────────────────────────────────────────────────────────── +public class WriteAllTextSafeTask : Task, IMultiThreadableTask +{ + public TaskEnvironment TaskEnvironment { get; set; } + public string OutputFile { get; set; } + + public override bool Execute() + { + AbsolutePath safePath = TaskEnvironment.GetAbsolutePath(OutputFile); + + // ✅ Safe: path is absolutized, "contents" param should NOT be flagged + File.WriteAllText(safePath, "these are contents, not a path"); + File.AppendAllText(safePath, "more contents"); + + // ❌ MSBuildTask0003: path param is NOT absolutized + File.WriteAllText(OutputFile, "contents"); + + return true; + } +}