Skip to content

Add MSTEST0070 analyzer validating [Condition] member references#9076

Open
Evangelink wants to merge 1 commit into
evangelink/issue-9070-condition-attributefrom
evangelink/issue-9070-condition-analyzer
Open

Add MSTEST0070 analyzer validating [Condition] member references#9076
Evangelink wants to merge 1 commit into
evangelink/issue-9070-condition-attributefrom
evangelink/issue-9070-condition-analyzer

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Follow-up to #9071 (per design review).

Summary

Adds MSTEST0070 — ConditionShouldBeValidAnalyzer, which catches at build time the same errors that ConditionAttribute.IsConditionMet throws at runtime, so typos and refactors don't silently break test gating.

This is the analyzer counterpart called out as a follow-up in #9071 and addresses the user feedback "add an analyzer to detect if the member targets (or type detected) is invalid".

⚠️ Stacked on top of #9071. This branch is cut from evangelink/issue-9070-condition-attribute, so the diff currently includes commits from that PR. Once #9071 merges, GitHub will rebase the diff to only show the analyzer-specific changes.

Diagnostics

One diagnostic is produced per invalid member name on each [Condition] attribute.

Rule Condition
MemberNotFoundRule The named member doesn't exist on the type.
MemberNotPublicRule Member is not public.
MemberNotStaticRule Member is not static.
MemberWrongKindRule Member is an event, nested type, or other unsupported kind.
MemberWrongReturnTypeRule Property / field / method doesn't return bool.
MethodHasParametersRule Referenced method has parameters.
PropertyNotReadableRule Property has no public getter (write-only).

All seven diagnostics share id MSTEST0070 with different message formats, matching how MSTEST0018 (DynamicData) is structured.

Example

public static class Conditions
{
    public static bool IsTrue => true;
    public static int NotBool => 0;
    public static bool WithParam(int x) => x > 0;
    internal static bool InternalIsTrue => true;
}

[TestClass]
public class MyTests
{
    [Condition(typeof(Conditions), "Mispelled")]                  // MSTEST0070: cannot be found
    [Condition(typeof(Conditions), nameof(Conditions.NotBool))]   // MSTEST0070: must return 'bool'
    [Condition(typeof(Conditions), nameof(Conditions.WithParam))] // MSTEST0070: must be parameterless
    [Condition(typeof(Conditions),
               nameof(Conditions.InternalIsTrue))]                // MSTEST0070: must be 'public'
    [TestMethod]
    public void MyTest() { }
}

Coverage

  • Fires on both test methods and test classes (the attribute is AttributeUsage(Class | Method)).
  • Validates every member name passed to the attribute — the simple (Type, string) ctor, the params string[] overload, and the ConditionMode-prefixed overloads.
  • Stacked [Condition] attributes are each validated independently.
  • Inherited public static members are recognized (walks BaseType chain), matching BindingFlags.Public | Static | FlattenHierarchy.
  • The resolution order matches the runtime (property → field → method), so if a property and method share a name only the property is validated — consistent with what would actually be invoked.

Tests

18 tests in test/UnitTests/MSTest.Analyzers.UnitTests/ConditionShouldBeValidAnalyzerTests.cs:

  • Valid: public static bool property / field / parameterless method, inherited public static member
  • Invalid: missing, non-public (internal), instance property, instance field, non-bool property, non-bool field, non-bool method, method with parameters, write-only property, event
  • Composition: multiple invalid member names in one attribute (each reported), multiple [Condition] attributes on one method (each validated), ConditionMode.Exclude ctor overload, attribute on class target

All 18 pass on net8.0.

Files touched

  • src/Analyzers/MSTest.Analyzers/ConditionShouldBeValidAnalyzer.cs
  • test/UnitTests/MSTest.Analyzers.UnitTests/ConditionShouldBeValidAnalyzerTests.cs
  • ✏️ src/Analyzers/MSTest.Analyzers/Helpers/DiagnosticIds.cs (added ConditionShouldBeValidRuleId = "MSTEST0070")
  • ✏️ src/Analyzers/MSTest.Analyzers/Helpers/WellKnownTypeNames.cs (added ConditionAttribute entry)
  • ✏️ src/Analyzers/MSTest.Analyzers/Resources.resx + 13 .xlf files (regenerated via dotnet msbuild /t:UpdateXlf)
  • ✏️ src/Analyzers/MSTest.Analyzers/AnalyzerReleases.Unshipped.md

Notes for reviewers

  • Severity is Warning, enabled by default — matches MSTEST0018 (DynamicData) which solves the same class of problem. Happy to downgrade to Info if you'd prefer a softer rollout.
  • Documentation URL https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0070 will be authored as a separate dotnet/docs PR after this merges.
  • No code fix provider — the right fix depends on user intent (rename member? change accessibility? change member kind?). Keeping it diagnostic-only matches DynamicDataShouldBeValidAnalyzer.

