Uh oh!
There was an error while loading. Please reload this page.
[test-improver] test: add edge case tests for IgnoreStringMethodReturnValueAnalyzer - #9468
Conversation
Add 3 edge case tests covering untested code paths in the analyzer's
guard conditions:
1. WhenDiscardAssignmentIgnoresReturnValue_NoDiagnostic
- `_ = str.Contains("test")` wraps an IAssignmentOperation (not
IInvocationOperation), so the early-return guard fires and no
diagnostic is reported.
2. WhenStringMethodReturnValueIgnoredInsideLambda_Diagnostic
- Expression statements inside a lambda block body are still
ExpressionStatement operations, so the analyzer fires inside
the lambda just as at method level.
3. WhenStringMethodResultUsedAsReceiverForChainedCall_NoDiagnostic
- `str.Contains("Hello").GetHashCode()` — the ExpressionStatement's
outermost operation is GetHashCode() on System.Boolean (not a
string method), so IsStringMethodCall returns false and no
diagnostic is reported. The Contains result IS used as a receiver.
All 8 tests pass (5 existing + 3 new).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>There was a problem hiding this comment.
Pull request overview
Adds additional unit-test coverage for the MSTest analyzer IgnoreStringMethodReturnValueAnalyzer (MSTEST0055) to cover previously untested guard/shape scenarios, without changing production analyzer behavior.
Changes:
- Added a test ensuring discard assignments (
_ = str.Contains(...)) do not produce MSTEST0055 diagnostics. - Added a test ensuring ignored string-method return values inside a lambda block body do produce diagnostics.
- Added a test ensuring chained calls where the string method result is used as a receiver (e.g.
str.Contains(...).GetHashCode()) do not produce diagnostics.
Show a summary per file
| File | Description |
|---|---|
| test/UnitTests/MSTest.Analyzers.UnitTests/IgnoreStringMethodReturnValueAnalyzerTests.cs | Adds three new test cases covering discard assignment, lambda-body expression statements, and chained-call receiver usage scenarios for MSTEST0055. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Low
This comment has been minimized.
This comment has been minimized.
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.
Expert Review — PR #9468
Reviewed against all 22 dimensions. Changed file: test/UnitTests/MSTest.Analyzers.UnitTests/IgnoreStringMethodReturnValueAnalyzerTests.cs
Verdict table
| # | Dimension | Result |
|---|---|---|
| 1 | Algorithmic Correctness | ✅ LGTM |
| 2 | Threading & Concurrency | N/A |
| 3 | Security & IPC Contract Safety | N/A |
| 4 | Public API & Binary Compatibility | N/A |
| 5 | Performance & Allocations | N/A |
| 6 | Cross-TFM Compatibility | N/A |
| 7 | Resource & IDisposable Management | N/A |
| 8 | Defensive Coding at Boundaries | N/A |
| 9 | Localization & Resources | N/A |
| 10 | Test Isolation | ✅ LGTM |
| 11 | Assertion Quality | ✅ LGTM |
| 12 | Flakiness Patterns | ✅ LGTM |
| 13 | Test Completeness & Coverage | ✅ LGTM |
| 14 | Data-Driven Test Coverage | N/A |
| 15 | Code Structure & Simplification | ✅ LGTM |
| 16 | Naming & Conventions | |
| 17 | Documentation Accuracy | |
| 18 | Analyzer & Code Fix Quality | ✅ LGTM |
| 19 | IPC Wire Compatibility | N/A |
| 20 | Build Infrastructure & Dependencies | N/A |
| 21 | Scope & PR Discipline | ✅ LGTM |
| 22 | PowerShell Scripting Hygiene | N/A |
15/15 applicable dimensions clean. 2 NIT-level findings (no blocking or major issues).
Summary
The three new tests correctly target the three distinct guard-logic paths in IgnoreStringMethodReturnValueAnalyzer.AnalyzeExpressionStatement:
| Test | Guard path exercised | Verified correct? |
|---|---|---|
WhenDiscardAssignmentIgnoresReturnValue_NoDiagnostic | expressionStatementOperation.Operation is not IInvocationOperation — the outer operation is ISimpleAssignmentOperation (a discard), so early return fires; no diagnostic | ✅ |
WhenStringMethodReturnValueIgnoredInsideLambda_Diagnostic | Block-body lambda still emits an IExpressionStatementOperation; analyzer fires correctly inside the lambda | ✅ |
WhenStringMethodResultUsedAsReceiverForChainedCall_NoDiagnostic | Outer IInvocationOperation is GetHashCode() on System.Boolean, not a string method; IsStringMethodCall returns false | ✅ |
All tests are properly isolated (no shared mutable state, no file system operations), non-flaky, and follow the project's CSharpCodeFixVerifier pattern. The using System; directive is correctly included only where System.Action is needed (the lambda test). No expression-bodied lambda gap exists because () => str.Contains(...) assigned to Action would not compile — the block-body form is the only valid pattern here.
Two NIT-level findings are attached as inline comments (naming precision on line 161, comment precision on line 170). Neither affects correctness or reviewer confidence in the PR.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…n comment Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Amaury Levé (Evangelink)
commented
Jun 28, 2026
🧪 Test quality grade — PR #94683 new test methods graded across 1 file (
This advisory comment was generated automatically. Grades are heuristic
|
Uh oh!
There was an error while loading. Please reload this page.
Goal and Rationale
IgnoreStringMethodReturnValueAnalyzer(MSTEST0055) had only 5 tests, all covering the happy path (diagnostic fires / doesn't fire based on whether the result ofContains,StartsWith, orEndsWithis used). Three distinct code paths in the analyzer's guard logic were completely untested:_ = str.Contains(...)expressionStatementOperation.Operation is not IInvocationOperationearly returnIsStringMethodCallon the outer call (not a string method) — should not fireApproach
Added three
[TestMethod]entries toIgnoreStringMethodReturnValueAnalyzerTests.cs:WhenDiscardAssignmentIgnoresReturnValue_NoDiagnostic_ = str.Contains("Hello")→ outer operation isIAssignmentOperation, notIInvocationOperation→ early return, no diagnosticWhenStringMethodReturnValueIgnoredInsideLambda_Diagnostic() => { str.Contains("Hello"); }→ ExpressionStatement inside lambda body triggers diagnosticWhenStringMethodResultUsedAsReceiverForChainedCall_NoDiagnosticstr.Contains("Hello").GetHashCode()→ outermost ExpressionStatement operation isGetHashCode()onSystem.Boolean, not a string method → no diagnostic onContainsTrade-offs
Purely additive — no production code changes, no new dependencies.
Test Status
All 8 tests pass (5 existing + 3 new):
Reproducibility
.dotnet/dotnet run --project test/UnitTests/MSTest.Analyzers.UnitTests/ -f net8.0 --no-build -- \ --filter "FullyQualifiedName~IgnoreStringMethodReturnValueAnalyzerTests"Add this agentic workflows to your repo
To install this agentic workflow, run