TaskAnalyzer: MSBuildTask0002 is not reported for code inherited from an unannotated base class
- Dominant language
- C#
- Stars
- 5.5k
- Forks
- 1.5k
- Avg merge
- 1d 8h
- Merged PRs (30d)
- 141
Description
### Issue Description
`MSBuildTask0002` (and, by the same mechanism, `0003`/`0005`) is not reported when the offending code lives in a **base class** of a task that carries `[MSBuildMultiThreadableTask]`.
#14772 already tracks base-class scope limitations, but only under **MSBuildTask0006** ("Base-class analysis") and **MSBuildTask0007**. This issue reports that the same limitation applies to the *path and environment* rules, which is where it actually causes silent correctness bugs.
This is not a theoretical concern: it is the dominant shape in the Arcade task migration ([dotnet/arcade#17381](https://github.com/dotnet/arcade/pull/17381), tracked by [dotnet/arcade#17378](https://github.com/dotnet/arcade/issues/17378)). 14 of 136 tasks had to be de-annotated *after* CI failures because their process-global state lived in a base class that the analyzer never looked at. The analyzer reported a clean build for every one of them.
### Steps to Reproduce
Analyzer `Microsoft.Build.TaskAuthoring.Analyzer` `18.11.0-1.26420.118`, `Microsoft.Build.Utilities.Core` `18.8.2`, with:
```ini
is_global = true
msbuild_task_analyzer.scope = multithreadable_only
```
```csharp
using System;
using System.IO;
using Microsoft.Build.Framework;
using Microsoft.Build.Utilities;
// ---------- CONTROL A: usage directly in the annotated class ----------
[MSBuildMultiThreadableTask]
public sealed class ControlDirect : Task, IMultiThreadableTask
{
public TaskEnvironment TaskEnvironment { get; set; } = TaskEnvironment.Fallback;
public override bool Execute()
{
string a = Environment.GetEnvironmentVariable("SYSTEM_ACCESSTOKEN");
string b = Directory.GetCurrentDirectory();
string c = Environment.CurrentDirectory;
Log.LogMessage(a + b + c);
return true;
}
}
// ---------- CONTROL B: same usage via a same-class static helper ----------
[MSBuildMultiThreadableTask]
public sealed class ControlSameClassHelper : Task, IMultiThreadableTask
{
public TaskEnvironment TaskEnvironment { get; set; } = TaskEnvironment.Fallback;
public override bool Execute()
{
Log.LogMessage(Read());
return true;
}
private static string Read() => Environment.GetEnvironmentVariable("SYSTEM_ACCESSTOKEN");
}
// ---------- THE GAP: identical usage, but in an unannotated base ----------
public abstract class BaseWithEnv : Task
{
protected string Token => Environment.GetEnvironmentVariable("SYSTEM_ACCESSTOKEN");
protected string Here() => Directory.GetCurrentDirectory();
public override bool Execute()
{
Log.LogMessage(Token + Here());
return ExecuteCore();
}
protected abstract bool ExecuteCore();
}
[MSBuildMultiThreadableTask]
public sealed class DerivedAnnotated : BaseWithEnv, IMultiThreadableTask
{
public TaskEnvironment TaskEnvironment { get; set; } = TaskEnvironment.Fallback;
protected override bool ExecuteCore() => true;
}
```
Build with `dotnet build -c Release --no-incremental`.
### Expected Behavior
`DerivedAnnotated` is the type the attribute is applied to, and it is the type MSBuild will schedule onto a shared node. The environment and CWD reads it inherits execute on that shared node, so they should be reported — as they are for the two controls.
### Actual Behavior
The two controls are reported. The inherited case is silent.
```
Control.cs(16,24): warning MSBuildTask0002: 'Environment.GetEnvironmentVariable(string)' should use TaskEnvironment alternative: use TaskEnvironment.GetEnvironmentVariable instead
Control.cs(17,24): warning MSBuildTask0002: 'Directory.GetCurrentDirectory()' should use TaskEnvironment alternative: use TaskEnvironment.ProjectDirectory instead
Control.cs(18,24): warning MSBuildTask0002: 'Environment.CurrentDirectory' should use TaskEnvironment alternative: use TaskEnvironment.ProjectDirectory instead
Control.cs(50,41): warning MSBuildTask0002: 'Environment.GetEnvironmentVariable(string)' should use TaskEnvironment alternative: use TaskEnvironment.GetEnvironmentVariable instead
(nothing for BaseWithEnv / DerivedAnnotated)
```
Control B is the significant one: it proves the analyzer **does** perform interprocedural analysis, and reaches a `private static` helper in the same type. The analysis therefore stops at the **type** boundary specifically, not at the method boundary.
`MSBuildTask0011` *does* fire on `DerivedAnnotated`, so the type is recognised as an annotated task — the rule simply never inspects its inherited members.
### Impact
This is the worst possible failure mode for a migration aid: it is silent, and it is silent precisely on the tasks that are hardest to reason about. A task author annotates a class, gets a clean analyzer run, ships it, and the shared-node violation only surfaces as a nondeterministic CI failure.
### Suggested Fix
When a type carries `[MSBuildMultiThreadableTask]`, analyze the transitive closure of members it inherits, not just members it declares. Diagnostics can be reported at the declaration site in the base with a message naming the annotated derived type (or on the derived type's attribute), whichever is less noisy when many tasks share a base.
A cheaper interim mitigation that would have caught all 14 Arcade cases: report an informational diagnostic on the annotated type whenever it derives from a base outside the current compilation, or from an unannotated base that itself contains banned APIs, telling the author the base was not verified.
### Versions & Configurations
- `Microsoft.Build.TaskAuthoring.Analyzer` `18.11.0-1.26420.118`
- `Microsoft.Build.Utilities.Core` `18.8.2`
- SDK `11.0.0-preview.6.26359.118`
### Related
- #14772 — tracks the same limitation for `MSBuildTask0006`/`0007`
- #14779, #14780
- [dotnet/arcade#17378](https://github.com/dotnet/arcade/issues/17378), [dotnet/arcade#17381](https://github.com/dotnet/arcade/pull/17381)
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.