Skip to content

Fix array assertion overload ambiguity with older C# versions - #11038

Open
Amaury Levé (Evangelink) wants to merge 3 commits into
microsoft:mainfrom
Evangelink:dev/amauryleve/fix-doesnotcontain-language-version
Open

Fix array assertion overload ambiguity with older C# versions#11038
Amaury Levé (Evangelink) wants to merge 3 commits into
microsoft:mainfrom
Evangelink:dev/amauryleve/fix-doesnotcontain-language-version

Conversation

@Evangelink

@Evangelink Amaury Levé (Evangelink) commented Sep 4, 2026

Copy link
Copy Markdown
Member

Summary

  • add exact array overloads across affected Assert families so C# 12 consumers, including explicit generic callers, do not encounter ambiguities between IEnumerable<T> and span overloads
  • add constrained collection forwarders that preserve inferred calls for all concrete IEnumerable<T> implementations, including string, ArraySegment<T>, mixed collection types, and user-defined span-convertible enumerables
  • preserve legacy behavior by forwarding compatibility calls to the existing IEnumerable<T> implementations
  • add a dedicated package-consuming C# 12 compilation oracle with representative calls for every public Assert method family and dual-conversion category
  • require future public overload changes to consider source compatibility in the repository Copilot instructions

Fixes #11022

Compatibility scope

Exact array overloads cover both inferred and explicit <T> calls for the reported/common case. Constrained forwarders cover inferred calls for arbitrary enumerable collection types. Explicit <T> calls for non-array dual-convertible types cannot use the additional inferred collection type parameter and would require an unscalable set of per-type overloads.

Validation

  • package build completed with 0 warnings and 0 errors
  • C# 12 AssertSourceCompatibilityTests package-consumer oracle passed
  • TestFramework tests passed (6,095 tests)
  • direct C# 12 probes passed for arrays, strings, ArraySegment<T>, ImmutableArray<T>, collection expressions, mixed types, and a custom dual-convertible enumerable
  • full Release solution rebuild completed with 0 warnings and 0 errors

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 4, 2026 09:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Predicate-based array calls remain ambiguous under C# 12.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 2 High severity

New issues introduced by this change (2)
Severity Finding
High severity src/​TestFramework/​TestFramework/​Assertions/​Assert.Contains.cs — The exact-array overload only covers the item form. Under C# 12, `Func<int, bool> predicate = ...;…
High severity src/​TestFramework/​TestFramework/​Assertions/​Assert.DoesNotContain.cs — The predicate form remains unfixed: with C# 12, `Func<int, bool> predicate = ...;…
What changed in this PR

Adds array-specific assertion overloads to avoid C# 12 overload ambiguity.

Changes:

  • Adds item and comparer array overloads with API baselines.
  • Adds unit and C# 12 acceptance coverage.
File Description
Assert.Contains.cs Adds array forwarding overloads.
Assert.DoesNotContain.cs Adds array forwarding overloads.
PublicAPI.Unshipped.txt Tracks new public APIs.
AssertTests.ContainsSpan.cs Tests array and comparer behavior.
SdkTests.cs Verifies compilation under C# 12.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/TestFramework/TestFramework/Assertions/Assert.Contains.cs Outdated
Comment thread src/TestFramework/TestFramework/Assertions/Assert.DoesNotContain.cs Outdated
public void TestMethod1()
{
int[] values = new[] { 1, 2, 3 };
Assert.Contains(1, values);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure it's only arrays that cause issues here? We don't want to end up needing to grow the list of overloads if we discover more issues.

It might even be safest to take a breaking change to the very new feature. What was the motivation to add the overloads?

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 4, 2026 12:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The compatibility guard omits attributed public Assert method families, making its completeness check inaccurate.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 1 Medium severity

New issues introduced by this change (1)
Severity Finding
Medium severity test/​IntegrationTests/​MSTest.Acceptance.IntegrationTests/​AssertSourceCompatibilityTests.cs — This regex skips attributed API entries because it requires the line to begin with static. The…
Issues resolved since last review (2)
Severity Finding
High severity src/​TestFramework/​TestFramework/​Assertions/​Assert.DoesNotContain.cs — The predicate form remains unfixed: with C# 12, `Func<int, bool> predicate = ...;… View resolved comment
High severity src/​TestFramework/​TestFramework/​Assertions/​Assert.Contains.cs — The exact-array overload only covers the item form. Under C# 12, `Func<int, bool> predicate = ...;… View resolved comment

Comment on lines +331 to +333
private static readonly Regex PublicAssertMethodRegex = new(
@"^static Microsoft\.VisualStudio\.TestTools\.UnitTesting\.Assert\.(?<name>[A-Za-z0-9]+)(?:<|\()",
RegexOptions.CultureInvariant);
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 4, 2026 14:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The broad public overload-resolution changes warrant final human API review despite comprehensive compatibility coverage.

Review tier: Balanced
Findings: 1 Medium severity

Pre-existing issues (1)
Severity Finding
Medium severity test/​IntegrationTests/​MSTest.Acceptance.IntegrationTests/​AssertSourceCompatibilityTests.cs — This regex skips attributed API entries because it requires the line to begin with static. The… View comment

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.

New Assert.DoesNotContain overloads break compilation if LangVersion=latest is not specified

3 participants