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

MSTEST0036: Add analyzer for when a test member is shadowing another member #3589

Merged
merged 21 commits into from
Aug 20, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ Rule ID | Category | Severity | Notes
MSTEST0018 | Usage | Warning | DynamicDataShouldBeValidAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0018)
MSTEST0034 | Usage | Info | UseClassCleanupBehaviorEndOfClassAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0034)
MSTEST0035 | Usage | Info | UseDeploymentItemWithTestMethodOrTestClassAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0035)
MSTEST0036 | Design | Warning | DoNotUseShadowingAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0036)
132 changes: 132 additions & 0 deletions src/Analyzers/MSTest.Analyzers/DoNotUseShadowingAnalyzer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
// 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 MSTest.Analyzers.Helpers;

namespace MSTest.Analyzers;

/// <summary>
/// MSTEST0036: <inheritdoc cref="Resources.DoNotUseShadowingTitle"/>.
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
public sealed class DoNotUseShadowingAnalyzer : DiagnosticAnalyzer
{
private static readonly LocalizableResourceString Title = new(nameof(Resources.DoNotUseShadowingTitle), Resources.ResourceManager, typeof(Resources));
private static readonly LocalizableResourceString Description = new(nameof(Resources.DoNotUseShadowingDescription), Resources.ResourceManager, typeof(Resources));
private static readonly LocalizableResourceString MessageFormat = new(nameof(Resources.DoNotUseShadowingMessageFormat), Resources.ResourceManager, typeof(Resources));

internal static readonly DiagnosticDescriptor DoNotUseShadowingRule = DiagnosticDescriptorHelper.Create(
DiagnosticIds.DoNotUseShadowingRuleId,
Title,
MessageFormat,
Description,
Category.Design,
DiagnosticSeverity.Warning,
isEnabledByDefault: true);

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

public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.EnableConcurrentExecution();

context.RegisterCompilationStartAction(context =>
{
if (context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingTestClassAttribute, out INamedTypeSymbol? testClassAttributeSymbol))
{
context.RegisterSymbolAction(
context => AnalyzeSymbol(context, testClassAttributeSymbol),
SymbolKind.NamedType);
}
});
}

private static void AnalyzeSymbol(SymbolAnalysisContext context, INamedTypeSymbol testClassAttributeSymbol)
{
var namedTypeSymbol = (INamedTypeSymbol)context.Symbol;
if (namedTypeSymbol.TypeKind != TypeKind.Class
|| !namedTypeSymbol.GetAttributes().Any(attr => SymbolEqualityComparer.Default.Equals(attr.AttributeClass, testClassAttributeSymbol)))
{
return;
}

Dictionary<string, List<ISymbol>> symbolGroups = GetBaseMembers(namedTypeSymbol);
foreach (ISymbol member in namedTypeSymbol.GetMembers())
{
foreach (ISymbol baseMember in symbolGroups.GetValueOrDefault(member.Name, new List<ISymbol>()))
{
// Check if the member is shadowing a base class member
if (IsMemberShadowing(member, baseMember))
{
context.ReportDiagnostic(member.CreateDiagnostic(DoNotUseShadowingRule, member.Name));
}
}
}
}

private static Dictionary<string, List<ISymbol>> GetBaseMembers(INamedTypeSymbol namedTypeSymbol)
{
Dictionary<string, List<ISymbol>> symbolGroups = new();
INamedTypeSymbol? currentType = namedTypeSymbol.BaseType;
while (currentType is not null)
{
foreach (ISymbol member in currentType.GetMembers())
{
if ((member is IMethodSymbol methodSymbol && (methodSymbol.MethodKind == MethodKind.PropertyGet || methodSymbol.MethodKind == MethodKind.PropertySet))
|| member.IsOverride || member.Name == ".ctor")
{
continue;
}

if (!symbolGroups.TryGetValue(member.Name, out List<ISymbol>? members))
{
members = new List<ISymbol>();
symbolGroups[member.Name] = members;
}

// Add the member to the list
members.Add(member);
}

currentType = currentType.BaseType;
}

return symbolGroups;
}

