Skip to content

Add CS8618 suppressor for required MSBuild task properties - #13926

Merged
JanProvaznik merged 2 commits into
mainfrom
aarnott/suppress-cs8618
Jun 4, 2026
Merged

Add CS8618 suppressor for required MSBuild task properties#13926
JanProvaznik merged 2 commits into
mainfrom
aarnott/suppress-cs8618

Conversation

@AArnott

@AArnott AArnott commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Context

MSBuild task authors can mark required inputs with [Required], and MSBuild guarantees those properties are populated before task execution. The C# compiler still reports CS8618 for these members, which forces task authors to add repetitive = null!; initializers that do not reflect how tasks are actually initialized.

This analyzer package has also grown beyond just thread-safe task checks, so the old ThreadSafeTaskAnalyzer project names were narrower than the current scope.

Changes Made

  • Added a Roslyn diagnostic suppressor that suppresses CS8618 for properties marked with Microsoft.Build.Framework.RequiredAttribute on types implementing Microsoft.Build.Framework.ITask.
  • Extended the Roslyn test helper infrastructure so tests can assert suppressed compiler diagnostics, and added focused coverage for direct task types, indirect task types, direct ITask implementations, explicit constructor cases, and negative cases like System.ComponentModel.DataAnnotations.RequiredAttribute.
  • Renamed the analyzer and test projects from ThreadSafeTaskAnalyzer / ThreadSafeTaskAnalyzer.Tests to TaskAnalyzer / TaskAnalyzer.Tests, and updated the solution, project references, docs, and CI comments to match.

Testing

  • dotnet test src\TaskAnalyzer.Tests\TaskAnalyzer.Tests.csproj -c Release -f net10.0
  • dotnet build src\TaskAnalyzer\TaskAnalyzer.csproj -c Release
  • Attempted ./build.cmd -v quiet, but it was blocked in this environment while downloading vswhere.

Notes

The suppressor is intentionally scoped to MSBuild's RequiredAttribute; it does not suppress CS8618 for other RequiredAttribute types.

Add a Roslyn suppressor for CS8618 on required MSBuild task properties, extend the analyzer tests to validate suppressed compiler diagnostics, and rename the analyzer projects from ThreadSafeTaskAnalyzer to TaskAnalyzer to match the package scope.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 2, 2026 19:50
@AArnott
AArnott requested a review from a team as a code owner June 2, 2026 19:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR evolves the MSBuild task authoring analyzer package by adding a Roslyn suppressor to remove noisy CS8618 warnings for [Microsoft.Build.Framework.Required] task properties (which MSBuild initializes), while also renaming the analyzer/test projects from ThreadSafeTaskAnalyzer* to the broader TaskAnalyzer* and updating references/docs accordingly.

Changes:

  • Added RequiredTaskPropertyInitializationSuppressor to suppress CS8618 on [Required] properties for ITask implementations.
  • Expanded analyzer infrastructure (shared helpers + transitive call-chain analyzer) and extended test helpers to assert suppressed compiler diagnostics.
  • Renamed analyzer projects and updated solution/CI/docs to reference TaskAnalyzer.

Reviewed changes

