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

Documentation/Diagnostics/PH2159.md

@@ -0,0 +1,64 @@
+# PH2159: Avoid Retry attribute
+
+| Property | Value  |
+|--|--|
+| Package | [Philips.CodeAnalysis.MsTestAnalyzers](https://www.nuget.org/packages/Philips.CodeAnalysis.MsTestAnalyzers) |
+| Diagnostic ID | PH2159 |
+| Category  | [MsTest](../MsTest.md) |
+| Analyzer | [AvoidAttributeAnalyzer](https://github.com/philips-software/roslyn-analyzers/blob/main/Philips.CodeAnalysis.MsTestAnalyzers/AvoidAttributeAnalyzer.cs)
+| CodeFix  | No |
+| Severity | Error |
+| Enabled By Default | Yes |
+
+## Introduction
+
+MSTest has a [Retry] attribute to attempt to overcome flaky tests. It would be preferable to avoid the flaky test.
+
+More thoroughly, one should avoid any attribute that derives from RetryBaseAttribute.
+
+## How to solve
+
+Remove the flaky test or fix the underlying cause of the flakiness. Rely on making your tests deterministic rather than retrying them.
+
+## Example
+
+Code that triggers a diagnostic:
+``` cs
+        [TestMethod]
+        [Retry(3)]
+        public void BadTestMethod()
+        {
+            Assert.AreEqual(1, 1);
+        }
+```
+
+Code that also triggers a diagnostic (custom attribute deriving from RetryBaseAttribute):
+``` cs
+        public class CustomRetryAttribute : RetryBaseAttribute
+        {
+            public CustomRetryAttribute(int count) : base(count) { }
+        }
+
+        [TestMethod]
+        [CustomRetry(5)]
+        public void BadTestMethod()
+        {
+            Assert.AreEqual(1, 1);
+        }
+```
+
+## Similar Analyzers
+
+The following analyzers detect other situations that prevent tests from executing (either unwittingly or nefariously):
+
+| ID | Title  |
+|--|--|
+| [PH2013](.\PH2013.md) | Avoid Ignore attribute |
+| [PH2034](.\PH2034.md) | Test methods must be in TestClass |
+| [PH2036](.\PH2036.md) | Test methods must be public |
+| [PH2038](.\PH2038.md) | Test classes must be public |
+| [PH2059](.\PH2059.md) | Public methods must be TestMethods |
+
+## Configuration
+
+This analyzer does not offer any special configuration. The general ways of [suppressing](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/suppress-warnings) diagnostics apply.

Philips.CodeAnalysis.Common/DiagnosticId.cs

@@ -144,5 +144,6 @@ public enum DiagnosticId
 		AvoidUnusedToString = 2153,
 		AvoidUnlicensedPackages = 2155,
 		AvoidPkcsPaddingWithRsaEncryption = 2158,
+		AvoidRetryAttribute = 2159,
 	}
 }

Philips.CodeAnalysis.MsTestAnalyzers/AvoidAttributeAnalyzer.cs

@@ -19,6 +19,15 @@ public class AvoidAttributeAnalyzer : DiagnosticAnalyzerBase
 
 		private static readonly ImmutableDictionary<string, ImmutableArray<AttributeModel>> Attributes = GetAttributeModels();
 
+		private static readonly DiagnosticDescriptor RetryBaseAttributeRule = new(
+			"PH2159",
+			"Retry attribute not allowed",
+			"Tests may not use attributes that derive from RetryBaseAttribute.",
+			Categories.Maintainability,
+			DiagnosticSeverity.Error,
+			isEnabledByDefault: true,
+			description: "Attributes that derive from RetryBaseAttribute attempt to overcome flaky tests. It would be preferable to avoid the flaky test.");
+
 		public static readonly ImmutableArray<DiagnosticDescriptor> Rules = GetRules(Attributes);
 
 		public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => Rules;
@@ -92,6 +101,49 @@ private void Analyze(ImmutableArray<AttributeModel> attributes, SyntaxNodeAnalys
 				var diagnostic = Diagnostic.Create(attribute.Rule, descriptionLocation, id);
 				context.ReportDiagnostic(diagnostic);
 			}
+
+			// Additional check for attributes that derive from RetryBaseAttribute
+			CheckForRetryBaseAttributeDerivatives(attributesNode, context);
+		}
+
+		private void CheckForRetryBaseAttributeDerivatives(AttributeListSyntax attributesNode, SyntaxNodeAnalysisContext context)
+		{
+			foreach (AttributeSyntax attributeSyntax in attributesNode.Attributes)
+			{
+				SymbolInfo symbolInfo = context.SemanticModel.GetSymbolInfo(attributeSyntax);
+				if (symbolInfo.Symbol is IMethodSymbol constructorSymbol)
+				{
+					INamedTypeSymbol attributeClass = constructorSymbol.ContainingType;
+
+					// Skip if this is the direct Retry attribute (already handled by the main check)
+					if (attributeClass.Name == "RetryAttribute" &&
+						attributeClass.ContainingNamespace.ToDisplayString() == "Microsoft.VisualStudio.TestTools.UnitTesting")
+					{
+						continue;
+					}
+
+					if (DerivesFromRetryBaseAttribute(attributeClass))
+					{
+						var diagnostic = Diagnostic.Create(RetryBaseAttributeRule, attributeSyntax.GetLocation());
+						context.ReportDiagnostic(diagnostic);
+					}
+				}
+			}
+		}
+
+		private bool DerivesFromRetryBaseAttribute(INamedTypeSymbol attributeType)
+		{
+			INamedTypeSymbol baseType = attributeType.BaseType;
+			while (baseType != null)
+			{
+				if (baseType.Name == "RetryBaseAttribute" &&
+					baseType.ContainingNamespace.ToDisplayString() == "Microsoft.VisualStudio.TestTools.UnitTesting")
+				{
+					return true;
+				}
+				baseType = baseType.BaseType;
+			}
+			return false;
 		}
 
 		private bool IsWhitelisted(ImmutableHashSet<string> whitelist, SemanticModel semanticModel, SyntaxNode node, out string id)
@@ -117,6 +169,9 @@ private static ImmutableArray<DiagnosticDescriptor> GetRules(ImmutableDictionary
 									.Select(x => x.Rule);
 			builder.AddRange(items);
 
+			// Add the RetryBaseAttribute rule
+			builder.Add(RetryBaseAttributeRule);
+
 			return builder.ToImmutable();
 		}
 
@@ -172,9 +227,18 @@ private static ImmutableDictionary<string, ImmutableArray<AttributeModel>> GetAt
 				isSuppressible: true,
 				isEnabledByDefault: true);
 
+			var retryAttribute = new AttributeModel(@"Retry",
+				@"Microsoft.VisualStudio.TestTools.UnitTesting.RetryAttribute",
+				@"Retry attribute not allowed",
+				@"Tests may not use the Retry attribute.",
+				@"The Retry attribute attempts to overcome flaky tests. It would be preferable to avoid the flaky test.",
+				DiagnosticId.AvoidRetryAttribute,
+				isSuppressible: false,
+				isEnabledByDefault: true);
+
 			ImmutableDictionary<string, ImmutableArray<AttributeModel>>.Builder builder = ImmutableDictionary.CreateBuilder<string, ImmutableArray<AttributeModel>>();
 
-			builder[StringConstants.AssertFullyQualifiedName] = ImmutableArray.Create(ownerAttribute, removedAttribute, testInitializeAttribute, testCleanupAttribute, classCleanupAttribute, classInitializeAttribute);
+			builder[StringConstants.AssertFullyQualifiedName] = ImmutableArray.Create(ownerAttribute, removedAttribute, testInitializeAttribute, testCleanupAttribute, classCleanupAttribute, classInitializeAttribute, retryAttribute);
 
 			return builder.ToImmutable();
 		}

