Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Respect custom ExpectedExceptionBase attribute implementations #4045

Merged
merged 2 commits into from
Nov 12, 2024
Merged

Respect custom ExpectedExceptionBase attribute implementations #4045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dface23
Respect custom ExpectedExceptionBase attribute implementations
Youssef1313 Nov 12, 2024
d6c15a7
Cleanup
Youssef1313 Nov 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions src/Analyzers/MSTest.Analyzers/AvoidExpectedExceptionAttributeAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,20 +41,21 @@ 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.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
Expand Down
1 change: 1 addition & 0 deletions src/Analyzers/MSTest.Analyzers/Helpers/WellKnownTypeNames.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
4 changes: 2 additions & 2 deletions src/Analyzers/MSTest.Analyzers/UseAttributeOnTestMethodAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
];
Expand Down Expand Up @@ -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));
}
Expand Down
24 changes: 23 additions & 1 deletion test/UnitTests/MSTest.Analyzers.UnitTests/AvoidExpectedExceptionAttributeAnalyzerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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
{
Expand All @@ -73,6 +89,12 @@ public void TestMethod2()
public void [|TestMethod4|]()
{
}

[MyExpectedException]
[TestMethod]
public void [|TestMethod5|]()
{
}
}
""";

Expand All @@ -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.
Expand Down
20 changes: 20 additions & 0 deletions test/UnitTests/MSTest.Analyzers.UnitTests/UseAttributeOnTestMethodAnalyzerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand All @@ -45,6 +53,8 @@ public async Task WhenMethodIsMarkedWithTestMethodAndTestAttributes_NoDiagnostic
string code = $$"""
using Microsoft.VisualStudio.TestTools.UnitTesting;

{{MyExpectedExceptionAttributeDeclaration}}

[TestClass]
public class MyTestClass
{
Expand All @@ -65,6 +75,8 @@ public async Task WhenMethodIsMarkedWithTestAttributeButNotWithTestMethod_Diagno
string code = $$"""
using Microsoft.VisualStudio.TestTools.UnitTesting;

{{MyExpectedExceptionAttributeDeclaration}}

[TestClass]
public class MyTestClass
{
Expand All @@ -77,6 +89,8 @@ public void TestMethod()

string fixedCode = $$"""
using Microsoft.VisualStudio.TestTools.UnitTesting;

{{MyExpectedExceptionAttributeDeclaration}}

[TestClass]
public class MyTestClass
Expand All @@ -101,6 +115,8 @@ public async Task WhenMethodIsMarkedWithMultipleTestAttributesButNotWithTestMeth
{
string code = $$"""
using Microsoft.VisualStudio.TestTools.UnitTesting;

{{MyExpectedExceptionAttributeDeclaration}}

[TestClass]
public class MyTestClass
Expand All @@ -115,6 +131,8 @@ public void TestMethod()

string fixedCode = $$"""
using Microsoft.VisualStudio.TestTools.UnitTesting;

{{MyExpectedExceptionAttributeDeclaration}}

[TestClass]
public class MyTestClass
Expand All @@ -137,6 +155,8 @@ public async Task WhenMethodIsMarkedWithTestAttributeAndCustomTestMethod_NoDiagn
string code = $$"""
using System;
using Microsoft.VisualStudio.TestTools.UnitTesting;

{{MyExpectedExceptionAttributeDeclaration}}

[TestClass]
public class MyTestClass
Expand Down