Skip to content
Open
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
16 changes: 14 additions & 2 deletions src/Analyzers/MSTest.Analyzers/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -323,10 +323,22 @@ The type declaring these methods should also respect the following rules:
<value>Test classes should have valid layout</value>
</data>
<data name="TestClassShouldHaveTestMethodDescription" xml:space="preserve">
<value>Test class should have at least one test method or be 'static' with method(s) marked by '[AssemblyInitialize]' and/or '[AssemblyCleanup]'.</value>
<value>Test classes (classes marked with '[TestClass]' attribute) should contain at least one test method.

A test method is any method marked with '[TestMethod]' or an attribute that derives from it (for example, custom test method attributes). The analyzer also considers test methods defined in base classes.

Exception: A 'static' test class is valid without test methods if it contains at least one method marked with '[AssemblyInitialize]', '[AssemblyCleanup]', '[GlobalTestInitialize]', or '[GlobalTestCleanup]'.

Common scenarios where this rule triggers:
- The class was intended to be a base class containing shared test setup but was incorrectly marked with '[TestClass]'. Consider removing the '[TestClass]' attribute from base classes.
- Test methods were accidentally removed or commented out.
- The class contains only helper methods and should not be marked as a test class.</value>
</data>
<data name="TestClassShouldHaveTestMethodMessageFormat" xml:space="preserve">
<value>Test class '{0}' should have at least one test method or be 'static' with method(s) marked by '[AssemblyInitialize]' and/or '[AssemblyCleanup]'</value>
<value>Test class '{0}' does not contain any test method (method marked with '[TestMethod]'). Add a test method, or remove '[TestClass]' if this is a base class or not intended to be a test class.</value>
</data>
<data name="TestClassShouldHaveTestMethodMessageFormat_BaseClassHasAssemblyAttributes" xml:space="preserve">
<value>Test class '{0}' does not contain any test method (method marked with '[TestMethod]'), but base class '{1}' contains assembly-level attributes. Make the class 'static' if it is only intended to contain assembly-level initialization or cleanup methods, or add a test method.</value>
</data>
<data name="TestClassShouldHaveTestMethodTitle" xml:space="preserve">
<value>Test class should have test method</value>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,13 @@ public sealed class TestClassShouldHaveTestMethodAnalyzer : DiagnosticAnalyzer
DiagnosticSeverity.Info,
isEnabledByDefault: true);

/// <inheritdoc cref="Resources.TestClassShouldHaveTestMethodMessageFormat_BaseClassHasAssemblyAttributes" />
public static readonly DiagnosticDescriptor TestClassShouldHaveTestMethodRuleBaseClassHasAssemblyAttributes = TestClassShouldHaveTestMethodRule
.WithMessage(new LocalizableResourceString(nameof(Resources.TestClassShouldHaveTestMethodMessageFormat_BaseClassHasAssemblyAttributes), Resources.ResourceManager, typeof(Resources)));

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

/// <inheritdoc />
public override void Initialize(AnalysisContext context)
Expand Down Expand Up @@ -78,10 +82,13 @@ private static void AnalyzeSymbol(SymbolAnalysisContext context, INamedTypeSymbo
return;
}

bool hasAssemblyAttribute = false;
bool hasAssemblyAttributeInCurrentClass = false;
bool hasAssemblyAttributeInBaseClass = false;
INamedTypeSymbol? baseClassWithAssemblyAttribute = null;
bool hasTestMethod = false;

INamedTypeSymbol? currentType = classSymbol;
bool isCurrentClass = true;
do
{
foreach (ISymbol classMember in currentType.GetMembers())
Expand All @@ -98,18 +105,44 @@ private static void AnalyzeSymbol(SymbolAnalysisContext context, INamedTypeSymbo
|| SymbolEqualityComparer.Default.Equals(attribute.AttributeClass, globalTestInitializeAttributeSymbol)
|| SymbolEqualityComparer.Default.Equals(attribute.AttributeClass, globalTestCleanupAttributeSymbol))
{
hasAssemblyAttribute = true;
if (isCurrentClass)
{
hasAssemblyAttributeInCurrentClass = true;
}
else
{
hasAssemblyAttributeInBaseClass = true;
baseClassWithAssemblyAttribute ??= currentType;
}
}
}
}

