From 489cdc7b549cf2c6fd23136d2bc3191f4d13f7df Mon Sep 17 00:00:00 2001
From: Antonio Nuno Monteiro <anmonteiro@gmail.com>
Date: Mon, 22 Jan 2024 17:55:54 -0800
Subject: [PATCH 1/2] feat: produce a hard error for `bs.*` attributes

---
 jscomp/core/ast_config.ml              | 49 ++++++--------
 jscomp/core/record_attributes_check.ml | 14 +++-
 ppx/ast_attributes.ml                  | 91 ++++++++++++++++----------
 ppx/ast_attributes.mli                 |  2 +-
 ppx/ast_external_process.ml            | 57 ++++++++--------
 ppx/mel_ast_invariant.ml               |  9 +--
 ppx/mel_ast_invariant.mli              |  1 -
 ppx/melange_ppx.ml                     | 56 ++++++++--------
 test/blackbox-tests/mel-attributes.t   | 37 +----------
 9 files changed, 155 insertions(+), 161 deletions(-)

diff --git a/jscomp/core/ast_config.ml b/jscomp/core/ast_config.ml
index 701407a870..1b3cbd77be 100644
--- a/jscomp/core/ast_config.ml
+++ b/jscomp/core/ast_config.ml
@@ -40,37 +40,28 @@ let signature_config_table : action_table ref = ref String.Map.empty
 let add_signature k v =
   signature_config_table := String.Map.add !signature_config_table k v
 
-let warn_if_non_namespaced ~loc txt =
-  let print_deprecated_unnamespaced_alert ~loc =
-    Location.prerr_alert loc
-      {
-        Warnings.kind = "deprecated";
-        message =
-          "FFI attributes without a namespace are deprecated and will be \
-           removed in the next release.\n\
-           Use `mel.*' instead.";
-        def = Location.none;
-        use = loc;
-      }
-  in
-  if not (String.starts_with txt ~prefix:"mel.") then
-    print_deprecated_unnamespaced_alert ~loc
+let namespace_error ~loc =
+  Location.raise_errorf ~loc
+    "`[@bs.*]' and non-namespaced attributes have been removed in favor of \
+     `[@mel.*]' attributes. Use `[@mel.config]' instead."
 
 let rec iter_on_mel_config_stru (x : Parsetree.structure) =
   match x with
   | [] -> ()
   | {
-      Parsetree.pstr_desc =
+      pstr_desc =
+        Pstr_attribute { attr_name = { txt = "bs.config" | "config"; loc }; _ };
+      _;
+    }
+    :: _ ->
+      namespace_error ~loc
+  | {
+      pstr_desc =
         Pstr_attribute
-          {
-            attr_name = { txt = ("mel.config" | "config") as txt; loc };
-            attr_payload = payload;
-            _;
-          };
+          { attr_name = { txt = "mel.config"; loc }; attr_payload = payload; _ };
       _;
     }
     :: _ ->
-      warn_if_non_namespaced ~loc txt;
       List.iter
         ~f:(fun x ->
           Ast_payload.table_dispatch !structural_config_table x |> ignore)
@@ -105,18 +96,20 @@ let rec iter_on_mel_config_stru (x : Parsetree.structure) =
 let rec iter_on_mel_config_sigi (x : Parsetree.signature) =
   match x with
   | [] -> ()
+  | {
+      psig_desc =
+        Psig_attribute { attr_name = { txt = "bs.config" | "config"; loc }; _ };
+      _;
+    }
+    :: _ ->
+      namespace_error ~loc
   | {
       psig_desc =
         Psig_attribute
-          {
-            attr_name = { txt = ("mel.config" | "config") as txt; loc };
-            attr_payload = payload;
-            _;
-          };
+          { attr_name = { txt = "mel.config"; loc }; attr_payload = payload; _ };
       _;
     }
     :: _ ->
-      warn_if_non_namespaced ~loc txt;
       List.iter
         ~f:(fun x ->
           Ast_payload.table_dispatch !signature_config_table x |> ignore)
diff --git a/jscomp/core/record_attributes_check.ml b/jscomp/core/record_attributes_check.ml
index 09767c9f74..c7736464f5 100644
--- a/jscomp/core/record_attributes_check.ml
+++ b/jscomp/core/record_attributes_check.ml
@@ -32,10 +32,18 @@ let rec find_with_default xs ~f ~default =
   | x :: l -> (
       match f x with Some v -> v | None -> find_with_default l ~f ~default)
 
+let namespace_error ~loc txt =
+  match txt with
+  | "bs.as" | "as" ->
+      Location.raise_errorf ~loc
+        "`[@bs.*]' and non-namespaced attributes have been removed in favor of \
+         `[@mel.*]' attributes. Use `[@mel.as]' instead."
+  | _ -> ()
+
 let find_name (attr : Parsetree.attribute) =
   match attr with
   | {
-   attr_name = { txt = "mel.as" | "as"; _ };
+   attr_name = { txt = ("mel.as" | "as" | "bs.as") as txt; loc };
    attr_payload =
      PStr
        [
@@ -48,6 +56,7 @@ let find_name (attr : Parsetree.attribute) =
        ];
    _;
   } ->
+      namespace_error ~loc txt;
       Some s
   | _ -> None
 
@@ -55,7 +64,7 @@ let find_name_with_loc (attr : Parsetree.attribute) : string Asttypes.loc option
     =
   match attr with
   | {
-   attr_name = { txt = "mel.as" | "as"; loc };
+   attr_name = { txt = ("mel.as" | "as" | "bs.as") as txt; loc };
    attr_payload =
      PStr
        [
@@ -68,6 +77,7 @@ let find_name_with_loc (attr : Parsetree.attribute) : string Asttypes.loc option
        ];
    _;
   } ->
