Add methods from #27912 (Flow System.Text.Rune through more APIs) #117168

Joy-less · 2025-06-30T17:58:57Z

Supercedes #111815.

This time the methods are fully implemented and ready to go, and the ref assemblies are added to match the implementations.

Copilot

Pull Request Overview

This PR adds a set of new API overloads to support System.Text.Rune across various string and text manipulation APIs, addressing methods such as Equals, Contains, IndexOf/LastIndexOf, Replace, Trim, Append, Insert, and Write. The changes span multiple libraries including System.Runtime and System.Private.CoreLib to fully integrate rune-based operations.

Added overloads for string comparison methods accepting System.Text.Rune.
Introduced new methods on StringBuilder, TextWriter, and TextInfo to handle rune conversion and manipulation.
Updated internal string splitting and trimming routines to support rune separators and trim characters.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/libraries/System.Runtime/ref/System.Runtime.cs	Added new overloads for string API methods accepting System.Text.Rune and StringComparison.
src/libraries/System.Private.CoreLib/src/System/Text/StringBuilder.cs	Added Append/Insert/Replace methods with System.Text.Rune and Rune retrieval overloads.
src/libraries/System.Private.CoreLib/src/System/Text/Rune.cs	Updated Equals implementations and added new overloads for comparing Runes with StringComparison.
src/libraries/System.Private.CoreLib/src/System/String.Searching.cs	Provided new overloads of Contains, IndexOf, and LastIndexOf for System.Text.Rune with careful iteration over surrogates.
src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs	Added Replace, Split, and Trim overloads using System.Text.Rune for enhanced string manipulation.
src/libraries/System.Private.CoreLib/src/System/String.Comparison.cs	Extended StartsWith and EndsWith to support System.Text.Rune comparisons.
src/libraries/System.Private.CoreLib/src/System/IO/TextWriter.cs	Integrated Write and WriteLine overloads that accept System.Text.Rune.
src/libraries/System.Private.CoreLib/src/System/Globalization/TextInfo.cs	Implemented ToLower and ToUpper for System.Text.Rune conversions.
src/libraries/System.Private.CoreLib/src/System/Char.cs	Added an overload for Equals accepting a StringComparison using modern array initialization.

Comments suppressed due to low confidence (1)

src/libraries/System.Private.CoreLib/src/System/Text/StringBuilder.cs:2850

In GetRuneAt(int index), consider throwing a more specific exception type (such as ArgumentOutOfRangeException) rather than a generic Exception for improved clarity and consistency with API design.

            throw new Exception($"Unable to get rune at {index}.");

src/libraries/System.Private.CoreLib/src/System/String.Searching.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

dotnet-policy-service · 2025-07-28T05:03:40Z

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

jeffhandley · 2025-09-01T21:17:34Z

@stephentoub - Since this has been open as long as it has, I wanted to check with you that we should still pursue adding the Rune overloads. Any concerns or hesitations?

Joy-less · 2025-09-05T16:00:46Z

@jeffhandley I have made some changes to my PR:

Fixed TextInfo.ToLower(Rune) and TextInfo.ToUpper(Rune) for runes which use two chars. Very niche but affects some characters like 𐐨 (\uD801\uDC28) -> 𐐀 (\uD801\uDC00).
Used the ordinal char.Equals(char) implementation for char.Equals(char, StringComparison) when StringComparison is StringComparison.Ordinal.

I think #111170 is a relevant pull request to this one.

Also, the original creator of the API proposals for both this pull request and the above pull request has this on their profile:

On other projects, not checking GitHub notifications - ping via Teams if urgent.

tarekgh · 2025-09-05T16:06:04Z

@Joy-less I'll get into this PR in the next few days. Thanks!

tarekgh · 2025-09-23T23:48:32Z

@Joy-less do you compile and run the tests locally before submitting changes? curious to know 😄.

Joy-less · 2025-09-24T00:01:58Z

As far as I know all of the tests have been implemented now, granted they are kind of all over the place so I may have missed some. Feel free to go through them now.

#117168 (comment)

I actually added these to an existing test, do you want them separate?
6b6fae9

Joy-less · 2025-09-24T00:03:22Z

@Joy-less do you compile and run the tests locally before submitting changes? curious to know 😄.

No haha, I'm not sure how especially with the lack of .NET 10.0 on my system and issues installing it.

tarekgh · 2025-09-24T02:35:15Z

No haha, I'm not sure how especially with the lack of .NET 10.0 on my system and issues installing it.

You don't need to install that. You can do:

assuming you cloned on a folder like C:\runtime
cd C:\runtime
Build from the root doing .\build.cmd -s libs+clr -c Release
then go to the test folder like: pushd .\src\libraries\System.Runtime\tests\System.Runtime.Tests\
Run the test C:\runtime\.dotnet\dotnet build /t:test /p:Configuration=Release
You can run any other tests same way

I'll try to clone your branch tomorrow and run it myself, would be faster push fixes 😄

tarekgh · 2025-09-24T02:41:00Z

I actually added these to an existing test, do you want them separate?

This works with a single character; we need to test surrogate pairs too. This is why I suggested doing something like the one using the string separator.

…into rune-overloads

tarekgh · 2025-09-25T02:03:38Z