currentType = currentType.BaseType;
isCurrentClass = false;
}
while (currentType is not null);

if (!hasTestMethod && (!classSymbol.IsStatic || (classSymbol.IsStatic && !hasAssemblyAttribute)))
if (hasTestMethod)
{
return;
}

// Static class with assembly attributes in the current class is valid
if (classSymbol.IsStatic && hasAssemblyAttributeInCurrentClass)
{
return;
}

// Non-static class that inherits assembly attributes from base class - suggest making it static
if (!classSymbol.IsStatic && hasAssemblyAttributeInBaseClass && baseClassWithAssemblyAttribute is not null)
{
context.ReportDiagnostic(classSymbol.CreateDiagnostic(TestClassShouldHaveTestMethodRule, classSymbol.Name));
context.ReportDiagnostic(classSymbol.CreateDiagnostic(TestClassShouldHaveTestMethodRuleBaseClassHasAssemblyAttributes, classSymbol.Name, baseClassWithAssemblyAttribute.Name));

return;
}
Comment on lines +137 to 143
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is really addressing the original reported complaint.

Per the description in the linked DC ticket, I think a repro would be something like this:

[TestClass]
public abstract class TestClass
{
    [AssemblyInitialize]
    public static void M(TestContext _) { }
}

So, asm init is in the same class being analyzed. Given that the class doesn't have any non-static members, the correct action user needs to take is to mark the class static instead of abstract, and they don't need to inherit from it anywhere.

A different scenario to consider in this case is when the abstract class have other non-static members. In this case, we might have two options:

  1. Not report a diagnostic at all and consider it okay.
  2. Report a diagnostic and suggest that the fixtures get moved to a different class that is marked as static.


// All other cases: class without test methods
context.ReportDiagnostic(classSymbol.CreateDiagnostic(TestClassShouldHaveTestMethodRule, classSymbol.Name));
}
}
22 changes: 18 additions & 4 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.cs.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,11 @@ Typ deklarující tyto metody by měl také respektovat následující pravidla:
<target state="translated">Testovací třída {0} by měla být platná.</target>
<note />
</trans-unit>
<trans-unit id="TestClassShouldHaveTestMethodMessageFormat_BaseClassHasAssemblyAttributes">
<source>Test class '{0}' does not contain any test method (method marked with '[TestMethod]'), but base class '{1}' contains assembly-level attributes. Make the class 'static' if it is only intended to contain assembly-level initialization or cleanup methods, or add a test method.</source>
<target state="new">Test class '{0}' does not contain any test method (method marked with '[TestMethod]'), but base class '{1}' contains assembly-level attributes. Make the class 'static' if it is only intended to contain assembly-level initialization or cleanup methods, or add a test method.</target>
<note />
</trans-unit>
<trans-unit id="TestContextShouldBeValidMessageFormat">
<source>Property 'TestContext' should be valid</source>
<target state="translated">Vlastnost TestContext by měla být platná.</target>
Expand Down Expand Up @@ -704,13 +709,22 @@ Typ deklarující tyto metody by měl také respektovat následující pravidla:
<note />
</trans-unit>
<trans-unit id="TestClassShouldHaveTestMethodDescription">
<source>Test class should have at least one test method or be 'static' with method(s) marked by '[AssemblyInitialize]' and/or '[AssemblyCleanup]'.</source>
<target state="translated">Testovací třída by měla mít aspoň jednu testovací metodu nebo by měla být static s metodami s označením [AssemblyInitialize] a/nebo [AssemblyCleanup].</target>
<source>Test classes (classes marked with '[TestClass]' attribute) should contain at least one test method.

A test method is any method marked with '[TestMethod]' or an attribute that derives from it (for example, custom test method attributes). The analyzer also considers test methods defined in base classes.

Exception: A 'static' test class is valid without test methods if it contains at least one method marked with '[AssemblyInitialize]', '[AssemblyCleanup]', '[GlobalTestInitialize]', or '[GlobalTestCleanup]'.

