[duplicate-code] Duplicate Code: Shared Analysis Helpers Duplicated in PreferAssertFail and ReviewAlwaysTrue Analyzers

[Evangelink]

Analysis of commit 916e3a0

Assignee: @copilot

Summary

PreferAssertFailOverAlwaysFalseConditionsAnalyzer and ReviewAlwaysTrueAssertConditionAnalyzer share ~97 duplicated lines across 10 clone pairs — including a private EqualityStatus enum, five parameter-name string constants, and four private utility methods (GetArgumentWithName, GetConditionArgument, GetValueArgument, GetEqualityStatus, IsNotNullableType). These helpers are logically the same and could be extracted to a shared internal helper class.

Duplication Details

Pattern: Duplicated Assert-Condition Analysis Helpers in Two Analyzers

• Severity: Medium
• Occurrences: 10 clone pairs, 97 duplicated lines
• Locations:
  • src/Analyzers/MSTest.Analyzers/PreferAssertFailOverAlwaysFalseConditionsAnalyzer.cs (131 lines)
  • src/Analyzers/MSTest.Analyzers/ReviewAlwaysTrueAssertConditionAnalyzer.cs (122 lines)

Duplicated blocks:

Block	PreferAssertFail (lines)	ReviewAlwaysTrue (lines)	Size
EqualityStatus enum	23–28	23–28	~6
Parameter name constants	30–35	30–34	~5
Initialize method body	54–68	53–67	15
GetArgumentWithName + helpers	109–116	99–106	~8
IsNotNullableType	101–107	91–97	7
GetEqualityStatus	118–130	108–120	13
Bulk of private helper section	97–131	87–121	35

Code Sample (identical private helpers in both files):

// Duplicated verbatim in both analyzers
private static bool IsNotNullableType(IOperation valueArgumentOperation)
{
    ITypeSymbol? valueArgType = valueArgumentOperation
        .GetReferencedMemberOrLocalOrParameter()
        .GetReferencedMemberOrLocalOrParameter();
    return valueArgType is not null
        && valueArgType.IsValueType
        && valueArgType.OriginalDefinition.SpecialType != SpecialType.System_Nullable_T;
}

private static IOperation? GetArgumentWithName(IInvocationOperation operation, string name)
    => operation.Arguments.FirstOrDefault(arg => arg.Parameter?.Name == name)
                          ?.Value.WalkDownConversion();

private static EqualityStatus GetEqualityStatus(IInvocationOperation operation, string expectedOrNotExpectedParameterName)
{
    if (GetArgumentWithName(operation, expectedOrNotExpectedParameterName) is { } expectedOrNotExpectedArgument &&
        GetArgumentWithName(operation, ActualParameterName) is { } actualArgument &&
        expectedOrNotExpectedArgument.ConstantValue.HasValue &&
        actualArgument.ConstantValue.HasValue)
    {
        return Equals(expectedOrNotExpectedArgument.ConstantValue.Value, actualArgument.ConstantValue.Value)
            ? EqualityStatus.Equal : EqualityStatus.NotEqual;
    }
    return EqualityStatus.Unknown;
}

The two analyzers differ only in the IsAlwaysFalse / IsAlwaysTrue switch expression (which checks for complementary conditions — e.g., IsTrue with Value: false vs Value: true), and in diagnostic IDs and messages. All shared helpers are otherwise byte-for-byte identical.

Impact Analysis

• Maintainability: Any fix to GetEqualityStatus, IsNotNullableType, or the Initialize pattern must be applied in both files. The two analyzers are closely related (one checks "always false" assertions, the other checks "always true" assertions) so they will likely evolve together.
• Bug Risk: A subtle difference in EqualityStatus lookup (e.g., a missing null-constant check) applied to one but not the other would create an inconsistency that is hard to notice in review.
• Code Bloat: ~70 lines of private utility code replicated in two files.

Refactoring Recommendations

1. Extract AssertConditionAnalyzerHelper internal class

   • Create src/Analyzers/MSTest.Analyzers/Helpers/AssertConditionAnalyzerHelper.cs
   • Move EqualityStatus enum, the five parameter-name constants, and all four private utility methods into the new class as internal static members
   • Replace the duplicate private implementations in both analyzers with calls to the shared helper
   • Estimated effort: 2 hours
   • Benefits: Single place to fix/extend equality-status reasoning; reduces each analyzer by ~40 lines

2. Keep IsAlwaysTrue / IsAlwaysFalse local

   • These switch expressions are intentionally complementary and should remain in their respective analyzers to preserve clarity of intent

Implementation Checklist

• Create src/Analyzers/MSTest.Analyzers/Helpers/AssertConditionAnalyzerHelper.cs
• Move EqualityStatus, parameter-name constants, and helper methods there
• Update PreferAssertFailOverAlwaysFalseConditionsAnalyzer to use the shared helper
• Update ReviewAlwaysTrueAssertConditionAnalyzer to use the shared helper
• Run MSTest.Analyzers.UnitTests to confirm no regression
• Verify PublicAPI.Unshipped.txt is unchanged (no public surface change)

Analysis Metadata

• Analyzed Files: 1003 C# source files
• Detection Method: jscpd semantic code analysis (threshold: minLines=6, minTokens=50)
• Commit: 916e3a0
• Analysis Date: 2026-06-13

🤖 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. · 734 AIC · ⌖ 14.1 AIC · [◷]( · ◷)

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

[copilot-swe-agent]

The agent encountered an error and was unable to start working on this issue: Please try again later, or contact support if the problem persists. (Request id: 142A:25BE3A:1497D2:4C5DB7:6A2CEFEE)