feat: produce a hard error for bs.* attributes (#1034)

* feat: produce a hard error for `bs.*` attributes

* chore: address code review items

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)

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,14 +56,15 @@ let find_name (attr : Parsetree.attribute) =
        ];
    _;
   } ->
+      namespace_error ~loc txt;
       Some s
   | _ -> None
 
 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
 

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 error_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" ->
+              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))
@@ -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" ->
+              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))
@@ -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 _) ->
+          error_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 _) ->
+          error_if_bs_or_non_namespaced ~loc txt;
           (Method attr, acc)
       | ("u" | "mel.this" | "this"), _ ->
           Error.err ~loc Conflict_u_mel_this_mel_meth
@@ -129,8 +139,12 @@ 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))
+    ~f:(fun (st, acc) ({ attr_name = { txt; loc }; _ } as attr) ->
+      match txt with
+      | "mel.open" | "bs.open" ->
+          error_if_bs_or_non_namespaced ~loc txt;
+          (true, acc)
+      | _ -> (st, attr :: acc))
     ~init:(false, []) attrs
 
 let process_uncurried attrs =
@@ -232,8 +246,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" ->
+          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
@@ -293,20 +307,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" ->
+          error_if_bs_or_non_namespaced ~loc txt;
           assign `String attr
-      | "mel.int" | "int" ->
-          warn_if_non_namespaced ~loc txt;
+      | "mel.int" | "bs.int" | "int" ->
+          error_if_bs_or_non_namespaced ~loc txt;
           assign `Int attr
-      | "mel.ignore" | "ignore" ->
-          warn_if_non_namespaced ~loc txt;
+      | "mel.ignore" | "bs.ignore" | "ignore" ->
+          error_if_bs_or_non_namespaced ~loc txt;
           assign `Ignore attr
-      | "mel.unwrap" | "unwrap" ->
-          warn_if_non_namespaced ~loc txt;
+      | "mel.unwrap" | "bs.unwrap" | "unwrap" ->
+          error_if_bs_or_non_namespaced ~loc txt;
           assign `Unwrap attr
-      | "mel.uncurry" | "uncurry" ->
-          warn_if_non_namespaced ~loc txt;
+      | "mel.uncurry" | "bs.uncurry" | "uncurry" ->
+          error_if_bs_or_non_namespaced ~loc txt;
           assign (`Uncurry (Ast_payload.is_single_int payload)) attr
       | _ -> ())
     attrs;
@@ -317,8 +331,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" ->
+          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
@@ -380,8 +394,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" ->
+          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
@@ -397,18 +411,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" ->
+          error_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" ->
+      error_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" ->
+      error_if_bs_or_non_namespaced ~loc txt;
+      false
+  | _ -> false
 
 let has_mel_as_payload attrs =
   List.fold_left

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