Common scenarios where this rule triggers:
- The class was intended to be a base class containing shared test setup but was incorrectly marked with '[TestClass]'. Consider removing the '[TestClass]' attribute from base classes.
- Test methods were accidentally removed or commented out.
- The class contains only helper methods and should not be marked as a test class.</source>
<target state="needs-review-translation">Testovací třída by měla mít aspoň jednu testovací metodu nebo by měla být static s metodami s označením [AssemblyInitialize] a/nebo [AssemblyCleanup].</target>
<note />
</trans-unit>
<trans-unit id="TestClassShouldHaveTestMethodMessageFormat">
<source>Test class '{0}' should have at least one test method or be 'static' with method(s) marked by '[AssemblyInitialize]' and/or '[AssemblyCleanup]'</source>
<target state="translated">Testovací třída {0} by měla mít aspoň jednu testovací metodu nebo by měla být static s metodami s označením [AssemblyInitialize] a/nebo [AssemblyCleanup].</target>
<source>Test class '{0}' does not contain any test method (method marked with '[TestMethod]'). Add a test method, or remove '[TestClass]' if this is a base class or not intended to be a test class.</source>
<target state="needs-review-translation">Testovací třída {0} by měla mít aspoň jednu testovací metodu nebo by měla být static s metodami s označením [AssemblyInitialize] a/nebo [AssemblyCleanup].</target>
<note />
</trans-unit>
<trans-unit id="TestClassShouldHaveTestMethodTitle">
Expand Down
22 changes: 18 additions & 4 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.de.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,11 @@ Der Typ, der diese Methoden deklariert, sollte auch die folgenden Regeln beachte
<target state="translated">Die Testklasse „{0}“ muss gültig sein</target>
<note />
</trans-unit>
<trans-unit id="TestClassShouldHaveTestMethodMessageFormat_BaseClassHasAssemblyAttributes">
<source>Test class '{0}' does not contain any test method (method marked with '[TestMethod]'), but base class '{1}' contains assembly-level attributes. Make the class 'static' if it is only intended to contain assembly-level initialization or cleanup methods, or add a test method.</source>
<target state="new">Test class '{0}' does not contain any test method (method marked with '[TestMethod]'), but base class '{1}' contains assembly-level attributes. Make the class 'static' if it is only intended to contain assembly-level initialization or cleanup methods, or add a test method.</target>
<note />
</trans-unit>
<trans-unit id="TestContextShouldBeValidMessageFormat">
<source>Property 'TestContext' should be valid</source>
<target state="translated">Die Eigenschaft „TestContext“ muss gültig sein</target>
Expand Down Expand Up @@ -705,13 +710,22 @@ Der Typ, der diese Methoden deklariert, sollte auch die folgenden Regeln beachte
<note />
</trans-unit>
<trans-unit id="TestClassShouldHaveTestMethodDescription">
<source>Test class should have at least one test method or be 'static' with method(s) marked by '[AssemblyInitialize]' and/or '[AssemblyCleanup]'.</source>
<target state="translated">Die Testklasse muss mindestens eine Testmethode aufweisen oder "statisch" sein, wenn Methoden mit "[AssemblyInitialize]" und/oder "[AssemblyCleanup]" markiert sind.</target>
<source>Test classes (classes marked with '[TestClass]' attribute) should contain at least one test method.

A test method is any method marked with '[TestMethod]' or an attribute that derives from it (for example, custom test method attributes). The analyzer also considers test methods defined in base classes.

Exception: A 'static' test class is valid without test methods if it contains at least one method marked with '[AssemblyInitialize]', '[AssemblyCleanup]', '[GlobalTestInitialize]', or '[GlobalTestCleanup]'.

