fix(util): quote top-level strings in util.format %o/%O (Node parity)#5106
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughA new internal helper Changesutil.inspect string quoting for %o/%O
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
7f4365f to
61f62a8
Compare
`util.format("%o", "hello")` / `console.log("%o", str)` printed the
string unquoted (`hello`) where Node renders it through `util.inspect`
(`'hello'`). The `%o`/`%O` arms called `format_jsvalue(val, 0)`, which
mirrors the bare `console.log(str)` behavior of leaving a *top-level*
string unquoted — but `%o`/`%O` should inspect the value, and inspect
quotes strings at every depth (nested strings already quoted correctly,
e.g. `%o` of `{a:"x"}` → `{ a: 'x' }`).
Route the `%o`/`%O` argument through a new `inspect_format_arg` helper
that mirrors `js_util_inspect`'s top-level handling: a primitive string
becomes `'<escaped>'`; everything else goes through `format_jsvalue`.
Verified vs Node v26.3.0: `%o`/`%O` of a string now yield `'hello'`;
numbers/objects/arrays/null/bool unchanged; `%s` still coerces without
quotes. Existing formatting/inspect unit tests pass (12).
(Out of scope: Node's `%o` also sets `showHidden`, so an array prints
`[ 1, 'two', [length]: 2 ]`; matching the `[length]` internal-slot
rendering is a separate showHidden-array feature and left as-is.)
61f62a8 to
0d172c4
Compare
Refresh floors from a clean node-26 full run (2810/2863, 98.1%): - object 23/23, util 86/86, tty 32/32, events 69/69 -> full (#5144/#5106-7/#5099) - stream 770, globals 111, diagnostics_channel 66, fs-promises 77 -> ratcheted up with small flake margins - http floor 19 -> 17: verified 19/19 in isolation but the full-suite harness flakes to 17 under port contention; a real http regression is a far larger drop (link break / behavior break), which still trips the guard. Stops the false-alarm seen on prior runs. Co-authored-by: Ralph Küpper <ralph2@skelpo.com>
Problem
util.format("%o", "hello")andconsole.log("%o", str)printed the string unquoted (hello), but Node renders%o/%Oarguments throughutil.inspect, which quotes strings:'hello'.Root cause
The
%o/%Oarms calledformat_jsvalue(val, 0), which mirrors the bareconsole.log(str)behavior of leaving a top-level string unquoted. But%o/%Oshould inspect the value, andutil.inspectquotes strings at every depth (nested strings already came out right —%oof{a:"x"}→{ a: 'x' }).js_util_inspectalready special-cases top-level strings (format!("'{}'", escape_string(&s))); the%o/%Opath just didn't.Fix
Route the
%o/%Oargument through a newinspect_format_arghelper that mirrorsjs_util_inspect's top-level handling: a primitive string becomes'<escaped>', everything else goes throughformat_jsvalue.Verification (vs Node v26.3.0)
Existing
perry-runtimeformatting + inspect unit tests pass (12). Found via autil.format/util.inspectdifferential sweep against Node; relates to the console/util-formatting parity gap noted inCLAUDE.mdand the parity roadmap (#4315).Out of scope: Node's
%oalso setsshowHidden, so an array prints[ 1, 'two', [length]: 2 ]; matching that internal-slot rendering is a separate showHidden-array feature, left as-is.No version bump / changelog per maintainer instruction.
Summary by CodeRabbit
%oand%Oso string arguments are now rendered with quotes and proper escaping, matchingutil.inspect-style output.