Uh oh!
There was an error while loading. Please reload this page.
[test-improver] test: add edge case tests for DoNotUseShadowingAnalyzer (MSTEST0036) - #9489
Conversation
Three new tests covering untested paths in IsMemberShadowing and GetBaseMembers: - WhenTestClassShadowsGrandparentMemberThroughIntermediateClass_Diagnostic: GetBaseMembers walks the full inheritance chain; a TestClass method that shadows a grandparent member (with an empty intermediate class) is detected. - WhenTestClassHasSamePropertyNameAsBaseClassButDifferentType_NoDiagnostic: IsMemberShadowing requires SymbolEqualityComparer equality on property types; same name but different type (int vs string) is not considered shadowing. - WhenTestClassHasSameFieldNameAsBaseClass_NoDiagnostic: IsMemberShadowing handles IMethodSymbol and IPropertySymbol only; fields (IFieldSymbol) fall through to 'return false', so field hiding is not flagged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds targeted unit tests for the MSTest analyzers project to cover previously untested edge paths in DoNotUseShadowingAnalyzer (MSTEST0036), improving confidence in inheritance-walking and member-type handling behavior.
Changes:
- Add a diagnostic test for shadowing a grandparent member through an empty intermediate base type.
- Add no-diagnostic tests for (1) same property name with different type and (2) same field name as base field.
Show a summary per file
| File | Description |
|---|---|
| test/UnitTests/MSTest.Analyzers.UnitTests/DoNotUseShadowingAnalyzerTests.cs | Adds 3 new analyzer tests covering grandparent-member shadowing, property type mismatch, and field hiding scenarios. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 3
- Review effort level: Low
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Amaury Levé (Evangelink)
commented
Jun 29, 2026
🧪 Test quality grade — PR #9489
This advisory comment was generated automatically. Grades are heuristic
|
Amaury Levé (Evangelink)
left a comment
There was a problem hiding this comment.
Note
🤖 Automated review 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 Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.
Review: PR #9489 — Edge case tests for DoNotUseShadowingAnalyzer (MSTEST0036)
This PR adds three test methods to cover previously untested code paths in DoNotUseShadowingAnalyzer. All three tests correctly reflect the analyzer's current behavior and are well-structured.
Dimension findings
| # | Dimension | Verdict |
|---|---|---|
| 17 | Documentation Accuracy | 🟡 2 MODERATE |
| 18 | Analyzer & Code Fix Quality | 🟡 2 MODERATE (same findings) |
✅ 20/22 dimensions clean.
- Test 2 (
WhenTestClassHasSamePropertyNameAsBaseClassButDifferentType_NoDiagnostic) — The comment says "not considered shadowing", which implies intentional design, but the production code has no corresponding explanation for why different-type same-name properties are excluded from the diagnostic. If intentional, the rationale should be documented inIsMemberShadowing; if a gap, a follow-up issue should be filed. - Test 3 (
WhenTestClassHasSameFieldNameAsBaseClass_NoDiagnostic) —GetBaseMemberscollectsIFieldSymbolmembers (they pass the skip filter), butIsMemberShadowinghas noIFieldSymbolbranch and returnsfalseby fallthrough — no comment in the production code explains this omission. Same ask: document the intent or track the gap.
Correctness verification
All three new tests correctly exercise the claimed code paths:
| Test | Path verified | Verdict |
|---|---|---|
WhenTestClassShadowsGrandparentMemberThroughIntermediateClass_Diagnostic | GetBaseMembers while-loop traverses IntermediateClass (no members), then GrandparentClass (finds Method) → diagnostic fires | ✅ |
WhenTestClassHasSamePropertyNameAsBaseClassButDifferentType_NoDiagnostic | IsMemberShadowing property branch: SymbolEqualityComparer.Default.Equals(string, int) → false → no diagnostic | ✅ |
WhenTestClassHasSameFieldNameAsBaseClass_NoDiagnostic | IsMemberShadowing: neither method nor property branch matches IFieldSymbol → falls through to return false → no diagnostic | ✅ |
Minor observation (NIT)
The new test names (WhenTestClassShadows..., WhenTestClassHas...) don't follow the existing pattern in the file (WhenTestClassHave...). This is inconsistent but a very minor style nit that does not need to block the PR.
Assertion style
VerifyCS.VerifyAnalyzerAsync is the correct assertion mechanism for this project. BannedSymbols.txt bans AwesomeAssertions; no violations found.
The tests are functionally correct and a welcome addition to the suite. The inline comments above ask the team to clarify design intent vs. gap for the two NoDiagnostic cases — no code changes are strictly required to merge, but the clarification would improve long-term maintainability.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Goal and Rationale
DoNotUseShadowingAnalyzer(MSTEST0036) had 18 tests, but three distinct code paths in the analyzer's logic were completely untested:GetBaseMemberswhile-loop walking allBaseTypeancestorsSymbolEqualityComparer.Default.Equals(propertySymbol.Type, basePropertySymbol.Type)guard inIsMemberShadowingIsMemberShadowingreturnsfalseforIFieldSymbol(onlyIMethodSymbolandIPropertySymbolare handled)Approach
Added three
[TestMethod]entries toDoNotUseShadowingAnalyzerTests.cs:WhenTestClassShadowsGrandparentMemberThroughIntermediateClass_Diagnostic[TestClass]DerivedClass : IntermediateClass : GrandparentClasswhere onlyGrandparentClassdeclaresMethod()andDerivedClassredeclares it →GetBaseMemberswalks through the emptyIntermediateClassto findGrandparentClass.Method, diagnostic firesWhenTestClassHasSamePropertyNameAsBaseClassButDifferentType_NoDiagnosticint Property, derived hasnew string Property— types differ,SymbolEqualityComparercheck returns false → no diagnosticWhenTestClassHasSameFieldNameAsBaseClass_NoDiagnosticint Field, derived hasnew int Field—IsMemberShadowinghas noIFieldSymbolbranch → falls through toreturn false→ no diagnosticCoverage Impact
DoNotUseShadowingAnalyzerTestsTrade-offs
Purely additive — no production code changes, no new dependencies.
Test Status
All 21/21 tests pass (
net8.0,Debug):Reproducibility
Add this agentic workflows to your repo
To install this agentic workflow, run