Analyze base classes of multithreadable tasks in TaskAnalyzer

plugins/mt-migration/skills/multithreaded-task-migration/SKILL.md

@@ -315,16 +315,16 @@ Helpers reached from `Execute()` can quietly depend on process state in any of t
 
 ### Unsafe Code in an Unannotated Base Class
 
-`[MSBuildMultiThreadableTask]` is `Inherited = false`, so it goes on each concrete task. The consequence people miss is the mirror image: **the base class runs multithreaded too, but nothing marks it as such** — and with `msbuild_task_analyzer.scope = multithreadable_only` (the recommended setting for incremental migration) the analyzer does not look inside it at all.
+`[MSBuildMultiThreadableTask]` is `Inherited = false`, so it goes on each concrete task. The consequence people miss is the mirror image: **the base class runs multithreaded too, but nothing marks it as such**. The analyzer now follows the base chain of an annotated task and reports inherited code at its declaration site under either scope setting — but only for base classes declared in the same compilation. A base living in a referenced assembly is still unverified, so migrate that assembly first.
 
-From dotnet/arcade: `CreateAkaMSLinks` and `DeleteAkaMSLinks` were both annotated, both analyzer-clean. Their shared base contained:
+From dotnet/arcade, before the analyzer looked at base classes: `CreateAkaMSLinks` and `DeleteAkaMSLinks` were both annotated, both analyzer-clean. Their shared base contained:
 
 ```csharp
-// AkaMSLinksBase - NOT annotated, so never analyzed
+// AkaMSLinksBase - NOT annotated, and at the time not analyzed either
 File.ReadAllText(ClientCertificate)   // ClientCertificate is a task input property
 ```
 
-A monitored API (`File`) on a raw task input — exactly what the analyzer exists to catch — invisible purely because of where it lived.
+A monitored API (`File`) on a raw task input — exactly what the analyzer exists to catch — invisible purely because of where it lived. Audit the base chain yourself whenever any part of it lives outside the compilation the analyzer sees.
 
 **Resolution:** when you annotate a task, audit its full base chain up to `Task`/`ToolTask`. If the base holds shared input properties or path handling, have the **base** implement `IMultiThreadableTask` and do the resolution there, so every derived task inherits the fix:
 

src/TaskAnalyzer.Tests/MultiThreadableTaskAnalyzerTests.cs

@@ -1909,4 +1909,161 @@ public override bool Execute()
         // IMultiThreadableTask SHOULD get MSBuildTask0002 even when scope is multithreadable_only
         diags.Where(d => d.Id == DiagnosticIds.TaskEnvironmentRequired).ShouldNotBeEmpty();
     }
