Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add analyzer for duplicate data row #5144

Merged
merged 3 commits into from
Mar 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/Analyzers/MSTest.Analyzers/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
; Unshipped analyzer release
; https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md

### New Rules

Rule ID | Category | Severity | Notes
--------|----------|----------|-------
MSTEST0042 | Usage | Warning | DuplicateDataRowAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0042)
145 changes: 145 additions & 0 deletions src/Analyzers/MSTest.Analyzers/DuplicateDataRowAnalyzer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System.Collections.Immutable;

using Analyzer.Utilities.Extensions;

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.Testing.Platform;

using MSTest.Analyzers.Helpers;

namespace MSTest.Analyzers;

/// <summary>
/// MSTEST0042: <inheritdoc cref="Resources.DuplicateDataRowTitle"/>.
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
public sealed class DuplicateDataRowAnalyzer : DiagnosticAnalyzer
{
internal static readonly DiagnosticDescriptor Rule = DiagnosticDescriptorHelper.Create(
DiagnosticIds.DuplicateDataRowRuleId,
new LocalizableResourceString(nameof(Resources.DuplicateDataRowTitle), Resources.ResourceManager, typeof(Resources)),
new LocalizableResourceString(nameof(Resources.DuplicateDataRowMessageFormat), Resources.ResourceManager, typeof(Resources)),
null,
Category.Usage,
DiagnosticSeverity.Warning,
isEnabledByDefault: true);

/// <inheritdoc />
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);

/// <inheritdoc />
public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.EnableConcurrentExecution();

context.RegisterCompilationStartAction(context =>
{
if (context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingDataRowAttribute, out INamedTypeSymbol? dataRowAttribute))
{
context.RegisterSymbolAction(
context => AnalyzeSymbol(context, dataRowAttribute),
SymbolKind.Method);
}
});
}

private static void AnalyzeSymbol(SymbolAnalysisContext context, INamedTypeSymbol dataRowAttribute)
{
var methodSymbol = (IMethodSymbol)context.Symbol;
var dataRowArguments = new Dictionary<ImmutableArray<TypedConstant>, int>(TypedConstantArrayComparer.Instance);

ImmutableArray<AttributeData> attributes = methodSymbol.GetAttributes();
for (int i = 0; i < attributes.Length; i++)
{
AttributeData attribute = attributes[i];
if (!dataRowAttribute.Equals(attribute.AttributeClass, SymbolEqualityComparer.Default))
{
continue;
}

if (dataRowArguments.TryGetValue(attribute.ConstructorArguments, out int existingIndex) &&
attribute.ApplicationSyntaxReference is not null)
{
context.ReportDiagnostic(attribute.ApplicationSyntaxReference.CreateDiagnostic(Rule, context.CancellationToken, existingIndex, i));
continue;
}

dataRowArguments[attribute.ConstructorArguments] = i;
}
}

private sealed class TypedConstantArrayComparer : IEqualityComparer<ImmutableArray<TypedConstant>>
{
public static TypedConstantArrayComparer Instance { get; } = new();

public bool Equals(ImmutableArray<TypedConstant> x, ImmutableArray<TypedConstant> y)
{
if (x.Length != y.Length)
{
return false;
}

for (int i = 0; i < x.Length; i++)
{
if (!AreTypedConstantEquals(x[i], y[i]))
{
return false;
}
}

return true;
}

private static bool AreTypedConstantEquals(TypedConstant typedConstant1, TypedConstant typedConstant2)
{
// If the Kind doesn't match or the Type doesn't match, they are not equal.
if (typedConstant1.Kind != typedConstant2.Kind ||
!SymbolEqualityComparer.Default.Equals(typedConstant1.Type, typedConstant2.Type))
{
return false;
}

// If IsNull is true and the Kind is array, Values will return default(ImmutableArray<TypedConstant>), not empty.
// To avoid dealing with that, we do the quick IsNull checks first.
// If both are nulls, then we are good, everything is equal.
if (typedConstant1.IsNull && typedConstant2.IsNull)
{
return true;
}

// If only one is null, then we are not equal.
if (typedConstant1.IsNull || typedConstant2.IsNull)
{
return false;
}

// If the kind is array (at this point we know both have the same Kind), we compare Values.
// Accessing `Value` property for arrays will throw so we need to have explicit check to decide whether
// we compare `Value` or `Values`.
if (typedConstant1.Kind == TypedConstantKind.Array)
{
return TypedConstantArrayComparer.Instance.Equals(typedConstant1.Values, typedConstant2.Values);
}

// At this point, the type is matching and the kind is matching and is not array.
return object.Equals(typedConstant1.Value, typedConstant2.Value);
}

public int GetHashCode(ImmutableArray<TypedConstant> obj)
{
var hashCode = default(RoslynHashCode);
foreach (TypedConstant typedConstant in obj)
{
hashCode.Add(typedConstant.Kind);
hashCode.Add(SymbolEqualityComparer.Default.GetHashCode(typedConstant.Type));
}

return hashCode.ToHashCode();
}
}
}
1 change: 1 addition & 0 deletions src/Analyzers/MSTest.Analyzers/Helpers/DiagnosticIds.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,5 @@ internal static class DiagnosticIds
public const string UseNewerAssertThrowsRuleId = "MSTEST0039";
public const string AvoidUsingAssertsInAsyncVoidContextRuleId = "MSTEST0040";
public const string UseConditionBaseWithTestClassRuleId = "MSTEST0041";
public const string DuplicateDataRowRuleId = "MSTEST0042";
}
4 changes: 4 additions & 0 deletions src/Analyzers/MSTest.Analyzers/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
#nullable enable
MSTest.Analyzers.DuplicateDataRowAnalyzer
MSTest.Analyzers.DuplicateDataRowAnalyzer.DuplicateDataRowAnalyzer() -> void
override MSTest.Analyzers.DuplicateDataRowAnalyzer.Initialize(Microsoft.CodeAnalysis.Diagnostics.AnalysisContext! context) -> void
override MSTest.Analyzers.DuplicateDataRowAnalyzer.SupportedDiagnostics.get -> System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.DiagnosticDescriptor!>
18 changes: 18 additions & 0 deletions src/Analyzers/MSTest.Analyzers/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions src/Analyzers/MSTest.Analyzers/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -573,4 +573,10 @@ The type declaring these methods should also respect the following rules:
<data name="UseConditionBaseWithTestClassMessageFormat" xml:space="preserve">
<value>The attribute '{0}' which derives from 'ConditionBaseAttribute' should be used only on classes marked with `TestClassAttribute`</value>
</data>
<data name="DuplicateDataRowTitle" xml:space="preserve">
<value>Avoid duplicated 'DataRow' entries</value>
</data>
<data name="DuplicateDataRowMessageFormat" xml:space="preserve">
<value>Do not duplicate 'DataRow' attributes. This is usually a copy/paste error. The attribute indices are '{0}' and '{1}'.</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,13 @@ public static Diagnostic CreateDiagnostic(
messageArgs: args);
}

public static Diagnostic CreateDiagnostic(
this SyntaxReference syntaxReference,
DiagnosticDescriptor rule,
CancellationToken cancellationToken,
params object[] args)
=> syntaxReference.GetSyntax(cancellationToken).CreateDiagnostic(rule, args);

