Skip to content

feat: Add PH2161 analyzer to avoid [Retry] attribute and RetryBaseAttribute derivatives#977

Draft
Copilot wants to merge 4 commits intomainfrom
copilot/fix-976
Draft

feat: Add PH2161 analyzer to avoid [Retry] attribute and RetryBaseAttribute derivatives#977
Copilot wants to merge 4 commits intomainfrom
copilot/fix-976

Conversation

Copy link
Contributor

Copilot AI commented Sep 6, 2025

This PR implements a new analyzer PH2161 that detects and prevents the use of MSTest's [Retry] attribute and any custom attributes that derive from RetryBaseAttribute.

Problem

MSTest provides a [Retry] attribute to automatically retry flaky tests, but this approach masks underlying issues rather than fixing them. As stated in the issue, "it would be preferable to avoid the flaky test" rather than attempting to overcome it with retries.

Solution

The new analyzer detects two scenarios:

  1. Direct usage of [Retry] attribute:
[TestMethod]
[Retry(3)]  // ❌ PH2161: Retry attribute not allowed
public void TestMethod() { }
  1. Custom attributes deriving from RetryBaseAttribute:
public class CustomRetryAttribute : RetryBaseAttribute
{
    public CustomRetryAttribute(int count) : base(count) { }
}

[TestMethod]
[CustomRetry(5)]  // ❌ PH2161: Tests may not use attributes that derive from RetryBaseAttribute
public void TestMethod() { }

Implementation Details

  • Diagnostic ID: PH2161
  • Severity: Error
  • Enabled by default: Yes
  • Category: MsTest

The implementation extends the existing AvoidAttributeAnalyzer with:

  • Direct attribute name checking for "Retry"
  • Semantic model analysis to detect inheritance from RetryBaseAttribute
  • Logic to avoid double-reporting when the same attribute matches both checks

Testing

Added comprehensive test coverage including:

  • Direct [Retry] attribute usage detection
  • Custom attributes deriving from RetryBaseAttribute detection
  • Verification that non-retry attributes are ignored
  • All existing tests continue to pass (2013 total)

Documentation

Created complete documentation at Documentation/Diagnostics/PH2161.md following the established pattern, including examples and guidance on how to resolve violations.

Fixes #976.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: bcollamore <57269455+bcollamore@users.noreply.github.com>
Copilot AI changed the title [WIP] Avoid [Retry] attribute feat: Add PH2159 analyzer to avoid [Retry] attribute and RetryBaseAttribute derivatives Sep 6, 2025
AvoidUnusedToString = 2153,
AvoidUnlicensedPackages = 2155,
AvoidPkcsPaddingWithRsaEncryption = 2158,
AvoidRetryAttribute = 2159,

Choose a reason for hiding this comment

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

Let's switch to 2161. (Another PR has claimed 2159 and 2160). We'll need to update our PH2159.md file name and contents as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to use PH2161 as requested. Changed the diagnostic ID in DiagnosticId.cs and AvoidAttributeAnalyzer.cs, renamed the documentation file from PH2159.md to PH2161.md, and updated all references. All 2013 tests pass. (commit 5d8c238)

@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 7, 2025

@bcollamore
Copy link
Member

Copilot see comments above

Co-authored-by: bcollamore <57269455+bcollamore@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Sep 7, 2025

Copilot see comments above

Addressed the diagnostic ID change from PH2159 to PH2161 as requested in the comment above. All changes have been made and validated. (commit 5d8c238)

Copilot AI changed the title feat: Add PH2159 analyzer to avoid [Retry] attribute and RetryBaseAttribute derivatives feat: Add PH2161 analyzer to avoid [Retry] attribute and RetryBaseAttribute derivatives Sep 7, 2025
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.

Avoid [Retry] attribute

2 participants