@Joy-less I pushed some changes to fix some behavior and ensure the runtime tests are passing now with no problem. Please take a look and let me know if there is anything else you want to add before I take a last look tomorrow and try to merge. Thanks!

Joy-less · 2025-09-25T02:34:45Z

src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs


-            return SplitInternal(separatorChars, count, options);
+            // Ensure matching the string separator overload.
+            return count <= 1 || Length == 0 ? CreateSplitArrayOfThisAsSoleValue(options, count) : Split(runeSeparator, count, options);


I think this would be easier to read with brackets:
(count <= 1 || Length == 0)

Change it if you want, 😄

Joy-less · 2025-09-25T02:36:58Z

src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs

        public string[] Split(Rune separator, int count, StringSplitOptions options = StringSplitOptions.None)
        {
-            if (Length == 0)
+            ReadOnlySpan<char> runeSeparator = separator.AsSpan(stackalloc char[Rune.MaxUtf16CharsPerRune]);


Maybe should be separatorChars instead of runeSeparator to be consistent with the other implementations.

Chars can indicate multiple separators, I wanted to be clear this is a single separator.

How about separatorSpan? runeSeparator implies that it's separator typed as a Rune which is actually the opposite of the case.

Joy-less · 2025-09-25T02:42:01Z

@Joy-less I pushed some changes to fix some behavior and ensure the runtime tests are passing now with no problem. Please take a look and let me know if there is anything else you want to add before I take a last look tomorrow and try to merge. Thanks!

I think that's everything done now. Let me know if there's something else I missed or need to look at. Thanks for your diligence.

Joy-less added 7 commits January 24, 2025 22:07

Add rune overloads

5e94820

Fix some compiler errors

ddecd49

More compiler error fixes

bedf656

Even more compiler error fixes

b67dfb9

Fix more compiler errors

725d1ee

Merge branch 'dotnet:main' into rune-overloads

b2d1828

Various improvements & add refs

1f6e247

Copilot AI review requested due to automatic review settings June 30, 2025 17:58

github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jun 30, 2025

dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jun 30, 2025

stephentoub requested a review from GrabYourPitchforks June 30, 2025 17:59

Copilot AI reviewed Jun 30, 2025

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/String.Searching.cs Outdated Show resolved Hide resolved

Joy-less and others added 2 commits June 30, 2025 19:04

Fix validation in LastIndexOf

b1e168f

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Fix replace core usage

5a43a0e

marek-safar added the area-System.Runtime label Jul 28, 2025

MihaZupan removed the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Aug 1, 2025

Merge branch 'main' into rune-overloads

180825f

jeffhandley requested a review from tarekgh September 1, 2025 21:16

jeffhandley assigned tarekgh Sep 1, 2025

Joy-less added 2 commits September 5, 2025 16:48

Fix TextInfo ToLower/ToUpper with 2-char runes

6c1bc2e

Optimize char.Equals for Ordinal StringComparison

74fbb49

build-analysis bot mentioned this pull request Sep 5, 2025

[android] Android.Device_Emulator.JIT.Test failing on emulators with CoreCLR #112633

Open

Joy-less and others added 19 commits September 24, 2025 20:34

Add dedicated SplitRuneSeparator test

c935a5f

Fix trim rune tests

9354640

Fix invalid collection expressions in tests

332a040

Fix InsertRune test

819a480

Fix replace rune tests

7a4742f

Fix unused comparisonType parameter

6104e54

Fix index of test names

bb1c1e9

Remove redundant test

c5ea928

Fix missing using

dc48f17

Fix replace method called with string comparison argument

0dd15b7

Fix tried to test split with rune on span

bae28a1

Remove start index from LastIndexOf tests

6bbea5b

Remove non-existent comparisonType parameter

e098468

Fix StartsWith test incorrect call

5f4e8b8

Fix LastIndexOf behaviour

f289829

Various rune test fixes

9e883b3

Fix error in LastIndexOf test

c48ff26

Some fixes and ensure the Runtime tests passes

73df815

Merge branch 'rune-overloads' of https://github.com/Joy-less/runtime …

59c0f39

…into rune-overloads

Joy-less commented Sep 25, 2025

View reviewed changes

This was referenced Sep 25, 2025

Checkout failure: "Git fetch failed with exit code 128" dotnet/arcade#9009

Open

Unable to checkout dotnet/dnceng#4115

Open

Add methods from #27912 (Flow System.Text.Rune through more APIs) #117168

Are you sure you want to change the base?

Add methods from #27912 (Flow System.Text.Rune through more APIs) #117168

Uh oh!

Conversation

Joy-less commented Jun 30, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

dotnet-policy-service bot commented Jul 28, 2025

Uh oh!

jeffhandley commented Sep 1, 2025

Uh oh!

Joy-less commented Sep 5, 2025

Uh oh!

tarekgh commented Sep 5, 2025

Uh oh!

tarekgh commented Sep 23, 2025

Uh oh!

Joy-less commented Sep 24, 2025

Uh oh!

Joy-less commented Sep 24, 2025

Uh oh!

tarekgh commented Sep 24, 2025

Uh oh!

tarekgh commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tarekgh commented Sep 25, 2025

Uh oh!

Joy-less Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

tarekgh Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

Joy-less Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

tarekgh Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

Joy-less Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

Joy-less commented Sep 25, 2025

Uh oh!

Uh oh!

tarekgh commented Sep 24, 2025 •

edited

Loading