private static bool IsMemberShadowing(ISymbol member, ISymbol baseMember)
{
// Ensure both members are of the same kind (Method, Property, etc.)
if (member.Kind != baseMember.Kind || member.IsOverride)
{
return false;
}

// Compare methods
if (member is IMethodSymbol methodSymbol && baseMember is IMethodSymbol baseMethodSymbol)
{
return methodSymbol.Name == baseMethodSymbol.Name &&
methodSymbol.Parameters.Length == baseMethodSymbol.Parameters.Length &&
methodSymbol.Parameters.Zip(baseMethodSymbol.Parameters, (p1, p2) =>
SymbolEqualityComparer.Default.Equals(p1.Type, p2.Type)).All(equal => equal);
}

// Compare properties
else if (member is IPropertySymbol propertySymbol && baseMember is IPropertySymbol basePropertySymbol)
{
return propertySymbol.Name == basePropertySymbol.Name &&
SymbolEqualityComparer.Default.Equals(propertySymbol.Type, basePropertySymbol.Type);
}

return false;
}
}
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 @@ -40,4 +40,5 @@ internal static class DiagnosticIds
public const string NonNullableReferenceNotInitializedSuppressorRuleId = "MSTEST0033";
public const string UseClassCleanupBehaviorEndOfClassRuleId = "MSTEST0034";
public const string UseDeploymentItemWithTestMethodOrTestClassRuleId = "MSTEST0035";
public const string DoNotUseShadowingRuleId = "MSTEST0036";
}
4 changes: 4 additions & 0 deletions src/Analyzers/MSTest.Analyzers/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
#nullable enable
MSTest.Analyzers.DoNotUseShadowingAnalyzer
MSTest.Analyzers.DoNotUseShadowingAnalyzer.DoNotUseShadowingAnalyzer() -> void
MSTest.Analyzers.DynamicDataShouldBeValidAnalyzer
MSTest.Analyzers.DynamicDataShouldBeValidAnalyzer.DynamicDataShouldBeValidAnalyzer() -> void
MSTest.Analyzers.NonNullableReferenceNotInitializedSuppressor
Expand All @@ -7,6 +9,8 @@ MSTest.Analyzers.UseClassCleanupBehaviorEndOfClassAnalyzer
MSTest.Analyzers.UseClassCleanupBehaviorEndOfClassAnalyzer.UseClassCleanupBehaviorEndOfClassAnalyzer() -> void
MSTest.Analyzers.UseDeploymentItemWithTestMethodOrTestClassAnalyzer
MSTest.Analyzers.UseDeploymentItemWithTestMethodOrTestClassAnalyzer.UseDeploymentItemWithTestMethodOrTestClassAnalyzer() -> void
override MSTest.Analyzers.DoNotUseShadowingAnalyzer.Initialize(Microsoft.CodeAnalysis.Diagnostics.AnalysisContext! context) -> void
override MSTest.Analyzers.DoNotUseShadowingAnalyzer.SupportedDiagnostics.get -> System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.DiagnosticDescriptor!>
override MSTest.Analyzers.DynamicDataShouldBeValidAnalyzer.Initialize(Microsoft.CodeAnalysis.Diagnostics.AnalysisContext! context) -> void
override MSTest.Analyzers.DynamicDataShouldBeValidAnalyzer.SupportedDiagnostics.get -> System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.DiagnosticDescriptor!>
override MSTest.Analyzers.NonNullableReferenceNotInitializedSuppressor.ReportSuppressions(Microsoft.CodeAnalysis.Diagnostics.SuppressionAnalysisContext context) -> void
Expand Down
27 changes: 27 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.