Adds `ConditionShouldBeValidAnalyzer` (MSTEST0070) which catches at build

time the same errors that `ConditionAttribute.IsConditionMet` throws at

runtime, so typos and refactors don't silently break test gating.

Rules emitted (one diagnostic per member name in the attribute):

- `MemberNotFoundRule` -- the named member doesn't exist on the type

- `MemberNotPublicRule` -- member is not `public`

- `MemberNotStaticRule` -- member is not `static`

- `MemberWrongKindRule` -- member is an event/nested type/etc.

- `MemberWrongReturnTypeRule` -- property/field/method doesn't return `bool`

- `MethodHasParametersRule` -- referenced method has parameters

- `PropertyNotReadableRule` -- property has no public getter

The analyzer fires on any symbol carrying `[Condition]` (test class or

test method) and validates every member name passed to the attribute,

including the `params string[]` overload and the `ConditionMode`-prefixed

overloads. Inherited public static members are recognized.

Resource strings localized via the standard `UpdateXlf` flow.

Listed in `AnalyzerReleases.Unshipped.md` as MSTEST0070, Usage, Warning.

Adds 18 analyzer tests covering all rules, multi-attribute stacking,

inherited members, `ConditionMode.Exclude`, and the additional-members

overload. All 18 tests pass on net8.0.

Follow-up to #9071 (per design review).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
Member Author

🧪 Test quality grade — PR #9076

ΔTestGradeBandNotes
new ConditionShouldBeValidAnalyzerTests.
WhenAdditionalMembersAreInvalid_
AllReported
A 90–100 Verifies two independent diagnostics (MemberNotFound + MemberWrongReturnType) for a single multi-member attribute call.
new ConditionShouldBeValidAnalyzerTests.
WhenAttributeIsOnTestClass_
StillValidated
A 90–100 Confirms class-level [Condition] triggers the same validation; checks exact rule, location, and type+member arguments.
new ConditionShouldBeValidAnalyzerTests.
WhenExplicitConditionMode_
StillValidated
A 90–100 Verifies that passing an explicit ConditionMode does not bypass member validation.
new ConditionShouldBeValidAnalyzerTests.
WhenFieldIsNonBool_
MemberWrongReturnType
A 90–100 No issues found.
new ConditionShouldBeValidAnalyzerTests.
WhenInheritedPublicStaticMember_
NoDiagnostic
A 90–100 No issues found; covers the inherited-member edge case to prevent false positives.
new ConditionShouldBeValidAnalyzerTests.
WhenMemberDoesNotExist_
MemberNotFound
A 90–100 No issues found.
new ConditionShouldBeValidAnalyzerTests.
WhenMemberIsEvent_
MemberWrongKind
A 90–100 No issues found.
new ConditionShouldBeValidAnalyzerTests.
WhenMemberIsInternal_
MemberNotPublic
A 90–100 No issues found.
new ConditionShouldBeValidAnalyzerTests.
WhenMemberIsValidPublicStaticBoolField_
NoDiagnostic
A 90–100 No issues found.
new ConditionShouldBeValidAnalyzerTests.
WhenMemberIsValidPublicStaticBoolMethod_
NoDiagnostic
A 90–100 No issues found.
new ConditionShouldBeValidAnalyzerTests.
WhenMemberIsValidPublicStaticBoolProperty_
NoDiagnostic
A 90–100 No issues found.
new ConditionShouldBeValidAnalyzerTests.
WhenMethodHasParameters_
MethodHasParameters
A 90–100 No issues found.
new ConditionShouldBeValidAnalyzerTests.
WhenMethodReturnsNonBool_
MemberWrongReturnType
A 90–100 No issues found.
new ConditionShouldBeValidAnalyzerTests.
WhenMultipleConditionAttributes_
EachValidated
A 90–100 Verifies each of two stacked [Condition] attributes is flagged at its own distinct location.
new ConditionShouldBeValidAnalyzerTests.
WhenPropertyIsInstance_
MemberNotStatic
A 90–100 No issues found.
new ConditionShouldBeValidAnalyzerTests.
WhenPropertyIsWriteOnly_
PropertyNotReadable
A 90–100 No issues found.
new ConditionShouldBeValidAnalyzerTests.
WhenPropertyReturnsNonBool_
MemberWrongReturnType
A 90–100 No issues found.
new ConditionShouldBeValidAnalyzerTests.
WhenStaticFieldIsInstance_
MemberNotStatic
A 90–100 No issues found.

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

🤖 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 Grade Tests on PR (on open / sync) workflow. · 272.1 AIC · ⌖ 25.2 AIC · [◷]( · )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant