feat: TUnit0074 analyzer for redundant hook attributes on overrides

TUnit.Analyzers.CodeFixers/VirtualHookOverrideCodeFixProvider.cs

@@ -0,0 +1,75 @@
+using System.Collections.Immutable;
+using System.Composition;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CodeActions;
+using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using TUnit.Analyzers;
+
+namespace TUnit.Analyzers.CodeFixers;
+
+[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(VirtualHookOverrideCodeFixProvider)), Shared]
+public class VirtualHookOverrideCodeFixProvider : CodeFixProvider
+{
+    private const string Title = "Remove redundant hook attribute";
+
+    public sealed override ImmutableArray<string> FixableDiagnosticIds { get; } =
+        ImmutableArray.Create(Rules.RedundantHookAttributeOnOverride.Id);
+
+    public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;
+
+    public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
+    {
+        var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
+        if (root is null)
+        {
+            return;
+        }
+
+        foreach (var diagnostic in context.Diagnostics)
+        {
+            var attributeSyntax = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true)
+                .FirstAncestorOrSelf<AttributeSyntax>();
+
+            if (attributeSyntax is null)
+            {
+                continue;
+            }
+
+            context.RegisterCodeFix(
+                CodeAction.Create(
+                    title: Title,
+                    createChangedDocument: c => RemoveAttributeAsync(context.Document, attributeSyntax, c),
+                    equivalenceKey: Title),
+                diagnostic);
+        }
+    }
+
+    private static async Task<Document> RemoveAttributeAsync(Document document, AttributeSyntax attributeSyntax, CancellationToken cancellationToken)
+    {
+        var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
+        if (root is null)
+        {
+            return document;
+        }
+
+        var attributeList = (AttributeListSyntax)attributeSyntax.Parent!;
+
+        SyntaxNode newRoot;
+        if (attributeList.Attributes.Count == 1)
+        {
+            // KeepLeadingTrivia so any comment above the attribute list is preserved by being
+            // re-attached to the next node (the method declaration).
+            newRoot = root.RemoveNode(attributeList, SyntaxRemoveOptions.KeepLeadingTrivia)!;
+        }
+        else
+        {
+            var newAttributeList = attributeList.WithAttributes(
+                attributeList.Attributes.Remove(attributeSyntax));
+            newRoot = root.ReplaceNode(attributeList, newAttributeList);
+        }
+
+        return document.WithSyntaxRoot(newRoot);
+    }
+}

TUnit.Analyzers.Tests/VirtualHookOverrideAnalyzerTests.cs

