From 34d10d59aedd958d1f22a3eb96947900994c08dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Nuno=20Mota?= Date: Wed, 16 Aug 2023 19:41:20 +0100 Subject: [PATCH] Add analyzer and fixer for test classes nested in generic classes (#159) --- .../Utility/CodeAnalysisExtensions.cs | 26 +++++ ...tClassCannotBeNestedInGenericClassFixer.cs | 38 +++++++ ...tClassCannotBeNestedInGenericClassTests.cs | 102 ++++++++++++++++++ ...sCannotBeNestedInGenericClassFixerTests.cs | 32 ++++++ src/xunit.analyzers/Utility/Descriptors.cs | 10 +- .../TestClassCannotBeNestedInGenericClass.cs | 59 ++++++++++ 6 files changed, 265 insertions(+), 2 deletions(-) create mode 100644 src/xunit.analyzers.fixes/X1000/TestClassCannotBeNestedInGenericClassFixer.cs create mode 100644 src/xunit.analyzers.tests/Analyzers/X1000/TestClassCannotBeNestedInGenericClassTests.cs create mode 100644 src/xunit.analyzers.tests/Fixes/X1000/TestClassCannotBeNestedInGenericClassFixerTests.cs create mode 100644 src/xunit.analyzers/X1000/TestClassCannotBeNestedInGenericClass.cs diff --git a/src/xunit.analyzers.fixes/Utility/CodeAnalysisExtensions.cs b/src/xunit.analyzers.fixes/Utility/CodeAnalysisExtensions.cs index 8cebdd08..59cea1ac 100644 --- a/src/xunit.analyzers.fixes/Utility/CodeAnalysisExtensions.cs +++ b/src/xunit.analyzers.fixes/Utility/CodeAnalysisExtensions.cs @@ -4,6 +4,8 @@ using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Editing; +using Microsoft.CodeAnalysis.Formatting; +using Microsoft.CodeAnalysis.Simplification; using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory; namespace Xunit.Analyzers.Fixes; @@ -142,6 +144,30 @@ public static async Task RemoveNode( return editor.GetChangedDocument(); } + public static async Task ExtractNodeFromParent( + this Document document, + SyntaxNode node, + CancellationToken cancellationToken) + { + var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); + var parent = node.Parent; + + if (parent is not null) + { + editor.RemoveNode(node); + + var formattedNode = + node + .WithLeadingTrivia(SyntaxFactory.ElasticMarker) + .WithTrailingTrivia(SyntaxFactory.ElasticMarker) + .WithAdditionalAnnotations(Formatter.Annotation, Simplifier.Annotation); + + editor.InsertAfter(parent, formattedNode); + } + + return editor.GetChangedDocument(); + } + public static async Task SetBaseClass( this Document document, ClassDeclarationSyntax declaration, diff --git a/src/xunit.analyzers.fixes/X1000/TestClassCannotBeNestedInGenericClassFixer.cs b/src/xunit.analyzers.fixes/X1000/TestClassCannotBeNestedInGenericClassFixer.cs new file mode 100644 index 00000000..0dc5e85d --- /dev/null +++ b/src/xunit.analyzers.fixes/X1000/TestClassCannotBeNestedInGenericClassFixer.cs @@ -0,0 +1,38 @@ +using System.Composition; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace Xunit.Analyzers.Fixes; + +[ExportCodeFixProvider(LanguageNames.CSharp), Shared] +public class TestClassCannotBeNestedInGenericClassFixer : BatchedCodeFixProvider +{ + public const string Key_ExtractTestClass = "xUnit1032_TestClassCannotBeNestedInGenericClass"; + + public TestClassCannotBeNestedInGenericClassFixer() : + base(Descriptors.X1032_TestClassCannotBeNestedInGenericClass.Id) + { } + + public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + if (root is null) + return; + + var classDeclaration = root.FindNode(context.Span).FirstAncestorOrSelf(); + if (classDeclaration is null) + return; + + context.RegisterCodeFix( + CodeAction.Create( + "Extract test class from parent class", + ct => context.Document.ExtractNodeFromParent(classDeclaration, ct), + Key_ExtractTestClass + ), + context.Diagnostics + ); + } +} diff --git a/src/xunit.analyzers.tests/Analyzers/X1000/TestClassCannotBeNestedInGenericClassTests.cs b/src/xunit.analyzers.tests/Analyzers/X1000/TestClassCannotBeNestedInGenericClassTests.cs new file mode 100644 index 00000000..cdc811a2 --- /dev/null +++ b/src/xunit.analyzers.tests/Analyzers/X1000/TestClassCannotBeNestedInGenericClassTests.cs @@ -0,0 +1,102 @@ +using System.Threading.Tasks; +using Xunit; +using Verify = CSharpVerifier; + +public class TestClassCannotBeNestedInGenericClassTests +{ + [Fact] + public async Task ReportsDiagnostic_WhenTestClassIsNestedInOpenGenericType() + { + var source = @" +public abstract class OpenGenericType +{ + public class NestedTestClass + { + [Xunit.Fact] + public void TestMethod() { } + } +}"; + + var expected = + Verify + .Diagnostic() + .WithLocation(4, 18); + + await Verify.VerifyAnalyzer(source, expected); + } + + [Fact] + public async Task ReportsDiagnostic_WhenDerivedTestClassIsNestedInOpenGenericType() + { + var source = @" +public abstract class BaseTestClass +{ + [Xunit.Fact] + public void TestMethod() { } +} + +public abstract class OpenGenericType +{ + public class NestedTestClass : BaseTestClass + { + } +}"; + + var expected = + Verify + .Diagnostic() + .WithLocation(10, 18); + + await Verify.VerifyAnalyzer(source, expected); + } + + [Fact] + public async Task DoesNotReportDiagnostic_WhenTestClassIsNestedInClosedGenericType() + { + var source = @" +public abstract class OpenGenericType +{ +} + +public abstract class ClosedGenericType : OpenGenericType +{ + public class NestedTestClass + { + [Xunit.Fact] + public void TestMethod() { } + } +}"; + + await Verify.VerifyAnalyzer(source); + } + + [Fact] + public async Task DoesNotReportDiagnostic_WhenNestedClassIsNotTestClass() + { + var source = @" +public abstract class OpenGenericType +{ + public class NestedClass + { + } +}"; + + await Verify.VerifyAnalyzer(source); + } + + [Fact] + public async Task DoesNotReportDiagnostic_WhenTestClassIsNotNestedInOpenGenericType() + { + var source = @" +public abstract class NonGenericType +{ + public class NestedTestClass + { + [Xunit.Fact] + public void TestMethod() { } + } +}"; + + await Verify.VerifyAnalyzer(source); + } +} diff --git a/src/xunit.analyzers.tests/Fixes/X1000/TestClassCannotBeNestedInGenericClassFixerTests.cs b/src/xunit.analyzers.tests/Fixes/X1000/TestClassCannotBeNestedInGenericClassFixerTests.cs new file mode 100644 index 00000000..88eddafe --- /dev/null +++ b/src/xunit.analyzers.tests/Fixes/X1000/TestClassCannotBeNestedInGenericClassFixerTests.cs @@ -0,0 +1,32 @@ +using Xunit; +using Xunit.Analyzers.Fixes; +using Verify = CSharpVerifier; + +public class TestClassCannotBeNestedInGenericClassFixerTests +{ + [Fact] + public async void MovesTestClassOutOfGenericParent() + { + const string before = @" +public abstract class OpenGenericType +{ + public class [|NestedTestClass|] + { + [Xunit.Fact] + public void TestMethod() { } + } +}"; + const string after = @" +public abstract class OpenGenericType +{ +} + +public class NestedTestClass +{ + [Xunit.Fact] + public void TestMethod() { } +}"; + + await Verify.VerifyCodeFix(before, after, TestClassCannotBeNestedInGenericClassFixer.Key_ExtractTestClass); + } +} diff --git a/src/xunit.analyzers/Utility/Descriptors.cs b/src/xunit.analyzers/Utility/Descriptors.cs index 51446792..b81ea84c 100644 --- a/src/xunit.analyzers/Utility/Descriptors.cs +++ b/src/xunit.analyzers/Utility/Descriptors.cs @@ -322,8 +322,14 @@ static DiagnosticDescriptor Rule( Error, "Test methods must not use blocking task operations, as they can cause deadlocks. Use an async test method and await instead." ); - - // Placeholder for rule X1032 + public static DiagnosticDescriptor X1032_TestClassCannotBeNestedInGenericClass { get; } = + Rule( + "xUnit1032", + "Test classes cannot be nested within a generic class", + Usage, + Error, + "Test classes cannot be nested within a generic class. Move the test class out of the class it is nested in." + ); public static DiagnosticDescriptor X1033_TestClassShouldHaveTFixtureArgument { get; } = Rule( diff --git a/src/xunit.analyzers/X1000/TestClassCannotBeNestedInGenericClass.cs b/src/xunit.analyzers/X1000/TestClassCannotBeNestedInGenericClass.cs new file mode 100644 index 00000000..8eb39ad0 --- /dev/null +++ b/src/xunit.analyzers/X1000/TestClassCannotBeNestedInGenericClass.cs @@ -0,0 +1,59 @@ +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Xunit.Analyzers; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class TestClassCannotBeNestedInGenericClass : XunitDiagnosticAnalyzer +{ + public TestClassCannotBeNestedInGenericClass() : + base(Descriptors.X1032_TestClassCannotBeNestedInGenericClass) + { } + + public override void AnalyzeCompilation( + CompilationStartAnalysisContext context, + XunitContext xunitContext) + { + context.RegisterSymbolAction(context => + { + if (xunitContext.Core.FactAttributeType is null) + return; + if (context.Symbol is not INamedTypeSymbol classSymbol) + return; + if (classSymbol.ContainingType is null) + return; + if (!classSymbol.ContainingType.IsGenericType) + return; + + var doesClassContainTests = DoesInheritenceTreeContainTests(classSymbol, xunitContext, depth: 3); + + if (!doesClassContainTests) + return; + + context.ReportDiagnostic( + Diagnostic.Create( + Descriptors.X1032_TestClassCannotBeNestedInGenericClass, + classSymbol.Locations.First() + ) + ); + }, SymbolKind.NamedType); + } + + private bool DoesInheritenceTreeContainTests( + INamedTypeSymbol classSymbol, + XunitContext xunitContext, + int depth) + { + var doesClassContainTests = + classSymbol + .GetMembers() + .OfType() + .Any(m => m.GetAttributes().Any(a => xunitContext.Core.FactAttributeType.IsAssignableFrom(a.AttributeClass))); + + if (!doesClassContainTests && classSymbol.BaseType is not null && depth > 0) + return DoesInheritenceTreeContainTests(classSymbol.BaseType, xunitContext, depth - 1); + + return doesClassContainTests; + } +}