Skip to content

Add tests for non-culture-sensitive string methods - #1626

Merged
meziantou merged 4 commits into
mainfrom
feature/gallant-maxwell-ovinjb
Oct 8, 2026
Merged

meziantou merged 4 commits into
mainfrom
feature/gallant-maxwell-ovinjb

Conversation

@meziantou

Copy link
Copy Markdown
Owner

Summary

This PR adds comprehensive test coverage for string methods that use ordinal or ordinal-ignore-case comparison by default, ensuring the analyzer correctly identifies when explicit StringComparison parameters are needed.

Key Changes

  • Test additions: Added a parameterized test StringMethods_ReportDiagnosticDependingOnTheDefaultComparison covering 38 different string method overloads with various parameter combinations (char, string, Rune arguments)
  • Verification test: Added NonCultureSensitiveMethod_DefaultComparisonIsOrdinal that validates the actual runtime behavior of non-culture-sensitive methods using soft hyphen (\u00AD) as a test character to demonstrate the difference between ordinal and culture-sensitive comparisons
  • Refactoring: Extracted the hardcoded list of non-culture-sensitive method documentation IDs into a public NonCultureSensitiveMethodDocumentationIds property in UseStringComparisonAnalyzer to enable test reuse and maintainability
  • Test infrastructure: Added necessary using statements for System.Security.Claims, System.Security.Principal, and System.Text namespaces
  • Helper classes: Added EmptyIdentity and CopiedClaimsIdentity test helper classes to support ClaimsIdentity testing

Notable Implementation Details

  • The verification test uses soft hyphen character (\u00AD) which is ignored in linguistic comparisons but not in ordinal comparisons, making it ideal for demonstrating the behavioral difference
  • Tests cover both instance methods (e.g., s.Contains()) and static methods (e.g., string.Equals())
  • Conditional compilation directives handle methods only available in .NET 11+ (Rune-based overloads)
  • Pragma directives suppress analyzer warnings during test execution since the test code intentionally calls methods without StringComparison parameters

https://claude.ai/code/session_01JGXMy71bpLs2UyvA8LToaa

claude added 4 commits October 8, 2026 13:34
The char and Rune overloads of string.Contains, StartsWith, EndsWith,
IndexOf, and LastIndexOf are ordinal, so specifying a StringComparison
is noise, as .NET 11 adds a StringComparison overload for each of them.
CA1307 and CA1310 do not report them either. They can still be reported
by setting MA0001.report_char_overloads to true.

Fixes #1624

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JGXMy71bpLs2UyvA8LToaa
Revert the MA0001.report_char_overloads option: the char and Rune
overloads of string.Contains, StartsWith, EndsWith, IndexOf, and
LastIndexOf are ordinal, so MA0001 keeps reporting them when an overload
with a StringComparison exists.

string.Contains(string) is ordinal too, so it is now reported by MA0001
instead of MA0074. Add tests that check the reported rule and the code fix
for all the string overloads that have a StringComparison overload.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JGXMy71bpLs2UyvA8LToaa
Expose the documentation IDs of the methods whose default comparison is
Ordinal or OrdinalIgnoreCase, and add a test for each of them that checks
the method behaves as an ordinal comparison, using an input whose result
differs with a culture-sensitive comparison. A method added to the list
without a test makes the test fail.

The Rune overloads of string are only available from .NET 11, so their
tests are skipped until the tests run on .NET 11.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JGXMy71bpLs2UyvA8LToaa
…s reference

The methods of Microsoft.Extensions.Primitives are excluded from the
test that checks the default comparison of the non culture-sensitive
methods. Also use escape sequences instead of invisible characters in the
test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JGXMy71bpLs2UyvA8LToaa
@meziantou
meziantou merged commit 75514c6 into main Oct 8, 2026
14 checks passed
@meziantou
meziantou deleted the feature/gallant-maxwell-ovinjb branch October 8, 2026 15:43
This was referenced Oct 8, 2026
This was referenced Oct 15, 2026
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.

2 participants