+      namespace_error ~loc txt;
       Some { txt = s; loc }
   | _ -> None
 
diff --git a/ppx/ast_attributes.ml b/ppx/ast_attributes.ml
index aed7ac0121..97ad8ead61 100644
--- a/ppx/ast_attributes.ml
+++ b/ppx/ast_attributes.ml
@@ -34,9 +34,19 @@ let assert_bool_lit (e : expression) =
       Location.raise_errorf ~loc:e.pexp_loc
         "expected this expression to be a boolean literal (`true` or `false`)"
 
-let warn_if_non_namespaced ~loc txt =
-  if not (Mel_ast_invariant.is_mel_attribute txt) then
-    Mel_ast_invariant.warn ~loc Deprecated_non_namespaced_attribute
+let warn_if_bs_or_non_namespaced ~loc txt =
+  match txt with
+  | "bs" ->
+      Location.raise_errorf ~loc
+        "The `[@bs]' attribute has been removed in favor of `[@u]'."
+  | other ->
+      if
+        String.starts_with ~prefix:"bs." other
+        || not (Mel_ast_invariant.is_mel_attribute txt)
+      then
+        Location.raise_errorf ~loc
+          "`[@bs.*]' and non-namespaced attributes have been removed in favor \
+           of `[@mel.*]' attributes."
 
 let process_method_attributes_rev attrs =
   let exception Local of Location.t * string in
@@ -48,8 +58,8 @@ let process_method_attributes_rev attrs =
             ({ attr_name = { txt; loc }; attr_payload = payload; _ } as attr)
           ->
           match txt with
-          | "mel.get" | "get" ->
-              warn_if_non_namespaced ~loc txt;
+          | "mel.get" | "bs.get" | "get" ->
+              warn_if_bs_or_non_namespaced ~loc txt;
               let result =
                 match Ast_payload.ident_or_record_as_config payload with
                 | Error s -> raise (Local (loc, s))
@@ -77,8 +87,8 @@ let process_method_attributes_rev attrs =
                       ~init:(false, false) config
               in
               ({ st with get = Some result }, acc)
-          | "mel.set" | "set" ->
-              warn_if_non_namespaced ~loc txt;
+          | "mel.set" | "bs.set" | "set" ->
+              warn_if_bs_or_non_namespaced ~loc txt;
               let result =
                 match Ast_payload.ident_or_record_as_config payload with
                 | Error s -> raise (Local (loc, s))
@@ -116,11 +126,11 @@ let process_attributes_rev attrs : attr_kind * attribute list =
       match (txt, st) with
       | "u", (Nothing | Uncurry _) ->
           (Uncurry attr, acc) (* TODO: warn unused/duplicated attribute *)
-      | ("mel.this" | "this"), (Nothing | Meth_callback _) ->
-          warn_if_non_namespaced ~loc txt;
+      | ("mel.this" | "bs.this" | "this"), (Nothing | Meth_callback _) ->
+          warn_if_bs_or_non_namespaced ~loc txt;
           (Meth_callback attr, acc)
-      | ("mel.meth" | "meth"), (Nothing | Method _) ->
-          warn_if_non_namespaced ~loc txt;
+      | ("mel.meth" | "bs.meth" | "meth"), (Nothing | Method _) ->
+          warn_if_bs_or_non_namespaced ~loc txt;
           (Method attr, acc)
       | ("u" | "mel.this" | "this"), _ ->
           Error.err ~loc Conflict_u_mel_this_mel_meth
@@ -130,7 +140,9 @@ let process_attributes_rev attrs : attr_kind * attribute list =
 let process_pexp_fun_attributes_rev attrs =
   List.fold_left
     ~f:(fun (st, acc) ({ attr_name = { txt; _ }; _ } as attr) ->
-      match txt with "mel.open" -> (true, acc) | _ -> (st, attr :: acc))
+      match txt with
+      | "mel.open" | "bs.open" -> (true, acc)
+      | _ -> (st, attr :: acc))
     ~init:(false, []) attrs
 
 let process_uncurried attrs =
