Keep @deprecated on let bindings in inferred signatures

changes/deprecated-let-without-interface.fix.md

@@ -0,0 +1 @@
+- Warn when another module uses a `@deprecated` `let` binding from a module without an interface file. As a result, `failwith` and `invalid_arg`, deprecated since v12, now emit their deprecation warning when used.

compiler/ml/builtin_attributes.ml

@@ -62,6 +62,9 @@ let rec deprecated_of_attrs = function
   | ({txt = "deprecated"; _}, p) :: _ -> Some (string_of_opt_payload p)
   | _ :: tl -> deprecated_of_attrs tl
 
+let deprecated_attributes attrs =
+  List.filter (fun ({txt}, _) -> txt = "deprecated") attrs
+
 let rec deprecated_of_attrs_with_migrate = function
   | [] -> None
   | ({txt = "deprecated"; _}, payload) :: _ -> (

compiler/ml/builtin_attributes.mli

@@ -40,6 +40,10 @@ val check_deprecated_inclusion :
   string ->
   unit
 val deprecated_of_attrs : Parsetree.attributes -> string option
+
+val deprecated_attributes : Parsetree.attributes -> Parsetree.attributes
+(** The [deprecated] attributes in a list *)
+
 val deprecated_of_sig : Parsetree.signature -> string option
 val deprecated_of_str : Parsetree.structure -> string option
 

compiler/ml/typemod.ml

@@ -1398,11 +1398,33 @@ and type_structure ?(toplevel = false) funct_body anchor env sstr =
       let () =
         if rec_flag = Recursive then Rec_check.check_recursive_bindings defs
       in
+      (* The environment keeps let-bound values without attributes, so uses
+         in this module do not warn. The signature carries their [deprecated]
+         attributes, so uses from other modules do. *)
+      let deprecated =
+        List.concat_map
+          (fun (vb : value_binding) ->
+            match Builtin_attributes.deprecated_attributes vb.vb_attributes with
+            | [] -> []
+            | attrs ->
+              List.map (fun id -> (id, attrs)) (pat_bound_idents vb.vb_pat))
+          defs
+      in
       (* Note: Env.find_value does not trigger the value_used event. Values
          will be marked as being used during the signature inclusion test. *)
       ( Tstr_value (rec_flag, defs),
         List.map
-          (fun id -> Sig_value (id, Env.find_value (Pident id) newenv))
+          (fun id ->
+            let vd = Env.find_value (Pident id) newenv in
+            let vd =
+              match
+                List.find_opt (fun (id', _) -> Ident.same id id') deprecated
+              with
+              | None -> vd
+              | Some (_, attrs) ->
+                {vd with val_attributes = attrs @ vd.val_attributes}
+            in
+            Sig_value (id, vd))
           (let_bound_idents defs),
         newenv )
     | Pstr_primitive sdesc ->

packages/@rescript/runtime/Pervasives.res

@@ -25,10 +25,10 @@ external throw: exn => 'a = "%raise"
 })
 external raise: exn => 'a = "%raise"
 
-@deprecated("Use custom exception instead")
+@deprecated("Use a custom exception or `JsError.throwWithMessage` instead.")
 let failwith = s => throw(Failure(s))
 
-@deprecated("Use custom exception instead")
+@deprecated("Use a custom exception or `JsError.throwWithMessage` instead.")
 let invalid_arg = s => throw(Invalid_argument(s))
 
 @deprecated("Use custom exception instead") exception Exit

rewatch/testrepo/packages/with-dev-deps/test/FileToTest_test.mjs

@@ -1,12 +1,15 @@
 // Generated by ReScript, PLEASE EDIT WITH CARE
 
 import * as FileToTest from "../src/FileToTest.mjs";
-import * as Pervasives from "@rescript/runtime/lib/es6/Pervasives.mjs";
 
 let res = FileToTest.add(1, 2);
 
 if (res !== 3) {
-  Pervasives.failwith("Expected " + (3).toString() + ", got " + res.toString());
+  throw {
+    RE_EXN_ID: "Failure",
+    _1: "Expected " + (3).toString() + ", got " + res.toString(),
+    Error: new Error()
+  };
 }
 
 let expected = 3;

rewatch/testrepo/packages/with-dev-deps/test/FileToTest_test.res

@@ -2,5 +2,5 @@ let res = FileToTest.add(1, 2)
 let expected = 3
 
 if res !== expected {
-  failwith("Expected " ++ expected->Int.toString ++ ", got " ++ res->Int.toString)
+  throw(Failure("Expected " ++ expected->Int.toString ++ ", got " ++ res->Int.toString))
 }

tests/build_tests/super_errors_multi/expected/Cross_deprecated_res_items.expected

@@ -0,0 +1,48 @@
+===== User.res =====
+
+  Warning number 3
+  /.../fixtures/Cross_deprecated_res_items/User.res:1:9-19
+
+  1 │ let a = Legacy.oldF(1)
+  2 │ let b = Legacy.compute(1)
+  3 │ let c = Legacy.OldM.v
+
+  deprecated: Legacy.oldF
+  Use newF instead
+
+
+  Warning number 3
+  /.../fixtures/Cross_deprecated_res_items/User.res:2:9-22
+
+  1 │ let a = Legacy.oldF(1)
+  2 │ let b = Legacy.compute(1)
+  3 │ let c = Legacy.OldM.v
+  4 │ let d: Legacy.oldT = 1
+
+  deprecated: Legacy.compute
+  Use computeNew instead
+
+
+  Warning number 3
+  /.../fixtures/Cross_deprecated_res_items/User.res:3:9-21
+
+  1 │ let a = Legacy.oldF(1)
+  2 │ let b = Legacy.compute(1)
+  3 │ let c = Legacy.OldM.v
+  4 │ let d: Legacy.oldT = 1
+  5 │ 
+
+  deprecated: module Legacy.OldM
+Use NewM instead
+
+
+  Warning number 3
+  /.../fixtures/Cross_deprecated_res_items/User.res:4:8-18
+
+  2 │ let b = Legacy.compute(1)
+  3 │ let c = Legacy.OldM.v
+  4 │ let d: Legacy.oldT = 1
+  5 │ 
+
+  deprecated: Legacy.oldT
+  Use newT instead

tests/build_tests/super_errors_multi/fixtures/Cross_deprecated_res_items/Legacy.res

@@ -0,0 +1,13 @@
+@deprecated("Use newF instead")
+let oldF = x => x + 1
+
+@deprecated("Use computeNew instead") @val
+external compute: int => int = "compute"
+
+@deprecated("Use NewM instead")
+module OldM = {
+  let v = 1
+}
+
+@deprecated("Use newT instead")
+type oldT = int

tests/build_tests/super_errors_multi/fixtures/Cross_deprecated_res_items/User.res

@@ -0,0 +1,4 @@
+let a = Legacy.oldF(1)
+let b = Legacy.compute(1)
+let c = Legacy.OldM.v
+let d: Legacy.oldT = 1