Skip to content

[duplicate-code] Duplicate Code: Scaffolding Pattern in StringAssertToAssertFixer and CollectionAssertToAssertFixer #8845

@Evangelink

Description

@Evangelink

Analysis of commit e5fea75 — triggered by @Evangelink

Assignee: @copilot

Summary

CollectionAssertToAssertFixer (added in commit 7a3558e) duplicates the RegisterCodeFixesAsync scaffolding and fix-method skeleton of the pre-existing StringAssertToAssertFixer. The two fixers share four structural patterns — GetFixAllProvider, the diagnostic-property extraction guard, the node-finding guard, and the document-root rewrite pattern — totalling ~22 near-identical lines before the argument-rewriting logic diverges.

Duplication Details

Pattern 1 — Identical GetFixAllProvider (2 lines)

  • Severity: Low
  • Occurrences: 2 identical implementations
  • Locations:
    • src/Analyzers/MSTest.Analyzers.CodeFixes/StringAssertToAssertFixer.cs
    • src/Analyzers/MSTest.Analyzers.CodeFixes/CollectionAssertToAssertFixer.cs
  • Code Sample:
    public sealed override FixAllProvider GetFixAllProvider()
        => WellKnownFixAllProviders.BatchFixer;

Pattern 2 — Near-identical RegisterCodeFixesAsync scaffolding (~18 lines)

  • Severity: High
  • Occurrences: 2
  • Locations:
    • src/Analyzers/MSTest.Analyzers.CodeFixes/StringAssertToAssertFixer.cs (RegisterCodeFixesAsync)
    • src/Analyzers/MSTest.Analyzers.CodeFixes/CollectionAssertToAssertFixer.cs (RegisterCodeFixesAsync)
  • Code Sample:
    // Both fixers follow this identical scaffold (only the resource key and
    // analyzer class name differ):
    SyntaxNode root = await context.Document
        .GetRequiredSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
    Diagnostic diagnostic = context.Diagnostics[0];
    if (!diagnostic.Properties.TryGetValue(XxxAnalyzer.ProperAssertMethodNameKey,
            out string? properAssertMethodName)
        || properAssertMethodName == null)
    {
        return;
    }
    if (root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true)
            is not InvocationExpressionSyntax invocationExpr)
    {
        return;
    }
    if (invocationExpr.Expression is not MemberAccessExpressionSyntax)
    {
        return;
    }
    string title = string.Format(CultureInfo.InvariantCulture, CodeFixResources.XxxTitle, properAssertMethodName);
    var action = CodeAction.Create(
        title: title,
        createChangedDocument: ct => FixXxxAsync(context.Document, invocationExpr, ..., ct),
        equivalenceKey: title);
    context.RegisterCodeFix(action, diagnostic);

Pattern 3 — Near-identical fix method skeleton (~6 lines)

  • Severity: Medium
  • Occurrences: 2
  • Locations:
    • src/Analyzers/MSTest.Analyzers.CodeFixes/StringAssertToAssertFixer.cs (FixStringAssertAsync)
    • src/Analyzers/MSTest.Analyzers.CodeFixes/CollectionAssertToAssertFixer.cs (FixCollectionAssertAsync)
  • Code Sample:
    // Both fix methods open with the same guard + root fetch, and close identically:
    if (invocationExpr.Expression is not MemberAccessExpressionSyntax memberAccessExpr)
    {
        return document;
    }
    SyntaxNode root = await document
        .GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
    // ... argument rewriting differs ...
    return document.WithSyntaxRoot(root.ReplaceNode(invocationExpr, newInvocationExpr));

Impact Analysis

  • Maintainability: Adding a new XxxAssertToAssertFixer in the future will copy-paste the same scaffolding a third time.
  • Bug Risk: A fix to the RegisterCodeFixesAsync guard logic (e.g., handling is null vs == null, or changing how the fix-all scope works) must be applied to multiple fixers manually.
  • Code Bloat: ~22 lines of scaffolding duplicated per fixer pair; will grow linearly with future *AssertToAssert rules.

Refactoring Recommendations

  1. Shared base class AssertToAssertFixerBase : CodeFixProvider

    • Implement GetFixAllProvider and the common RegisterCodeFixesAsync scaffold (property extraction, node finding, MemberAccessExpressionSyntax guard, CodeAction.Create) in the base.
    • Subclasses override a single abstract method BuildFixedDocumentAsync(Document, InvocationExpressionSyntax, string assertMethodName, string fixKind, CancellationToken) for the argument-rewriting logic.
    • Estimated effort: 2–3 hours.
    • Benefits: single place for the scaffold; future fixers are ~20 lines instead of ~80.
  2. Alternative: shared static helper AssertToAssertFixerHelper

    • If inheritance is undesirable (e.g. to keep sealed), extract a static helper with the common guard steps as a TryExtractFixInfo method that both fixers call.
    • Estimated effort: 1–2 hours.

Implementation Checklist

  • Review duplication findings
  • Choose between base-class and static-helper approach
  • Create the shared infrastructure in src/Analyzers/MSTest.Analyzers.CodeFixes/
  • Refactor StringAssertToAssertFixer to use it
  • Refactor CollectionAssertToAssertFixer to use it
  • Verify all existing fixer unit tests still pass
  • Run unit tests: MSTest.Analyzers.UnitTests

Analysis Metadata

  • Analyzed Files: 2 source files
  • Detection Method: Semantic code analysis — structural comparison of recently-added code fixer
  • Commit: 7a3558e (Add MSTEST0068)
  • Analysis Date: 2026-06-05

Generated by Duplicate Code Detector · sonnet46 3M ·

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 7, 2026, 5:55 AM UTC

Metadata

Metadata

Labels

type/automationCreated or maintained by an agentic workflow.type/tech-debtCode health, refactoring, simplification.

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions