Detect inherited fixtures/tests from a different MSTest version (MSTEST0082)

[Jakub Jareš (nohwnd)]

Fixes the silent v3/v4 inheritance breakage from #10505 by detecting it at build time.

The problem

When a [TestClass] compiled against MSTest v4 inherits a [TestInitialize] / [TestMethod] / … from a base class compiled against v3, the inherited member does not run and inherited test methods are not discovered — with no build error and no discovery error. The framework assembly was renamed:

MSTest	framework assembly
v3	Microsoft.VisualStudio.TestPlatform.TestFramework
v4	MSTest.TestFramework

so the attribute baked into the v3-compiled base is a different CLR type identity than the one the v4 adapter matches, and it is silently ignored.

The rule

InheritedMemberFromDifferentMSTestVersionAnalyzer (MSTEST0082, Usage, Warning). For a test class it walks the base types and warns when an inherited lifecycle or test-method attribute resolves to a framework assembly whose name differs from the one the test project compiles against. Assembly identity is matched by name, so a benign version bump of the same framework is not flagged — only the rename is.

One rule covers the six inherited attributes (TestInitialize, TestCleanup, ClassInitialize, ClassCleanup, TestMethod, DataTestMethod), with the attribute name passed as a message argument. AssemblyInitialize/AssemblyCleanup are excluded: assembly fixtures are discovered per-assembly, never inherited, so recompiling the base cannot make them run.

It only warns when recompiling the base would actually fix the problem:

• both the framework assembly and each inherited attribute are resolved through the canonical MSTest attribute in the applied attribute's base chain, so custom [TestClass] / [TestMethod] subclasses are handled — no false positive on a custom test-class attribute, and a custom [RetryTest : TestMethodAttribute] from a v3 base is still caught;
• the member must be one MSTest would run/discover: public, correct static-ness, valid signature (parameterless instance fixtures, TestContext-shaped class fixtures, void/Task/ValueTask returns), class fixtures only with InheritanceBehavior.BeforeEachDerivedClass, and members overridden or hidden with new in a more-derived type are suppressed.

It's a SymbolKind.NamedType action that early-outs via IsTestClass and skips same-assembly base types.

Tests

15 unit tests. Warns: inherited [TestInitialize], inherited [TestMethod], a custom [RetryTest : TestMethodAttribute] subclass, an inheritable [ClassInitialize], and the realistic graph where the old framework is reached only through the base-library metadata (not a direct reference of the test project). Silent: same-version cross-assembly base, intra-assembly inheritance, custom [TestClass] with a same-version inherited fixture, private / overridden / new-hidden / invalid-signature members, non-inheritable [ClassInitialize], [AssemblyInitialize], and a non-test-class derived type.

🤖

[Copilot]

Pull request overview

Adds MSTEST0082 to detect inherited MSTest fixtures or tests compiled against a differently named framework assembly.

Changes:

• Implements cross-version inherited-member analysis.
• Adds analyzer tests and release tracking.
• Adds localized diagnostic resources.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/Analyzers/MSTest.Analyzers/InheritedMemberFromDifferentMSTestVersionAnalyzer.cs	Implements MSTEST0082.
src/Analyzers/MSTest.Analyzers/Helpers/DiagnosticIds.cs	Registers the rule ID.
src/Analyzers/MSTest.Analyzers/AnalyzerReleases.Unshipped.md	Tracks the new rule.
src/Analyzers/MSTest.Analyzers/Resources.resx	Adds diagnostic text.
src/Analyzers/MSTest.Analyzers/xlf/Resources.cs.xlf	Adds Czech localization units.
src/Analyzers/MSTest.Analyzers/xlf/Resources.de.xlf	Adds German localization units.
src/Analyzers/MSTest.Analyzers/xlf/Resources.es.xlf	Adds Spanish localization units.
src/Analyzers/MSTest.Analyzers/xlf/Resources.fr.xlf	Adds French localization units.
src/Analyzers/MSTest.Analyzers/xlf/Resources.it.xlf	Adds Italian localization units.
src/Analyzers/MSTest.Analyzers/xlf/Resources.ja.xlf	Adds Japanese localization units.
src/Analyzers/MSTest.Analyzers/xlf/Resources.ko.xlf	Adds Korean localization units.
src/Analyzers/MSTest.Analyzers/xlf/Resources.pl.xlf	Adds Polish localization units.
src/Analyzers/MSTest.Analyzers/xlf/Resources.pt-BR.xlf	Adds Portuguese localization units.
src/Analyzers/MSTest.Analyzers/xlf/Resources.ru.xlf	Adds Russian localization units.
src/Analyzers/MSTest.Analyzers/xlf/Resources.tr.xlf	Adds Turkish localization units.
src/Analyzers/MSTest.Analyzers/xlf/Resources.zh-Hans.xlf	Adds Simplified Chinese units.
src/Analyzers/MSTest.Analyzers/xlf/Resources.zh-Hant.xlf	Adds Traditional Chinese units.
test/UnitTests/MSTest.Analyzers.UnitTests/InheritedMemberFromDifferentMSTestVersionAnalyzerTests.cs	Tests core diagnostic scenarios.

[Amaury Levé (Evangelink)]

Additional review notes after validating the analyzer against MSTest's custom-attribute extensibility. I am submitting these as comments only—not requesting changes—so the PR is not rejected or blocked while the author is away.

[Jakub Jareš (nohwnd)]

Addressed the automated review as well (3998b0c):

• Dropped [AssemblyInitialize]/[AssemblyCleanup] — assembly fixtures are discovered per-assembly, never inherited, so recompiling the base cannot make them run.
• [ClassInitialize]/[ClassCleanup] now warn only when the attribute specifies InheritanceBehavior.BeforeEachDerivedClass; the default None does not flow to derived classes even in the same version.
• Filtered to the effective inherited-member set: public instance test methods and instance fixtures (overridden base lifecycle methods are suppressed), and public static class fixtures.
• Added a test for the unresolved/missing framework graph — the framework is compiled separately and omitted from the consumer references, so the base attribute is reached as a missing type through the base library metadata.
• The rule description now lists [DataTestMethod], and same-assembly base types are skipped.

13 tests, all green.

🤖

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/Analyzers/MSTest.Analyzers/InheritedMemberFromDifferentMSTestVersionAnalyzer.cs:190

• This check does not establish that the member would actually run after recompilation. It only checks accessibility/staticness, while MSTest also rejects instance fixtures with parameters or invalid return types and class fixtures without the required TestContext shape (see MethodInfoExtensions.cs:20-30,59-66); test methods with invalid return types are likewise skipped (TestMethodValidator.cs:52-73). Such members will now receive MSTEST0082 even though the prescribed recompilation cannot make them run. Please validate the complete per-kind signature before reporting.

    private static bool WouldRunOrBeDiscoveredIfSameVersion(IMethodSymbol method, MSTestMemberKind kind, AttributeData attribute, INamedTypeSymbol testClass, INamedTypeSymbol declaringBaseType)

src/Analyzers/MSTest.Analyzers/InheritedMemberFromDifferentMSTestVersionAnalyzer.cs:197

• Checking only overrides misses members hidden with new. MSTest suppresses an inherited initialize/cleanup whenever a valid method with the same name was seen in a more-derived type (TypeCache.ClassInfo.cs:314-316,341-343), and CLR method enumeration similarly hides a same-signature base test method. MSTEST0082 therefore warns even though recompiling the base cannot restore that member. Account for hidden declarations as well as override chains.

                    && !IsOverriddenInDerivedType(method, testClass, declaringBaseType),

src/Analyzers/MSTest.Analyzers/InheritedMemberFromDifferentMSTestVersionAnalyzer.cs:186

• The PR description says this rule covers all eight listed attributes, including AssemblyInitialize and AssemblyCleanup, but this mapping explicitly excludes both and the new test asserts no diagnostic. Since the exclusion matches the adapter's per-assembly discovery model, update the PR description to describe the six supported inherited attributes rather than promising eight.

        // AssemblyInitialize/AssemblyCleanup are intentionally excluded: they are discovered per-assembly, not
        // inherited, so a base library's assembly fixture never runs on a derived test class even after recompilation.

test/UnitTests/MSTest.Analyzers.UnitTests/InheritedMemberFromDifferentMSTestVersionAnalyzerTests.cs:235

• This positive case uses an invalid class-initialize signature: MSTest requires exactly one TestContext parameter (MethodInfoExtensions.cs:20-30). Even with a same-version attribute, this method would be rejected, so the test currently codifies the analyzer's false positive instead of proving the inheritance scenario. Define the legacy TestContext type and pass it here.

                    public static void BaseClassInitialize() { }

[Jakub Jareš (nohwnd)]

Second pass (8555a7f), addressing the follow-up review:

• Full per-kind signature validation before reporting: parameterless instance fixtures, TestContext-shaped class fixtures, void/Task/ValueTask returns, correct static-ness. A member with a mismatched attribute but an invalid signature (which would not run even after recompiling) is no longer flagged.
• new-hidden members are now suppressed as well as overrides — MSTest suppresses an inherited member whenever a more-derived type declares one with the same name.
• Fixed the [ClassInitialize] positive test to use a valid signature (a TestContext parameter) so it demonstrates a real scenario rather than an already-invalid member; added tests for a new-hidden member and an invalid-signature fixture.
• GetFrameworkAssembly and HasBeforeEachDerivedClassBehavior converted to LINQ per the code-quality analyzer.
• PR description corrected to six inherited attributes (assembly fixtures excluded).

15 tests, all green; analyzer builds clean (0 warnings).

🤖

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/Analyzers/MSTest.Analyzers/InheritedMemberFromDifferentMSTestVersionAnalyzer.cs:244

