bun_core: make bun_core::strings a direct alias of string::immutable

[robobun]

What

bun_core::strings was a back-compat pub mod strings { ... } in bun_core/lib.rs that glob-merged the crate root (which carried scalar fallbacks from an internal strings_impl module) with crate::string::immutable (the highway/simdutf-backed canonical bun.strings namespace), then hand-disambiguated ~30 colliding names. string::immutable in turn re-exported pieces of crate::strings back into itself, forming a mutual re-export fixpoint, and the same module was reachable as bun_core::strings, bun_core::immutable, bun_core::string_immutable, and bun_core::string::strings.

The practical cost was two implementations per name: bun_core::contains resolved to a bstr scalar while bun_core::strings::contains resolved to the highway one, bun_core::index_of_char returned Option<usize> while bun_core::strings::index_of_char returned Option<u32>, and so on.

Change

• bun_core::strings is now literally pub use crate::string::immutable as strings;. The bun_core::immutable, bun_core::string_immutable, and bun_core::string::strings aliases are deleted and their callers migrated.
• string::immutable's back-references to crate::strings now go through crate::strings_impl (the tier-0 module that actually defines the transcoding primitives), so the re-export cycle is gone.
• Every scalar twin in strings_impl that shadowed a highway/simdutf implementation in immutable is deleted: contains, includes, index_of_char, starts_with, ends_with, eql, eql_long, eql_comptime_ignore_len, index_of_any, index_of_any_t, last_index_of_char, last_index_of_char_t, first_non_ascii16, a duplicate ares_inet_pton extern plus is_ipv6_address, a duplicate Encoding enum, the CodepointIterator stub fmt.rs used, and bun_alloc::copy_lowercase_if_needed (shadowed by immutable's). Their callers move to bun_core::strings::*.
• bun_resolver's local copy of BOM (a near-verbatim duplicate of bun_core::strings::BOM, justified by a comment claiming BOM was not exported, which was false) becomes pub use bun_core::strings::BOM;. The canonical impl is a strict superset and the bun_resolver::fs_full::BOM re-export is unchanged.
• index_of_any / index_of_any_t / index_of_any16 had two live signatures (the alias served a scalar Option<usize> form to half the callers and a highway Option<u32> form to the other half). They are unified to one: non-'static chars and Option<usize>, backed by highway::index_of_any_char. The OptionalUsize type alias this leaves unused is removed, along with the as usize casts that became identity.
• strings_impl::first_non_ascii is renamed first_non_ascii_usize so strings::first_non_ascii (the Option<u32> form) is the only spelling of that name, matching the existing index_of_char / index_of_char_usize pair.

After this, every public name in strings_impl is either re-exported from immutable (same item) or has a distinct name, so bun_core::X and bun_core::strings::X can never refer to two different functions.

src/bun_core/lib.rs alone loses 175 lines; the whole diff is +310/-642 across 54 files (most of the caller churn is mechanical path renames).

Verification

• cargo check --workspace passes on all 10 CI target triples (bun run rust:check-all: 10 ok, 0 failed).
• cargo clippy --workspace --no-deps reports nothing.
• cargo check --workspace --all-targets has exactly the same pre-existing cfg(test) errors as main (verified by stashing the diff and re-running); this change adds none.
• bun bd builds.
• The call sites that actually switch implementation (from a scalar strings_impl loop to the highway one) are concentrated in src/bun_core/fmt.rs: the JS syntax highlighter's secret-redaction arms, PowerShell argument escaping, and path-separator normalization. The tests that drive those directly pass against the debug build: test/js/bun/util/highlighter.test.ts (which carries the regression tests for three prior out-of-bounds bugs in exactly that boundary arithmetic), test/internal/highlighter.test.ts, test/internal/powershell-escape.test.ts, and test/cli/install/redacted-config-logs.test.ts.
• A broader slice over the renamed call sites also passes: bundler_naming, bundler_string, glob match/scan, css, shell brace/lex/parse, md-spec, gfm-compat, inspect, escapeRegExp, escapeHTML, and the test/regression/issue/23275.test.ts BOM test. (Note: the escapeRegExp/escapeHTML/MutableString callers did not change implementation; they were already on the highway immutable::index_of_any path on main, and the only diff there is dropping as usize casts the return-type unification made identity.)

The only intentional behavioral deltas are removals of panics that no caller depends on: index_of_any(slice, b"") now returns None instead of hitting unreachable!(), and index_of_any / index_of_any16 no longer panic on inputs larger than 4 GB (they returned u32 indices via try_from(..).unwrap()).

Why there is no new test file

This change is behavior-preserving by construction, and I could not find a JavaScript-observable input on which main and this branch differ:

• Every call site whose implementation switches moves to a function that computes the same value for the same input (each pairing is spelled out in the Change section, and the suites above exercise them).
• The two panic removals above are unreachable from JS: every index_of_any caller passes a compile-time, non-empty character set, and a JSC string tops out at 2^31 code units, below the 4 GB u32 overflow the old code asserted on.
• Everything else is a module-path rename the compiler checks on all 10 target triples.

A regression test for this PR would therefore pass identically with and without the src/ changes, which makes it worthless as a guard. The compile checks and the existing suites listed under Verification are the proof this change admits.

[robobun]

^{Updated 5:13 AM PT - Jun 29th, 2026}

❌ @robobun, your commit dae7fce has 1 failures in Build #66709 (All Failures):

• test/js/node/test/parallel/test-net-connect-memleak.js - code 1 on 🐧 3.23 x64-baseline
• test/js/node/test/parallel/test-net-connect-memleak.js - code 1 on 🐧 3.23 x64

---

🧪   To try this PR locally:

bunx bun-pr 33035

That installs a local version of the PR into your bun-33035 executable, so you can run:

bun-33035 --bun

[coderabbitai]

Walkthrough

Consolidates bun_core::strings around crate::string::immutable, moves canonical implementations into strings_impl, updates index_of_any-style helpers to use usize indices, removes deprecated helpers, and migrates core and downstream call sites to the new string namespace.

Changes

strings namespace consolidation

Layer / File(s)	Summary
Core strings surface src/bun_core/lib.rs, src/bun_alloc/lib.rs	Reworks the crate-root strings aliasing and removes deprecated helpers from the exported strings surface.
Immutable unicode and re-export routing src/bun_core/string/immutable.rs, src/bun_core/string/immutable/unicode.rs, src/resolver/fs.rs, src/resolver/resolver.rs, src/paths/lib.rs	Redirects immutable/unicode helpers and BOM/path re-exports to crate::strings_impl or bun_core::strings, and updates related module docs/comments.
bun_core internal call sites src/bun_core/Global.rs, src/bun_core/fmt.rs, src/bun_core/util.rs	Switches internal formatter, breakpoint, and boundary-scanning logic to the new crate::strings helpers and usize index types.
Core string submodules src/bun_core/string/MutableString.rs, src/bun_core/string/StringBuilder.rs, src/bun_core/string/StringJoiner.rs, src/bun_core/string/escapeRegExp.rs, src/bun_core/string/identifier.rs, src/bun_core/string/wtf.rs, src/bun_core/string/mod.rs	Updates string submodule imports and scanning code to the new namespace and index return types.
Downstream crate migrations src/ast/..., src/bundler/..., src/crash_handler/..., src/css/..., src/http/lib.rs, src/ini/lib.rs, src/install/..., src/js_printer/..., src/jsc/..., src/md/links.rs, src/paths/string_paths.rs, src/runtime/..., src/shell_parser/..., src/url/lib.rs	Migrates remaining call sites across AST, bundler, runtime, shell, URL, and related crates to bun_core::strings and the updated helper signatures.

Possibly related PRs

• oven-sh/bun#30993: Both PRs touch src/bun_core/lib.rs around strings_impl::CodepointIterator and related iterator exports.
• oven-sh/bun#31557: Both PRs are tied to the first_non_ascii / first_non_ascii_usize string-scanning helper surface.
• oven-sh/bun#32924: Both PRs modify src/jsc/ConsoleObject.rs formatting paths.

Suggested reviewers

• alii
• dylan-conway

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change by describing bun_core::strings as a direct alias of string::immutable.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The description covers the PR purpose and verification details, though the headings differ slightly from the repository template.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/jsc/ConsoleObject.rs`:
- Around line 3168-3171: The UTF-16 key-path branch in
ConsoleObject::write_object_property formatting is writing embedded quote
characters back out unescaped, unlike the Latin-1 path. Update the loop that
uses strings::index_of_any16 and writes through writer.write_16_bit /
writer.write_all so that any embedded QUOTE_U16 is emitted with the same
JSON-style escaping as the Latin-1 branch, keeping quoted property names
consistent for non-Latin1 keys.

In `@src/jsc/webcore_types.rs`:
- Around line 831-838: The trailing slash handling in S3 path normalization is
incorrect in the path trimming logic inside the S3 path routine in
webcore_types.rs. Update the branch that checks
bun_core::strings::ends_with(path_name, b"/") so it removes the final byte like
the backslash branch already does, and keep the normalization behavior
consistent in S3::path() for both "/" and "\\" suffixes.

In `@src/runtime/cli/create_command.rs`:
- Around line 1764-1770: Restore the second-slash lookup in
create_command::parse_template_positional so the GitHub-repo detection only
triggers when there is exactly one slash. The current repeated call to
bun_core::strings::index_of_char_usize on positional makes first_slash_index ==
last_slash_index for any slash-containing input, so update the logic to use the
last-slash check as intended and keep foo/bar/baz resolving as a template path
instead of a repo shorthand.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: b1ae73e2-34cc-4c51-a35f-707c555f122f

📥 Commits

Reviewing files that changed from the base of the PR and between f789198 and ee965c2.

📒 Files selected for processing (54)

• src/ast/expr.rs
• src/ast/lib.rs
• src/bun_alloc/lib.rs
• src/bun_core/Global.rs
• src/bun_core/fmt.rs
• src/bun_core/lib.rs
• src/bun_core/string/MutableString.rs
• src/bun_core/string/StringBuilder.rs
• src/bun_core/string/StringJoiner.rs
• src/bun_core/string/escapeRegExp.rs
• src/bun_core/string/identifier.rs
• src/bun_core/string/immutable.rs
• src/bun_core/string/immutable/unicode.rs
• src/bun_core/string/mod.rs
• src/bun_core/string/wtf.rs
• src/bun_core/util.rs
• src/bundler/Chunk.rs
• src/bundler/defines.rs
• src/bundler/linker_context/postProcessJSChunk.rs
• src/bundler/linker_context/writeOutputFilesToDisk.rs
• src/bundler/transpiler.rs
• src/crash_handler/lib.rs
• src/css/generics.rs
• src/css/properties/animation.rs
• src/css/rules/keyframes.rs
• src/http/lib.rs
• src/ini/lib.rs
• src/install/dependency.rs
• src/install/lib.rs
• src/js_printer/lib.rs
• src/js_printer/renamer.rs
• src/jsc/ConsoleObject.rs
• src/jsc/VirtualMachine.rs
• src/jsc/ipc.rs
• src/jsc/webcore_types.rs
• src/md/links.rs
• src/paths/lib.rs
• src/paths/string_paths.rs
• src/resolver/fs.rs
• src/resolver/resolver.rs
• src/runtime/api/cron.rs
• src/runtime/api/filesystem_router.rs
• src/runtime/bake/FrameworkRouter.rs
• src/runtime/bake/dev_server/mod.rs
• src/runtime/cli/create_command.rs
• src/runtime/cli/mod.rs
• src/runtime/jsc_hooks.rs
• src/runtime/node/fs_events.rs
• src/runtime/server/ServerConfig.rs
• src/runtime/socket/udp_socket.rs
• src/runtime/webcore/s3/client.rs
• src/shell_parser/braces.rs
• src/shell_parser/parse.rs
• src/url/lib.rs

💤 Files with no reviewable changes (2)

• src/runtime/bake/FrameworkRouter.rs
• src/resolver/resolver.rs

[robobun]

All three CodeRabbit findings land on lines this PR only renames; none is a regression from this change. Each one is also byte-for-byte identical in the original Zig source before the Rust rewrite (commit c8b4c36):

• src/jsc/webcore_types.rs:832: the no-op path_name[0..path_name.len()] is src/runtime/webcore/blob/Store.zig:317.
• src/runtime/cli/create_command.rs: the repeated first-slash lookup is src/cli/create_command.zig:1776-1777, which also calls std.mem.indexOfScalar twice.
• src/jsc/ConsoleObject.rs:3170: the bare quote in the UTF-16 key loop is src/jsc/ConsoleObject.zig:1993.

They look like real latent bugs worth fixing, but each is a user-visible behavior change (S3 key normalization, bun create template resolution, Bun.inspect output) that needs its own PR and test. This PR's contract is that nothing observable changes, so I left them alone and replied on each review thread with the evidence. Happy to send follow-ups if a maintainer confirms the intended behavior for any of them.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/resolver/fs.rs`:
- Line 27: The resolver’s BOM handling now depends on bun_core::strings::BOM,
but that implementation misses UTF-16LE BOM-only inputs because it returns None
for 2-byte buffers. Update the BOM logic used by src/resolver/fs.rs so BOM-only
UTF-16LE data (FF FE) is still detected and stripped/converted correctly before
relying on the shared alias. Keep the fix in the BOM parsing path itself so
callers of BOM continue to handle this edge case.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ffb32bb8-0662-4b8e-a892-feb9a7135d25

📥 Commits

Reviewing files that changed from the base of the PR and between ee965c2 and 4ae9bf9.

📒 Files selected for processing (2)

• src/bun_core/string/immutable.rs
• src/resolver/fs.rs

[claude]

I didn't find any issues beyond the pre-existing ones already discussed and resolved, but this is a 54-file refactor of the core bun.strings namespace (return-type unification, scalar→highway swaps in fmt.rs, dedup of BOM/is_ipv6_address/etc.), so it's worth a human pass.

Extended reasoning...

Overview

This PR collapses bun_core::strings from a back-compat pub mod that glob-merged the crate root's scalar fallbacks with string::immutable (and hand-disambiguated ~30 colliding names) into a direct pub use crate::string::immutable as strings; alias. It deletes the bun_core::immutable / string_immutable / string::strings alternate spellings, removes every scalar twin in strings_impl that shadowed a highway/simdutf implementation (contains, includes, index_of_char, eql_long, first_non_ascii16, is_ipv6_address, the duplicate Encoding enum, the CodepointIterator stub, etc.), unifies index_of_any{,_t,16} to return Option<usize> with non-'static needles, replaces bun_resolver's near-verbatim BOM duplicate with a re-export, and migrates ~50 call sites across ast/bundler/css/http/install/jsc/md/paths/resolver/runtime/shell/url. Net: +310/−642.

Security risks

None introduced. The change is a namespace consolidation; the only non-rename code paths affected are string-search/transcode primitives. No auth, crypto, permissions, deserialization, or network input-handling logic changes. The deleted duplicate is_ipv6_address / ares_inet_pton extern resolves to the identical canonical impl already in immutable.rs.

Level of scrutiny

Moderate-to-high. Most of the diff is mechanical path renames (bun_core::X → bun_core::strings::X, bun_core::immutable → bun_core::strings) and dropped now-identity as usize casts. But several call sites in src/bun_core/fmt.rs (JS syntax-highlighter redaction arms, PowerShell escaping, path-separator normalization, format_latin1) genuinely switch implementation from a scalar strings_impl loop to the highway-backed immutable one, and index_of_any* changed both signature and edge-case behavior (empty needle now returns None instead of unreachable!(); no longer panics on >4 GB input via the removed u32::try_from(..).unwrap()). The fmt.rs CodepointIterator swap (scalar stub → canonical immutable one) changes cursor.i from usize to a type requiring as usize, which the diff handles. These are exactly the spots a human reviewer should eyeball — the author already ran the targeted regression tests (highlighter.test.ts, powershell-escape.test.ts, redacted-config-logs.test.ts) covering them.

Other factors

• The bug-hunting system found nothing on this revision.
• All four prior inline threads (three CodeRabbit, one mine) were correctly identified as pre-existing behavior carried from the Zig source and resolved as out-of-scope; the author's reasoning for not taking drive-by fixes in a behavior-preserving refactor is sound.
• CI shows 2 failures (terminal.test.ts on macOS x64, test-net-connect-memleak.js on Linux x64) that appear unrelated to string-search code paths and look like known-flaky tests, but worth confirming.
• cargo check --workspace passes on all 10 target triples per the description; the PR is well-verified.

The refactor is clean and the description is unusually thorough, but 54 files touching the string layer that the resolver, bundler, parser, shell, and JSC formatters all sit on top of is outside the bar for bot approval.

[robobun]

The red test-bun lanes on this PR's CI (test/js/node/test/parallel/test-net-connect-memleak.js on both linux-x64-musl configs) are a repo-wide failure that started around 2026-06-28 23:00 UTC and currently hits about half of all PR builds on unrelated branches (13 of the most recent 28, versus 0 of the 40 before that window). It is the same JSC FinalizationRegistry-vs-setImmediate race that test/expectations.txt:28 already marks FLAKY on LINUX-X64-MUSL for the test's tls twin. Filed with the data as #33044.

This diff touches no GC, refcount, event-loop, or net code (it is a string-namespace refactor). Build 66657 finished with every other lane green across the whole matrix (macOS x64 and aarch64, Linux glibc x64 and aarch64 including the ASAN lane, musl aarch64, Windows, FreeBSD); the two musl-x64 shards running this one test were the only red. I pushed one ci: retrigger to re-roll them. If they flake again, the fix belongs in #33044, not here, and this PR just needs a maintainer to merge over it.

[claude]

No bugs found, but this is a 54-file refactor of the core bun_core::strings namespace that swaps scalar scanners for highway-backed ones and changes the index_of_any return type — worth a human sign-off given how widely these primitives are used.

Extended reasoning...

Overview

This PR collapses bun_core::strings from a hand-disambiguated glob-merge of crate-root scalar fallbacks + string::immutable into a direct pub use crate::string::immutable as strings; alias. It deletes ~15 scalar twin functions from strings_impl (the ones shadowed by highway/simdutf implementations in immutable), unifies the index_of_any{,_t,16} return type from Option<u32> to Option<usize> (dropping the 'static bound and the unreachable!() on empty needles), renames strings_impl::first_non_ascii → first_non_ascii_usize, replaces the resolver's duplicate BOM enum with a re-export of the canonical one, and migrates ~40 downstream call sites to the new paths. Net +310/-642 across 54 files; bun_core/lib.rs alone loses 175 lines.

Security risks

None identified. The change is a namespace consolidation with no auth, crypto, permission, or input-validation surface. The only behavioral deltas are panic removals (index_of_any(slice, b"") now returns None instead of unreachable!(); the u32::try_from(..).unwrap() on >4 GB inputs is gone), neither of which is reachable from JS per the description.

Level of scrutiny

Moderate-to-high. While the vast majority of the diff is mechanical path renames the compiler verifies, the substantive parts are not:

• Several fmt.rs call sites silently switch from a scalar strings_impl loop to the highway-backed immutable one (syntax highlighter redaction, PowerShell escaping, path-sep normalization). The author ran the regression tests covering exactly those (highlighter.test.ts, powershell-escape.test.ts, redacted-config-logs.test.ts).
• fmt.rs's FormatValidIdentifier swaps the local CodepointIterator stub for the canonical strings::CodepointIterator (whose cursor.i is u32, hence the new as usize casts).
• The index_of_any signature change ripples through MutableString, escapeRegExp, ConsoleObject, FrameworkRouter, md/links, etc., dropping now-identity as usize casts.
• The resolver BOM dedup was verified line-by-line in the thread to be a strict superset with identical detect() semantics.

These are core string-scanning primitives reached from the bundler, resolver, JS printer, shell, CSS, S3, and console — broad enough blast radius that a maintainer should confirm the scalar→highway equivalences are sound.

Other factors

• All four CodeRabbit findings and my own prior inline note were established as pre-existing (verified against main and the original Zig) and resolved; the author deliberately kept this PR behavior-neutral and deferred those to follow-ups.
• Verification is unusually thorough: cargo check on all 10 CI triples, clippy clean, bun bd builds, and a targeted test slice over every implementation-switching call site passes. The red musl-x64 lane is a documented repo-wide flake (#33044) unrelated to this diff.
• No new tests are added; the description argues (reasonably) that a regression test would pass identically on main, and the compile checks across 10 triples are the proof for the rename portion.
• CodeRabbit suggested alii and dylan-conway as reviewers.