Improve stdlib stringSplit behavior for empty separator by shargon · Pull Request #4589 · neo-project/neo

shargon · 2026-07-23T18:02:24Z

Description

Unit Testing
Run Application
Local Computer Tests
No Testing

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

codecov · 2026-07-23T18:04:32Z

Codecov Report

❌ Patch coverage is 60.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.90%. Comparing base (3e0ed07) to head (dee1671).

Files with missing lines	Patch %	Lines
src/Neo/SmartContract/Native/StdLib.cs	60.00%	3 Missing and 3 partials ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##           master-n3    #4589      +/-   ##
=============================================
- Coverage      81.96%   81.90%   -0.07%     
=============================================
  Files            236      236              
  Lines          16490    16504      +14     
  Branches        2402     2406       +4     
=============================================
+ Hits           13516    13517       +1     
- Misses          2198     2206       +8     
- Partials         776      781       +5

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

cschuchardt88 · 2026-07-24T06:28:37Z

@shargon — apologies for the overlap. I opened #4590 with a related implementation that builds on the same goal as this PR (empty-separator stringSplit behavior for #4585).

I should have coordinated on this branch instead of opening a separate PR. #4590 is meant as an incremental improvement (hardfork-gated rune split, expanded tests, and a small StrLen cleanup). Please take a look when you have a chance; happy to align, close #4590 in favor of this one, or fold the changes here—whichever you prefer.

Also, I don't have access to modify or create branches in neo-project anymore. So I couldn't update the current branch you're working on.

shargon · 2026-07-25T12:38:47Z

@shargon — apologies for the overlap. I opened #4590 with a related implementation that builds on the same goal as this PR (empty-separator stringSplit behavior for #4585).

I should have coordinated on this branch instead of opening a separate PR. #4590 is meant as an incremental improvement (hardfork-gated rune split, expanded tests, and a small StrLen cleanup). Please take a look when you have a chance; happy to align, close #4590 in favor of this one, or fold the changes here—whichever you prefer.

Also, I don't have access to modify or create branches in neo-project anymore. So I couldn't update the current branch you're working on.

Could you open a pr to this one with the ut only?

cschuchardt88 · 2026-07-25T15:05:10Z

@shargon as long as you use my stdlib.stringsplit method changes. Because I updated both stringsplit methods, so they both behave the same.

vncoelho

Another option would be to create a new method that has this behavior.

shargon · 2026-08-05T07:54:17Z

@roman-khimov do you have any tests about how go deal with UTF8, or unicode characteres?

cschuchardt88 · 2026-08-09T00:14:57Z

@roman-khimov do you have any tests about how go deal with UTF8, or unicode characteres?

maybe this can help:

https://github.com/neo-project/neo/pull/4590/changes#diff-291683c268fc3408d5df727e7e771c1be7c64b6fa64db99134788564d4443867

cschuchardt88

Summary

Hardfork-gates empty-separator StdLib.stringSplit to a UTF-16/rune split ("abc","" → ["a","b","c"]) to match Go strings.Split, closing #4585. Only the 2-arg overload is changed. Tests rewrite the existing "abc","" case to the Huyao result; Codecov patch coverage is 60%.

Empty-string + empty-separator returns [] outside the Huyao check, so genesis replay of stringSplit("", "") changes from [""] to [] before Huyao. The 3-arg overload is untouched, so after Huyao stringSplit(s, "") and stringSplit(s, "", false) disagree. No pre-Huyao, empty+empty, 3-arg, or Unicode coverage.

Also note: using Neo.VM looks unused (ApplicationEngine is Neo.SmartContract). neo-go still returns identity for empty sep (C# compat); Huyao rune split needs a matching neo-go change or mixed nodes fork. PR is mergeable but behind master-n3.

Issues

bug src/Neo/SmartContract/Native/StdLib.cs:259 — empty+empty [] is not Huyao-gated. Status: open
bug src/Neo/SmartContract/Native/StdLib.cs:252 — 3-arg stringSplit still identity on empty separator. Status: open
suggestion src/Neo/SmartContract/Native/StdLib.cs:268 — ConvertToUtf32 vs Go runes; unpaired surrogates FAULT. Status: open
suggestion tests/Neo.UnitTests/SmartContract/Native/UT_StdLib.cs:266 — tests miss pre-Huyao, empty+empty, 3-arg, Unicode. Status: open

cschuchardt88 · 2026-09-20T01:26:39Z

+            {
+                if (string.IsNullOrEmpty(str))
+                {
+                    return [];


bug: return [] for empty str + empty separator sits outside IsHardforkEnabled(Hardfork.HF_Huyao).

Pre-Huyao (and current mainnet) stringSplit("", "") is .NET "".Split("") → [""]. This path changes that to [] at every height, including genesis replay, as soon as this binary is running. That is an ungated consensus break.

Go strings.Split("", "") is [], which is the right Huyao result — it just has to be inside the Huyao branch:

if (string.IsNullOrEmpty(separator)) { if (engine.IsHardforkEnabled(Hardfork.HF_Huyao)) return string.IsNullOrEmpty(str) ? [] : /* rune split */; } return str.Split(separator, StringSplitOptions.None);

Please add a pre-Huyao test that stringSplit("", "") is still [""].

cschuchardt88 · 2026-09-20T01:26:39Z


        [ContractMethod(CpuFee = 1 << 8)]
-        private static string[] StringSplit([MaxLength(MaxInputLength)] string str, string separator)
+        private static string[] StringSplit(ApplicationEngine engine, [MaxLength(MaxInputLength)] string str, string separator)


bug: Only this 2-arg overload is Huyao-gated. The 3-arg sibling is unchanged:

private static string[] StringSplit(string str, string separator, bool removeEmptyEntries) => str.Split(separator, removeEmptyEntries ? StringSplitOptions.RemoveEmptyEntries : StringSplitOptions.None);

After Huyao:

stringSplit("abc", "") → ["a","b","c"]

stringSplit("abc", "", false) → ["abc"] (still identity)

stringSplit("", "") → [] (this PR, even pre-Huyao)

stringSplit("", "", false) → [""]

Compiler/framework callers often hit the 3-arg method. Both overloads must share the empty-separator path (delegate 2-arg → 3-arg, as in #4590). Please also cover stringSplit(s, "", true/false) in tests.

cschuchardt88 · 2026-09-20T01:26:39Z

+                    var result = new List<string>(str.Length);
+                    for (var i = 0; i < str.Length; i += char.IsHighSurrogate(str[i]) ? 2 : 1)
+                    {
+                        result.Add(char.ConvertFromUtf32(char.ConvertToUtf32(str, i)));


suggestion: char.ConvertToUtf32(str, i) throws ArgumentException on an unpaired high/low surrogate (including a trailing high surrogate, because IsHighSurrogate still advances by 2). That FAULTs the VM after Huyao; pre-Huyao Split does not throw.

Go strings.Split(s, "") splits by UTF-8 runes. The C# equivalent is str.EnumerateRunes() (unpaired surrogates become Rune values, no throw):

return [.. str.EnumerateRunes().Select(static r => r.ToString())];

If FAULT on invalid UTF-16 is intentional, please document it and add a test. Also add an emoji / surrogate-pair case ("A😀👍🚀" → 4 parts) — asked on this PR, still uncovered.

cschuchardt88 · 2026-09-20T01:26:39Z

            arr = engine.ResultStack.Pop<VM.Types.Array>();
-            Assert.HasCount(1, arr);
-            Assert.AreEqual("abc", arr[0].GetString());
+            Assert.HasCount(3, arr);


suggestion: This only asserts Huyao 2-arg ASCII ("abc","" → 3 parts). TestProtocolSettings.Default enables every hardfork at height 0, so the pre-Huyao identity path, the empty-str branch, and the Huyao-false fallthrough are untested (Codecov patch 60%, 6 lines missing).

Please add:

Pre-Huyao (omit HF_Huyao from ProtocolSettings.Hardforks; PersistingBlock is null so IsHardforkEnabled is ContainsKey): stringSplit("abc", "") → ["abc"], stringSplit("", "") → [""].

Huyao empty+empty: stringSplit("", "") → [].

3-arg empty separator, both removeEmptyEntries values.

At least one scalar/emoji split (and a non-empty separator that is itself a surrogate pair).

#4590 already has this layout if you want to fold the tests in.

Related to #4585

038d83e

github-actions Bot added the N3 label Jul 23, 2026

shargon changed the title ~~Related to https://github.com/neo-project/neo/issues/4585~~ Improve stdlib stringSplit behavior for empty separator Jul 24, 2026

Merge branch 'master-n3' into improve-string-split

e246adf

Merge branch 'master-n3' into improve-string-split

6e8c32a

vncoelho approved these changes Jul 29, 2026

View reviewed changes

Wi1l-B0t and others added 2 commits August 3, 2026 08:25

Merge branch 'master-n3' into improve-string-split

5fad960

Merge branch 'master-n3' into improve-string-split

dee1671

cschuchardt88 mentioned this pull request Sep 20, 2026

Review tracker: pending reviews on open neo-project PRs #4756

Open

47 tasks

cschuchardt88 reviewed Sep 20, 2026

View reviewed changes

Navigation Menu

Improve stdlib stringSplit behavior for empty separator - #4589

Improve stdlib stringSplit behavior for empty separator#4589
shargon wants to merge 5 commits into
master-n3from
improve-string-split

shargon commented Jul 23, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Jul 23, 2026 •

edited

Loading

Uh oh!

cschuchardt88 commented Jul 24, 2026 •

edited

Loading

Uh oh!

shargon commented Jul 25, 2026

Uh oh!

cschuchardt88 commented Jul 25, 2026

Uh oh!

vncoelho left a comment

Uh oh!

shargon commented Aug 5, 2026

Uh oh!

cschuchardt88 commented Aug 9, 2026

Uh oh!

cschuchardt88 left a comment

Uh oh!

cschuchardt88 Sep 20, 2026

Uh oh!

cschuchardt88 Sep 20, 2026

Uh oh!

cschuchardt88 Sep 20, 2026

Uh oh!

cschuchardt88 Sep 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

shargon commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist:

Uh oh!

codecov Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

cschuchardt88 commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shargon commented Jul 25, 2026

Uh oh!

cschuchardt88 commented Jul 25, 2026

Uh oh!

vncoelho left a comment

Choose a reason for hiding this comment

Uh oh!

shargon commented Aug 5, 2026

Uh oh!

cschuchardt88 commented Aug 9, 2026

Uh oh!

cschuchardt88 left a comment

Choose a reason for hiding this comment

Summary

Issues

Uh oh!

cschuchardt88 Sep 20, 2026

Choose a reason for hiding this comment

Uh oh!

cschuchardt88 Sep 20, 2026

Choose a reason for hiding this comment

Uh oh!

cschuchardt88 Sep 20, 2026

Choose a reason for hiding this comment

Uh oh!

cschuchardt88 Sep 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

shargon commented Jul 23, 2026 •

edited

Loading

codecov Bot commented Jul 23, 2026 •

edited

Loading

cschuchardt88 commented Jul 24, 2026 •

edited

Loading