@@ -0,0 +1,242 @@
+using Verifier = TUnit.Analyzers.Tests.Verifiers.CSharpAnalyzerVerifier<TUnit.Analyzers.VirtualHookOverrideAnalyzer>;
+
+namespace TUnit.Analyzers.Tests;
+
+public class VirtualHookOverrideAnalyzerTests
+{
+    [Test]
+    public async Task Before_Test_On_Both_Base_And_Override_Reports_Error()
+    {
+        await Verifier
+            .VerifyAnalyzerAsync(
+                """
+                using System.Threading.Tasks;
+                using TUnit.Core;
+                using static TUnit.Core.HookType;
+
+                public class BaseClass
+                {
+                    [Before(Test)]
+                    public virtual Task SetupAsync() => Task.CompletedTask;
+                }
+
+                public class DerivedClass : BaseClass
+                {
+                    [{|#0:Before(Test)|}]
+                    public override Task SetupAsync() => base.SetupAsync();
+                }
+                """,
+                Verifier.Diagnostic(Rules.RedundantHookAttributeOnOverride)
+                    .WithLocation(0)
+                    .WithArguments("Before", "BaseClass", "SetupAsync")
+            );
+    }
+
+    [Test]
+    public async Task After_Test_On_Both_Base_And_Override_Reports_Error()
+    {
+        await Verifier
+            .VerifyAnalyzerAsync(
+                """
+                using System.Threading.Tasks;
+                using TUnit.Core;
+                using static TUnit.Core.HookType;
+
+                public class BaseClass
+                {
+                    [After(Test)]
+                    public virtual Task TeardownAsync() => Task.CompletedTask;
+                }
+
+                public class DerivedClass : BaseClass
+                {
+                    [{|#0:After(Test)|}]
+                    public override Task TeardownAsync() => base.TeardownAsync();
+                }
+                """,
+                Verifier.Diagnostic(Rules.RedundantHookAttributeOnOverride)
+                    .WithLocation(0)
+                    .WithArguments("After", "BaseClass", "TeardownAsync")
+            );
+    }
+
+    [Test]
+    public async Task Attribute_Only_On_Base_Is_Fine()
+    {
+        await Verifier
+            .VerifyAnalyzerAsync(
+                """
+                using System.Threading.Tasks;
+                using TUnit.Core;
+                using static TUnit.Core.HookType;
+
+                public class BaseClass
+                {
+                    [Before(Test)]
+                    public virtual Task SetupAsync() => Task.CompletedTask;
+                }
+
+                public class DerivedClass : BaseClass
+                {
+                    public override Task SetupAsync() => base.SetupAsync();
+                }
+                """
+            );
+    }
+
+    [Test]
+    public async Task Attribute_Only_On_Override_Is_Fine()
+    {
+        await Verifier
+            .VerifyAnalyzerAsync(
+                """
+                using System.Threading.Tasks;
+                using TUnit.Core;
+                using static TUnit.Core.HookType;
+
+                public class BaseClass
+                {
+                    public virtual Task SetupAsync() => Task.CompletedTask;
+                }
+
+                public class DerivedClass : BaseClass
+                {
+                    [Before(Test)]
+                    public override Task SetupAsync() => base.SetupAsync();
+                }
+                """
+            );
+    }
+
+    [Test]
+    public async Task Abstract_Intermediate_With_InheritsTests_Reports_Error_On_Intermediate()
+    {
+        // Regression shape for https://github.com/thomhurst/TUnit/issues/5450 — an abstract
+        // intermediate overrides the base hook and then concrete subclasses pick up the tests via
+        // [InheritsTests]. The duplication happens on the intermediate's override, so that's where
+        // the diagnostic should fire.
+        await Verifier
+            .VerifyAnalyzerAsync(
+                """
+                using System.Threading.Tasks;
+                using TUnit.Core;
+                using static TUnit.Core.HookType;
+
+                public class BaseClass
+                {
+                    [Before(Test)]
+                    public virtual Task SetupAsync() => Task.CompletedTask;
+                }
+
+                public abstract class IntermediateClass : BaseClass
+                {
+                    [{|#0:Before(Test)|}]
+                    public override Task SetupAsync() => base.SetupAsync();
+
+                    [Test]
+                    public void DoTest() { }
+                }
+
+                [InheritsTests]
+                public class ConcreteA : IntermediateClass;
+
+                [InheritsTests]
+                public class ConcreteB : IntermediateClass;
+                """,
+                Verifier.Diagnostic(Rules.RedundantHookAttributeOnOverride)
+                    .WithLocation(0)
+                    .WithArguments("Before", "BaseClass", "SetupAsync")
+            );
+    }
+
+    [Test]
+    public async Task Chain_With_Gap_Still_Reports_Error_Via_Transitive_Ancestor()
+    {
+        // A has the hook attribute, B overrides without it (fine — virtual dispatch), C overrides
+        // with the hook attribute again — C is the source of the duplication because A is still
+        // in the ancestor chain.
+        await Verifier
+            .VerifyAnalyzerAsync(
+                """
+                using System.Threading.Tasks;
+                using TUnit.Core;
+                using static TUnit.Core.HookType;
+
+                public class A
+                {
+                    [Before(Test)]
+                    public virtual Task SetupAsync() => Task.CompletedTask;
+                }
+
+                public class B : A
+                {
+                    public override Task SetupAsync() => base.SetupAsync();
+                }
+
+                public class C : B
+                {
+                    [{|#0:Before(Test)|}]
+                    public override Task SetupAsync() => base.SetupAsync();
+                }
+                """,
+                Verifier.Diagnostic(Rules.RedundantHookAttributeOnOverride)
+                    .WithLocation(0)
+                    .WithArguments("Before", "A", "SetupAsync")
+            );
+    }
+
+    [Test]
+    public async Task Mismatched_Hook_Types_Are_Fine()
+    {
+        // [Before(Test)] on the base and [After(Test)] on the override are different hooks; both
+        // run in their own phase, so there is no duplication.
+        await Verifier
+            .VerifyAnalyzerAsync(
+                """
+                using System.Threading.Tasks;
+                using TUnit.Core;
+                using static TUnit.Core.HookType;
+
+                public class BaseClass
+                {
+                    [Before(Test)]
+                    public virtual Task SetupAsync() => Task.CompletedTask;
+                }
+
+                public class DerivedClass : BaseClass
+                {
+                    [After(Test)]
+                    public override Task SetupAsync() => base.SetupAsync();
+                }
+                """
+            );
+    }
+
+    [Test]
+    public async Task New_Method_Is_Fine()
+    {
+        // `new` is not `override` — derived's method is a different method, so both hooks are
+        // registered on different targets and neither double-fires.
+        await Verifier
+            .VerifyAnalyzerAsync(
+                """
+                using System.Threading.Tasks;
+                using TUnit.Core;
+                using static TUnit.Core.HookType;
+
+                public class BaseClass
+                {
+                    [Before(Test)]
+                    public virtual Task SetupAsync() => Task.CompletedTask;
+                }
+
+                public class DerivedClass : BaseClass
+                {
+                    [Before(Test)]
+                    public new Task SetupAsync() => Task.CompletedTask;
+                }
+                """
+            );
+    }
+
+}