Copilot reviewed 11 out of 24 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/Tasks/Microsoft.Build.Tasks.csproj Switches analyzer ProjectReference to TaskAnalyzer.
src/TaskAnalyzer/WellKnownTypeNames.cs Adds metadata name constant for MSBuild RequiredAttribute.
src/TaskAnalyzer/TransitiveCallChainAnalyzer.cs Adds compilation-wide call graph analyzer for transitive unsafe API usage (MSBuildTask0005).
src/TaskAnalyzer/TaskAnalyzer.csproj Updates InternalsVisibleTo to renamed test assembly.
src/TaskAnalyzer/SharedAnalyzerHelpers.cs Adds shared helper utilities for banned API and path-safety analysis.
src/TaskAnalyzer/RequiredTaskPropertyInitializationSuppressor.cs Introduces CS8618 suppressor for [Required] task properties.
src/TaskAnalyzer/README.md Documents suppressor behavior and updates paths/names for renamed projects.
src/TaskAnalyzer/MultiThreadableTaskCodeFixProvider.cs Adds/updates code fixes for MSBuildTask0002/0003 diagnostics.
src/TaskAnalyzer/MultiThreadableTaskAnalyzer.cs Adds/updates main analyzer logic and scope option handling.
src/TaskAnalyzer/DiagnosticIds.cs Defines MSBuildTask0001–0005 IDs.
src/TaskAnalyzer/DiagnosticDescriptors.cs Defines descriptors/severities, including compilation-end rule for transitive diagnostics.
src/TaskAnalyzer/BannedApiDefinitions.cs Declares banned API list + categories for analyzer lookups.
src/TaskAnalyzer/AnalyzerReleases.Unshipped.md Adds unshipped analyzer rule list (0001–0005).
src/TaskAnalyzer/AnalyzerReleases.Shipped.md Initializes shipped file (none shipped yet).
src/TaskAnalyzer.Tests/WriteAllTextDetailedTest.cs Adds a focused diagnostic-count regression test.
src/TaskAnalyzer.Tests/TransitiveCallChainAnalyzerTests.cs Adds tests for transitive call-chain diagnostics and message formatting.
src/TaskAnalyzer.Tests/TestHelpers.cs Extends test harness to return suppressed diagnostics and improves framework stubs.
src/TaskAnalyzer.Tests/TaskAnalyzer.Tests.csproj Updates project reference to renamed analyzer project.
src/TaskAnalyzer.Tests/RequiredTaskPropertyInitializationSuppressorTests.cs Adds test coverage for CS8618 suppression scenarios and negative cases.
src/TaskAnalyzer.Tests/MultiThreadableTaskCodeFixProviderTests.cs Adds code fix tests for TaskEnvironment/path fixes.
src/TaskAnalyzer.Tests/MultiThreadableTaskAnalyzerTests.cs Adds extensive analyzer behavior coverage including scope option cases.
MSBuild.slnx Updates solution entries to TaskAnalyzer projects.
eng/dependabot/Directory.Packages.props Updates comment to reflect renamed analyzer project usage.
.vsts-dotnet-ci.yml Updates CI comment to reference TaskAnalyzer.

Comment thread src/TaskAnalyzer/RequiredTaskPropertyInitializationSuppressor.cs
Comment thread src/TaskAnalyzer/RequiredTaskPropertyInitializationSuppressor.cs
Remove the unused suppressor using directive and narrow CS8618 suppression to required task properties that MSBuild can actually assign, with test coverage for get-only properties.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@rainersigwald rainersigwald left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very nice, thanks!

@ViktorHofer

Copy link
Copy Markdown
Member

@AArnott note that the current task analyzer package is non-shipping, it doesn't go to nuget.org is currently only meant for internal consumption. We want to eventually ship it but it's not yet clear when and in which form (separate package vs analyzer moved into Microsoft.Build.Framework package).

@AArnott

AArnott commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

@ViktorHofer That is indeed relevant. I guess the question for now is, do you want to take this PR as-is toward that final outcome, or do you want changes made first? I'm OK if this doesn't ship immediately.

@baronfel

baronfel commented Jun 3, 2026

Copy link
Copy Markdown
Member

I'm super happy to take this and dogfood it on the MSBuild repo - I would like us to decide when the ship bar for the analyzers package is soon though. cc @jankratochvilcz

@rainersigwald

Copy link
Copy Markdown
Member

Agreed: this is the type of thing I want in the analyzer whenever it ships and there's no reason to not dogfood it here (and in SDK) ASAP.

@ViktorHofer

Copy link
Copy Markdown
Member

+1

@JanProvaznik JanProvaznik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the functionality addition is welcome, thanks. We'll need to think about the final naming before release I don't like "TaskAnalyzer", "MSBuildAnalyzer" would be more appropriate I think, but I'll not bikeshed that here, but when we actually want to release it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants