Add matchLength-returning APIs to CompareInfo

[GrabYourPitchforks]

Fixes #27935.

The ICU logic is a little weird since we need to call ucol_next and ucol_previous to discover the attributes of the current collation element, but those methods have the side effect of mutating the offset. So I end up calling ucol_getOffset defensively before the iterator is advanced, which allows us to make a local copy. This is guarded behind a "is the caller actually asking for this?" check so that we don't take this hit in the common case where the caller doesn't care. (/cc @adamsitnik and @safern since I think you were spelunking in this code recently.)

I also modified the Utf8String.TryFind and string.Replace methods to call the new public API instead of the internal API. This has the side effect of allowing us to remove the unsafe annotation from string.Replace, since we're no longer working with raw pointers.

There's a decent amount of churn in the unit test files because I annotated all of the test cases with "what should matchLength be when the method returns?" If your diff tool shows per-character differences rather than highlighting the entire line this will make a lot more sense.

[ghost]

Tagging subscribers to this area: @tarekgh, @safern, @krwq
See info in area-owners.md if you want to be subscribed.

[Dotnet-GitSync-Bot]

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

[GrabYourPitchforks]

This was incorrectly returning the target length instead of the source length and was causing some newly introduced unit tests to fail.

[adamsitnik]

great catch @GrabYourPitchforks ! it's not a common case code path:

runtime/src/libraries/Native/Unix/System.Globalization.Native/pal_collation.c

Line 483 in 5137178

// It's possible somebody passed us (source = <empty>, target = <non-empty>).

but should we consider backporting this fix to older 3.1/2.1?

[GrabYourPitchforks]

Don't know if it requires backporting, to be honest. Here's a simplified repro:

var newString = "".Replace("\u200d", "x", StringComparison.InvariantCulture);

Before this fix, this throws an ArgumentOutOfRangeException (due to attempting to slice a span in an invalid fashion). After the fix, this correctly returns "".

To trigger the bug, the "this" string must be empty, and the "old" string must be non-empty and consist only of zero-weight chars, and the caller must be passing a linguistic comparison such as InvariantCulture. I think the likelihood of all three happening at the same time are low enough where we couldn't justify the backport.

[tarekgh]

I agree we don't have to service this till we get a case need it.

[adamsitnik]

Overall looks great to me, however, it would be great if you could add more edge case test cases and check if we can replace the call to ucol_getOffset with math on our side.

[adamsitnik]

great catch @GrabYourPitchforks ! it's not a common case code path:

runtime/src/libraries/Native/Unix/System.Globalization.Native/pal_collation.c

Line 483 in 5137178

// It's possible somebody passed us (source = <empty>, target = <non-empty>).

but should we consider backporting this fix to older 3.1/2.1?

[GrabYourPitchforks]

CI failure is known issue #40416.

[GrabYourPitchforks]

PR feedback was:

• Reduce parameter count taken in SimpleAffix native methods
• Rename parameters in the p/invoke declaration to match existing conventions
• Simplify newly-added IsPrefix / IsSuffix methods by delegating to existing overloads where possible
• Additional unit test edge case coverage
• Use named parameters instead of /* comments */

[GrabYourPitchforks]

Thank you both for the excellent feedback! :D