Enable MSBuildTask0001 task-authoring analyzer at error severity#55053
Open
baronfel wants to merge 2 commits into
Open
Enable MSBuildTask0001 task-authoring analyzer at error severity#55053baronfel wants to merge 2 commits into
baronfel wants to merge 2 commits into
Conversation
Re-enable the Microsoft.Build.TaskAuthoring.Analyzer MSBuildTask0001 rule (never-safe APIs such as Console.*, Environment.Exit/FailFast, Process.Kill, Directory.SetCurrentDirectory) at its natural 'error' severity, as the first incremental step out of the report-only 'suggestion' onboarding. Its fixes do not depend on the IMultiThreadableTask/TaskEnvironment migration that 0002-0005 require. Updated both the repo-root .editorconfig and src/StaticWebAssetsSdk/.editorconfig (root=true, keeps its own copy). Fixed the only banned-API call site in task code paths: ContainerBuilder.cs used Console.WriteLine on an error path; switched to logger.LogError. All 12 IsMSBuildTaskProject projects build green with the rule at error. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR incrementally ratchets up enforcement of the MSBuild task-authoring analyzer by promoting MSBuildTask0001 back to its default error severity, and fixes the only known task-path call site that violated the rule by removing a Console.WriteLine usage in container task code.
Changes:
- Re-enabled
dotnet_diagnostic.MSBuildTask0001.severity = errorin the repo-root.editorconfig. - Kept
src/StaticWebAssetsSdk/.editorconfig(which setsroot = true) in sync by re-enabling MSBuildTask0001 there as well. - Replaced a
Console.WriteLine(...)error-path call inContainerBuilderwithlogger.LogError(Resource.FormatString(...)).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/StaticWebAssetsSdk/.editorconfig |
Re-enables MSBuildTask0001 at error severity while keeping 0002–0005 at suggestion, with updated onboarding commentary. |
src/Containers/Microsoft.NET.Build.Containers/ContainerBuilder.cs |
Removes Console.WriteLine on an error path to comply with MSBuildTask0001, using existing logging/resource formatting patterns. |
.editorconfig |
Promotes MSBuildTask0001 to error severity globally while leaving 0002–0005 as suggestion, with expanded rationale/comments. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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
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
The
Microsoft.Build.TaskAuthoring.Analyzer(MSBuild thread-safe task analyzer) was recently referenced from allIsMSBuildTaskProjectprojects, but every rule (MSBuildTask0001–0005) was downgraded tosuggestionfor report-only onboarding. This PR takes one rule and fully re-enables it across the codebase as an example of the incremental ratcheting workflow.What changed
MSBuildTask0001at its naturalerrorseverity in both the repo-root.editorconfigandsrc/StaticWebAssetsSdk/.editorconfig(the latter setsroot = true, so it keeps its own in-sync copy). Rules 0002–0005 remain atsuggestion.ContainerBuilder.cscalledConsole.WriteLine(...)on an error path; replaced it withlogger.LogError(Resource.FormatString(...)), matching the surrounding error-handling pattern.Why MSBuildTask0001 first
MSBuildTask0001 flags APIs that are never safe in any MSBuild task (
Console.*,Environment.Exit/FailFast,Process.Kill,Directory.SetCurrentDirectory,ThreadPool.SetMin/MaxThreads,CultureInfo.DefaultThreadCurrent*). Crucially, its fixes (e.g.Console.*→Log.LogMessage) do not depend on theIMultiThreadableTask/TaskEnvironmentmigration that rules 0002/0003 require — so it can be enabled today without any task rewrites.Verification
Console.WriteLinein a real task class producederror MSBuildTask0001(then reverted).IsMSBuildTaskProjectprojects — every one succeeds with zero MSBuildTask0001 diagnostics.Follow-ups
The same one-rule-at-a-time pattern can be repeated for MSBuildTask0002–0005 as tasks migrate to
IMultiThreadableTask/TaskEnvironment.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com