@@ -232,8 +244,8 @@ let iter_process_mel_string_or_int_as (attrs : attributes) =
   List.iter
     ~f:(fun ({ attr_name = { txt; loc }; attr_payload = payload; _ } as attr) ->
       match txt with
-      | "mel.as" | "as" ->
-          warn_if_non_namespaced ~loc txt;
+      | "mel.as" | "bs.as" | "as" ->
+          warn_if_bs_or_non_namespaced ~loc txt;
           if !st = None then (
             Mel_ast_invariant.mark_used_mel_attribute attr;
             match Ast_payload.is_single_int payload with
@@ -293,20 +305,20 @@ let iter_process_mel_string_int_unwrap_uncurry attrs =
   List.iter
     ~f:(fun ({ attr_name = { txt; loc }; attr_payload = payload; _ } as attr) ->
       match txt with
-      | "mel.string" | "string" ->
-          warn_if_non_namespaced ~loc txt;
+      | "mel.string" | "bs.string" | "string" ->
+          warn_if_bs_or_non_namespaced ~loc txt;
           assign `String attr
-      | "mel.int" | "int" ->
-          warn_if_non_namespaced ~loc txt;
+      | "mel.int" | "bs.int" | "int" ->
+          warn_if_bs_or_non_namespaced ~loc txt;
           assign `Int attr
-      | "mel.ignore" | "ignore" ->
-          warn_if_non_namespaced ~loc txt;
+      | "mel.ignore" | "bs.ignore" | "ignore" ->
+          warn_if_bs_or_non_namespaced ~loc txt;
           assign `Ignore attr
-      | "mel.unwrap" | "unwrap" ->
-          warn_if_non_namespaced ~loc txt;
+      | "mel.unwrap" | "bs.unwrap" | "unwrap" ->
+          warn_if_bs_or_non_namespaced ~loc txt;
           assign `Unwrap attr
-      | "mel.uncurry" | "uncurry" ->
-          warn_if_non_namespaced ~loc txt;
+      | "mel.uncurry" | "bs.uncurry" | "uncurry" ->
+          warn_if_bs_or_non_namespaced ~loc txt;
           assign (`Uncurry (Ast_payload.is_single_int payload)) attr
       | _ -> ())
     attrs;
@@ -317,8 +329,8 @@ let iter_process_mel_string_as attrs : string option =
   List.iter
     ~f:(fun ({ attr_name = { txt; loc }; attr_payload = payload; _ } as attr) ->
       match txt with
-      | "mel.as" | "as" ->
-          warn_if_non_namespaced ~loc txt;
+      | "mel.as" | "bs.as" | "as" ->
+          warn_if_bs_or_non_namespaced ~loc txt;
           if !st = None then (
             match Ast_payload.is_single_string payload with
             | None -> Error.err ~loc Expect_string_literal
@@ -380,8 +392,8 @@ let iter_process_mel_int_as attrs =
   List.iter
     ~f:(fun ({ attr_name = { txt; loc }; attr_payload = payload; _ } as attr) ->
       match txt with
-      | "mel.as" | "as" ->
-          warn_if_non_namespaced ~loc txt;
+      | "mel.as" | "bs.as" | "as" ->
+          warn_if_bs_or_non_namespaced ~loc txt;
           if !st = None then (
             match Ast_payload.is_single_int payload with
             | None -> Error.err ~loc Expect_int_literal
@@ -397,18 +409,31 @@ let has_mel_optional attrs : bool =
   List.exists
     ~f:(fun ({ attr_name = { txt; loc }; _ } as attr) ->
       match txt with
-      | "mel.optional" | "optional" ->
-          warn_if_non_namespaced ~loc txt;
+      | "mel.optional" | "bs.optional" | "optional" ->
+          warn_if_bs_or_non_namespaced ~loc txt;
           Mel_ast_invariant.mark_used_mel_attribute attr;
           true
       | _ -> false)
     attrs
 
-let is_inline { attr_name = { txt; _ }; _ } =
-  txt = "mel.inline" || txt = "inline"
+let is_inline : attribute -> bool =
+ fun { attr_name = { txt; loc }; _ } ->
+  match txt with
+  | "mel.inline" -> true
+  | "bs.inline" ->
+      warn_if_bs_or_non_namespaced ~loc txt;
+      false
+  | _ -> false
 
 let has_inline_payload attrs = List.find_opt ~f:is_inline attrs
-let is_mel_as { attr_name = { txt; _ }; _ } = txt = "mel.as" || txt = "as"
+
+let is_mel_as { attr_name = { txt; loc }; _ } =
+  match txt with
+  | "mel.as" -> true
+  | "bs.as" | "as" ->
+      warn_if_bs_or_non_namespaced ~loc txt;
+      false
+  | _ -> false
 
 let has_mel_as_payload attrs =
   List.fold_left
diff --git a/ppx/ast_attributes.mli b/ppx/ast_attributes.mli
index 09c5d8b50e..ee0adfa6a3 100644
--- a/ppx/ast_attributes.mli
+++ b/ppx/ast_attributes.mli
@@ -38,7 +38,7 @@ type attr_kind =
   | Uncurry of attribute
   | Method of attribute
 
-val warn_if_non_namespaced : loc:location -> label -> unit
+val warn_if_bs_or_non_namespaced : loc:location -> label -> unit
 val process_attributes_rev : attribute list -> attr_kind * attribute list
 val process_pexp_fun_attributes_rev : attribute list -> bool * attribute list
 val process_uncurried : attribute list -> bool * attribute list
diff --git a/ppx/ast_external_process.ml b/ppx/ast_external_process.ml
index 5e6ad57754..6f2170bfb7 100644
--- a/ppx/ast_external_process.ml
+++ b/ppx/ast_external_process.ml
@@ -294,8 +294,8 @@ let parse_external_attributes (prim_name_check : string)
       else *)
       let action () =
         match txt with
-        | "mel.module" | "module" -> (
-            Ast_attributes.warn_if_non_namespaced ~loc txt;
+        | "mel.module" | "bs.module" | "module" -> (
+            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
             match Ast_payload.assert_strings loc payload with
             | [ bundle ] ->
                 {
@@ -325,8 +325,8 @@ let parse_external_attributes (prim_name_check : string)
                 Location.raise_errorf ~loc
                   "`[%@mel.module ..]' expects, at most, a tuple of two \
                    strings (module name, variable name)")
-        | "mel.scope" | "scope" -> (
-            Ast_attributes.warn_if_non_namespaced ~loc txt;
+        | "mel.scope" | "bs.scope" | "scope" -> (
+            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
             match Ast_payload.assert_strings loc payload with
             | [] ->
                 Location.raise_errorf ~loc
@@ -334,14 +334,14 @@ let parse_external_attributes (prim_name_check : string)
             (* We need err on empty scope, so we can tell the difference
                between unset/set *)
             | scopes -> { st with scopes })
-        | "mel.variadic" | "variadic" ->
-            Ast_attributes.warn_if_non_namespaced ~loc txt;
+        | "mel.variadic" | "bs.variadic" | "variadic" ->
+            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
             { st with variadic = true }
-        | "mel.send" | "send" ->
-            Ast_attributes.warn_if_non_namespaced ~loc txt;
+        | "mel.send" | "bs.send" | "send" ->
+            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
             { st with val_send = name_from_payload_or_prim ~loc payload }
-        | "mel.send.pipe" ->
-            Ast_attributes.warn_if_non_namespaced ~loc txt;
+        | "mel.send.pipe" | "bs.send.pipe" | "send.pipe" ->
+            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
             {
               st with
               val_send_pipe =
@@ -352,34 +352,34 @@ let parse_external_attributes (prim_name_check : string)
                       "expected a type after `[%@mel.send.pipe]', e.g. \
                        `[%@mel.send.pipe: t]'");
             }
-        | "mel.set" | "set" ->
-            Ast_attributes.warn_if_non_namespaced ~loc txt;
+        | "mel.set" | "bs.set" | "set" ->
+            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
             { st with set_name = name_from_payload_or_prim ~loc payload }
-        | "mel.get" | "get" ->
-            Ast_attributes.warn_if_non_namespaced ~loc txt;
+        | "mel.get" | "bs.get" | "get" ->
+            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
             { st with get_name = name_from_payload_or_prim ~loc payload }
-        | "mel.new" | "new" ->
-            Ast_attributes.warn_if_non_namespaced ~loc txt;
+        | "mel.new" | "bs.new" | "new" ->
+            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
             { st with new_name = name_from_payload_or_prim ~loc payload }
-        | "mel.set_index" | "set_index" ->
-            Ast_attributes.warn_if_non_namespaced ~loc txt;
+        | "mel.set_index" | "bs.set_index" | "set_index" ->
+            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
             if String.length prim_name_check <> 0 then
               Location.raise_errorf ~loc
                 "`%@mel.set_index' requires its `external' payload to be the \
                  empty string";
             { st with set_index = true }
-        | "mel.get_index" | "get_index" ->
-            Ast_attributes.warn_if_non_namespaced ~loc txt;
+        | "mel.get_index" | "bs.get_index" | "get_index" ->
+            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
             if String.length prim_name_check <> 0 then
               Location.raise_errorf ~loc
                 "`%@mel.get_index' requires its `external' payload to be the \
                  empty string";
             { st with get_index = true }
-        | "mel.obj" | "obj" ->
-            Ast_attributes.warn_if_non_namespaced ~loc txt;
+        | "mel.obj" | "bs.obj" | "obj" ->
+            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
             { st with mk_obj = true }
-        | "mel.return" | "return" -> (
-            Ast_attributes.warn_if_non_namespaced ~loc txt;
+        | "mel.return" | "bs.return" | "return" -> (
+            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
             match Ast_payload.ident_or_record_as_config payload with
             | Ok [ ({ txt; _ }, None) ] ->
                 { st with return_wrapper = return_wrapper loc txt }
@@ -393,8 +393,13 @@ let parse_external_attributes (prim_name_check : string)
 
 let has_mel_uncurry (attrs : attribute list) =
   List.exists
-    ~f:(fun { attr_name = { txt; loc = _ }; _ } ->
-      txt = "mel.uncurry" || txt = "uncurry")
+    ~f:(fun { attr_name = { txt; loc }; _ } ->
+      match txt with
+      | "mel.uncurry" -> true
+      | "bs.uncurry" | "uncurry" ->
+          Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
+          false
+      | _ -> false)
     attrs
 
 let is_user_option ty =
diff --git a/ppx/mel_ast_invariant.ml b/ppx/mel_ast_invariant.ml
index fbdddc4332..e20711d0ba 100644
--- a/ppx/mel_ast_invariant.ml
+++ b/ppx/mel_ast_invariant.ml
@@ -29,13 +29,11 @@ module Warnings = struct
     | Unused_attribute of string
     | Fragile_external of string
     | Redundant_mel_string
-    | Deprecated_non_namespaced_attribute
 
   let kind = function
     | Unused_attribute _ -> "unused"
     | Fragile_external _ -> "fragile"
     | Redundant_mel_string -> "redundant"
-    | Deprecated_non_namespaced_attribute -> "deprecated"
 
   let pp fmt t =
     match t with
@@ -53,11 +51,6 @@ module Warnings = struct
     | Redundant_mel_string ->
         Format.fprintf fmt
           "[@mel.string] is redundant here, you can safely remove it"
-    | Deprecated_non_namespaced_attribute ->
-        Format.fprintf fmt
-          "FFI attributes without a namespace are deprecated and will be \
-           removed in the next release.@\n\
-           Use `mel.*' instead."
 end
 
 let warn =
@@ -106,7 +99,7 @@ let emit_external_warnings : Ast_traverse.iter =
       List.iter
         ~f:(fun attr ->
           match attr with
-          | { attr_name = { txt = "mel.as" | "as"; _ }; _ } ->
+          | { attr_name = { txt = "mel.as"; _ }; _ } ->
               mark_used_mel_attribute attr
           | _ -> ())
         lbl.pld_attributes;
diff --git a/ppx/mel_ast_invariant.mli b/ppx/mel_ast_invariant.mli
index 44a19b535e..756d7909d4 100644
--- a/ppx/mel_ast_invariant.mli
+++ b/ppx/mel_ast_invariant.mli
@@ -29,7 +29,6 @@ module Warnings : sig
     | Unused_attribute of string
     | Fragile_external of string
     | Redundant_mel_string
-    | Deprecated_non_namespaced_attribute
 end
 
 val is_mel_attribute : string -> bool
diff --git a/ppx/melange_ppx.ml b/ppx/melange_ppx.ml
index f486e64120..b7d59ada7b 100644
--- a/ppx/melange_ppx.ml
+++ b/ppx/melange_ppx.ml
@@ -597,10 +597,9 @@ module Mapper = struct
               ( Ast_attributes.has_inline_payload pvb_attributes,
                 pvb_expr.pexp_desc )
             with
-            | ( Some ({ attr_name = { txt; loc }; _ } as attr),
-                Pexp_constant (Pconst_string (s, _, None)) ) ->
-                Ast_attributes.warn_if_non_namespaced ~loc txt;
+            | Some attr, Pexp_constant (Pconst_string (s, _, None)) ->
                 succeed attr pvb_attributes;
+                let loc = pvb_loc in
                 {
                   str with
                   pstr_desc =
@@ -608,15 +607,14 @@ module Mapper = struct
                       {
                         pval_name;
                         pval_type = [%type: string];
-                        pval_loc = pvb_loc;
+                        pval_loc = loc;
                         pval_attributes = [];
                         pval_prim =
                           Melange_ffi.External_ffi_types.inline_string_primitive
                             s None;
                       };
                 }
-            | ( Some ({ attr_name = { txt; loc }; _ } as attr),
-                Pexp_constant (Pconst_string (s, _, Some dec)) ) -> (
+            | Some attr, Pexp_constant (Pconst_string (s, loc, Some dec)) -> (
                 match
                   Melange_ffi.Utf8_string.Interp.transform ~loc ~delim:dec
                     (Melange_compiler_libs.Ast_helper.Exp.constant
@@ -625,7 +623,6 @@ module Mapper = struct
                 with
                 | { pexp_desc = Pexp_constant (Pconst_string (s, _, dec)); _ }
                   ->
-                    Ast_attributes.warn_if_non_namespaced ~loc txt;
                     succeed attr pvb_attributes;
                     {
                       str with
@@ -642,11 +639,10 @@ module Mapper = struct
                           };
                     }
                 | _ -> str)
-            | ( Some ({ attr_name = { txt; loc }; _ } as attr),
-                Pexp_constant (Pconst_integer (s, None)) ) ->
-                Ast_attributes.warn_if_non_namespaced ~loc txt;
+            | Some attr, Pexp_constant (Pconst_integer (s, None)) ->
                 let s = Int32.of_string s in
                 succeed attr pvb_attributes;
+                let loc = pvb_loc in
                 {
                   str with
                   pstr_desc =
@@ -654,17 +650,16 @@ module Mapper = struct
                       {
                         pval_name;
                         pval_type = [%type: int];
-                        pval_loc = pvb_loc;
+                        pval_loc = loc;
                         pval_attributes = [];
                         pval_prim =
                           Melange_ffi.External_ffi_types.inline_int_primitive s;
                       };
                 }
-            | ( Some ({ attr_name = { txt; loc }; _ } as attr),
-                Pexp_constant (Pconst_integer (s, Some 'L')) ) ->
-                Ast_attributes.warn_if_non_namespaced ~loc txt;
+            | Some attr, Pexp_constant (Pconst_integer (s, Some 'L')) ->
                 let s = Int64.of_string s in
                 succeed attr pvb_attributes;
+                let loc = pvb_loc in
                 {
                   str with
                   pstr_desc =
@@ -672,17 +667,16 @@ module Mapper = struct
                       {
                         pval_name;
                         pval_type = [%type: int64];
-                        pval_loc = pvb_loc;
+                        pval_loc = loc;
                         pval_attributes = [];
                         pval_prim =
                           Melange_ffi.External_ffi_types.inline_int64_primitive
                             s;
                       };
                 }
-            | ( Some ({ attr_name = { txt; loc }; _ } as attr),
-                Pexp_constant (Pconst_float (s, None)) ) ->
-                Ast_attributes.warn_if_non_namespaced ~loc txt;
+            | Some attr, Pexp_constant (Pconst_float (s, None)) ->
                 succeed attr pvb_attributes;
+                let loc = pvb_loc in
                 {
                   str with
                   pstr_desc =
@@ -690,18 +684,18 @@ module Mapper = struct
                       {
                         pval_name;
                         pval_type = [%type: float];
-                        pval_loc = pvb_loc;
+                        pval_loc = loc;
                         pval_attributes = [];
                         pval_prim =
                           Melange_ffi.External_ffi_types.inline_float_primitive
                             s;
                       };
                 }
-            | ( Some ({ attr_name = { txt; loc }; _ } as attr),
+            | ( Some attr,
                 Pexp_construct
                   ({ txt = Lident (("true" | "false") as bool); _ }, None) ) ->
-                Ast_attributes.warn_if_non_namespaced ~loc txt;
                 succeed attr pvb_attributes;
+                let loc = pvb_loc in
                 {
                   str with
                   pstr_desc =
@@ -709,7 +703,7 @@ module Mapper = struct
                       {
                         pval_name;
                         pval_type = [%type: bool];
-                        pval_loc = pvb_loc;
+                        pval_loc = loc;
                         pval_attributes = [];
                         pval_prim =
                           Melange_ffi.External_ffi_types.inline_bool_primitive
@@ -733,9 +727,12 @@ module Mapper = struct
                   (r, Ast_tuple_pattern_flatten.value_bindings_mapper self vbs);
             }
         | Pstr_attribute
-            ({ attr_name = { txt = ("mel.config" | "config") as txt; loc }; _ }
-             as attr) ->
-            Ast_attributes.warn_if_non_namespaced ~loc txt;
+            ({
+               attr_name =
+                 { txt = ("mel.config" | "bs.config" | "config") as txt; loc };
+               _;
+             } as attr) ->
+            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
             Mel_ast_invariant.mark_used_mel_attribute attr;
             str
         | Pstr_module
@@ -936,9 +933,12 @@ module Mapper = struct
                           };
                     })
         | Psig_attribute
-            ({ attr_name = { txt = ("mel.config" | "config") as txt; loc }; _ }
-             as attr) ->
-            Ast_attributes.warn_if_non_namespaced ~loc txt;
+            ({
+               attr_name =
+                 { txt = ("mel.config" | "bs.config" | "config") as txt; loc };
+               _;
+             } as attr) ->
+            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
             Mel_ast_invariant.mark_used_mel_attribute attr;
             sigi
         | Psig_module
diff --git a/test/blackbox-tests/mel-attributes.t b/test/blackbox-tests/mel-attributes.t
index 3a1aa568a9..6b4e80c654 100644
--- a/test/blackbox-tests/mel-attributes.t
+++ b/test/blackbox-tests/mel-attributes.t
@@ -34,38 +34,8 @@
   File "x.ml", line 2, characters 63-66:
   2 | external clipboardData : t -> < .. > Js.t = "clipboardData" [@@get]
                                                                      ^^^
-  Alert deprecated: FFI attributes without a namespace are deprecated and will be removed in the next release.
-  Use `mel.*' instead.
-  
-  File "x.ml", line 3, characters 54-57:
-  3 | external set_title : t -> string -> unit = "title" [@@set]
-                                                            ^^^
-  Alert deprecated: FFI attributes without a namespace are deprecated and will be removed in the next release.
-  Use `mel.*' instead.
-  
-  File "x.ml", line 5, characters 35-41:
-  5 |   x:([`a of int | `b of string ] [@string]) ->
-                                         ^^^^^^
-  Alert deprecated: FFI attributes without a namespace are deprecated and will be removed in the next release.
-  Use `mel.*' instead.
-  
-  File "x.ml", line 7, characters 48-52:
-  7 | external set_onload : t -> ((t -> int -> unit)[@this]) -> unit = "onload"
-                                                      ^^^^
-  Alert deprecated: FFI attributes without a namespace are deprecated and will be removed in the next release.
-  Use `mel.*' instead.
-  
-  File "x.ml", line 16, characters 43-46:
-  16 | external mk : ?hi:int -> unit -> _ = "" [@@obj]
-                                                  ^^^
-  Alert deprecated: FFI attributes without a namespace are deprecated and will be removed in the next release.
-  Use `mel.*' instead.
-  
-  File "x.ml", line 17, characters 6-12:
-  17 | let [@inline] _x = 42
-             ^^^^^^
-  Alert deprecated: FFI attributes without a namespace are deprecated and will be removed in the next release.
-  Use `mel.*' instead.
+  Error: `[@bs.*]' and non-namespaced attributes have been removed in favor of `[@mel.*]' attributes.
+  [1]
 
 Skip processing with PPX but still use `@mel.config`
 
@@ -82,7 +52,6 @@ Skip processing with PPX but still use `@mel.config`
   File "x.ml", line 1, characters 4-10:
   1 | [@@@config { flags = [| "-w"; "-32" |] }]
           ^^^^^^
-  Error (alert deprecated): FFI attributes without a namespace are deprecated and will be removed in the next release.
-  Use `mel.*' instead.
+  Error: `[@bs.*]' and non-namespaced attributes have been removed in favor of `[@mel.*]' attributes. Use `[@mel.config]' instead.
   [1]
 

From 88a9bbe68ed1aa7474a1f0a2324e1376eb14969a Mon Sep 17 00:00:00 2001
From: Antonio Nuno Monteiro <anmonteiro@gmail.com>
Date: Wed, 24 Jan 2024 19:36:27 -0800
Subject: [PATCH 2/2] chore: address code review items

---
 ppx/ast_attributes.ml       | 38 +++++++++++++++++++------------------
 ppx/ast_attributes.mli      |  2 +-
 ppx/ast_external_process.ml | 29 +++++++++++++++-------------
 ppx/melange_ppx.ml          |  4 ++--
 4 files changed, 39 insertions(+), 34 deletions(-)

diff --git a/ppx/ast_attributes.ml b/ppx/ast_attributes.ml
index 97ad8ead61..11ada12bcd 100644
--- a/ppx/ast_attributes.ml
+++ b/ppx/ast_attributes.ml
@@ -34,7 +34,7 @@ let assert_bool_lit (e : expression) =
       Location.raise_errorf ~loc:e.pexp_loc
         "expected this expression to be a boolean literal (`true` or `false`)"
 
-let warn_if_bs_or_non_namespaced ~loc txt =
+let error_if_bs_or_non_namespaced ~loc txt =
   match txt with
   | "bs" ->
       Location.raise_errorf ~loc
@@ -59,7 +59,7 @@ let process_method_attributes_rev attrs =
           ->
           match txt with
           | "mel.get" | "bs.get" | "get" ->
-              warn_if_bs_or_non_namespaced ~loc txt;
+              error_if_bs_or_non_namespaced ~loc txt;
               let result =
                 match Ast_payload.ident_or_record_as_config payload with
                 | Error s -> raise (Local (loc, s))
@@ -88,7 +88,7 @@ let process_method_attributes_rev attrs =
               in
               ({ st with get = Some result }, acc)
           | "mel.set" | "bs.set" | "set" ->
-              warn_if_bs_or_non_namespaced ~loc txt;
+              error_if_bs_or_non_namespaced ~loc txt;
               let result =
                 match Ast_payload.ident_or_record_as_config payload with
                 | Error s -> raise (Local (loc, s))
@@ -127,10 +127,10 @@ let process_attributes_rev attrs : attr_kind * attribute list =
       | "u", (Nothing | Uncurry _) ->
           (Uncurry attr, acc) (* TODO: warn unused/duplicated attribute *)
       | ("mel.this" | "bs.this" | "this"), (Nothing | Meth_callback _) ->
-          warn_if_bs_or_non_namespaced ~loc txt;
+          error_if_bs_or_non_namespaced ~loc txt;
           (Meth_callback attr, acc)
       | ("mel.meth" | "bs.meth" | "meth"), (Nothing | Method _) ->
-          warn_if_bs_or_non_namespaced ~loc txt;
+          error_if_bs_or_non_namespaced ~loc txt;
           (Method attr, acc)
       | ("u" | "mel.this" | "this"), _ ->
           Error.err ~loc Conflict_u_mel_this_mel_meth
@@ -139,9 +139,11 @@ let process_attributes_rev attrs : attr_kind * attribute list =
 
 let process_pexp_fun_attributes_rev attrs =
   List.fold_left
-    ~f:(fun (st, acc) ({ attr_name = { txt; _ }; _ } as attr) ->
+    ~f:(fun (st, acc) ({ attr_name = { txt; loc }; _ } as attr) ->
       match txt with
-      | "mel.open" | "bs.open" -> (true, acc)
+      | "mel.open" | "bs.open" ->
+          error_if_bs_or_non_namespaced ~loc txt;
+          (true, acc)
       | _ -> (st, attr :: acc))
     ~init:(false, []) attrs
 
@@ -245,7 +247,7 @@ let iter_process_mel_string_or_int_as (attrs : attributes) =
     ~f:(fun ({ attr_name = { txt; loc }; attr_payload = payload; _ } as attr) ->
       match txt with
       | "mel.as" | "bs.as" | "as" ->
-          warn_if_bs_or_non_namespaced ~loc txt;
+          error_if_bs_or_non_namespaced ~loc txt;
           if !st = None then (
             Mel_ast_invariant.mark_used_mel_attribute attr;
             match Ast_payload.is_single_int payload with
@@ -306,19 +308,19 @@ let iter_process_mel_string_int_unwrap_uncurry attrs =
     ~f:(fun ({ attr_name = { txt; loc }; attr_payload = payload; _ } as attr) ->
       match txt with
       | "mel.string" | "bs.string" | "string" ->
-          warn_if_bs_or_non_namespaced ~loc txt;
+          error_if_bs_or_non_namespaced ~loc txt;
           assign `String attr
       | "mel.int" | "bs.int" | "int" ->
-          warn_if_bs_or_non_namespaced ~loc txt;
+          error_if_bs_or_non_namespaced ~loc txt;
           assign `Int attr
       | "mel.ignore" | "bs.ignore" | "ignore" ->
-          warn_if_bs_or_non_namespaced ~loc txt;
+          error_if_bs_or_non_namespaced ~loc txt;
           assign `Ignore attr
       | "mel.unwrap" | "bs.unwrap" | "unwrap" ->
-          warn_if_bs_or_non_namespaced ~loc txt;
+          error_if_bs_or_non_namespaced ~loc txt;
           assign `Unwrap attr
       | "mel.uncurry" | "bs.uncurry" | "uncurry" ->
-          warn_if_bs_or_non_namespaced ~loc txt;
+          error_if_bs_or_non_namespaced ~loc txt;
           assign (`Uncurry (Ast_payload.is_single_int payload)) attr
       | _ -> ())
     attrs;
@@ -330,7 +332,7 @@ let iter_process_mel_string_as attrs : string option =
     ~f:(fun ({ attr_name = { txt; loc }; attr_payload = payload; _ } as attr) ->
       match txt with
       | "mel.as" | "bs.as" | "as" ->
-          warn_if_bs_or_non_namespaced ~loc txt;
+          error_if_bs_or_non_namespaced ~loc txt;
           if !st = None then (
             match Ast_payload.is_single_string payload with
             | None -> Error.err ~loc Expect_string_literal
@@ -393,7 +395,7 @@ let iter_process_mel_int_as attrs =
     ~f:(fun ({ attr_name = { txt; loc }; attr_payload = payload; _ } as attr) ->
       match txt with
       | "mel.as" | "bs.as" | "as" ->
-          warn_if_bs_or_non_namespaced ~loc txt;
+          error_if_bs_or_non_namespaced ~loc txt;
           if !st = None then (
             match Ast_payload.is_single_int payload with
             | None -> Error.err ~loc Expect_int_literal
@@ -410,7 +412,7 @@ let has_mel_optional attrs : bool =
     ~f:(fun ({ attr_name = { txt; loc }; _ } as attr) ->
       match txt with
       | "mel.optional" | "bs.optional" | "optional" ->
-          warn_if_bs_or_non_namespaced ~loc txt;
+          error_if_bs_or_non_namespaced ~loc txt;
           Mel_ast_invariant.mark_used_mel_attribute attr;
           true
       | _ -> false)
@@ -421,7 +423,7 @@ let is_inline : attribute -> bool =
   match txt with
   | "mel.inline" -> true
   | "bs.inline" ->
-      warn_if_bs_or_non_namespaced ~loc txt;
+      error_if_bs_or_non_namespaced ~loc txt;
       false
   | _ -> false
 
@@ -431,7 +433,7 @@ let is_mel_as { attr_name = { txt; loc }; _ } =
   match txt with
   | "mel.as" -> true
   | "bs.as" | "as" ->
-      warn_if_bs_or_non_namespaced ~loc txt;
+      error_if_bs_or_non_namespaced ~loc txt;
       false
   | _ -> false
 
diff --git a/ppx/ast_attributes.mli b/ppx/ast_attributes.mli
index ee0adfa6a3..fb1ae7afb2 100644
--- a/ppx/ast_attributes.mli
+++ b/ppx/ast_attributes.mli
@@ -38,7 +38,7 @@ type attr_kind =
   | Uncurry of attribute
   | Method of attribute
 
-val warn_if_bs_or_non_namespaced : loc:location -> label -> unit
+val error_if_bs_or_non_namespaced : loc:location -> label -> unit
 val process_attributes_rev : attribute list -> attr_kind * attribute list
 val process_pexp_fun_attributes_rev : attribute list -> bool * attribute list
 val process_uncurried : attribute list -> bool * attribute list
diff --git a/ppx/ast_external_process.ml b/ppx/ast_external_process.ml
index 6f2170bfb7..b2da499cc7 100644
--- a/ppx/ast_external_process.ml
+++ b/ppx/ast_external_process.ml
@@ -295,7 +295,7 @@ let parse_external_attributes (prim_name_check : string)
       let action () =
         match txt with
         | "mel.module" | "bs.module" | "module" -> (
-            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
+            Ast_attributes.error_if_bs_or_non_namespaced ~loc txt;
             match Ast_payload.assert_strings loc payload with
             | [ bundle ] ->
                 {
@@ -326,7 +326,7 @@ let parse_external_attributes (prim_name_check : string)
                   "`[%@mel.module ..]' expects, at most, a tuple of two \
                    strings (module name, variable name)")
         | "mel.scope" | "bs.scope" | "scope" -> (
-            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
+            Ast_attributes.error_if_bs_or_non_namespaced ~loc txt;
             match Ast_payload.assert_strings loc payload with
             | [] ->
                 Location.raise_errorf ~loc
@@ -334,14 +334,17 @@ let parse_external_attributes (prim_name_check : string)
             (* We need err on empty scope, so we can tell the difference
                between unset/set *)
             | scopes -> { st with scopes })
+        | "mel.splice" | "bs.splice" | "splice" ->
+            Location.raise_errorf ~loc
+              "`%s' has been removed. Use `@mel.variadic' instead." txt
         | "mel.variadic" | "bs.variadic" | "variadic" ->
-            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
+            Ast_attributes.error_if_bs_or_non_namespaced ~loc txt;
             { st with variadic = true }
         | "mel.send" | "bs.send" | "send" ->
-            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
+            Ast_attributes.error_if_bs_or_non_namespaced ~loc txt;
             { st with val_send = name_from_payload_or_prim ~loc payload }
         | "mel.send.pipe" | "bs.send.pipe" | "send.pipe" ->
-            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
+            Ast_attributes.error_if_bs_or_non_namespaced ~loc txt;
             {
               st with
               val_send_pipe =
@@ -353,33 +356,33 @@ let parse_external_attributes (prim_name_check : string)
                        `[%@mel.send.pipe: t]'");
             }
         | "mel.set" | "bs.set" | "set" ->
-            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
+            Ast_attributes.error_if_bs_or_non_namespaced ~loc txt;
             { st with set_name = name_from_payload_or_prim ~loc payload }
         | "mel.get" | "bs.get" | "get" ->
-            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
+            Ast_attributes.error_if_bs_or_non_namespaced ~loc txt;
             { st with get_name = name_from_payload_or_prim ~loc payload }
         | "mel.new" | "bs.new" | "new" ->
-            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
+            Ast_attributes.error_if_bs_or_non_namespaced ~loc txt;
             { st with new_name = name_from_payload_or_prim ~loc payload }
         | "mel.set_index" | "bs.set_index" | "set_index" ->
-            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
+            Ast_attributes.error_if_bs_or_non_namespaced ~loc txt;
             if String.length prim_name_check <> 0 then
               Location.raise_errorf ~loc
                 "`%@mel.set_index' requires its `external' payload to be the \
                  empty string";
             { st with set_index = true }
         | "mel.get_index" | "bs.get_index" | "get_index" ->
-            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
+            Ast_attributes.error_if_bs_or_non_namespaced ~loc txt;
             if String.length prim_name_check <> 0 then
               Location.raise_errorf ~loc
                 "`%@mel.get_index' requires its `external' payload to be the \
                  empty string";
             { st with get_index = true }
         | "mel.obj" | "bs.obj" | "obj" ->
-            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
+            Ast_attributes.error_if_bs_or_non_namespaced ~loc txt;
             { st with mk_obj = true }
         | "mel.return" | "bs.return" | "return" -> (
-            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
+            Ast_attributes.error_if_bs_or_non_namespaced ~loc txt;
             match Ast_payload.ident_or_record_as_config payload with
             | Ok [ ({ txt; _ }, None) ] ->
                 { st with return_wrapper = return_wrapper loc txt }
@@ -397,7 +400,7 @@ let has_mel_uncurry (attrs : attribute list) =
       match txt with
       | "mel.uncurry" -> true
       | "bs.uncurry" | "uncurry" ->
-          Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
+          Ast_attributes.error_if_bs_or_non_namespaced ~loc txt;
           false
       | _ -> false)
     attrs
diff --git a/ppx/melange_ppx.ml b/ppx/melange_ppx.ml
index b7d59ada7b..062a872c32 100644
--- a/ppx/melange_ppx.ml
+++ b/ppx/melange_ppx.ml
@@ -732,7 +732,7 @@ module Mapper = struct
                  { txt = ("mel.config" | "bs.config" | "config") as txt; loc };
                _;
              } as attr) ->
-            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
+            Ast_attributes.error_if_bs_or_non_namespaced ~loc txt;
             Mel_ast_invariant.mark_used_mel_attribute attr;
             str
         | Pstr_module
@@ -938,7 +938,7 @@ module Mapper = struct
                  { txt = ("mel.config" | "bs.config" | "config") as txt; loc };
                _;
              } as attr) ->
-            Ast_attributes.warn_if_bs_or_non_namespaced ~loc txt;
+            Ast_attributes.error_if_bs_or_non_namespaced ~loc txt;
             Mel_ast_invariant.mark_used_mel_attribute attr;
             sigi
         | Psig_module