• Checking only the method name suppresses real diagnostics. For example, a derived BaseInitialize(int) does not suppress a valid parameterless base initializer (the adapter only records same-name lifecycle methods with valid fixture signatures), and a differently-shaped test-method overload does not hide the inherited test. This helper nevertheless returns true for either case, so the mismatched base attribute is missed. Please apply the adapter's kind-specific hiding/signature rules rather than treating every same-name method as a replacement.

            if (type.GetMembers(baseMethod.Name).Any(member => member is IMethodSymbol))
            {

src/Analyzers/MSTest.Analyzers/InheritedMemberFromDifferentMSTestVersionAnalyzer.cs:217

• This return-type check accepts Task<T>/ValueTask<T> because Roslyn reports their names as Task/ValueTask, and it accepts async void through ReturnsVoid. The adapter rejects all three shapes, so this rule can warn even though recompiling the base would not make the member runnable. Please use exact Task/ValueTask symbol equality and exclude async void, as FixtureUtils.HasValidTestMethodSignature/HasValidFixtureMethodSignature already do.

        => method.ReturnsVoid
            || (method.ReturnType.ContainingNamespace is { } containingNamespace
                && string.Equals(containingNamespace.ToDisplayString(), "System.Threading.Tasks", StringComparison.Ordinal)
                && (string.Equals(method.ReturnType.Name, "Task", StringComparison.Ordinal)
                    || string.Equals(method.ReturnType.Name, "ValueTask", StringComparison.Ordinal)));

src/Analyzers/MSTest.Analyzers/InheritedMemberFromDifferentMSTestVersionAnalyzer.cs:207

• HasClassFixtureParameterShape allows zero parameters for both class fixture kinds, but the adapter requires [ClassInitialize] to take exactly one TestContext; only [ClassCleanup] permits zero or one. A zero-argument legacy class initializer therefore receives MSTEST0082 even though recompilation cannot make it run. Please retain whether the canonical attribute is initialize or cleanup and validate each parameter shape separately.

                MSTestMemberKind.ClassFixture
                    => method.IsStatic
                        && HasClassFixtureParameterShape(method)
                        && ReturnsVoidTaskOrValueTask(method)
                        && HasBeforeEachDerivedClassBehavior(attribute),

src/Analyzers/MSTest.Analyzers/InheritedMemberFromDifferentMSTestVersionAnalyzer.cs:181

• The PR description says MSTEST0082 covers all eight attributes, including AssemblyInitialize and AssemblyCleanup, while this switch intentionally supports only the six inheritable class/member attributes. Please align the stated scope with the implementation, or add the intended assembly-fixture detection path if those two attributes are still required.

        // AssemblyInitialize/AssemblyCleanup are intentionally excluded: they are discovered per-assembly, not
        // inherited, so a base library's assembly fixture never runs on a derived test class even after recompilation.
        _ => MSTestMemberKind.None,

[Amaury Levé (Evangelink)]

Fresh review of the latest head. The current automated review already captures the method-hiding, return-type, and ClassInitialize parameter-shape mismatches, so I am not duplicating those inline. This additional comment covers the remaining unresolved-metadata case. Comment-only review; no rejection or request-changes state.

[Amaury Levé (Evangelink)]

Fresh code + MSTest expert review (8555a7ff)

I ran two independent reviews of the complete latest diff. They converged on the same four open semantic gaps and found no additional correctness, concurrency, localization, API, or performance issues. The PR head has not changed since the previous review, so I am consolidating the result here rather than duplicating the existing inline threads.

1. Missing legacy PE metadata can hide inherited class fixtures. HasBeforeEachDerivedClassBehavior depends on decoded AttributeData.ConstructorArguments. In the real three-PE graph (legacy framework PE → base-library PE → consumer omitting the legacy framework), Roslyn retains the missing attribute identity but cannot bind its constructor, leaving the arguments empty. An explicit BeforeEachDerivedClass class initialize/cleanup therefore escapes MSTEST0082. The current project-reference test covers only [TestInitialize] and preserves dependency symbols. Add an emitted-PE test with MetadataReference.CreateFromImage; complete support requires metadata-blob decoding, otherwise document/limit this scope. Existing thread: InheritedMemberFromDifferentMSTestVersionAnalyzer.cs:228.

2. Any same-name derived method suppresses a real warning. IsHiddenOrOverriddenInDerivedType treats every derived method with the same name as a replacement. The adapter is kind-specific: an invalid/private Initialize(int) does not suppress base Initialize(), different-signature test overloads remain discoverable, and class fixtures have no ordinary name-based suppression. Make hiding signature- and member-kind-aware and add C#/VB coverage for each case. Existing current-head automated finding: line 244.

3. Invalid return types pass analyzer validation. Name-only matching accepts Task<T> and ValueTask<T>, while ReturnsVoid also accepts async void/VB Async Sub. The adapter requires exact non-generic Task/ValueTask and rejects asynchronous void. Resolve the task symbols at compilation start, compare with SymbolEqualityComparer.Default, and require !method.IsAsync for void. Existing current-head automated finding: line 217.

4. ClassInitialize and ClassCleanup parameter rules are conflated. The shared ClassFixture branch allows zero or one TestContext, but initialize requires exactly one while cleanup permits zero or one. Preserve distinct member kinds and add a no-diagnostic test for parameterless legacy ClassInitialize. Existing current-head automated finding: line 207.

Previously reported custom [TestClass], custom TestMethodAttribute, assembly-fixture, [DataTestMethod], private/override, and PR-scope issues are fixed. CI is green, but it does not exercise the four cases above.

This is a comment-only review: no rejection or request-changes state.

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/Analyzers/MSTest.Analyzers/InheritedMemberFromDifferentMSTestVersionAnalyzer.cs:226

• This accepts async void, Task<T>, and ValueTask<T> because it checks only ReturnsVoid or the return type's namespace/name. MSTest rejects all three (FixtureUtils.cs:74-76,140-143; TestMethodValidator.cs:55-67), so MSTEST0082 can claim that recompilation fixes a member that would remain invalid. Require non-async void and arity-zero Task/ValueTask.

        => method.ReturnsVoid
            || (method.ReturnType.ContainingNamespace is { } containingNamespace
                && string.Equals(containingNamespace.ToDisplayString(), "System.Threading.Tasks", StringComparison.Ordinal)
                && (string.Equals(method.ReturnType.Name, "Task", StringComparison.Ordinal)
                    || string.Equals(method.ReturnType.Name, "ValueTask", StringComparison.Ordinal)));

src/Analyzers/MSTest.Analyzers/InheritedMemberFromDifferentMSTestVersionAnalyzer.cs:255

• This name-only suppression does not match MSTest and causes false negatives. Instance fixtures are suppressed only when the derived same-name method itself has a valid fixture signature (TypeCache.ClassInfo.cs:312-317), class fixtures are accumulated independently (TypeCache.AssemblyInfo.cs:122-139), and test-method hiding is signature-based. Thus a private/static/parameterized same-name method—or any same-name class fixture—currently hides MSTEST0082 even though recompiling the base changes execution. Make this check member-kind-aware and mirror each adapter path's hiding rules.

            if (type.GetMembers(baseMethod.Name).Any(member => member is IMethodSymbol))

src/Analyzers/MSTest.Analyzers/InheritedMemberFromDifferentMSTestVersionAnalyzer.cs:214

• ClassInitialize and ClassCleanup cannot share this parameter check. MSTest requires ClassInitialize to take exactly one TestContext (MethodInfoExtensions.cs:20-30 and ClassInitializeShouldBeValidAnalyzer.cs:37-41), whereas only ClassCleanup may be parameterless. As written, a parameterless legacy [ClassInitialize] receives MSTEST0082 even though recompiling it would still leave an invalid fixture. Preserve the canonical initialize/cleanup kind and apply their distinct parameter rules.

This issue also appears in the following locations of the same file:

• line 222
• line 255

                        && HasClassFixtureParameterShape(method)

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/Analyzers/MSTest.Analyzers/InheritedMemberFromDifferentMSTestVersionAnalyzer.cs:169

• This visibility check accepts protected and protected-internal nested classes because GetResultantVisibility() treats them as public, but the adapter rejects those classes even with DiscoverInternals. This produces MSTEST0082 for a class that discovery will never run. Mirror TypeValidator.TypeHasValidAccessibility by requiring every containing type to be public, or exactly internal when internals discovery is enabled (as DependsOnShouldBeValidAnalyzer.HasValidAccessibility already does).

        SymbolVisibility resultantVisibility = classSymbol.GetResultantVisibility();
        return canDiscoverInternals
            ? resultantVisibility != SymbolVisibility.Private
            : resultantVisibility == SymbolVisibility.Public;

src/Analyzers/MSTest.Analyzers/InheritedMemberFromDifferentMSTestVersionAnalyzer.cs:362

• A same-signature new declaration does not by itself suppress the base test. TypeEnumerator.GetTests de-duplicates only after the derived method passes IsValidTestMethod; therefore the added public new void Run() without [TestMethod] still leaves the recompiled base [TestMethod] discoverable. This branch currently suppresses MSTEST0082 for that case (and for private/otherwise non-test declarations). Require the derived declaration to be a discoverable current-version test before treating it as hiding the base test.

                    // Test-method hiding is signature-based: only a same-signature method shadows the inherited test;
                    // a different-arity, by-ref, or otherwise different-signature overload stays discoverable.
                    case MSTestMemberKind.TestMethod when HaveSameSignature(derivedMethod, baseMethod):
                        return true;

src/Analyzers/MSTest.Analyzers/InheritedMemberFromDifferentMSTestVersionAnalyzer.cs:399

• SymbolEqualityComparer treats method type parameters from separate declarations as different symbols, although their names are not part of the CLR signature. Consequently Run<T>(T) and a derived current-version [TestMethod] Run<U>(U) are considered different here, and MSTEST0082 is reported even though the derived test hides the base test. Normalize method type parameters by ordinal when comparing parameter types; DependsOnShouldBeValidAnalyzer.BuildSignatureKey already handles this case.

            // ref/out/in all share the same by-ref CLR signature, so compare by-value versus by-ref rather than the
            // exact RefKind (a derived 'Run(out int)' still hides a base 'Run(ref int)').
            if ((leftParameter.RefKind == RefKind.None) != (rightParameter.RefKind == RefKind.None)
                || !SymbolEqualityComparer.Default.Equals(leftParameter.Type, rightParameter.Type))
            {

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/Analyzers/MSTest.Analyzers/InheritedMemberFromDifferentMSTestVersionAnalyzer.cs:368

• This comparison does not recognize alpha-equivalent generic signatures: Run<T>(T) and Run<U>(U) hide one another in CLR metadata, but their parameter type symbols are unequal, so the analyzer reports the inherited method even though recompiling it would not make it discoverable. DependsOnShouldBeValidAnalyzer.BuildSignatureKey already normalizes method type parameters by ordinal (lines 833-907); extract/reuse that signature logic here and add this renamed-type-parameter case.

            // ref/out/in all share the same by-ref CLR signature, so compare by-value versus by-ref rather than the
            // exact RefKind (a derived 'Run(out int)' still hides a base 'Run(ref int)').
            if ((leftParameter.RefKind == RefKind.None) != (rightParameter.RefKind == RefKind.None)
                || !SymbolEqualityComparer.Default.Equals(leftParameter.Type, rightParameter.Type))
            {

src/Analyzers/MSTest.Analyzers/InheritedMemberFromDifferentMSTestVersionAnalyzer.cs:169

• GetResultantVisibility() treats nested protected and protected internal types as public, but MSTest discovery explicitly accepts only nested public types (or nested internal types with DiscoverInternals) and rejects those protected variants (TypeValidator.cs:165-178, covered by TypeValidatorTests.cs:353-370). This therefore emits MSTEST0082 for classes the adapter never discovers. Check the exact accessibility of the class and each containing type instead.

    private static bool IsDiscoverableTestClassVisibility(INamedTypeSymbol classSymbol, bool canDiscoverInternals)
    {
        SymbolVisibility resultantVisibility = classSymbol.GetResultantVisibility();
        return canDiscoverInternals
            ? resultantVisibility != SymbolVisibility.Private
            : resultantVisibility == SymbolVisibility.Public;

[Amaury Levé (Evangelink)]

Fresh review of the latest head (53457772) using independent code and MSTest-expert passes plus a direct CLR reflection probe. The existing current-head automated review already records the protected nested-class visibility issue, so this review adds only the new signature-hiding defect below. Comment-only; no rejection or request-changes state.

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/Analyzers/MSTest.Analyzers/InheritedMemberFromDifferentMSTestVersionAnalyzer.cs:363

• With [assembly: DiscoverInternals], the adapter treats an internal [TestMethod] as discoverable (TestMethodValidator.cs:52-58). This public-only check therefore misses a valid derived new internal test method that de-duplicates and replaces the inherited base test, causing MSTEST0082 to be reported even though recompiling the base would not change discovery. Pass canDiscoverInternals into this path and apply the same accessibility rule as the adapter.

    private static bool IsDiscoverableTestMethod(IMethodSymbol method, IAssemblySymbol referenceAssembly, INamedTypeSymbol? taskSymbol, INamedTypeSymbol? valueTaskSymbol)
        => method is { DeclaredAccessibility: Accessibility.Public, IsStatic: false }
            && ReturnsVoidTaskOrValueTask(method, taskSymbol, valueTaskSymbol)

src/Analyzers/MSTest.Analyzers/InheritedMemberFromDifferentMSTestVersionAnalyzer.cs:392

• This compares parameter types by symbol identity, which is not equivalent to the CLR signature for generic methods. For example, derived Run<U>(U) and base Run<T>(T) have the same reflected signature and MSTest keeps the derived test, but T and U are distinct Roslyn symbols, so this returns false and emits a false-positive MSTEST0082. Normalize method type parameters by ordinal (and compare constructed types structurally), as DependsOnShouldBeValidAnalyzer.BuildSignatureKey already does.

            // ref/out/in all share the same by-ref CLR signature, so compare by-value versus by-ref rather than the
            // exact RefKind (a derived 'Run(out int)' still hides a base 'Run(ref int)').
            if ((leftParameter.RefKind == RefKind.None) != (rightParameter.RefKind == RefKind.None)
                || !SymbolEqualityComparer.Default.Equals(leftParameter.Type, rightParameter.Type))

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/Analyzers/MSTest.Analyzers/InheritedMemberFromDifferentMSTestVersionAnalyzer.cs:378

• With DiscoverInternals, GetResultantVisibility() != Private also admits protected, protected internal, and private protected methods. TestMethodValidator only discovers methods that are public or exactly internal, so such a derived method cannot hide the inherited test; returning true here suppresses the MSTEST0082 warning even though the inherited legacy test remains missing. Check DeclaredAccessibility against Public/Internal directly.

        SymbolVisibility resultantVisibility = method.GetResultantVisibility();
        return canDiscoverInternals
            ? resultantVisibility != SymbolVisibility.Private
            : resultantVisibility == SymbolVisibility.Public && method.DeclaredAccessibility == Accessibility.Public;

src/Analyzers/MSTest.Analyzers/InheritedMemberFromDifferentMSTestVersionAnalyzer.cs:169

• This visibility check treats nested protected, protected internal, and private protected classes as discoverable because GetResultantVisibility does not preserve those accessibilities. The adapter only accepts nested classes whose entire containing chain is public (or public/internal with DiscoverInternals; see TypeValidator.TypeHasValidAccessibility), so MSTEST0082 can warn on a class that will never be discovered. Mirror that exact containing-type accessibility check here.

This issue also appears on line 375 of the same file.

        SymbolVisibility resultantVisibility = classSymbol.GetResultantVisibility();
        return canDiscoverInternals
            ? resultantVisibility != SymbolVisibility.Private
            : resultantVisibility == SymbolVisibility.Public;

[github-actions]

🧵 Parallel-safety audit — PR #10508

Nothing audited here touches process-global state, shared filesystem paths, or
[ResourceLock] / [DoNotParallelize] declarations. Nothing to flag for
parallel-safety.

The only change is a new test file,
InheritedMemberFromDifferentMSTestVersionAnalyzerTests.cs, added to
MSTest.Analyzers.UnitTests. Its test methods are pure Roslyn analyzer
verifications (VerifyCS/VerifyVB against in-memory source strings) with no
filesystem I/O, no environment/culture/console mutation, and no static mutable
fields. The [TestInitialize]/[ClassInitialize]/[AssemblyInitialize]
occurrences visible in the diff are all inside embedded string literals
(simulated source code fed to the analyzer under test), not real test
lifecycle members of the test class itself — so they are out of scope for this
audit. No [ResourceLock] or [DoNotParallelize] declarations were added,
removed, or touched, and no .runsettings/testconfig.json/MSBuild
parallelization setting changed.

Audited MSTest.Analyzers.UnitTests at scope MethodLevel, workers CPU count (via [assembly: Parallelize(Scope = ExecutionScope.MethodLevel, Workers = 0)] in Program.cs, unchanged by this PR).

_{Re-run with /parallel-audit.}

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 44.5 AIC · ⌖ 4.66 AIC · ⊞ 24.8K · [◷]( · ◷)

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/Analyzers/MSTest.Analyzers/InheritedMemberFromDifferentMSTestVersionAnalyzer.cs:347

• GetRuntimeMethods delegates to Type.GetMethods(Everything) (ReflectionOperations.cs:68-69), which removes a base method hidden by a same-signature derived declaration before TestMethodValidator examines attributes. Requiring the derived method itself to be discoverable therefore produces a false MSTEST0082 for cases such as public new void Run() without [TestMethod]: the legacy base Run is absent both before and after recompilation. Treat any same-signature derived method as hiding the base method, and update the corresponding WhenInheritedTestMethodShadowedByNonTestMethod_Diagnostic expectation.

                    case MSTestMemberKind.TestMethod
                        when IsDiscoverableTestMethod(derivedMethod, referenceAssembly, canDiscoverInternals, taskSymbol, valueTaskSymbol)
                            && HaveSameSignature(derivedMethod, baseMethod):

[github-actions]

🧪 Expert test review — PR #10508

The only test file changed in this PR is a newly-added file: test/UnitTests/MSTest.Analyzers.UnitTests/InheritedMemberFromDifferentMSTestVersionAnalyzerTests.cs (47 new [TestMethod]-decorated async Task test methods, all using Microsoft.CodeAnalysis.Testing's VerifyCS/VerifyVB analyzer test harness).

Overall assessment: This is an exceptionally thorough and well-structured analyzer test suite. Each test:

• Has a clear, descriptive name (WhenX_Diagnostic / WhenX_NoDiagnostic) stating the exact scenario and expected outcome.
• Includes an explanatory comment describing why the scenario does or doesn't trigger the analyzer, tying back to the CLR semantics being validated (signature identity, generic arity, ordinal-based type parameter matching, static/instance mismatch, accessibility, DiscoverInternals, etc.).
• Uses VerifyCS.Diagnostic(...).WithLocation(0).WithArguments(...) to assert the exact diagnostic and its message arguments (not just "a diagnostic fired"), or omits ExpectedDiagnostics entirely for the negative cases — both are meaningful, specific assertions.
• Awaits test.RunAsync() in every case (verified no un-awaited async calls).
• Cross-checks correctly against the production analyzer's actual matching logic (ordinal-based method type-parameter comparison, AllGenericTypeParametersAreInferable, arity/signature/static-vs-instance distinctions in InheritedMemberFromDifferentMSTestVersionAnalyzer.cs), so the asserted behavior matches the implementation rather than being coincidentally correct.
• Covers a wide, deliberate scenario matrix: C# and Visual Basic; real project references vs. emitted/missing-PE cross-assembly cases; TestInitialize/ClassInitialize/ClassCleanup/AssemblyInitialize/TestMethod/DataTestMethod; shadowing by valid/invalid/different-signature/generic/by-ref/static/return-type-mismatched derived methods; internal/protected accessibility with and without DiscoverInternals; abstract and generic test classes; nested classes.

No high-confidence actionable defects were found that warrant an inline suggestion — the suite is internally consistent, each assertion is specific (exact diagnostic arguments), and no test asserts a tautology or omits verification of its stated claim.

Grade	Test	Mutation	Notes	How to improve
A (90–100)	new InheritedMemberFromDifferentMSTestVersionAnalyzerTests. WhenInheritedTestInitializeComesFromDifferentFrameworkAssembly_ Diagnostic	N/A (analyzer test)	Asserts exact diagnostic location and arguments for cross-assembly TestInitialize mismatch.	—
A (90–100)	new InheritedMemberFromDifferentMSTestVersionAnalyzerTests. WhenInheritedTestMethodComesFromDifferentFrameworkAssembly_ Diagnostic	N/A	Covers silent non-discovery of inherited [TestMethod] from a different framework identity.	—
A (90–100)	new InheritedMemberFromDifferentMSTestVersionAnalyzerTests. WhenInheritedCustomTestMethodSubclassComesFromDifferentFrameworkAssembly_ Diagnostic	N/A	Exercises custom TestMethodAttribute-derived subclasses compiled against legacy framework.	—
A (90–100)	new InheritedMemberFromDifferentMSTestVersionAnalyzerTests. WhenInheritedGenericTestMethodShadowedByRenamedTypeParameter_ NoDiagnostic	N/A	Validates ordinal-based (not name-based) method type-parameter equality against the analyzer's actual comparer.	—
A (90–100)	new InheritedMemberFromDifferentMSTestVersionAnalyzerTests. WhenInheritedTestMethodShadowedByByRefOverload_ Diagnostic	N/A	Confirms by-ref parameter overloads are treated as distinct signatures.	—
A (90–100)	new InheritedMemberFromDifferentMSTestVersionAnalyzerTests. WhenInheritedTestInitializeFromDifferentFrameworkAssemblyInVisualBasic_ Diagnostic	N/A	Confirms the analyzer is symbol-based and equally applies to VB consumers.	—
Remaining 41 tests (all Grade A — same rigor and pattern) All other 41 new tests in this file follow the identical high-quality pattern: descriptive naming, explanatory comments tied to CLR/analyzer semantics, exact diagnostic-argument assertions or explicit no-diagnostic negative cases, and awaited RunAsync calls. No actionable issues found in any of them.

_{This advisory comment was generated automatically. Grades are heuristic and informational — they do not block merging. Re-run with /review-tests.}

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 93.6 AIC · ⌖ 3.74 AIC · ⊞ 16.9K · [◷]( · ◷)

[Jan Krivanek (JanKrivanek)]

Code looks good to go at head 824705c9; no major correctness, API, or test-coverage concerns found.