Add analyzer for TaskEnvironment constructor injection#14400
Closed
jankratochvilcz wants to merge 2 commits into
Closed
Conversation
jankratochvilcz
temporarily deployed
to
copilot-pat-pool
July 16, 2026 14:49 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
jankratochvilcz
temporarily deployed
to
copilot-pat-pool
July 16, 2026 14:50 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
jankratochvilcz
temporarily deployed
to
copilot-pat-pool
July 16, 2026 14:50 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
jankratochvilcz
temporarily deployed
to
copilot-pat-pool
July 16, 2026 14:55 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
jankratochvilcz
temporarily deployed
to
copilot-pat-pool
July 16, 2026 14:56 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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.allowedlist in your workflow frontmatter:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
Generated by Expert Code Review (on open) for #14400 · 52.2 AIC · ⌖ 7.39 AIC · ⊞ 4.8K
|
|
||
| /// <summary>Transitive unsafe API usage detected in task call chain.</summary> | ||
| public const string TransitiveUnsafeCall = "MSBuildTask0005"; | ||
|
|
Contributor
There was a problem hiding this comment.
[MINOR — Diagnostic ID Gap] IDs jump from MSBuildTask0005 to MSBuildTask0010. Is this intentional to reserve 0006-0009 for future warnings? If so, a brief comment here would help future contributors understand the numbering scheme (e.g., // 0006-0009 reserved for future warning-severity rules).
| public sealed class TaskEnvironmentConstructorInjectionAnalyzer : DiagnosticAnalyzer | ||
| { | ||
| public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => | ||
| ImmutableArray.Create(DiagnosticDescriptors.PreferTaskEnvironmentConstructorInjection); |
Contributor
There was a problem hiding this comment.
[MINOR - Performance] SupportedDiagnostics calls ImmutableArray.Create(...) on every property access. Cache in a private static readonly field to avoid repeated allocations.
jankratochvilcz
requested a deployment
to
copilot-pat-pool
July 16, 2026 14:57 — with
GitHub Actions
Abandoned
GitHub Actions
Abandoned
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 175855cd-9522-42fd-ad8a-928f21c9e4f5
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 175855cd-9522-42fd-ad8a-928f21c9e4f5
jankratochvilcz
force-pushed
the
jankratochvilcz-taskenvironment-injection-analyzer
branch
from
July 16, 2026 16:29
995fdac to
57ae245
Compare
baronfel
pushed a commit
that referenced
this pull request
Jul 20, 2026
## 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` - #14387 BenchmarkDotNet no-op and exact-count 1/10/100 diagnostic scenarios Fixes #14378
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MSBuildTask0010at Info severity for concreteIMultiThreadableTaskimplementations that do not expose a public single-TaskEnvironmentconstructorThis is a fast-follow stacked on #14315. The rule uses ID 0010 because IDs 0006-0009 are already assigned on
main, although this parent branch predates those rules.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.
MSBuildTask0009is the closest existing symbol-action analyzer.The initial no-reference path measured 48.08 us / 32.41 KB. Short-circuiting after the mandatory
IMultiThreadableTasklookup reduced that to 35.52 us / 30.56 KB, close toMSBuildTask0009at 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 quietdotnet test --project src/TaskAnalyzer.Tests/TaskAnalyzer.Tests.csproj -c Releasesrc/Samples/Dependency/Dependency.csprojMSBuild.dll --helpFixes #14378