Skip to content

Add DiagnosticId to warning-level [Obsolete] attributes#8194

Open
Evangelink wants to merge 2 commits into
mainfrom
dev/amauryleve/obsolete-diagnostic-ids
Open

Add DiagnosticId to warning-level [Obsolete] attributes#8194
Evangelink wants to merge 2 commits into
mainfrom
dev/amauryleve/obsolete-diagnostic-ids

Conversation

@Evangelink
Copy link
Copy Markdown
Member

Summary

Add specific DiagnosticId (and UrlFormat) to all warning-level [Obsolete] attributes that previously relied on the default CS0618 warning. This enables downstream users to selectively suppress specific deprecation warnings using #pragma warning disable MSTEST0XXX or project-level <NoWarn>, following the pattern established by SYSLIB0051.

Diagnostic ID scheme

ID API Reason
MSTEST0100 Assert.Equals Do not use for assertions
MSTEST0101 Assert.ReferenceEquals Do not use for assertions
MSTEST0102 StringAssert.Equals Do not use for assertions
MSTEST0103 StringAssert.ReferenceEquals Do not use for assertions
MSTEST0104 CollectionAssert.Equals Do not use for assertions
MSTEST0105 CollectionAssert.ReferenceEquals Do not use for assertions
MSTEST0106 MSTestExecutor.RunTests Use RunTestsAsync instead
MTP0001 CancelledTestNodeStateProperty Use OperationCanceledException instead
MTP0002 TestApplication.CreateServerModeBuilderAsync Use CreateBuilderAsync instead

Notes

  • DiagnosticId and UrlFormat properties on ObsoleteAttribute require .NET 5+, so they are guarded with #if NET8_0_OR_GREATER (the minimum .NET Core TFM in this repo). On netstandard2.0 and net462, the attributes remain unchanged.
  • The MSTEST0100+ range is deliberately separated from the analyzer rule IDs (MSTEST0001MSTEST0063) to avoid confusion.
  • The MSTestExecutor.RunTests overloads share MSTEST0106 since they are deprecated for the same reason.
  • The MTP0001/MTP0002 prefix distinguishes Microsoft.Testing.Platform deprecations from MSTest deprecations.

Add specific diagnostic identifiers to all warning-level [Obsolete] attributes
that previously relied on the default CS0618 warning. This enables downstream
users to selectively suppress specific deprecation warnings using
#pragma warning disable or project-level <NoWarn>.

Diagnostic ID scheme:
- MSTEST0100-0105: Assert/StringAssert/CollectionAssert Equals/ReferenceEquals
- MSTEST0106: MSTestExecutor.RunTests (use RunTestsAsync)
- MTP0001: CancelledTestNodeStateProperty
- MTP0002: TestApplication.CreateServerModeBuilderAsync

The DiagnosticId and UrlFormat properties are guarded with
#if NET8_0_OR_GREATER since they are not available on netstandard2.0/net462.
Copilot AI review requested due to automatic review settings May 13, 2026 17:06
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 adds custom diagnostic IDs and help URLs to obsolete MSTest and Microsoft.Testing.Platform APIs where supported by newer target frameworks, enabling more targeted downstream suppression and diagnostics.

Changes:

  • Adds DiagnosticId/UrlFormat to warning-level obsolete MSTest assertion APIs and adapter RunTests overloads under NET8_0_OR_GREATER.
  • Adds MTP-specific obsolete diagnostic IDs for deprecated platform APIs.
  • Preserves existing obsolete attributes for older target frameworks.
Show a summary per file
File Description
src/TestFramework/TestFramework/Assertions/StringAssert.cs Adds MSTEST obsolete diagnostic IDs for Equals and ReferenceEquals.
src/TestFramework/TestFramework/Assertions/CollectionAssert.cs Adds MSTEST obsolete diagnostic IDs for collection assertion blocked APIs.
src/TestFramework/TestFramework/Assertions/Assert.cs Adds MSTEST obsolete diagnostic IDs for base assertion blocked APIs.
src/Platform/Microsoft.Testing.Platform/Messages/TestNodeStateProperties.cs Adds MTP obsolete diagnostic metadata for cancelled test node state property.
src/Platform/Microsoft.Testing.Platform/Builder/TestApplication.cs Adds MTP obsolete diagnostic metadata for server mode builder creation API.
src/Adapter/MSTest.TestAdapter/VSTestAdapter/MSTestExecutor.cs Adds MSTEST obsolete diagnostic metadata for sync RunTests adapter overloads.

Copilot's findings

  • Files reviewed: 6/6 changed files
  • Comments generated: 0

@Evangelink
Copy link
Copy Markdown
Member Author

Review: Add DiagnosticId to warning-level [Obsolete] attributes

🟡 MAJOR — Source-compatibility break for #pragma warning disable CS0619 users

Confirmed finding (validated against actual PR branch files).

For the five error: true methods (Assert.Equals, Assert.ReferenceEquals, StringAssert.Equals, CollectionAssert.Equals, CollectionAssert.ReferenceEquals), the pre-PR compiler diagnostic was CS0619. After this PR, on NET8_0_OR_GREATER non-DEBUG builds, the diagnostic becomes MSTEST0100MSTEST0105.

Concrete breaking scenario: A user on .NET 8 with:

#pragma warning disable CS0619
Assert.Equals(a, b);   // was suppressed via CS0619
#pragma warning restore CS0619

After upgrading to the new package, the compiler emits MSTEST0100 instead. The CS0619 pragma no longer suppresses it, so the build either fails (if -WarnAsError is set) or silently emits a new warning/error the user didn't opt into.

Recommendation: Explicitly call this out as a source-breaking change in the release notes / changelog. Ideally also add a note in the XML doc or a migration guide pointing users from CS0619/CS0618MSTEST0100 etc. The change itself is intentional and correct — the user simply needs to be informed.


🔵 NIT — UrlFormat convention inconsistency between MSTEST and MTP

  • MSTEST IDs hardcode the full URL per attribute: `UrlFormat = "(aka.ms/redacted)
  • MTP IDs use the SYSLIB {0} placeholder: `UrlFormat = "(aka.ms/redacted)

Both resolve correctly, but using {0} (as in MTP and the SYSLIB pattern) is less repetitive and less error-prone. Consider unifying to `UrlFormat = "(aka.ms/redacted) for MSTEST IDs for consistency.


All other dimensions reviewed: Cross-TFM guard logic (NET8_0_OR_GREATER) is correct for this repo's minimum .NET Core TFM. No ID conflicts with existing analyzer rules (MSTEST0001–0063). No PublicAPI.Unshipped.txt changes required. CollectionAssert diff annotations were misleading — the actual file is correct. Test coverage exception applies (purely compiler-metadata change).

Generated by Expert Code Review (on open) for issue #8194 · ● 12.4M ·

Copy link
Copy Markdown
Member Author

@Evangelink Evangelink left a comment

Choose a reason for hiding this comment

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

Code review complete. One MAJOR source-compatibility concern (CS0619 suppressions breaking on NET8_0_OR_GREATER builds) and one NIT (UrlFormat convention inconsistency). No blocking issues. All other dimensions LGTM.

Generated by Expert Code Review (on open) for issue #8194 · ● 12.4M

Update all #pragma warning disable/restore CS0618 at CancelledTestNodeStateProperty
call sites to also suppress MTP0001 (the new diagnostic ID on NET8_0_OR_GREATER).

Also unify UrlFormat to use {0} placeholder across all MSTEST Obsolete attributes,
matching the MTP convention and the SYSLIB pattern.
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.

2 participants