Skip to content

MA0002: treat the xunit assertions that take an IEqualityComparer<T> as ordinal with report_only_non_ordinal - #1625

Merged
meziantou merged 2 commits into
meziantou:mainfrom
vencyk:xunit-assert-known-ordinal
Oct 8, 2026
Merged

meziantou merged 2 commits into
meziantou:mainfrom
vencyk:xunit-assert-known-ordinal

Conversation

@vencyk

@vencyk vencyk commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

With MA0002.report_only_non_ordinal = true, a call to a Xunit.Assert method is no longer reported when the overload it lacks takes an IEqualityComparer<string>: Equal, NotEqual, Contains, DoesNotContain and Distinct. This mirrors the existing special case for Meziantou.Framework.Assertions.Assert.

Why: xunit compares strings ordinally, in its own Assert.Equal(string, string) overloads and, for items and collections, through IEquatable<string>. In a test project with the option on, Assert.Equal(expectedStrings, actualStrings) and Assert.Contains("a", strings) were the only calls left reported, and passing StringComparer.Ordinal to them changes nothing.

What stays reported: a method whose missing overload takes an IComparer<string> (InRange, NotInRange), since Comparer<string>.Default is culture-sensitive. The check is on the comparer kind of the overload, not on method names.

Tests: four added to UseStringComparerAnalyzerTests. The equality assertions are tested against the real xunit v3 and v2 packages (AddXunitV3() / AddXunitV2()). The ordering test uses a stub Xunit.Assert, because the overload finder does not detect the real generic InRange<T> overload today; that gap is independent of this change. UseStringComparerAnalyzerTests passes on Roslyn 5.9 (91 tests) and Roslyn 4.8 (67 tests). dotnet run --project src/DocumentationGenerator was run; the only documentation change is the note in docs/Rules/MA0002.md.

🤖 Generated with Claude Code

…al in MA0002's report_only_non_ordinal mode

With MA0002.report_only_non_ordinal = true, a call to a method of
Xunit.Assert is no longer reported when the overload it lacks takes an
IEqualityComparer<string>: Equal, NotEqual, Contains, DoesNotContain and
Distinct. xunit compares strings ordinally, in its own string overloads
and, for items and collections, through IEquatable<string>, so passing
StringComparer.Ordinal to those assertions changes nothing. A method
whose missing overload takes an IComparer<string> (InRange, NotInRange)
stays reported, as Comparer<string>.Default is culture-sensitive.

The tests reference the real xunit v3 and v2 assemblies for the
equality assertions. The ordering test uses a stub Xunit.Assert because
the overload finder does not detect the real InRange<T> overload today,
which is independent of this change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

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.

🟢 Approval recommended

The implementation and tests cover the requested behavior; only a non-blocking documentation nit remains.

1 open finding
What changed in this PR

Updates MA0002 to treat xUnit equality-comparer assertions as ordinal when report_only_non_ordinal is enabled.

Changes:

  • Added xUnit v2/v3 comparer handling.
  • Added equality and ordering tests.
  • Updated MA0002 documentation.
File Summary
tests/​Meziantou.Analyzer.Test/​Rules/​UseStringComparerAnalyzerTests.cs Adds xUnit behavior coverage.
src/​Meziantou.Analyzer/​Rules/​UseStringComparerAnalyzer.cs Handles xUnit assertion comparer overloads.
docs/​Rules/​MA0002.md Documents the new behavior; includes a minor punctuation nit.

🧠 Review effort: Lite


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Comment thread docs/Rules/MA0002.md Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@meziantou
meziantou merged commit ecd47ef into meziantou:main Oct 8, 2026
14 checks passed
This was referenced Oct 8, 2026
This was referenced Oct 9, 2026
IhateTrains pushed a commit to ParadoxGameConverters/ImperatorToCK3 that referenced this pull request Oct 10, 2026
Updated
[Meziantou.Analyzer](https://github.com/meziantou/Meziantou.Analyzer)
from 3.0.291 to 3.0.296.

<details>
<summary>Release notes</summary>

_Sourced from [Meziantou.Analyzer's
releases](https://github.com/meziantou/Meziantou.Analyzer/releases)._

## 3.0.296

NuGet package:
<https://www.nuget.org/packages/Meziantou.Analyzer/3.0.296>

## What's Changed
* Add tests for non-culture-sensitive string methods by @​meziantou in
meziantou/Meziantou.Analyzer#1626


**Full Changelog**:
meziantou/Meziantou.Analyzer@3.0.295...3.0.296

## 3.0.295

NuGet package:
<https://www.nuget.org/packages/Meziantou.Analyzer/3.0.295>

## What's Changed
* MA0002: treat the xunit assertions that take an IEqualityComparer<T>
as ordinal with report_only_non_ordinal by @​vencyk in
meziantou/Meziantou.Analyzer#1625

## New Contributors
* @​vencyk made their first contribution in
meziantou/Meziantou.Analyzer#1625

**Full Changelog**:
meziantou/Meziantou.Analyzer@3.0.294...3.0.295

## 3.0.294

NuGet package:
<https://www.nuget.org/packages/Meziantou.Analyzer/3.0.294>

## What's Changed
* Do not suggest instance members in static contexts in MA0166/MA0167 by
@​meziantou in meziantou/Meziantou.Analyzer#1623


**Full Changelog**:
meziantou/Meziantou.Analyzer@3.0.293...3.0.294

## 3.0.293

NuGet package:
<https://www.nuget.org/packages/Meziantou.Analyzer/3.0.293>

## What's Changed
* Check for a set instance only before an MA0002 report by
@​alexander-efremov in
meziantou/Meziantou.Analyzer#1622


**Full Changelog**:
meziantou/Meziantou.Analyzer@3.0.292...3.0.293

## 3.0.292

NuGet package:
<https://www.nuget.org/packages/Meziantou.Analyzer/3.0.292>

## What's Changed
* Cache LookupSymbols results in OverloadFinder by @​alexander-efremov
in meziantou/Meziantou.Analyzer#1621
* chore(deps): update all dependencies to 1.0.179 by @​renovate[bot] in
meziantou/Meziantou.Analyzer#1618

## New Contributors
* @​alexander-efremov made their first contribution in
meziantou/Meziantou.Analyzer#1621

**Full Changelog**:
meziantou/Meziantou.Analyzer@3.0.291...3.0.292

Commits viewable in [compare
view](meziantou/Meziantou.Analyzer@3.0.291...3.0.296).
</details>

[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=Meziantou.Analyzer&package-manager=nuget&previous-version=3.0.291&new-version=3.0.296)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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.

3 participants