TUnit.Analyzers/AnalyzerReleases.Unshipped.md

@@ -5,6 +5,7 @@ Rule ID | Category | Severity | Notes
 TUnit0061 | Usage | Error | ClassDataSource type requires parameterless constructor
 TUnit0062 | Usage | Warning | CancellationToken must be the last parameter
 TUnit0073 | Usage | Error | Missing polyfill types required by TUnit
+TUnit0074 | Usage | Error | Hook attribute is redundant on an override
 
 ### Removed Rules
 

TUnit.Analyzers/Resources.resx

@@ -534,6 +534,15 @@
     <data name="TUnit0073Title" xml:space="preserve">
         <value>Missing polyfill types required by TUnit</value>
     </data>
+    <data name="TUnit0074Description" xml:space="preserve">
+        <value>When a virtual instance hook method ([Before(Test)] / [After(Test)]) is already declared on a base class, applying the same hook attribute to an override causes the hook to be registered twice. Because virtual dispatch routes both registrations to the same override, the override's body runs twice per test. Remove the attribute from the override — the base class hook will still call through to the override via virtual dispatch.</value>
+    </data>
+    <data name="TUnit0074MessageFormat" xml:space="preserve">
+        <value>[{0}(Test)] is already declared on base method '{1}.{2}'. Remove this attribute from the override — virtual dispatch will still invoke this override when the base hook runs.</value>
+    </data>
+    <data name="TUnit0074Title" xml:space="preserve">
+        <value>Hook attribute is redundant on an override</value>
+    </data>
     <data name="TUnit0301Description" xml:space="preserve">
         <value>Tuple types require reflection for property/field access which is not AOT-compatible. Consider using concrete types or value deconstruction.</value>
     </data>

TUnit.Analyzers/Rules.cs

@@ -173,6 +173,9 @@ public static class Rules
             customTags: [WellKnownDiagnosticTags.CompilationEnd],
             helpLinkUri: "https://www.nuget.org/packages/Polyfill");
 
+    public static readonly DiagnosticDescriptor RedundantHookAttributeOnOverride =
+        CreateDescriptor("TUnit0074", UsageCategory, DiagnosticSeverity.Error);
+
     public static readonly DiagnosticDescriptor GenericTypeNotAotCompatible =
         CreateDescriptor("TUnit0300", UsageCategory, DiagnosticSeverity.Warning);