+
+    [Fact]
+    public async Task Scope_MultithreadableOnly_UnannotatedBaseOfAnnotatedTask_GetsDiagnostic()
+    {
+        var diags = await GetDiagnosticsWithScopeAsync("""
+            using System;
+            using System.IO;
+            using Microsoft.Build.Framework;
+
+            public abstract class BaseWithEnv : Microsoft.Build.Utilities.Task
+            {
+                protected string? Token => Environment.GetEnvironmentVariable("SYSTEM_ACCESSTOKEN");
+                protected string Here() => Directory.GetCurrentDirectory();
+            }
+
+            [MSBuildMultiThreadableTask]
+            public sealed class DerivedAnnotated : BaseWithEnv, IMultiThreadableTask
+            {
+                public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment();
+                public override bool Execute() => Token is not null && Here() is not null;
+            }
+            """, SharedAnalyzerHelpers.ScopeMultiThreadableOnly);
+
+        // The base class runs on the shared node as part of the annotated task, so its
+        // environment and working-directory reads are reported.
+        diags.Where(d => d.Id == DiagnosticIds.TaskEnvironmentRequired).Count().ShouldBe(2);
+    }
+
+    [Fact]
+    public async Task Scope_MultithreadableOnly_UnannotatedBaseUsedByPlainTask_NoDiagnostic()
+    {
+        var diags = await GetDiagnosticsWithScopeAsync("""
+            using System;
+
+            public abstract class BaseWithEnv : Microsoft.Build.Utilities.Task
+            {
+                protected string? Token => Environment.GetEnvironmentVariable("SYSTEM_ACCESSTOKEN");
+            }
+
+            public sealed class PlainDerived : BaseWithEnv
+            {
+                public override bool Execute() => Token is not null;
+            }
+            """, SharedAnalyzerHelpers.ScopeMultiThreadableOnly);
+
+        // No task in the hierarchy opted into multithreaded execution
+        diags.Where(d => d.Id == DiagnosticIds.TaskEnvironmentRequired).ShouldBeEmpty();
+    }
+
+    [Fact]
+    public async Task Scope_MultithreadableOnly_GrandparentOfAnnotatedTask_GetsDiagnostic()
+    {
+        var diags = await GetDiagnosticsWithScopeAsync("""
+            using System.IO;
+            using Microsoft.Build.Framework;
+
+            public abstract class Grandparent : Microsoft.Build.Utilities.Task
+            {
+                protected string Read(string path) => File.ReadAllText(path);
+            }
+
+            public abstract class Parent : Grandparent
+            {
+            }
+
+            [MSBuildMultiThreadableTask]
+            public sealed class DerivedAnnotated : Parent, IMultiThreadableTask
+            {
+                public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment();
+                public string ClientCertificate { get; set; } = "cert.pfx";
+                public override bool Execute() => Read(ClientCertificate) is not null;
+            }
+            """, SharedAnalyzerHelpers.ScopeMultiThreadableOnly);
+
+        // The whole base chain is in scope, not just the immediate base
+        diags.Where(d => d.Id == DiagnosticIds.FilePathRequiresAbsolute).Count().ShouldBe(1);
+    }
+
+    [Fact]
+    public async Task Scope_MultithreadableOnly_NonTaskBaseOfAnnotatedTask_GetsDiagnostic()
+    {
+        var diags = await GetDiagnosticsWithScopeAsync("""
+            using System;
+            using Microsoft.Build.Framework;
+
+            public abstract class NonTaskBase
+            {
+                protected string? Token => Environment.GetEnvironmentVariable("SYSTEM_ACCESSTOKEN");
+            }
+
+            [MSBuildMultiThreadableTask]
+            public sealed class DerivedAnnotated : NonTaskBase, IMultiThreadableTask
+            {
+                public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment();
+                public IBuildEngine BuildEngine { get; set; } = new BuildEngineStub();
+                public bool Execute() => Token is not null;
+            }
+            """, SharedAnalyzerHelpers.ScopeMultiThreadableOnly);
+
+        // A base class that is not itself an ITask still runs as part of the annotated task
+        diags.Where(d => d.Id == DiagnosticIds.TaskEnvironmentRequired).Count().ShouldBe(1);
+    }
+
+    [Fact]
+    public async Task Scope_MultithreadableOnly_GenericBaseOfAnnotatedTask_GetsDiagnostic()
+    {
+        var diags = await GetDiagnosticsWithScopeAsync("""
+            using System;
+            using Microsoft.Build.Framework;
+
+            public abstract class GenericBase<T> : Microsoft.Build.Utilities.Task
+            {
+                protected string? Token => Environment.GetEnvironmentVariable("SYSTEM_ACCESSTOKEN");
+            }
+
+            [MSBuildMultiThreadableTask]
+            public sealed class DerivedAnnotated : GenericBase<string>, IMultiThreadableTask
+            {
+                public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment();
+                public override bool Execute() => Token is not null;
+            }
+            """, SharedAnalyzerHelpers.ScopeMultiThreadableOnly);
+
+        // The base is referenced as GenericBase<string> but declared as GenericBase<T>
+        diags.Where(d => d.Id == DiagnosticIds.TaskEnvironmentRequired).Count().ShouldBe(1);
+    }
+
+    [Fact]
+    public async Task Scope_MultithreadableOnly_BaseSharedByTwoAnnotatedTasks_ReportedOnce()
+    {
+        var diags = await GetDiagnosticsWithScopeAsync("""
+            using System;
+            using Microsoft.Build.Framework;
+
+            public abstract class SharedBase : Microsoft.Build.Utilities.Task
+            {
+                protected string? Token => Environment.GetEnvironmentVariable("SYSTEM_ACCESSTOKEN");
+            }
+
+            [MSBuildMultiThreadableTask]
+            public sealed class FirstTask : SharedBase, IMultiThreadableTask
+            {
+                public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment();
+                public override bool Execute() => Token is not null;
+            }
+
+            [MSBuildMultiThreadableTask]
+            public sealed class SecondTask : SharedBase, IMultiThreadableTask
+            {
+                public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment();
+                public override bool Execute() => Token is not null;
+            }
+            """, SharedAnalyzerHelpers.ScopeMultiThreadableOnly);
+
+        // The shared base is analyzed once, not once per derived task
+        diags.Where(d => d.Id == DiagnosticIds.TaskEnvironmentRequired).Count().ShouldBe(1);
+    }
 }

src/TaskAnalyzer.Tests/TestHelpers.cs

