Fix CA1305 in Assert.That test (int.ToString() needs IFormatProvider)#8358
Merged
Merged
Conversation
…keRight Use CultureInfo.InvariantCulture for int.ToString() to satisfy CA1305. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a CA1305 globalization analyzer failure in the Assert.That unit tests by providing an explicit IFormatProvider when converting an int to string, unblocking Windows builds that treat this warning as an error.
Changes:
- Update
counter.Increment().ToString()tocounter.Increment().ToString(CultureInfo.InvariantCulture)in theThat_Coalesce_ShortCircuit_PassingAssertion_DoesNotInvokeRighttest.
Show a summary per file
| File | Description |
|---|---|
| test/UnitTests/TestFramework.UnitTests/Assertions/AssertTests.That.cs | Passes CultureInfo.InvariantCulture to int.ToString to satisfy CA1305 in the Assert.That coalesce short-circuit test. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 0
Evangelink
commented
May 19, 2026
Member
Author
Evangelink
left a comment
Evangelink
left a comment
There was a problem hiding this comment.
✅ 21/21 dimensions clean — no findings.
The fix correctly resolves CA1305 by passing CultureInfo.InvariantCulture to int.ToString(). For integer formatting, InvariantCulture is semantically equivalent to the default, so test behavior is unchanged. The change is minimal, correctly scoped to the test file, and unblocks the CI build.
Generated by Expert Code Review (on open) for issue #8358 · ● 3.2M
Evangelink
pushed a commit
that referenced
this pull request
May 19, 2026
…ted reference Resets PR #8308 onto current main and re-adds the unique contribution from @jakubjares as a single test plus its helper types. After PR #8307 was merged via #8306 and PR #8358 added the CA1305 fix, the old branch was hopelessly diverged from main. Instead of trying to thread the needle through 16+ source conflicts, this commit takes main's source/test files verbatim and only adds: * That_TwoCallsReturningSameMutatedReference_DeduplicatesInDetails * Shape (helper) * BoxOfShapes (helper) The test pins down the current UX limitation: when two calls return the SAME mutable reference and the comparison is reference-inequality, the cache stores the same reference for both call expressions; by the time details are extracted the object has been mutated, both slots resolve to the same value, and TryAddExpressionValue's same-value dedupe means only one entry surfaces (`Shape: Circle`). Documenting this behavior gives us a regression guard if we ever decide to improve the diagnostic. Replaces the previous `WithMessage("non")` placeholder with the real expected message. All 76 That_* tests pass. Co-authored-by: Jakub Jareš <me@jakubjares.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Evangelink
pushed a commit
that referenced
this pull request
May 19, 2026
…ted reference Resets PR #8308 onto current main and re-adds the unique contribution from @jakubjares as a single test plus its helper types. After PR #8307 was merged via #8306 and PR #8358 added the CA1305 fix, the old branch was hopelessly diverged from main. Instead of trying to thread the needle through 16+ source conflicts, this commit takes main's source/test files verbatim and only adds: * That_TwoCallsReturningSameMutatedReference_DeduplicatesInDetails * Shape (helper) * BoxOfShapes (helper) The test pins down the current UX limitation: when two calls return the SAME mutable reference and the comparison is reference-inequality, the cache stores the same reference for both call expressions; by the time details are extracted the object has been mutated, both slots resolve to the same value, and TryAddExpressionValue's same-value dedupe means only one entry surfaces (`Shape: Circle`). Documenting this behavior gives us a regression guard if we ever decide to improve the diagnostic. Replaces the previous `WithMessage("non")` placeholder with the real expected message. All 76 That_* tests pass. Co-authored-by: Jakub Jareš <me@jakubjares.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Windows build is failing with CA1305 in
That_Coalesce_ShortCircuit_PassingAssertion_DoesNotInvokeRightbecauseint.ToString()is called without a format provider. This blocks PR #8348 (and any other PR built againstmain).Fix: pass
CultureInfo.InvariantCulturetoint.ToString.The bug was introduced in #8307 / #8306.