CallSite: return 1-based getColumnNumber() to match V8

src/jsc/bindings/CallSitePrototype.cpp

@@ -134,15 +134,22 @@ JSC_DEFINE_HOST_FUNCTION(callSiteProtoFuncGetFileName, (JSGlobalObject * globalO
 JSC_DEFINE_HOST_FUNCTION(callSiteProtoFuncGetLineNumber, (JSGlobalObject * globalObject, JSC::CallFrame* callFrame))
 {
     ENTER_PROTO_FUNC();
-    // https://github.com/mozilla/source-map/blob/60adcb064bf033702d954d6d3f9bc3635dcb744b/lib/source-map-consumer.js#L484-L486
-    return JSC::JSValue::encode(jsNumber(std::max(callSite->lineNumber().oneBasedInt(), 1)));
+    auto line = callSite->lineNumber();
+    if (line == OrdinalNumber::beforeFirst())
+        return JSC::JSValue::encode(JSC::jsNull());
+    return JSC::JSValue::encode(jsNumber(line.oneBasedInt()));
 }
 
+// V8's CallSite#getColumnNumber() is 1-based (https://v8.dev/docs/stack-trace-api). It returns null
+// when no position is available. source-map-support relies on the 1-based value to compute a
+// 0-based offset for originalPositionFor().
 JSC_DEFINE_HOST_FUNCTION(callSiteProtoFuncGetColumnNumber, (JSGlobalObject * globalObject, JSC::CallFrame* callFrame))
 {
     ENTER_PROTO_FUNC();
-    // https://github.com/mozilla/source-map/blob/60adcb064bf033702d954d6d3f9bc3635dcb744b/lib/source-map-consumer.js#L488-L489
-    return JSC::JSValue::encode(jsNumber(std::max(callSite->columnNumber().zeroBasedInt(), 0)));
+    auto column = callSite->columnNumber();
+    if (column == OrdinalNumber::beforeFirst())
+        return JSC::JSValue::encode(JSC::jsNull());
+    return JSC::JSValue::encode(jsNumber(column.oneBasedInt()));
 }
 
 // TODO:
@@ -256,9 +263,11 @@ JSC_DEFINE_HOST_FUNCTION(callSiteProtoFuncToJSON, (JSGlobalObject * globalObject
 {
     ENTER_PROTO_FUNC();
     JSObject* obj = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), 4);
+    auto line = callSite->lineNumber();
+    auto column = callSite->columnNumber();
     obj->putDirect(vm, JSC::Identifier::fromString(vm, "sourceURL"_s), callSite->sourceURL());
-    obj->putDirect(vm, JSC::Identifier::fromString(vm, "lineNumber"_s), jsNumber(callSite->lineNumber().oneBasedInt()));
-    obj->putDirect(vm, JSC::Identifier::fromString(vm, "columnNumber"_s), jsNumber(callSite->columnNumber().zeroBasedInt()));
+    obj->putDirect(vm, JSC::Identifier::fromString(vm, "lineNumber"_s), line == OrdinalNumber::beforeFirst() ? JSC::jsNull() : jsNumber(line.oneBasedInt()));
+    obj->putDirect(vm, JSC::Identifier::fromString(vm, "columnNumber"_s), column == OrdinalNumber::beforeFirst() ? JSC::jsNull() : jsNumber(column.oneBasedInt()));
     obj->putDirect(vm, JSC::Identifier::fromString(vm, "functionName"_s), callSite->functionName());
     return JSC::JSValue::encode(obj);
 }

test/js/node/test/common/index.js

@@ -641,7 +641,8 @@ function canCreateSymLink() {
 
 function getCallSite(top) {
   const originalStackFormatter = Error.prepareStackTrace;
-  Error.prepareStackTrace = (err, stack) => `${stack[0].getFileName()}:${stack[0].getLineNumber()}:${stack[0].getColumnNumber()}`;
+  Error.prepareStackTrace = (err, stack) =>
+    `${stack[0].getFileName()}:${stack[0].getLineNumber()}`;
   const err = new Error();
   Error.captureStackTrace(err, top);
   // With the V8 Error API, the stack is not formatted until it is accessed

test/js/node/test/parallel/test-common-must-not-call.js

@@ -26,14 +26,14 @@ const createValidate = (line, args = []) => common.mustCall((e) => {
   assert.strictEqual(rest, line + argsInfo);
 });
 
-const validate1 = createValidate('9:29');
+const validate1 = createValidate('9');
 try {
   testFunction1();
 } catch (e) {
   validate1(e);
 }
 
-const validate2 = createValidate('11:29', ['hello', 42]);
+const validate2 = createValidate('11', ['hello', 42]);
 try {
   testFunction2('hello', 42);
 } catch (e) {

test/js/node/v8/capture-stack-trace.test.js

@@ -551,6 +551,57 @@ test("CallFrame.p.isNative", () => {
   Error.prepareStackTrace = prevPrepareStackTrace;
 });
 
+// https://github.com/oven-sh/bun/issues/17303
+// https://github.com/oven-sh/bun/issues/18250
+test("CallFrame.p.getColumnNumber is 1-based and matches toString()", async () => {
+  // Run in a subprocess so the column isn't shifted by the CJS-wrapper transform applied
+  // to .js test files: the function-call column must be observable at exactly 1.
+  const src = [
+    `let frames;`,
+    `Error.prepareStackTrace = (_, s) => s;`,
+    `function callee() { frames = new Error().stack; }`,
+    `callee();`,
+    `Error.prepareStackTrace = undefined;`,
+    `const callee0 = frames[0];`,
+    `const caller = frames[1];`,
+    `const colOf = f => Number(/:(\\d+):(\\d+)\\)?$/.exec(f.toString())[2]);`,
+    `console.log(JSON.stringify({`,
+    `  calleeMatch: callee0.getColumnNumber() === colOf(callee0),`,
+    `  callerCol: caller.getColumnNumber(),`,
+    `  callerStrCol: colOf(caller),`,
+    `  json: callee0.toJSON().columnNumber === callee0.getColumnNumber(),`,
+    `}));`,
+  ].join("\n");
+  await using proc = Bun.spawn({ cmd: [bunExe(), "-e", src], env: bunEnv, stdout: "pipe", stderr: "pipe" });
+  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+  expect(stderr).toBe("");
+  // `callee()` starts at column 1; V8 reports 1, source-map-support subtracts 1 to get 0.
+  expect(JSON.parse(stdout)).toEqual({ calleeMatch: true, callerCol: 1, callerStrCol: 1, json: true });
+  expect(exitCode).toBe(0);
+});
+
+test("CallFrame.p.getLineNumber/getColumnNumber return null for native frames", () => {
+  let prevPrepareStackTrace = Error.prepareStackTrace;
+  Error.prepareStackTrace = (e, s) => s;
+  let frames;
+  nativeFrameForTesting(() => {
+    const err = new Error("");
+    Error.captureStackTrace(err);
+    frames = err.stack;
+    return 0;
+  });
+  Error.prepareStackTrace = prevPrepareStackTrace;
+  const nativeFrame = frames[1];
+  expect(nativeFrame.isNative()).toBe(true);
+  expect(nativeFrame.getLineNumber()).toBeNull();
+  expect(nativeFrame.getColumnNumber()).toBeNull();
+  const json = nativeFrame.toJSON();
+  expect({ lineNumber: json.lineNumber, columnNumber: json.columnNumber }).toEqual({
+    lineNumber: null,
+    columnNumber: null,
+  });
+});
+
 test("return non-strings from Error.prepareStackTrace", () => {
   // This behavior is allowed by V8 and used by the node-depd npm package.
   let prevPrepareStackTrace = Error.prepareStackTrace;