9 changes: 9 additions & 0 deletions src/Analyzers/MSTest.Analyzers/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -520,4 +520,13 @@ The type declaring these methods should also respect the following rules:
<data name="TestClassShouldBeValidMessageFormat" xml:space="preserve">
<value>Test class '{0}' should be valid</value>
</data>
<data name="DoNotUseShadowingDescription" xml:space="preserve">
<value>Shadowing test members could cause testing issues (such as NRE).</value>
</data>
<data name="DoNotUseShadowingMessageFormat" xml:space="preserve">
<value>Member '{0}' is already exist in the base class</value>
</data>
<data name="DoNotUseShadowingTitle" xml:space="preserve">
<value>Do not use shadowing</value>
</data>
</root>
15 changes: 15 additions & 0 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.cs.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,21 @@ Typ deklarující tyto metody by měl také respektovat následující pravidla:
<target state="translated">Neukládejte TestContext ve statickém členu</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseShadowingDescription">
<source>Shadowing test members could cause testing issues (such as NRE).</source>
<target state="new">Shadowing test members could cause testing issues (such as NRE).</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseShadowingMessageFormat">
<source>Member '{0}' is already exist in the base class</source>
<target state="new">Member '{0}' is already exist in the base class</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseShadowingTitle">
<source>Do not use shadowing</source>
<target state="new">Do not use shadowing</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseSystemDescriptionAttributeDescription">
<source>'System.ComponentModel.DescriptionAttribute' has no effect in the context of tests and you likely wanted to use 'Microsoft.VisualStudio.TestTools.UnitTesting.DescriptionAttribute' instead.</source>
<target state="translated">System.ComponentModel.DescriptionAttribute nemá v kontextu testů žádný účinek a pravděpodobně jste místo toho chtěli použít Microsoft.VisualStudio.TestTools.UnitTesting.DescriptionAttribute.</target>
Expand Down
15 changes: 15 additions & 0 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.de.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,21 @@ Der Typ, der diese Methoden deklariert, sollte auch die folgenden Regeln beachte
<target state="translated">TestContext nicht in einem statischen Member speichern</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseShadowingDescription">
<source>Shadowing test members could cause testing issues (such as NRE).</source>
<target state="new">Shadowing test members could cause testing issues (such as NRE).</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseShadowingMessageFormat">
<source>Member '{0}' is already exist in the base class</source>
<target state="new">Member '{0}' is already exist in the base class</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseShadowingTitle">
<source>Do not use shadowing</source>
<target state="new">Do not use shadowing</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseSystemDescriptionAttributeDescription">
<source>'System.ComponentModel.DescriptionAttribute' has no effect in the context of tests and you likely wanted to use 'Microsoft.VisualStudio.TestTools.UnitTesting.DescriptionAttribute' instead.</source>
<target state="translated">"System.ComponentModel.DescriptionAttribute" hat im Kontext von Tests keine Auswirkungen, und Sie wollten wahrscheinlich stattdessen "Microsoft.VisualStudio.TestTools.UnitTesting.DescriptionAttribute" verwenden.</target>
Expand Down
15 changes: 15 additions & 0 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.es.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,21 @@ El tipo que declara estos métodos también debe respetar las reglas siguientes:
<target state="translated">No almacenar TestContext en un miembro estático</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseShadowingDescription">
<source>Shadowing test members could cause testing issues (such as NRE).</source>
<target state="new">Shadowing test members could cause testing issues (such as NRE).</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseShadowingMessageFormat">
<source>Member '{0}' is already exist in the base class</source>
<target state="new">Member '{0}' is already exist in the base class</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseShadowingTitle">
<source>Do not use shadowing</source>
<target state="new">Do not use shadowing</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseSystemDescriptionAttributeDescription">
<source>'System.ComponentModel.DescriptionAttribute' has no effect in the context of tests and you likely wanted to use 'Microsoft.VisualStudio.TestTools.UnitTesting.DescriptionAttribute' instead.</source>
<target state="translated">"System.ComponentModel.DescriptionAttribute" no tiene ningún efecto en el contexto de las pruebas y es probable que quiera usar "Microsoft.VisualStudio.TestTools.UnitTesting.DescriptionAttribute" en su lugar.</target>
Expand Down
15 changes: 15 additions & 0 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.fr.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,21 @@ Le type doit être une classe
<target state="translated">Ne pas stocker TestContext dans un membre statique</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseShadowingDescription">
<source>Shadowing test members could cause testing issues (such as NRE).</source>
<target state="new">Shadowing test members could cause testing issues (such as NRE).</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseShadowingMessageFormat">
<source>Member '{0}' is already exist in the base class</source>
<target state="new">Member '{0}' is already exist in the base class</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseShadowingTitle">
<source>Do not use shadowing</source>
<target state="new">Do not use shadowing</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseSystemDescriptionAttributeDescription">
<source>'System.ComponentModel.DescriptionAttribute' has no effect in the context of tests and you likely wanted to use 'Microsoft.VisualStudio.TestTools.UnitTesting.DescriptionAttribute' instead.</source>
<target state="translated">« System.ComponentModel.DescriptionAttribute » n’a aucun effet dans le contexte des tests et vous vouliez probablement utiliser « Microsoft.VisualStudio.TestTools.UnitTesting.DescriptionAttribute » à la place.</target>
Expand Down
15 changes: 15 additions & 0 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.it.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,21 @@ Anche il tipo che dichiara questi metodi deve rispettare le regole seguenti:
<target state="translated">Non archiviare TestContext in un membro statico</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseShadowingDescription">
<source>Shadowing test members could cause testing issues (such as NRE).</source>
<target state="new">Shadowing test members could cause testing issues (such as NRE).</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseShadowingMessageFormat">
<source>Member '{0}' is already exist in the base class</source>
<target state="new">Member '{0}' is already exist in the base class</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseShadowingTitle">
<source>Do not use shadowing</source>
<target state="new">Do not use shadowing</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseSystemDescriptionAttributeDescription">
<source>'System.ComponentModel.DescriptionAttribute' has no effect in the context of tests and you likely wanted to use 'Microsoft.VisualStudio.TestTools.UnitTesting.DescriptionAttribute' instead.</source>
<target state="translated">'System.ComponentModel.DescriptionAttribute' non ha alcun effetto in un contesto di test ed è probabile che si intendesse usare 'Microsoft.VisualStudio.TestTools.UnitTesting.DescriptionAttribute'.</target>
Expand Down
15 changes: 15 additions & 0 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.ja.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,21 @@ The type declaring these methods should also respect the following rules:
<target state="translated">静的メンバーに TestContext を格納しない</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseShadowingDescription">
<source>Shadowing test members could cause testing issues (such as NRE).</source>
<target state="new">Shadowing test members could cause testing issues (such as NRE).</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseShadowingMessageFormat">
<source>Member '{0}' is already exist in the base class</source>
<target state="new">Member '{0}' is already exist in the base class</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseShadowingTitle">
<source>Do not use shadowing</source>
<target state="new">Do not use shadowing</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseSystemDescriptionAttributeDescription">
<source>'System.ComponentModel.DescriptionAttribute' has no effect in the context of tests and you likely wanted to use 'Microsoft.VisualStudio.TestTools.UnitTesting.DescriptionAttribute' instead.</source>
<target state="translated">'System.ComponentModel.DescriptionAttribute' はテストのコンテキストでは効果がないため、代わりに 'Microsoft.VisualStudio.TestTools.UnitTesting.DescriptionAttribute' を使用する必要がある可能性があります。</target>
Expand Down
Loading
Loading