Fix %todo with a local JsError module, and the location in its message

changes/todo-jserror-shadowing.fix.md

@@ -0,0 +1,2 @@
+- Fix `%todo` calling a user-defined `JsError` module instead of the standard library one.
+- Fix the location in the `%todo` error message: the file's base name instead of its absolute path, and columns instead of character offsets.

compiler/frontend/ast_exp_extension.ml

@@ -33,17 +33,20 @@ let handle_extension e (_self : Ast_mapper.mapper)
       | None -> None
     in
     Location.prerr_warning e.Parsetree.pexp_loc (Bs_todo todo_message);
+    (* Like [__LOC__]: the file's base name, so the generated code does not
+       depend on where it was built, and columns rather than offsets. *)
+    let file, line, start_col = Location.get_pos_info loc.loc_start in
+    let end_col = loc.loc_end.pos_cnum - loc.loc_start.pos_cnum + start_col in
     let pretext =
-      loc.loc_start.pos_fname ^ ":"
-      ^ string_of_int loc.loc_start.pos_lnum
-      ^ ":"
-      ^ string_of_int loc.loc_start.pos_cnum
-      ^ "-"
-      ^ string_of_int loc.loc_end.pos_cnum
+      Printf.sprintf "%s:%d:%d-%d" (Filename.basename file) line start_col
+        end_col
     in
 
     Exp.apply ~loc
-      (Exp.ident ~loc {txt = Longident.parse "JsError.throwWithMessage"; loc})
+      (* The compilation unit, not [JsError]: a user module of that name
+         would shadow the [Stdlib] alias. *)
+      (Exp.ident ~loc
+         {txt = Ldot (Lident "Stdlib_JsError", "throwWithMessage"); loc})
       [
         ( Nolabel,
           Exp.constant ~loc

tests/tests/rescript.json

@@ -15,6 +15,6 @@
   "compiler-flags": [
     "-check-lam",
     "-w -3-6-26-27-29-30-32..40-44-45-52-60-9-106+104",
-    "-warn-error A"
+    "-warn-error A-110"
   ]
 }

tests/tests/src/todo_shadowing_test.mjs

@@ -0,0 +1,29 @@
+// Generated by ReScript, PLEASE EDIT WITH CARE
+
+import * as Mocha from "mocha";
+import * as Test_utils from "./test_utils.mjs";
+import * as Stdlib_JsError from "@rescript/runtime/lib/es6/Stdlib_JsError.mjs";
+
+function throwWithMessage(s) {
+  return s;
+}
+
+let JsError = {
+  throwWithMessage: throwWithMessage
+};
+
+function notImplemented(param) {
+  return Stdlib_JsError.throwWithMessage("todo_shadowing_test.res:9:38-43 - Todo: later");
+}
+
+Mocha.describe("Todo_shadowing_test", () => {
+  Mocha.test("%todo throws even with a local JsError module", () => Test_utils.throws("File \"todo_shadowing_test.res\", line 13, characters 11-18", () => {
+    Stdlib_JsError.throwWithMessage("todo_shadowing_test.res:9:38-43 - Todo: later");
+  }));
+});
+
+export {
+  JsError,
+  notImplemented,
+}
+/*  Not a pure module */

tests/tests/src/todo_shadowing_test.res

@@ -0,0 +1,15 @@
+open Mocha
+open Test_utils
+
+// A local module named JsError must not change what %todo expands to.
+module JsError = {
+  let throwWithMessage = (s: string) => s
+}
+
+let notImplemented = (_: int): int => %todo("later")
+
+describe(__MODULE__, () => {
+  test("%todo throws even with a local JsError module", () => {
+    throws(__LOC__, () => ignore(notImplemented(1)))
+  })
+})