console,util: route every %s through one formatPercentS (Node's rule)

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)

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::<ENABLE_ANSI_COLORS>(
-                                    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<const C: bool>(
             &mut self,

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));
+}
+
 }

src/jsc/bindings/ZigGlobalObject.cpp

@@ -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));

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<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)                  \

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");

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,