[duplicate-code] Duplicate Code: Near-Identical TestInitializeShouldBeValidAnalyzer and TestCleanupShouldBeValidAnalyzer

[Evangelink]

Analysis of commit d8c1e43

Assignee: @copilot

Summary

TestInitializeShouldBeValidAnalyzer and TestCleanupShouldBeValidAnalyzer are structurally identical 42-line files. Every single line is the same except for the diagnostic ID reference, four resource string names, and the MSTest attribute type name. Both delegate to FixtureMethodAnalyzerHelper.AnalyzeInstanceFixtureMethod with the same logic. The broader set of fixture lifecycle analyzers (ClassInitializeShouldBeValidAnalyzer, ClassCleanupShouldBeValidAnalyzer, AssemblyInitializeShouldBeValidAnalyzer, AssemblyCleanupShouldBeValidAnalyzer) follow the same structural template with variations only in validation flags, so the TestInitialize/TestCleanup pair is the clearest and lowest-risk consolidation target.

Duplication Details

Pattern: Near-identical fixture lifecycle analyzer files

• Severity: Medium
• Occurrences: 2 (exact pair) / 6 (broader family)
• Locations:
  • src/Analyzers/MSTest.Analyzers/TestInitializeShouldBeValidAnalyzer.cs (lines 1–42)
  • src/Analyzers/MSTest.Analyzers/TestCleanupShouldBeValidAnalyzer.cs (lines 1–42)
• Code Sample (files differ only in the highlighted parts):

// TestInitializeShouldBeValidAnalyzer vs TestCleanupShouldBeValidAnalyzer
public sealed class TestInitializeShouldBeValidAnalyzer : DiagnosticAnalyzer   // <-- differs
{
    public static readonly DiagnosticDescriptor Rule = DiagnosticDescriptorHelper.Create(
        DiagnosticIds.TestInitializeShouldBeValidRuleId,                        // <-- differs
        FixtureMethodAnalyzerHelper.CreateResourceString(nameof(Resources.TestInitializeShouldBeValidTitle)),    // <-- differs
        FixtureMethodAnalyzerHelper.CreateResourceString(nameof(Resources.TestInitializeShouldBeValidMessageFormat)), // <-- differs
        FixtureMethodAnalyzerHelper.CreateResourceString(nameof(Resources.TestInitializeShouldBeValidDescription)),  // <-- differs
        Category.Usage, DiagnosticSeverity.Warning, isEnabledByDefault: true);

    public override void Initialize(AnalysisContext context)
    {
        context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
        context.EnableConcurrentExecution();
        FixtureMethodAnalyzerHelper.RegisterFixtureMethodSymbolAction(
            context,
            WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingTestInitializeAttribute, // <-- differs
            static (symbolContext, symbols) => FixtureMethodAnalyzerHelper.AnalyzeInstanceFixtureMethod(symbolContext, symbols, Rule));
    }
}

Impact Analysis

• Maintainability: Any change to instance-fixture validation logic (e.g., adding a new signature constraint) must be applied to both files identically. Overlooking one results in inconsistent behavior between [TestInitialize] and [TestCleanup] methods.
• Bug Risk: Copy-paste errors — if a future fix is applied to one analyzer but not the other, the two lifecycle hooks would be validated under different rules without any compile-time signal.
• Code Bloat: ~42 lines of structurally duplicated code that could be expressed as a single data-driven definition.

Refactoring Recommendations

1. Merge into a single data-driven or parameterized implementation

   • Keep both public DiagnosticDescriptor fields (TestInitializeShouldBeValidAnalyzer.Rule and TestCleanupShouldBeValidAnalyzer.Rule) but move the registration logic to a shared helper in FixtureMethodAnalyzerHelper:

     internal static void RegisterInstanceFixtureAnalyzer(
         AnalysisContext context,
         string attributeTypeName,
         DiagnosticDescriptor rule)
     {
         RegisterFixtureMethodSymbolAction(
             context,
             attributeTypeName,
             (ctx, symbols) => AnalyzeInstanceFixtureMethod(ctx, symbols, rule));
     }

   • Both analyzer Initialize methods then become one-liners.
   • Estimated effort: ~1 hour
   • Benefits: Shared logic updated in one place; new instance-fixture analyzers get it for free.

2. Consider consolidating the whole *ShouldBeValid family

   • ClassInitializeShouldBeValidAnalyzer / ClassCleanupShouldBeValidAnalyzer differ only in FixtureParameterMode and inheritance flags — these could be table-driven as well, though each has subtler validation differences that warrant careful review first.

Implementation Checklist

• Review duplication findings
• Add RegisterInstanceFixtureAnalyzer (or equivalent) to FixtureMethodAnalyzerHelper
• Refactor TestInitializeShouldBeValidAnalyzer and TestCleanupShouldBeValidAnalyzer to use shared helper
• Ensure both public Rule fields remain accessible (consumed by fixer/test projects)
• Run analyzer unit tests to confirm no behavioral change

Analysis Metadata

• Analyzed Files: 2 primary (TestInitializeShouldBeValidAnalyzer.cs, TestCleanupShouldBeValidAnalyzer.cs); 4 related (ClassInitialize, ClassCleanup, AssemblyInitialize, AssemblyCleanup)
• Detection Method: Semantic code analysis — structural diff showed only identifier/string differences across the two files
• Commit: d8c1e43
• Analysis Date: 2026-06-28

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Duplicate Code Detector workflow. · 1.2K AIC · ⌖ 25.3 AIC · ⊞ 43.5K · [◷]( · ◷)

Add this agentic workflows to your repo

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/duplicate-code-detector.md@main

• expires on Jun 30, 2026, 5:52 AM UTC

[Evangelink]

Addressed in #9487 via a shared FixtureMethodAnalyzerHelper.RegisterInstanceFixtureAnalyzer helper (the ConfigureGeneratedCodeAnalysis/EnableConcurrentExecution calls stay in each Initialize to satisfy RS1025/RS1026).