Common scenarios where this rule triggers:
- The class was intended to be a base class containing shared test setup but was incorrectly marked with '[TestClass]'. Consider removing the '[TestClass]' attribute from base classes.
- Test methods were accidentally removed or commented out.
- The class contains only helper methods and should not be marked as a test class.</source>
<target state="needs-review-translation">Die Testklasse muss mindestens eine Testmethode aufweisen oder "statisch" sein, wenn Methoden mit "[AssemblyInitialize]" und/oder "[AssemblyCleanup]" markiert sind.</target>
<note />
</trans-unit>
<trans-unit id="TestClassShouldHaveTestMethodMessageFormat">
<source>Test class '{0}' should have at least one test method or be 'static' with method(s) marked by '[AssemblyInitialize]' and/or '[AssemblyCleanup]'</source>
<target state="translated">Die Testklasse "{0}" muss mindestens eine Testmethode aufweisen oder "statisch" sein, wenn Methoden mit "[AssemblyInitialize]" und/oder "[AssemblyCleanup]" markiert sind.</target>
<source>Test class '{0}' does not contain any test method (method marked with '[TestMethod]'). Add a test method, or remove '[TestClass]' if this is a base class or not intended to be a test class.</source>
<target state="needs-review-translation">Die Testklasse "{0}" muss mindestens eine Testmethode aufweisen oder "statisch" sein, wenn Methoden mit "[AssemblyInitialize]" und/oder "[AssemblyCleanup]" markiert sind.</target>
<note />
</trans-unit>
<trans-unit id="TestClassShouldHaveTestMethodTitle">
Expand Down
22 changes: 18 additions & 4 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.es.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,11 @@ El tipo que declara estos métodos también debe respetar las reglas siguientes:
<target state="translated">La clase de prueba '{0}' debe ser válida</target>
<note />
</trans-unit>
<trans-unit id="TestClassShouldHaveTestMethodMessageFormat_BaseClassHasAssemblyAttributes">
<source>Test class '{0}' does not contain any test method (method marked with '[TestMethod]'), but base class '{1}' contains assembly-level attributes. Make the class 'static' if it is only intended to contain assembly-level initialization or cleanup methods, or add a test method.</source>
<target state="new">Test class '{0}' does not contain any test method (method marked with '[TestMethod]'), but base class '{1}' contains assembly-level attributes. Make the class 'static' if it is only intended to contain assembly-level initialization or cleanup methods, or add a test method.</target>
<note />
</trans-unit>
<trans-unit id="TestContextShouldBeValidMessageFormat">
<source>Property 'TestContext' should be valid</source>
<target state="translated">La propiedad 'TestContext' debe ser válida</target>
Expand Down Expand Up @@ -704,13 +709,22 @@ El tipo que declara estos métodos también debe respetar las reglas siguientes:
<note />
</trans-unit>
<trans-unit id="TestClassShouldHaveTestMethodDescription">
<source>Test class should have at least one test method or be 'static' with method(s) marked by '[AssemblyInitialize]' and/or '[AssemblyCleanup]'.</source>
<target state="translated">La clase de prueba debe tener al menos un método de prueba o ser "static" con métodos marcados por "[AssemblyInitialize]" o "[AssemblyCleanup]".</target>
<source>Test classes (classes marked with '[TestClass]' attribute) should contain at least one test method.

A test method is any method marked with '[TestMethod]' or an attribute that derives from it (for example, custom test method attributes). The analyzer also considers test methods defined in base classes.

Exception: A 'static' test class is valid without test methods if it contains at least one method marked with '[AssemblyInitialize]', '[AssemblyCleanup]', '[GlobalTestInitialize]', or '[GlobalTestCleanup]'.

Common scenarios where this rule triggers:
- The class was intended to be a base class containing shared test setup but was incorrectly marked with '[TestClass]'. Consider removing the '[TestClass]' attribute from base classes.
- Test methods were accidentally removed or commented out.
- The class contains only helper methods and should not be marked as a test class.</source>
<target state="needs-review-translation">La clase de prueba debe tener al menos un método de prueba o ser "static" con métodos marcados por "[AssemblyInitialize]" o "[AssemblyCleanup]".</target>
<note />
</trans-unit>
<trans-unit id="TestClassShouldHaveTestMethodMessageFormat">
<source>Test class '{0}' should have at least one test method or be 'static' with method(s) marked by '[AssemblyInitialize]' and/or '[AssemblyCleanup]'</source>
<target state="translated">La clase de prueba "{0}" debe tener al menos un método de prueba o ser "static" con métodos marcados por "[AssemblyInitialization]" o "[AssemblyCleanup]"</target>
<source>Test class '{0}' does not contain any test method (method marked with '[TestMethod]'). Add a test method, or remove '[TestClass]' if this is a base class or not intended to be a test class.</source>
<target state="needs-review-translation">La clase de prueba "{0}" debe tener al menos un método de prueba o ser "static" con métodos marcados por "[AssemblyInitialization]" o "[AssemblyCleanup]"</target>
<note />
</trans-unit>
<trans-unit id="TestClassShouldHaveTestMethodTitle">
Expand Down
Loading