Uh oh!
There was an error while loading. Please reload this page.
Add Span/Memory overloads to Assert.HasCount and extend MSTEST0037 - #9176
Conversation
Fixes#9143. Adds ReadOnlySpan<T>/Span<T>/ReadOnlyMemory<T>/Memory<T> overloads to Assert.HasCount (with interpolated-message variants) guarded for NETCOREAPP3_1_OR_GREATER, and extends MSTEST0037 to flag Assert.AreEqual(x, span/memory.Length) and suggest Assert.HasCount(x, span). Span/memory always map to HasCount (never IsEmpty/IsNotEmpty, which have no span overloads). Interpolated-handler variants preserve lazy message evaluation now that arrays bind to the ReadOnlySpan<T> overload. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR extends MSTest’s Assert.HasCount to support modern Span<T>/Memory<T> shapes (plus lazy interpolated-string failure messages) and updates analyzer MSTEST0037 so Assert.AreEqual(expected, spanOrMemory.Length) can be rewritten to Assert.HasCount(expected, spanOrMemory).
Changes:
- Added
HasCountoverloads forSpan<T>,ReadOnlySpan<T>,Memory<T>, andReadOnlyMemory<T>(guarded toNETCOREAPP3_1_OR_GREATER), including interpolated-string-handler variants. - Updated MSTEST0037 recognition to treat span/memory
.Lengthas aHasCountcandidate (neverIsEmpty/IsNotEmpty). - Added/updated unit tests covering the new overload behavior and the analyzer/code-fix behavior (with TFM guards where needed).
Show a summary per file
| File | Description |
|---|---|
| test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.Items.cs | Adds unit coverage for new HasCount overloads and lazy-message behavior (including array overload-resolution note). |
| test/UnitTests/MSTest.Analyzers.UnitTests/UseProperAssertMethodsAnalyzerTests.cs | Adds analyzer/code-fix tests for span/memory .Length → HasCount transformations (guarded to net8+). |
| src/TestFramework/TestFramework/PublicAPI/net/PublicAPI.Unshipped.txt | Declares the newly added net-only public APIs (overloads + handler ctors). |
| src/TestFramework/TestFramework/Assertions/Assert.Count.cs | Implements the span/memory HasCount overload set and handler constructors, with a span-based private implementation. |
| src/Analyzers/MSTest.Analyzers/UseProperAssertMethodsAnalyzer.Collection.cs | Extends count-pattern recognition to handle span/memory .Length and route it to HasCount. |
Copilot's findings
- Files reviewed: 5/5 changed files
- Comments generated: 2
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
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.
| # | Dimension | Verdict |
|---|---|---|
| 4 | Public API & Binary Compatibility | 🟡 1 MODERATE |
| 13 | Test Completeness & Coverage | 🟡 1 MODERATE |
| 17 | Documentation Accuracy | ⚪ 1 NIT |
| 18 | Analyzer & Code Fix Quality | ⚪ 1 NIT |
✅ 18/22 dimensions clean.
- Public API — source-breaking change for arrays with async messages: Adding
ReadOnlySpan<T>overloads causesint[](and other array types) to rebind fromIEnumerable<T>toReadOnlySpan<T>during overload resolution. Any existing call site of the formAssert.HasCount(n, someArray, $"{await task}")will fail to compile with CS4007 ("a ref struct cannot be preserved across an await point"). The PR description and a test change already document this trade-off, but it is a source-breaking change for callers mixing arrays and async interpolated messages. This should be captured in the changelog/release notes so upgrading users know to switch arrays toList<T>(or a localIEnumerable<T>variable) if they hit CS4007. - Test Coverage — missing
Span<T>/ReadOnlyMemory<T>failure-path tests: see inline comment onAssertTests.Items.csline 202. - NIT — private span method comment mentions
"IsEmpty"which is unreachable in the span code path: see inline comment onAssert.Count.csline 664. - NIT — no analyzer test for non-
intexpected value against span.Length: see inline comment onUseProperAssertMethodsAnalyzerTests.cs.
🤖 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 Expert Code Review (on PR ready) workflow. · 707.8 AIC · ⌖ 12.9 AIC · ◷
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.
- Gate MSTEST0037 span/memory '.Length' suggestion on a matching BCL HasCount overload being present (avoids non-compiling fixes on .NET Framework targets) and require the span/memory type to be a BCL symbol. - Fix doc comment in ReadOnlySpan<T> HasCount private overload (assertionName is always HasCount). - Add failure tests for Span<T>/ReadOnlyMemory<T> public HasCount overloads and their interpolated-string handlers. - Add analyzer test asserting no MSTEST0037 for non-int expected against span.Length. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
…verload Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
Amaury Levé (Evangelink)
commented
Jun 16, 2026
🧪 Test quality grade — PR #917621 tests graded across 2 files. 17 A, 4 B — a strong contribution overall. The four B-grade tests are minimal happy-path smoke tests with a single assertion each; they are adequately paired with dedicated
This advisory comment was generated automatically. Grades are heuristic
|
Uh oh!
There was an error while loading. Please reload this page.
Amaury Levé (@Evangelink), YuliiaKovalova, this is still broken when targeting .NET Framework 4.8, etc. MSTEST0037 is triggered, but Assert.HasCount overload does not exist. |
Fixes#9143.
What
Adds
Span<T>/ReadOnlySpan<T>/Memory<T>/ReadOnlyMemory<T>overloads toAssert.HasCountand extends analyzer MSTEST0037 to flagAssert.AreEqual(x, span/memory.Length)and offer a code fix toAssert.HasCount(x, span).Changes
Assert.Count.cs— 4 span/memoryHasCountoverloads, each with a plainstring?message and an interpolated-string-handler variant, plus 4 handler constructors and a span-based private impl. Guarded#if NETCOREAPP3_1_OR_GREATER(System.Memoryisn't referenced for netstandard2.0/net462).PublicAPI/net/PublicAPI.Unshipped.txt— declared the 12 new net-only public APIs.UseProperAssertMethodsAnalyzer.Collection.cs— addedIsSpanOrMemoryTypeplus a dedicated branch so span/memory.Lengthalways maps toHasCount(neverIsEmpty/IsNotEmpty, which have no span overloads). TheIsTrue/IsFalseempty/not-empty path is intentionally left untouched for span/memory.AssertTests.Items.cs) + 7 analyzer tests (UseProperAssertMethodsAnalyzerTests.cs, guarded#if NET8_0_OR_GREATER).Behavioral note / trade-off
Span/ReadOnlySpanareref structs, so they need explicit overloads. A side effect is that arrays now bind to theReadOnlySpan<T>overload during overload resolution (previouslyIEnumerable<T>). The interpolated-handler variants preserve lazy failure-message evaluation, butAssert.HasCount(<array>, …, $"…{await …}…")becomes a compile error (CS4007— a span can't be preserved acrossawait). This is inherent to ref-struct overloads; one existing test that used that pattern was switched to aList<int>.Verification
AssertTests: 967 pass (net8.0); net48 span tests correctly guarded out.UseProperAssertMethodsAnalyzerTests: 125 pass (net8.0), 118 (net472, span tests excluded by the guard).Microsoft.Testing.Platform.UnitTests(heavyHasCountconsumer) builds clean — no rebind breakage.