Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 2 commits into from
Jan 25, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
489cdc7
feat: produce a hard error for `bs.*` attributes
anmonteiro Jan 23, 2024
88a9bbe
chore: address code review items
anmonteiro Jan 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 21 additions & 28 deletions jscomp/core/ast_config.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
14 changes: 12 additions & 2 deletions jscomp/core/record_attributes_check.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
[
Expand All @@ -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
[
Expand All @@ -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

Expand Down
95 changes: 61 additions & 34 deletions ppx/ast_attributes.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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
Expand All @@ -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 =
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion ppx/ast_attributes.mli
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading