Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 39 additions & 24 deletions src/js/internal/util/inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -344,12 +344,22 @@ function isURL(value) {

const SymbolToPrimitive = Symbol.toPrimitive;

const builtInObjects = new SafeSet(
ArrayPrototypeFilter(
ObjectGetOwnPropertyNames(globalThis),
e => RegExpPrototypeExec(/^[A-Z][a-zA-Z0-9]+$/, e) !== null,
),
);
// Node computes this at bootstrap before any host globals (Buffer, URL, Request, ...)
// are installed, so its set contains only ECMAScript language intrinsics. Bun already
// has every host global on globalThis when this module loads, so the dynamic scrape
// would wrongly include them and `%s` would inspect instead of String()-ing them.
// prettier-ignore
Comment thread
robobun marked this conversation as resolved.
Outdated
const builtInObjects = new SafeSet([
"AggregateError", "Array", "ArrayBuffer", "AsyncDisposableStack", "Atomics",
"BigInt", "BigInt64Array", "BigUint64Array", "Boolean", "DataView", "Date",
"DisposableStack", "Error", "EvalError", "FinalizationRegistry", "Float16Array",
"Float32Array", "Float64Array", "Function", "Int16Array", "Int32Array",
"Int8Array", "Intl", "Iterator", "JSON", "Map", "Math", "Number", "Object",
"Promise", "Proxy", "RangeError", "ReferenceError", "Reflect", "RegExp", "Set",
"SharedArrayBuffer", "String", "SuppressedError", "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;
Expand Down Expand Up @@ -2779,6 +2789,26 @@ function formatBigIntNoColor(bigint, options) {
return formatBigInt(stylizeNoColor, bigint, options?.numericSeparator ?? inspectDefaultOptions.numericSeparator);
}

// Node's `%s` rule, shared by util.format, Console instances, and the native global
// console (via Bun__callFormatPercentS) so all three agree on one value.
Comment thread
robobun marked this conversation as resolved.
Outdated
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;
Expand All @@ -2798,25 +2828,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;
Expand Down Expand Up @@ -2995,6 +3009,7 @@ export default {
inspect,
format,
formatWithOptions,
formatPercentS,
getStringWidth,
stripVTControlCharacters,
//! non-standard properties, should these be kept? (not currently exposed)
Expand Down
38 changes: 32 additions & 6 deletions src/jsc/ConsoleObject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2526,12 +2526,7 @@ pub mod formatter {
const MIN_BEFORE_E_NOTATION: f64 = 0.000001;
match token {
PercentTag::S => {
self.print_as::<ENABLE_ANSI_COLORS>(
Tag::String,
writer_,
next_value,
next_value.js_type(),
)?;
self.print_percent_s(writer_, next_value)?;
Comment thread
robobun marked this conversation as resolved.
writer = WrappedWriter {
ctx: writer_,
failed: false,
Expand Down Expand Up @@ -3318,6 +3313,11 @@ pub mod formatter {
max_depth: u32,
colors: bool,
) -> JSValue;

/// C++ helper (`UtilInspect.cpp`) — calls the single `formatPercentS`
/// routine from `internal/util/inspect.js` so the native console `%s`
/// uses the exact same decision tree as `util.format` and `Console`.
Comment thread
robobun marked this conversation as resolved.
Outdated
safe fn Bun__callFormatPercentS(global: &JSGlobalObject, arg: JSValue) -> JSValue;
}

// ───────────────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -3891,6 +3891,32 @@ pub mod formatter {
Ok(())
}

/// Node's `util.format` `%s` semantics. Strings pass straight through;
/// everything else is delegated to the shared JS `formatPercentS`
/// routine so the global console, `Console` instances, and
/// `util.format` cannot disagree.
Comment thread
robobun marked this conversation as resolved.
Outdated
#[inline(never)]
fn print_percent_s(
&mut self,
writer_: &mut dyn bun_io::Write,
value: JSValue,
) -> JsResult<()> {
let result = if value.is_string_literal() {
value
} else {
crate::from_js_host_call(self.global_this, || {
Bun__callFormatPercentS(self.global_this, value)
})?
};
if writer_
.write_fmt(format_args!("{}", result.fmt_string(self.global_this)))
.is_err()
{
self.failed = true;
}
Ok(())
}

#[inline(never)]
fn print_custom_formatted_object<const C: bool>(
&mut self,
Expand Down
19 changes: 19 additions & 0 deletions src/jsc/bindings/UtilInspect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

}
12 changes: 12 additions & 0 deletions src/jsc/bindings/ZigGlobalObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2291,6 +2291,18 @@ void GlobalObject::finishCreation(VM& vm)
init.set(uncheckedDowncast<JSFunction>(prop));
});

m_utilInspectFormatPercentSFunction.initLater(
[](const Initializer<JSFunction>& init) {
auto scope = DECLARE_THROW_SCOPE(init.vm);
JSValue mod = uncheckedDowncast<Zig::GlobalObject>(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<JSFunction>(prop));
});

m_utilInspectOptionsStructure.initLater(
[](const Initializer<Structure>& init) {
init.set(Bun::createUtilInspectOptionsStructure(init.vm, init.owner));
Expand Down
2 changes: 2 additions & 0 deletions src/jsc/bindings/ZigGlobalObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -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); }

Expand Down Expand Up @@ -600,6 +601,7 @@ class GlobalObject : public Bun::GlobalScope {
V(private, LazyPropertyOfGlobalObject<JSFunction>, m_nativeMicrotaskTrampoline) \
V(private, LazyPropertyOfGlobalObject<JSFunction>, m_performMicrotaskVariadicFunction) \
V(private, LazyPropertyOfGlobalObject<JSFunction>, m_utilInspectFunction) \
V(private, LazyPropertyOfGlobalObject<JSFunction>, m_utilInspectFormatPercentSFunction) \
V(private, LazyPropertyOfGlobalObject<Structure>, m_utilInspectOptionsStructure) \
V(private, LazyPropertyOfGlobalObject<JSFunction>, m_utilInspectStylizeColorFunction) \
V(private, LazyPropertyOfGlobalObject<JSFunction>, m_utilInspectStylizeNoColorFunction) \
Expand Down
19 changes: 19 additions & 0 deletions test/js/node/util/node-inspect-tests/parallel/util-format.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
135 changes: 134 additions & 1 deletion test/js/web/console/console-log.test.ts
Original file line number Diff line number Diff line change
@@ -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 () => {
Expand Down Expand Up @@ -143,6 +143,139 @@ 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)],
["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] {}",
"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) []");
Expand Down
Loading