/// <summary>
/// TODO: Revert this reflection based workaround once we move to Microsoft.CodeAnalysis version 3.0
/// </summary>
Expand Down
10 changes: 10 additions & 0 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.cs.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,16 @@ Typ deklarující tyto metody by měl také respektovat následující pravidla:
<target state="translated">System.ComponentModel.DescriptionAttribute nemá žádný vliv na testovací metody</target>
<note />
</trans-unit>
<trans-unit id="DuplicateDataRowMessageFormat">
<source>Do not duplicate 'DataRow' attributes. This is usually a copy/paste error. The attribute indices are '{0}' and '{1}'.</source>
<target state="new">Do not duplicate 'DataRow' attributes. This is usually a copy/paste error. The attribute indices are '{0}' and '{1}'.</target>
<note />
</trans-unit>
<trans-unit id="DuplicateDataRowTitle">
<source>Avoid duplicated 'DataRow' entries</source>
<target state="new">Avoid duplicated 'DataRow' entries</target>
<note />
</trans-unit>
<trans-unit id="DynamicDataShouldBeValidDescription">
<source>'DynamicData' entry should have the following layout to be valid:
- should only be set on a test method;
Expand Down
10 changes: 10 additions & 0 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.de.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,16 @@ Der Typ, der diese Methoden deklariert, sollte auch die folgenden Regeln beachte
<target state="translated">"System.ComponentModel.DescriptionAttribute" hat keine Auswirkungen auf Testmethoden.</target>
<note />
</trans-unit>
<trans-unit id="DuplicateDataRowMessageFormat">
<source>Do not duplicate 'DataRow' attributes. This is usually a copy/paste error. The attribute indices are '{0}' and '{1}'.</source>
<target state="new">Do not duplicate 'DataRow' attributes. This is usually a copy/paste error. The attribute indices are '{0}' and '{1}'.</target>
<note />
</trans-unit>
<trans-unit id="DuplicateDataRowTitle">
<source>Avoid duplicated 'DataRow' entries</source>
<target state="new">Avoid duplicated 'DataRow' entries</target>
<note />
</trans-unit>
<trans-unit id="DynamicDataShouldBeValidDescription">
<source>'DynamicData' entry should have the following layout to be valid:
- should only be set on a test method;
Expand Down
10 changes: 10 additions & 0 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.es.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,16 @@ El tipo que declara estos métodos también debe respetar las reglas siguientes:
<target state="translated">"System.ComponentModel.DescriptionAttribute" no tiene ningún efecto en los métodos de prueba</target>
<note />
</trans-unit>
<trans-unit id="DuplicateDataRowMessageFormat">
<source>Do not duplicate 'DataRow' attributes. This is usually a copy/paste error. The attribute indices are '{0}' and '{1}'.</source>
<target state="new">Do not duplicate 'DataRow' attributes. This is usually a copy/paste error. The attribute indices are '{0}' and '{1}'.</target>
<note />
</trans-unit>
<trans-unit id="DuplicateDataRowTitle">
<source>Avoid duplicated 'DataRow' entries</source>
<target state="new">Avoid duplicated 'DataRow' entries</target>
<note />
</trans-unit>
<trans-unit id="DynamicDataShouldBeValidDescription">
<source>'DynamicData' entry should have the following layout to be valid:
- should only be set on a test method;
Expand Down
10 changes: 10 additions & 0 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.fr.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,16 @@ Le type doit être une classe
<target state="translated">« System.ComponentModel.DescriptionAttribute » n’a aucun effet sur les méthodes de test</target>
<note />
</trans-unit>
<trans-unit id="DuplicateDataRowMessageFormat">
<source>Do not duplicate 'DataRow' attributes. This is usually a copy/paste error. The attribute indices are '{0}' and '{1}'.</source>
<target state="new">Do not duplicate 'DataRow' attributes. This is usually a copy/paste error. The attribute indices are '{0}' and '{1}'.</target>
<note />
</trans-unit>
<trans-unit id="DuplicateDataRowTitle">
<source>Avoid duplicated 'DataRow' entries</source>
<target state="new">Avoid duplicated 'DataRow' entries</target>
<note />
</trans-unit>
<trans-unit id="DynamicDataShouldBeValidDescription">
<source>'DynamicData' entry should have the following layout to be valid:
- should only be set on a test method;
Expand Down
10 changes: 10 additions & 0 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.it.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,16 @@ Anche il tipo che dichiara questi metodi deve rispettare le regole seguenti:
<target state="translated">'System.ComponentModel.DescriptionAttribute' non ha alcun effetto sui metodi di test.</target>
<note />
</trans-unit>
<trans-unit id="DuplicateDataRowMessageFormat">
<source>Do not duplicate 'DataRow' attributes. This is usually a copy/paste error. The attribute indices are '{0}' and '{1}'.</source>
<target state="new">Do not duplicate 'DataRow' attributes. This is usually a copy/paste error. The attribute indices are '{0}' and '{1}'.</target>
<note />
</trans-unit>
<trans-unit id="DuplicateDataRowTitle">
<source>Avoid duplicated 'DataRow' entries</source>
<target state="new">Avoid duplicated 'DataRow' entries</target>
<note />
</trans-unit>
<trans-unit id="DynamicDataShouldBeValidDescription">
<source>'DynamicData' entry should have the following layout to be valid:
- should only be set on a test method;
Expand Down
10 changes: 10 additions & 0 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.ja.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,16 @@ The type declaring these methods should also respect the following rules:
<target state="translated">'System.ComponentModel.DescriptionAttribute' はテスト メソッドに影響しません</target>
<note />
</trans-unit>
<trans-unit id="DuplicateDataRowMessageFormat">
<source>Do not duplicate 'DataRow' attributes. This is usually a copy/paste error. The attribute indices are '{0}' and '{1}'.</source>
<target state="new">Do not duplicate 'DataRow' attributes. This is usually a copy/paste error. The attribute indices are '{0}' and '{1}'.</target>
<note />
</trans-unit>
<trans-unit id="DuplicateDataRowTitle">
<source>Avoid duplicated 'DataRow' entries</source>
<target state="new">Avoid duplicated 'DataRow' entries</target>
<note />
</trans-unit>
<trans-unit id="DynamicDataShouldBeValidDescription">
<source>'DynamicData' entry should have the following layout to be valid:
- should only be set on a test method;
Expand Down
10 changes: 10 additions & 0 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.ko.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,16 @@ The type declaring these methods should also respect the following rules:
<target state="translated">'System.ComponentModel.DescriptionAttribute'는 테스트 메서드에 영향을 주지 않습니다.</target>
<note />
</trans-unit>
<trans-unit id="DuplicateDataRowMessageFormat">
<source>Do not duplicate 'DataRow' attributes. This is usually a copy/paste error. The attribute indices are '{0}' and '{1}'.</source>
<target state="new">Do not duplicate 'DataRow' attributes. This is usually a copy/paste error. The attribute indices are '{0}' and '{1}'.</target>
<note />
</trans-unit>
<trans-unit id="DuplicateDataRowTitle">
<source>Avoid duplicated 'DataRow' entries</source>
<target state="new">Avoid duplicated 'DataRow' entries</target>
<note />
</trans-unit>
<trans-unit id="DynamicDataShouldBeValidDescription">
<source>'DynamicData' entry should have the following layout to be valid:
- should only be set on a test method;
Expand Down
10 changes: 10 additions & 0 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.pl.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,16 @@ Typ deklarujący te metody powinien również przestrzegać następujących regu
<target state="translated">Element „System.ComponentModel.DescriptionAttribute” nie ma wpływu na metody testowe</target>
<note />
</trans-unit>
<trans-unit id="DuplicateDataRowMessageFormat">
<source>Do not duplicate 'DataRow' attributes. This is usually a copy/paste error. The attribute indices are '{0}' and '{1}'.</source>
<target state="new">Do not duplicate 'DataRow' attributes. This is usually a copy/paste error. The attribute indices are '{0}' and '{1}'.</target>
<note />
</trans-unit>
<trans-unit id="DuplicateDataRowTitle">
<source>Avoid duplicated 'DataRow' entries</source>
<target state="new">Avoid duplicated 'DataRow' entries</target>
<note />
</trans-unit>
<trans-unit id="DynamicDataShouldBeValidDescription">
<source>'DynamicData' entry should have the following layout to be valid:
- should only be set on a test method;
Expand Down
10 changes: 10 additions & 0 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.pt-BR.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,16 @@ O tipo que declara esses métodos também deve respeitar as seguintes regras:
<target state="translated">'System.ComponentModel.DescriptionAttribute' não tem efeito sobre métodos de teste</target>
<note />
</trans-unit>
<trans-unit id="DuplicateDataRowMessageFormat">
<source>Do not duplicate 'DataRow' attributes. This is usually a copy/paste error. The attribute indices are '{0}' and '{1}'.</source>
<target state="new">Do not duplicate 'DataRow' attributes. This is usually a copy/paste error. The attribute indices are '{0}' and '{1}'.</target>
<note />
</trans-unit>
<trans-unit id="DuplicateDataRowTitle">
<source>Avoid duplicated 'DataRow' entries</source>
<target state="new">Avoid duplicated 'DataRow' entries</target>
<note />
</trans-unit>
<trans-unit id="DynamicDataShouldBeValidDescription">
<source>'DynamicData' entry should have the following layout to be valid:
- should only be set on a test method;
Expand Down
10 changes: 10 additions & 0 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.ru.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,16 @@ The type declaring these methods should also respect the following rules:
<target state="translated">"System.ComponentModel.DescriptionAttribute" не действует на методы тестирования</target>
<note />
</trans-unit>
<trans-unit id="DuplicateDataRowMessageFormat">
<source>Do not duplicate 'DataRow' attributes. This is usually a copy/paste error. The attribute indices are '{0}' and '{1}'.</source>
<target state="new">Do not duplicate 'DataRow' attributes. This is usually a copy/paste error. The attribute indices are '{0}' and '{1}'.</target>
<note />
</trans-unit>
<trans-unit id="DuplicateDataRowTitle">
<source>Avoid duplicated 'DataRow' entries</source>
<target state="new">Avoid duplicated 'DataRow' entries</target>
<note />
</trans-unit>
<trans-unit id="DynamicDataShouldBeValidDescription">
<source>'DynamicData' entry should have the following layout to be valid:
- should only be set on a test method;
Expand Down
Loading