diff --git a/src/js/internal/util/inspect.js b/src/js/internal/util/inspect.js index 09d55ecafcbe..76ed84ef844a 100644 --- a/src/js/internal/util/inspect.js +++ b/src/js/internal/util/inspect.js @@ -344,12 +344,17 @@ function isURL(value) { const SymbolToPrimitive = Symbol.toPrimitive; -const builtInObjects = new SafeSet( - ArrayPrototypeFilter( - ObjectGetOwnPropertyNames(globalThis), - e => RegExpPrototypeExec(/^[A-Z][a-zA-Z0-9]+$/, e) !== null, - ), -); +// prettier-ignore +const builtInObjects = new SafeSet([ + "AggregateError", "Array", "ArrayBuffer", "Atomics", "BigInt", "BigInt64Array", + "BigUint64Array", "Boolean", "DataView", "Date", "Error", "EvalError", + "FinalizationRegistry", "Float32Array", "Float64Array", "Function", "Infinity", + "Int16Array", "Int32Array", "Int8Array", "Intl", "Iterator", "JSON", "Map", + "Math", "NaN", "Number", "Object", "Promise", "Proxy", "RangeError", + "ReferenceError", "Reflect", "RegExp", "Set", "String", "Symbol", "SyntaxError", + "TypeError", "URIError", "Uint16Array", "Uint32Array", "Uint8Array", + "Uint8ClampedArray", "WeakMap", "WeakRef", "WeakSet", +]); // https://tc39.es/ecma262/#sec-IsHTMLDDA-internal-slot const isUndetectableObject = v => typeof v === "undefined" && v !== undefined; @@ -2779,6 +2784,24 @@ function formatBigIntNoColor(bigint, options) { return formatBigInt(stylizeNoColor, bigint, options?.numericSeparator ?? inspectDefaultOptions.numericSeparator); } +function formatPercentS(inspectOptions, arg) { + if (typeof arg === "number") { + return formatNumberNoColor(arg, inspectOptions); + } + if (typeof arg === "bigint") { + return formatBigIntNoColor(arg, inspectOptions); + } + if (typeof arg !== "object" || arg === null || !hasBuiltInToString(arg)) { + return String(arg); + } + return inspect(arg, { + ...inspectOptions, + compact: 3, + colors: false, + depth: 0, + }); +} + function formatWithOptionsInternal(inspectOptions, args) { const first = args[0]; let a = 0; @@ -2798,25 +2821,9 @@ function formatWithOptionsInternal(inspectOptions, args) { const nextChar = StringPrototypeCharCodeAt(first, ++i); if (a + 1 !== args.length) { switch (nextChar) { - case 115: { - // 's' - const tempArg = args[++a]; - if (typeof tempArg === "number") { - tempStr = formatNumberNoColor(tempArg, inspectOptions); - } else if (typeof tempArg === "bigint") { - tempStr = formatBigIntNoColor(tempArg, inspectOptions); - } else if (typeof tempArg !== "object" || tempArg === null || !hasBuiltInToString(tempArg)) { - tempStr = String(tempArg); - } else { - tempStr = inspect(tempArg, { - ...inspectOptions, - compact: 3, - colors: false, - depth: 0, - }); - } + case 115: // 's' + tempStr = formatPercentS(inspectOptions, args[++a]); break; - } case 106: // 'j' tempStr = tryStringify(args[++a]); break; @@ -2995,6 +3002,7 @@ export default { inspect, format, formatWithOptions, + formatPercentS, getStringWidth, stripVTControlCharacters, //! non-standard properties, should these be kept? (not currently exposed) diff --git a/src/jsc/ConsoleObject.rs b/src/jsc/ConsoleObject.rs index e9f3b7261673..59023c846912 100644 --- a/src/jsc/ConsoleObject.rs +++ b/src/jsc/ConsoleObject.rs @@ -2526,12 +2526,7 @@ pub mod formatter { const MIN_BEFORE_E_NOTATION: f64 = 0.000001; match token { PercentTag::S => { - self.print_as::( - Tag::String, - writer_, - next_value, - next_value.js_type(), - )?; + self.print_percent_s(writer_, next_value)?; writer = WrappedWriter { ctx: writer_, failed: false, @@ -3318,6 +3313,9 @@ pub mod formatter { max_depth: u32, colors: bool, ) -> JSValue; + + /// `formatPercentS` from `internal/util/inspect.js` (see `UtilInspect.cpp`). + safe fn Bun__callFormatPercentS(global: &JSGlobalObject, arg: JSValue) -> JSValue; } // ─────────────────────────────────────────────────────────────────────── @@ -3646,7 +3644,6 @@ pub mod formatter { value: JSValue, js_type: jsc::JSType, ) -> JsResult<()> { - // This is called from the '%s' formatter, so it can actually be any value use crate::StringJsc as _; let str = OwnedString::new(BunString::from_js(value, self.global_this)?); let mut writer = WrappedWriter { @@ -3891,6 +3888,28 @@ pub mod formatter { Ok(()) } + #[inline(never)] + fn print_percent_s( + &mut self, + writer_: &mut dyn bun_io::Write, + value: JSValue, + ) -> JsResult<()> { + use crate::StringJsc as _; + let result = if value.is_string_literal() { + value + } else { + crate::from_js_host_call(self.global_this, || { + Bun__callFormatPercentS(self.global_this, value) + })? + }; + let str = OwnedString::new(BunString::from_js(result, self.global_this)?); + self.add_for_new_line(str.length()); + if writer_.write_fmt(format_args!("{}", *str)).is_err() { + self.failed = true; + } + Ok(()) + } + #[inline(never)] fn print_custom_formatted_object( &mut self, diff --git a/src/jsc/bindings/UtilInspect.cpp b/src/jsc/bindings/UtilInspect.cpp index ae09d5b11cdb..8f81b7991e52 100644 --- a/src/jsc/bindings/UtilInspect.cpp +++ b/src/jsc/bindings/UtilInspect.cpp @@ -65,4 +65,23 @@ extern "C" JSC::EncodedJSValue JSC__JSValue__callCustomInspectFunction( RELEASE_AND_RETURN(scope, JSValue::encode(inspectRet)); } +extern "C" JSC::EncodedJSValue Bun__callFormatPercentS( + Zig::GlobalObject* globalObject, + JSC::EncodedJSValue encodedArg) +{ + auto& vm = JSC::getVM(globalObject); + auto scope = DECLARE_THROW_SCOPE(vm); + + JSFunction* fn = globalObject->utilInspectFormatPercentSFunction(); + RETURN_IF_EXCEPTION(scope, {}); + + MarkedArgumentBuffer arguments; + arguments.append(jsUndefined()); + arguments.append(JSValue::decode(encodedArg)); + + auto result = JSC::profiledCall(globalObject, ProfilingReason::API, fn, JSC::getCallData(fn), jsUndefined(), arguments); + RETURN_IF_EXCEPTION(scope, {}); + RELEASE_AND_RETURN(scope, JSValue::encode(result)); +} + } diff --git a/src/jsc/bindings/ZigGlobalObject.cpp b/src/jsc/bindings/ZigGlobalObject.cpp index a45cf0016dfe..5088884b3499 100644 --- a/src/jsc/bindings/ZigGlobalObject.cpp +++ b/src/jsc/bindings/ZigGlobalObject.cpp @@ -2291,6 +2291,18 @@ void GlobalObject::finishCreation(VM& vm) init.set(uncheckedDowncast(prop)); }); + m_utilInspectFormatPercentSFunction.initLater( + [](const Initializer& init) { + auto scope = DECLARE_THROW_SCOPE(init.vm); + JSValue mod = uncheckedDowncast(init.owner)->internalModuleRegistry()->requireId(init.owner, init.vm, Bun::InternalModuleRegistry::Field::InternalUtilInspect); + RETURN_IF_EXCEPTION(scope, ); + RELEASE_ASSERT(mod.isObject()); + auto prop = mod.getObject()->getIfPropertyExists(init.owner, Identifier::fromString(init.vm, "formatPercentS"_s)); + RETURN_IF_EXCEPTION(scope, ); + ASSERT(prop); + init.set(uncheckedDowncast(prop)); + }); + m_utilInspectOptionsStructure.initLater( [](const Initializer& init) { init.set(Bun::createUtilInspectOptionsStructure(init.vm, init.owner)); diff --git a/src/jsc/bindings/ZigGlobalObject.h b/src/jsc/bindings/ZigGlobalObject.h index ca759a74da9e..7d0343dca743 100644 --- a/src/jsc/bindings/ZigGlobalObject.h +++ b/src/jsc/bindings/ZigGlobalObject.h @@ -287,6 +287,7 @@ class GlobalObject : public Bun::GlobalScope { JSC::Structure* utilInspectOptionsStructure() const { return m_utilInspectOptionsStructure.getInitializedOnMainThread(this); } JSC::JSFunction* utilInspectFunction() const { return m_utilInspectFunction.getInitializedOnMainThread(this); } + JSC::JSFunction* utilInspectFormatPercentSFunction() const { return m_utilInspectFormatPercentSFunction.getInitializedOnMainThread(this); } JSC::JSFunction* utilInspectStylizeColorFunction() const { return m_utilInspectStylizeColorFunction.getInitializedOnMainThread(this); } JSC::JSFunction* utilInspectStylizeNoColorFunction() const { return m_utilInspectStylizeNoColorFunction.getInitializedOnMainThread(this); } @@ -600,6 +601,7 @@ class GlobalObject : public Bun::GlobalScope { V(private, LazyPropertyOfGlobalObject, m_nativeMicrotaskTrampoline) \ V(private, LazyPropertyOfGlobalObject, m_performMicrotaskVariadicFunction) \ V(private, LazyPropertyOfGlobalObject, m_utilInspectFunction) \ + V(private, LazyPropertyOfGlobalObject, m_utilInspectFormatPercentSFunction) \ V(private, LazyPropertyOfGlobalObject, m_utilInspectOptionsStructure) \ V(private, LazyPropertyOfGlobalObject, m_utilInspectStylizeColorFunction) \ V(private, LazyPropertyOfGlobalObject, m_utilInspectStylizeNoColorFunction) \ diff --git a/test/js/node/util/node-inspect-tests/parallel/util-format.test.js b/test/js/node/util/node-inspect-tests/parallel/util-format.test.js index 57bbac75fb0c..9092e83d5f1e 100644 --- a/test/js/node/util/node-inspect-tests/parallel/util-format.test.js +++ b/test/js/node/util/node-inspect-tests/parallel/util-format.test.js @@ -238,6 +238,25 @@ test("no assertion failures", () => { assert.strictEqual(util.format("%s", { __proto__: null }), "[Object: null prototype] {}"); } + // `%s` with values whose inherited `toString` comes from a host global + // (Buffer, URL, ...) must call String(value), not inspect it. + assert.strictEqual(util.format("%s", Buffer.from("ab")), "ab"); + assert.strictEqual(util.format("%s", Buffer.from([0xe2, 0x82, 0xac])), "\u20ac"); + assert.strictEqual(util.format("%s", new URL("http://a/b")), "http://a/b"); + assert.strictEqual(util.format("%s", new URLSearchParams("a=1&b=2")), "a=1&b=2"); + { + class MyBuffer extends Buffer {} + const sub = new MyBuffer(2); + sub[0] = 0x68; + sub[1] = 0x69; + assert.strictEqual(util.format("%s", sub), "hi"); + } + // Uint8Array inherits toString from %TypedArray%.prototype, a language built-in. + assert.strictEqual(util.format("%s", new Uint8Array([65])), "65"); + // Language built-ins with their own toString keep the inspect path. + assert.strictEqual(util.format("%s", [1, 2]), "[ 1, 2 ]"); + assert.strictEqual(util.format("%s", new Map([["k", "v"]])), "Map(1) { 'k' => 'v' }"); + // JSON format specifier assert.strictEqual(util.format("%j"), "%j"); assert.strictEqual(util.format("%j", 42), "42"); diff --git a/test/js/node/util/node-inspect-tests/parallel/util-inspect.test.js b/test/js/node/util/node-inspect-tests/parallel/util-inspect.test.js index cc88c7ff7367..bed52577c828 100644 --- a/test/js/node/util/node-inspect-tests/parallel/util-inspect.test.js +++ b/test/js/node/util/node-inspect-tests/parallel/util-inspect.test.js @@ -237,8 +237,10 @@ test("inspect from a different context", () => { }); test("no assertion failures 2", () => { + // Float16Array is intentionally absent: it is not in Node's bootstrap-time + // `builtInObjects`, so showHidden inspects its prototype getters and the + // output diverges from the other typed arrays in Node as well. [ - Float16Array, Float32Array, Float64Array, Int16Array, @@ -267,10 +269,10 @@ test("no assertion failures 2", () => { ); assert.strictEqual(util.inspect(array, false), `${constructor.name}(${length}) [ 65, 97 ]`); }); + assert.ok(util.inspect(new Float16Array(1), { showHidden: true }).includes("[buffer]: [Getter]")); // Now check that declaring a TypedArray in a different context works the same. [ - Float16Array, Float32Array, Float64Array, Int16Array, diff --git a/test/js/web/console/console-log.test.ts b/test/js/web/console/console-log.test.ts index 4356ae993262..c8b034a8a49e 100644 --- a/test/js/web/console/console-log.test.ts +++ b/test/js/web/console/console-log.test.ts @@ -1,6 +1,6 @@ import { file, spawn } from "bun"; import { expect, it } from "bun:test"; -import { bunEnv, bunExe } from "harness"; +import { bunEnv, bunExe, tempDir } from "harness"; import { join } from "node:path"; it("should log to console correctly", async () => { @@ -143,6 +143,145 @@ NamedError: console.error a named error `); }); +it("console.log %s matches util.format and Console instances (Node's rule)", async () => { + // The three `%s` implementations (util.format, the native global console, and + // JS `Console` instances) historically disagreed: the native console forced + // engine ToString, while util.format ported Node's decision tree. All three + // now route through a single `formatPercentS` in internal/util/inspect. + using dir = tempDir("console-percent-s", { + "run.mjs": ` + import util from "node:util"; + import { Console } from "node:console"; + import { Writable } from "node:stream"; + import fs from "node:fs"; + + let captured = ""; + const jsConsole = new Console(new Writable({ + write(chunk, enc, cb) { captured += chunk; cb(); }, + })); + + const cases = [ + ["string", "hi"], + ["number", 3.5], + ["-0", -0], + ["NaN", NaN], + ["Infinity", Infinity], + ["42n", 42n], + ["true", true], + ["null", null], + ["undefined", undefined], + ["Symbol(q)", Symbol("q")], + ["Symbol()", Symbol()], + ["[1,2]", [1, 2]], + ["{a:1}", { a: 1 }], + ["arrow", () => 1], + ["Map", new Map([["k", "v"]])], + ["Set", new Set([1, 2])], + ["Date(0)", new Date(0)], + ["Buffer", Buffer.from("ab")], + ["URL", new URL("http://a/b")], + ["URLSearchParams", new URLSearchParams("a=1&b=2")], + ["Uint8Array", new Uint8Array([65])], + ["Number(5)", new Number(5)], + ["String(x)", new String("x")], + ["ArrayBuffer", new ArrayBuffer(4)], + ["null-proto", Object.create(null)], + ["toString-null", { toString: null }], + ["[Symbol]", [Symbol("a")]], + ["revoked-proxy", (() => { const { proxy, revoke } = Proxy.revocable({}, {}); revoke(); return proxy; })()], + ["RegExp", /re/g], + ["own-toString", { toString() { return "own"; } }], + ["own-toPrim", { [Symbol.toPrimitive]() { return "prim"; } }], + ["class-toString", new (class C { toString() { return "C!"; } })()], + ["nested", { a: { b: 1 } }], + ]; + + const marker = String.fromCharCode(30); + const rows = []; + for (const [label, v] of cases) { + const uf = util.format("%s", v); + captured = ""; + jsConsole.log("%s", v); + const jc = captured.endsWith("\\n") ? captured.slice(0, -1) : captured; + process.stdout.write(marker); + try { + console.log("%s", v); + } catch (e) { + process.stdout.write("THREW:" + e.constructor.name + "\\n"); + } + rows.push({ label, uf, jc }); + } + // The global console writes synchronously to this process's stdout, so + // flush the metadata to a side file we read back in the parent. + fs.writeFileSync("rows.json", JSON.stringify(rows)); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "run.mjs"], + env: { ...bunEnv, TZ: "UTC", NO_COLOR: "1" }, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [out, err, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(err).toBe(""); + + const rows: { label: string; uf: string; jc: string }[] = JSON.parse( + await Bun.file(join(String(dir), "rows.json")).text(), + ); + const gc = out + .replaceAll("\r\n", "\n") + .split(String.fromCharCode(30)) + .slice(1) + .map(s => (s.endsWith("\n") ? s.slice(0, -1) : s)); + expect(gc.length).toBe(rows.length); + + // (1) all three paths must produce identical text for every value + const actual = rows.map((r, i) => ({ label: r.label, uf: r.uf, gc: gc[i], jc: r.jc })); + const expected = rows.map(r => ({ label: r.label, uf: r.uf, gc: r.uf, jc: r.uf })); + expect(actual).toEqual(expected); + + // (2) and the shared value is Node's `%s` rule + const byLabel = Object.fromEntries(rows.map(r => [r.label, r.uf])); + expect(byLabel).toMatchObject({ + "string": "hi", + "number": "3.5", + "-0": "-0", + "NaN": "NaN", + "Infinity": "Infinity", + "42n": "42n", + "true": "true", + "null": "null", + "undefined": "undefined", + "Symbol(q)": "Symbol(q)", + "Symbol()": "Symbol()", + "[1,2]": "[ 1, 2 ]", + "{a:1}": "{ a: 1 }", + "arrow": "() => 1", + "Map": "Map(1) { 'k' => 'v' }", + "Set": "Set(2) { 1, 2 }", + "Date(0)": "1970-01-01T00:00:00.000Z", + "Buffer": "ab", + "URL": "http://a/b", + "URLSearchParams": "a=1&b=2", + "Uint8Array": "65", + "Number(5)": "[Number: 5]", + "String(x)": "[String: 'x']", + "null-proto": "[Object: null prototype] {}", + "toString-null": "{ toString: null }", + "[Symbol]": "[ Symbol(a) ]", + "revoked-proxy": "", + "RegExp": "/re/g", + "own-toString": "own", + "own-toPrim": "prim", + "class-toString": "C!", + "nested": "{ a: [Object] }", + }); + + expect(exitCode).toBe(0); +}); + it("console.log with SharedArrayBuffer", () => { // console.log(x) === Bun.inspect(x) + "\n" written to stdout. expect(Bun.inspect(new ArrayBuffer(0))).toBe("ArrayBuffer(0) []");