Philips.CodeAnalysis.Test/MsTest/AvoidRetryAttributeAnalyzerTest.cs

@@ -0,0 +1,132 @@
+// © 2025 Koninklijke Philips N.V. See License.md in the project root for license information.
+
+using System.Text.RegularExpressions;
+using System.Threading.Tasks;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using Philips.CodeAnalysis.Common;
+using Philips.CodeAnalysis.MsTestAnalyzers;
+using Philips.CodeAnalysis.Test.Helpers;
+using Philips.CodeAnalysis.Test.Verifiers;
+
+namespace Philips.CodeAnalysis.Test.MsTest
+{
+	[TestClass]
+	public class AvoidRetryAttributeAnalyzerTest : DiagnosticVerifier
+	{
+		[TestMethod]
+		[TestCategory(TestDefinitions.UnitTests)]
+		public async Task AvoidRetryAttributeTestAsync()
+		{
+			var givenText = @"
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+namespace Microsoft.VisualStudio.TestTools.UnitTesting
+{
+    public class RetryAttribute : System.Attribute
+    {
+        public RetryAttribute(int count) { }
+    }
+}
+
+[TestClass]
+class Foo 
+{
+  [TestMethod]
+  [Retry(3)]
+  public void TestMethod()
+  {
+    Assert.IsTrue(true);
+  }
+}
+";
+
+			DiagnosticResult expected = new()
+			{
+				Id = DiagnosticId.AvoidRetryAttribute.ToId(),
+				Message = new Regex(".*"),
+				Severity = DiagnosticSeverity.Error,
+				Locations = new[]
+				{
+					new DiagnosticResultLocation("Test0.cs", 16, 4)
+				}
+			};
+
+			await VerifyDiagnostic(givenText, expected).ConfigureAwait(false);
+		}
+
+		[TestMethod]
+		[TestCategory(TestDefinitions.UnitTests)]
+		public async Task AvoidCustomRetryAttributeTestAsync()
+		{
+			var givenText = @"
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+namespace Microsoft.VisualStudio.TestTools.UnitTesting
+{
+    public class RetryBaseAttribute : System.Attribute
+    {
+        public RetryBaseAttribute(int count) { }
+    }
+}
+
+public class CustomRetryAttribute : Microsoft.VisualStudio.TestTools.UnitTesting.RetryBaseAttribute
+{
+    public CustomRetryAttribute(int count) : base(count) { }
+}
+
+[TestClass]
+class Foo 
+{
+  [TestMethod]
+  [CustomRetry(5)]
+  public void TestMethod()
+  {
+    Assert.IsTrue(true);
+  }
+}
+";
+
+			DiagnosticResult expected = new()
+			{
+				Id = DiagnosticId.AvoidRetryAttribute.ToId(),
+				Message = new Regex(".*"),
+				Severity = DiagnosticSeverity.Error,
+				Locations = new[]
+				{
+					new DiagnosticResultLocation("Test0.cs", 21, 4)
+				}
+			};
+
+			await VerifyDiagnostic(givenText, expected).ConfigureAwait(false);
+		}
+
+		[TestMethod]
+		[TestCategory(TestDefinitions.UnitTests)]
+		public async Task NonRetryAttributesAreIgnoredAsync()
+		{
+			var givenText = @"
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+[TestClass]
+class Foo 
+{
+  [TestMethod]
+  [TestCategory(""Unit"")]
+  public void TestMethod()
+  {
+    Assert.IsTrue(true);
+  }
+}
+";
+
+			await VerifySuccessfulCompilation(givenText).ConfigureAwait(false);
+		}
+
+		protected override DiagnosticAnalyzer GetDiagnosticAnalyzer()
+		{
+			return new AvoidAttributeAnalyzer();
+		}
+	}
+}