Add analyzer for TaskEnvironment constructor injection

[jankratochvilcz]

Summary

• add MSBuildTask0011 at Info severity for concrete IMultiThreadableTask implementations that do not expose a public single-TaskEnvironment constructor
• cover valid, invalid, abstract, and inherited-constructor cases
• document the recommended constructor-injection pattern in the analyzer README and thread-safe task specification

Replaces #14400, which GitHub automatically closed when its stacked base branch was deleted after #14315 merged. The branch is now rebased directly onto main. The rule moved from the originally proposed ID 0010 to 0011 because main assigned MSBuildTask0010 in #13974.

Compatibility

The task-authoring analyzer is currently non-shipping/opt-in, and this diagnostic defaults to Info severity, so it does not break builds using warnings-as-errors. No ChangeWave is needed.

Performance

Measured with the BenchmarkDotNet harness from #14387 in a disposable integration checkout, using the default job on an Apple M4 Max with .NET 10.0.8 / SDK 10.0.300. MSBuildTask0009 is the closest existing symbol-action analyzer.

Diagnostics	MSBuildTask0009	MSBuildTask0011	0009 allocated	0011 allocated
1	358.4 us	357.9 us	352.02 KB	352.26 KB
10	563.5 us	588.8 us	693.73 KB	683.04 KB
100	2.731 ms	2.835 ms	3,570.28 KB	3,584.23 KB

The initial no-reference path measured 48.08 us / 32.41 KB. Short-circuiting after the mandatory IMultiThreadableTask lookup reduced that to 35.52 us / 30.56 KB, close to MSBuildTask0009 at 31.46 us / 30.56 KB. Diagnostic time, allocations, and 1/10/100 scaling are comparable, so no further complexity is justified.

Validation

• ./build.sh -v quiet
• dotnet test --project src/TaskAnalyzer.Tests/TaskAnalyzer.Tests.csproj -c Release
• Add task analyzer performance benchmarks #14387 BenchmarkDotNet no-op and exact-count 1/10/100 diagnostic scenarios

Fixes #14378

[Copilot]

Pull request overview

This pull request adds a new Roslyn analyzer rule to the MSBuild task-authoring analyzer package to encourage constructor-based TaskEnvironment injection for concrete IMultiThreadableTask implementations, aligning author guidance with the engine’s newer constructor-injection capability.

Changes:

• Introduces MSBuildTask0011 (Info) to flag concrete IMultiThreadableTask classes missing a public single-TaskEnvironment constructor.
• Adds analyzer unit tests covering valid/invalid/abstract/inherited-constructor scenarios.
• Updates analyzer documentation and the thread-safe tasks specification to document the recommended pattern and the new rule.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/TaskAnalyzer/TaskEnvironmentConstructorInjectionAnalyzer.cs	Implements the new MSBuildTask0011 analyzer logic.
src/TaskAnalyzer/README.md	Documents the new rule, rationale, and scope/severity guidance.
src/TaskAnalyzer/DiagnosticIds.cs	Adds the MSBuildTask0011 diagnostic ID constant.
src/TaskAnalyzer/DiagnosticDescriptors.cs	Adds the MSBuildTask0011 descriptor and registers it in the descriptor set.
src/TaskAnalyzer/AnalyzerReleases.Unshipped.md	Records the new analyzer rule in the unshipped release notes.
src/TaskAnalyzer.Tests/TaskEnvironmentConstructorInjectionAnalyzerTests.cs	Adds unit tests validating analyzer behavior across key cases.
documentation/specs/multithreading/thread-safe-tasks.md	Updates the spec to mention the analyzer recommendation and constructor-injection guidance.

[github-actions]

Review Summary — MSBuildTask0011 TaskEnvironmentConstructorInjectionAnalyzer

Overall: Looks good. Clean, well-scoped analyzer with solid test coverage.

Findings

Minor (non-blocking):

1. SupportedDiagnostics allocation (Performance): The => expression body allocates on every call. Use { get; } = pattern like PreferTypedParameterAnalyzer. Pre-existing inconsistency with TransitiveCallChainAnalyzer.

2. Potential false positive: If a concrete IMultiThreadableTask does NOT have a TaskEnvironment property at all (e.g. a task that delegates environment usage to another object), the diagnostic still fires. This seems acceptable for an Info-severity suggestion but could be confusing. Consider adding a test for this scenario.

3. No code fix: Unlike MSBuildTask0008, there is no code fix provider. Fine for initial PR but worth noting for follow-up.

Correctness ✅

• Analyzer logic is correct: checks concrete, non-abstract classes implementing IMultiThreadableTask, looks for public single-TaskEnvironment-parameter constructors on the declaring type only.
• The inherited-constructor test correctly validates that base class constructors do not satisfy the check (the engine needs a public ctor on the concrete type).
• Well-known type resolution with early-return is correct.
• Thread safety: EnableConcurrentExecution() is called, no shared mutable state.

Test Coverage ✅

• Positive case, negative cases (abstract, regular task, has ctor), edge cases (private ctor, multi-param ctor, inherited ctor, inherited interface).
• Missing: task without TaskEnvironment property at all (minor).

Documentation ✅

• README, spec, and AnalyzerReleases.Unshipped.md all updated consistently.

Breaking Changes: None ✅

• Info severity does not trigger WarnAsError. No ChangeWave needed for new analyzer diagnostics.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• awmgmcpg

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

Generated by Expert Code Review (on open) for #14401 · 64.1 AIC · ⌖ 5.16 AIC · ⊞ 4.8K

[baronfel]

Looks straightforward to me! I do agree with Copilot about caching the SupportedDiagnostics member though.

[baronfel]

@jankratochvilcz I think the one failing test is a flaky test not related to this change. Re-approved!