@@ -244,9 +244,24 @@ public static CSharpCompilation CreateCompilation(string source)
     /// Runs the MultiThreadableTaskAnalyzer with a specific scope option and returns analyzer diagnostics.
     /// </summary>
     public static async System.Threading.Tasks.Task<ImmutableArray<Diagnostic>> GetDiagnosticsWithScopeAsync(string source, string scope)
+    {
+        return await GetDiagnosticsWithScopeAsync(source, scope, new MultiThreadableTaskAnalyzer());
+    }
+
+    /// <summary>
+    /// Runs BOTH the direct and transitive analyzers with a specific scope option.
+    /// </summary>
+    public static async System.Threading.Tasks.Task<ImmutableArray<Diagnostic>> GetAllDiagnosticsWithScopeAsync(string source, string scope)
+    {
+        return await GetDiagnosticsWithScopeAsync(source, scope, new MultiThreadableTaskAnalyzer(), new TransitiveCallChainAnalyzer());
+    }
+
+    private static async System.Threading.Tasks.Task<ImmutableArray<Diagnostic>> GetDiagnosticsWithScopeAsync(
+        string source,
+        string scope,
+        params DiagnosticAnalyzer[] analyzers)
     {
         var compilation = CreateCompilation(source);
-        var analyzer = new MultiThreadableTaskAnalyzer();
 
         var globalOptions = new Dictionary<string, string>
         {
@@ -255,8 +270,7 @@ public static async System.Threading.Tasks.Task<ImmutableArray<Diagnostic>> GetD
         var optionsProvider = new TestAnalyzerConfigOptionsProvider(globalOptions);
         var options = new AnalyzerOptions(ImmutableArray<AdditionalText>.Empty, optionsProvider);
 
-        var compilationWithAnalyzers = compilation.WithAnalyzers(
-            ImmutableArray.Create<DiagnosticAnalyzer>(analyzer), options);
+        var compilationWithAnalyzers = compilation.WithAnalyzers(ImmutableArray.Create(analyzers), options);
         return await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync();
     }
 

src/TaskAnalyzer.Tests/TransitiveCallChainAnalyzerTests.cs

@@ -244,4 +244,104 @@ public override bool Execute()
         msg.ShouldContain("A.Step1");
         msg.ShouldContain("B.Step2");
     }
-}
+
+    [Fact]
+    public async Task Scope_MultithreadableOnly_BaseClassMethodCallingHelper_ProducesDiagnostic()
+    {
+        var diags = await GetAllDiagnosticsWithScopeAsync("""
+            using System;
+            using Microsoft.Build.Framework;
+
+            public class UnsafeHelper
+            {
+                public static string? ReadToken() => Environment.GetEnvironmentVariable("SYSTEM_ACCESSTOKEN");
+            }
+
+            public abstract class BaseWithHelperCall : Microsoft.Build.Utilities.Task
+            {
+                public override bool Execute() => UnsafeHelper.ReadToken() is not null && ExecuteCore();
+
+                protected abstract bool ExecuteCore();
+            }
+
+            [MSBuildMultiThreadableTask]
+            public sealed class DerivedAnnotated : BaseWithHelperCall, IMultiThreadableTask
+            {
+                public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment();
+
+                protected override bool ExecuteCore() => true;
+            }
+            """, SharedAnalyzerHelpers.ScopeMultiThreadableOnly);
+
+        // The call chain starts in the unannotated base, which the annotated task inherits
+        var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ToArray();
+        transitive.Length.ShouldBe(1);
+        transitive[0].GetMessage().ShouldContain("UnsafeHelper.ReadToken");
+    }
+
+    [Fact]
+    public async Task Scope_MultithreadableOnly_BaseSharedByTwoAnnotatedTasks_ReportedOnce()
+    {
+        var diags = await GetAllDiagnosticsWithScopeAsync("""
+            using System;
+            using Microsoft.Build.Framework;
+
+            public class UnsafeHelper
+            {
+                public static string? ReadToken() => Environment.GetEnvironmentVariable("SYSTEM_ACCESSTOKEN");
+            }
+
+            public abstract class SharedBase : Microsoft.Build.Utilities.Task
+            {
+                public override bool Execute() => UnsafeHelper.ReadToken() is not null && ExecuteCore();
+
+                protected abstract bool ExecuteCore();
+            }
+
+            [MSBuildMultiThreadableTask]
+            public sealed class FirstTask : SharedBase, IMultiThreadableTask
+            {
+                public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment();
+
+                protected override bool ExecuteCore() => true;
+            }
+
+            [MSBuildMultiThreadableTask]
+            public sealed class SecondTask : SharedBase, IMultiThreadableTask
+            {
+                public TaskEnvironment TaskEnvironment { get; set; } = new TaskEnvironment();
+
+                protected override bool ExecuteCore() => true;
+            }
+            """, SharedAnalyzerHelpers.ScopeMultiThreadableOnly);
+
+        // The shared base is walked once, not once per derived task
+        var transitive = diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ToArray();
+        transitive.Length.ShouldBe(1);
+    }
+
+    [Fact]
+    public async Task Scope_MultithreadableOnly_BaseOfPlainTaskOnly_NoDiagnostic()
+    {
+        var diags = await GetAllDiagnosticsWithScopeAsync("""
+            using System;
+
+            public class UnsafeHelper
+            {
+                public static string? ReadToken() => Environment.GetEnvironmentVariable("SYSTEM_ACCESSTOKEN");
+            }
+
+            public abstract class PlainBase : Microsoft.Build.Utilities.Task
+            {
+                public override bool Execute() => UnsafeHelper.ReadToken() is not null;
+            }
+
+            public sealed class PlainDerived : PlainBase
+            {
+            }
+            """, SharedAnalyzerHelpers.ScopeMultiThreadableOnly);
+
+        // No task in the hierarchy opted into multithreaded execution
+        diags.Where(d => d.Id == DiagnosticIds.TransitiveUnsafeCall).ShouldBeEmpty();
+    }
+}

