Respect custom ExpectedExceptionBase attribute implementations

microsoft · Nov 12, 2024 · dface23 · dface23
1 parent be23aac
commit dface23
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 8 deletions.
diff --git a/src/Analyzers/MSTest.Analyzers/AvoidExpectedExceptionAttributeAnalyzer.cs b/src/Analyzers/MSTest.Analyzers/AvoidExpectedExceptionAttributeAnalyzer.cs
@@ -41,20 +41,22 @@ public override void Initialize(AnalysisContext context)
 
         context.RegisterCompilationStartAction(context =>
         {
-            if (context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingExpectedExceptionAttribute, out INamedTypeSymbol? expectedExceptionAttributeSymbol))
+            if (context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingExpectedExceptionBaseAttribute, out INamedTypeSymbol? expectedExceptionBaseAttributeSymbol) &&
+                context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingExpectedExceptionAttribute, out INamedTypeSymbol? expectedExceptionAttributeSymbol))
             {
-                context.RegisterSymbolAction(context => AnalyzeSymbol(context, expectedExceptionAttributeSymbol), SymbolKind.Method);
+                context.RegisterSymbolAction(context => AnalyzeSymbol(context, expectedExceptionBaseAttributeSymbol), SymbolKind.Method);
             }
         });
     }
 
-    private static void AnalyzeSymbol(SymbolAnalysisContext context, INamedTypeSymbol expectedExceptionAttributeSymbol)
+    private static void AnalyzeSymbol(SymbolAnalysisContext context, INamedTypeSymbol expectedExceptionBaseAttributeSymbol)
     {
         var methodSymbol = (IMethodSymbol)context.Symbol;
         if (methodSymbol.GetAttributes().FirstOrDefault(
-            attr => SymbolEqualityComparer.Default.Equals(attr.AttributeClass, expectedExceptionAttributeSymbol)) is { } expectedExceptionAttribute)
+            attr => attr.AttributeClass.Inherits(expectedExceptionBaseAttributeSymbol)) is { } expectedExceptionBaseAttribute)
         {
-            bool allowsDerivedTypes = expectedExceptionAttribute.NamedArguments.FirstOrDefault(n => n.Key == "AllowDerivedTypes").Value.Value is true;
+            bool allowsDerivedTypes = expectedExceptionBaseAttribute.NamedArguments.FirstOrDefault(n => n.Key == "AllowDerivedTypes").Value.Value is true;
+
             // Assert.ThrowsException checks the exact Exception type. So, we cannot offer a fix to ThrowsException if the user sets AllowDerivedTypes to true.
             context.ReportDiagnostic(
                 allowsDerivedTypes

diff --git a/src/Analyzers/MSTest.Analyzers/Helpers/WellKnownTypeNames.cs b/src/Analyzers/MSTest.Analyzers/Helpers/WellKnownTypeNames.cs
@@ -24,6 +24,7 @@ internal static class WellKnownTypeNames
     public const string MicrosoftVisualStudioTestToolsUnitTestingDynamicDataAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.DynamicDataAttribute";
     public const string MicrosoftVisualStudioTestToolsUnitTestingDynamicDataSourceType = "Microsoft.VisualStudio.TestTools.UnitTesting.DynamicDataSourceType";
     public const string MicrosoftVisualStudioTestToolsUnitTestingExpectedExceptionAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.ExpectedExceptionAttribute";
+    public const string MicrosoftVisualStudioTestToolsUnitTestingExpectedExceptionBaseAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.ExpectedExceptionBaseAttribute";
     public const string MicrosoftVisualStudioTestToolsUnitTestingIgnoreAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.IgnoreAttribute";
     public const string MicrosoftVisualStudioTestToolsUnitTestingInheritanceBehavior = "Microsoft.VisualStudio.TestTools.UnitTesting.InheritanceBehavior";
     public const string MicrosoftVisualStudioTestToolsUnitTestingOwnerAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.OwnerAttribute";

diff --git a/src/Analyzers/MSTest.Analyzers/UseAttributeOnTestMethodAnalyzer.cs b/src/Analyzers/MSTest.Analyzers/UseAttributeOnTestMethodAnalyzer.cs
@@ -122,7 +122,7 @@ public sealed class UseAttributeOnTestMethodAnalyzer : DiagnosticAnalyzer
         (WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingTestPropertyAttribute, TestPropertyRule),
         (WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingWorkItemAttribute, WorkItemRule),
         (WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingDescriptionAttribute, DescriptionRule),
-        (WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingExpectedExceptionAttribute, ExpectedExceptionRule),
+        (WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingExpectedExceptionBaseAttribute, ExpectedExceptionRule),
         (WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingCssIterationAttribute, CssIterationRule),
         (WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingCssProjectStructureAttribute, CssProjectStructureRule)
     ];
@@ -188,7 +188,7 @@ private static void AnalyzeSymbol(
             // Get all test attributes decorating the current method.
             foreach ((INamedTypeSymbol attributeSymbol, DiagnosticDescriptor rule) in attributeRuleTuples)
             {
-                if (SymbolEqualityComparer.Default.Equals(methodAttribute.AttributeClass, attributeSymbol))
+                if (methodAttribute.AttributeClass.Inherits(attributeSymbol))
                 {
                     attributes.Add((methodAttribute, rule));
                 }

diff --git a/test/UnitTests/MSTest.Analyzers.UnitTests/AvoidExpectedExceptionAttributeAnalyzerTests.cs b/test/UnitTests/MSTest.Analyzers.UnitTests/AvoidExpectedExceptionAttributeAnalyzerTests.cs
@@ -17,6 +17,11 @@ public async Task WhenUsed_Diagnostic()
         string code = """
             using Microsoft.VisualStudio.TestTools.UnitTesting;
 
+            public class MyExpectedExceptionAttribute : ExpectedExceptionBaseAttribute
+            {
+                protected override void Verify(System.Exception exception) { }
+            }
+
             [TestClass]
             public class TestClass
             {
@@ -43,12 +48,23 @@ public class TestClass
                 public void [|TestMethod4|]()
                 {
                 }
+
+                [MyExpectedException]
+                [TestMethod]
+                public void [|TestMethod5|]()
+                {
+                }
             }
             """;
 
         string fixedCode = """
             using Microsoft.VisualStudio.TestTools.UnitTesting;
 
+            public class MyExpectedExceptionAttribute : ExpectedExceptionBaseAttribute
+            {
+                protected override void Verify(System.Exception exception) { }
+            }
+
             [TestClass]
             public class TestClass
             {
@@ -73,6 +89,12 @@ public void TestMethod2()
                 public void [|TestMethod4|]()
                 {
                 }
+
+                [MyExpectedException]
+                [TestMethod]
+                public void [|TestMethod5|]()
+                {
+                }
             }
             """;
 
@@ -86,7 +108,7 @@ public void TestMethod2()
                     fixedCode,
                 },
                 // ExpectedException with AllowDerivedTypes = True cannot be simply converted
-                // to Assert.ThrowsException as the semantics are different.
+                // to Assert.ThrowsException as the semantics are different (same for custom attributes that may have some special semantics).
                 // For now, the user needs to manually fix this to use Assert.ThrowsException and specify the actual (exact) exception type.
                 // We *could* provide a codefix that uses Assert.ThrowsException<SameExceptionType> but that's most likely going to be wrong.
                 // If the user explicitly has AllowDerivedTypes, it's likely because he doesn't specify the exact exception type.

diff --git a/test/UnitTests/MSTest.Analyzers.UnitTests/UseAttributeOnTestMethodAnalyzerTests.cs b/test/UnitTests/MSTest.Analyzers.UnitTests/UseAttributeOnTestMethodAnalyzerTests.cs
@@ -20,10 +20,18 @@ public sealed class UseAttributeOnTestMethodAnalyzerTests(ITestExecutionContext
         (UseAttributeOnTestMethodAnalyzer.WorkItemRule, "WorkItem(100)"),
         (UseAttributeOnTestMethodAnalyzer.DescriptionRule, """Description("description")"""),
         (UseAttributeOnTestMethodAnalyzer.ExpectedExceptionRule, "ExpectedException(null)"),
+        (UseAttributeOnTestMethodAnalyzer.ExpectedExceptionRule, "MyExpectedException"),
         (UseAttributeOnTestMethodAnalyzer.CssIterationRule, "CssIteration(null)"),
         (UseAttributeOnTestMethodAnalyzer.CssProjectStructureRule, "CssProjectStructure(null)")
     ];
 
+    private const string MyExpectedExceptionAttributeDeclaration = """
+        public class MyExpectedExceptionAttribute : ExpectedExceptionBaseAttribute
+        {
+            protected override void Verify(System.Exception exception) { }
+        }
+        """;
+
     internal static IEnumerable<(DiagnosticDescriptor Rule, string AttributeUsageExample)> GetAttributeUsageExampleAndRuleTuples()
         => RuleUsageExamples.Select(tuple => (tuple.Rule, tuple.AttributeUsageExample));
 
@@ -45,6 +53,8 @@ public async Task WhenMethodIsMarkedWithTestMethodAndTestAttributes_NoDiagnostic
         string code = $$"""
             using Microsoft.VisualStudio.TestTools.UnitTesting;
 
+            {{MyExpectedExceptionAttributeDeclaration}}
+
             [TestClass]
             public class MyTestClass
             {
@@ -65,6 +75,8 @@ public async Task WhenMethodIsMarkedWithTestAttributeButNotWithTestMethod_Diagno
         string code = $$"""
             using Microsoft.VisualStudio.TestTools.UnitTesting;
 
+            {{MyExpectedExceptionAttributeDeclaration}}
+
             [TestClass]
             public class MyTestClass
             {
@@ -77,6 +89,8 @@ public void TestMethod()
 
         string fixedCode = $$"""
             using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+            {{MyExpectedExceptionAttributeDeclaration}}
 
             [TestClass]
             public class MyTestClass
@@ -101,6 +115,8 @@ public async Task WhenMethodIsMarkedWithMultipleTestAttributesButNotWithTestMeth
     {
         string code = $$"""
             using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+            {{MyExpectedExceptionAttributeDeclaration}}
 
             [TestClass]
             public class MyTestClass
@@ -115,6 +131,8 @@ public void TestMethod()
 
         string fixedCode = $$"""
             using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+            {{MyExpectedExceptionAttributeDeclaration}}
 
             [TestClass]
             public class MyTestClass
@@ -137,6 +155,8 @@ public async Task WhenMethodIsMarkedWithTestAttributeAndCustomTestMethod_NoDiagn
         string code = $$"""
             using System;
             using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+            {{MyExpectedExceptionAttributeDeclaration}}
 
             [TestClass]
             public class MyTestClass