src/TaskAnalyzer/MultiThreadableTaskAnalyzer.cs

@@ -21,6 +21,9 @@ namespace Microsoft.Build.TaskAuthoring.Analyzer
     /// - "multithreadable_only": MSBuildTask0002, 0003 fire only on IMultiThreadableTask or [MSBuildMultiThreadableTask]
     ///   (MSBuildTask0001 and MSBuildTask0004 always fire on all tasks regardless)
     /// 
+    /// Base classes of a multithreadable task are analyzed as multithreadable under either scope, since the
+    /// members a task inherits run on the shared node just like the ones it declares.
+    /// 
     /// Per review feedback from @rainersigwald:
     /// - Console.* promoted to MSBuildTask0001 (always wrong in tasks)
     /// - Helper classes can opt in via [MSBuildMultiThreadableTaskAnalyzed] attribute
@@ -72,6 +75,16 @@ private void OnCompilationStart(CompilationStartAnalysisContext compilationConte
             // Build set of file-path types for MSBuildTask0003
             var filePathTypes = ResolveFilePathTypes(compilationContext.Compilation);
 
+            // [MSBuildMultiThreadableTask] is not inherited, but the members a task inherits still run on
+            // the shared node. Base classes of a multithreadable task are therefore analyzed as
+            // multithreadable themselves. Computed once per compilation, on first use.
+            var multiThreadableBaseTypes = new Lazy<ImmutableHashSet<INamedTypeSymbol>>(() =>
+                CollectMultiThreadableBaseTypes(
+                    compilationContext.Compilation,
+                    iMultiThreadableTaskType,
+                    multiThreadableTaskAttributeType,
+                    analyzedAttributeType));
+
             // Use RegisterSymbolStartAction for efficient per-type scoping
             compilationContext.RegisterSymbolStartAction(symbolStartContext =>
             {
@@ -89,13 +102,17 @@ private void OnCompilationStart(CompilationStartAnalysisContext compilationConte
                 bool hasMultiThreadableAttribute = multiThreadableTaskAttributeType is not null &&
                     namedType.GetAttributes().Any(a => SymbolEqualityComparer.Default.Equals(a.AttributeClass, multiThreadableTaskAttributeType));
 
-                if (!isTask && !hasAnalyzedAttribute)
+                // Base classes of a multithreadable task are in scope even without an annotation of their own
+                bool isMultiThreadableBaseType = !isMultiThreadableTask && !hasAnalyzedAttribute && !hasMultiThreadableAttribute &&
+                    multiThreadableBaseTypes.Value.Contains(namedType);
+
+                if (!isTask && !hasAnalyzedAttribute && !isMultiThreadableBaseType)
                 {
                     return;
                 }
 
                 // Helper classes with the attribute or tasks with [MSBuildMultiThreadableTask] are treated as IMultiThreadableTask
-                bool analyzeAsMultiThreadable = isMultiThreadableTask || hasAnalyzedAttribute || hasMultiThreadableAttribute;
+                bool analyzeAsMultiThreadable = isMultiThreadableTask || hasAnalyzedAttribute || hasMultiThreadableAttribute || isMultiThreadableBaseType;
 
                 // When scope is "multithreadable_only", only analyze MSBuildTask0002/0003 for multithreadable tasks
                 bool reportEnvironmentRules = analyzeAllTasks || analyzeAsMultiThreadable;