From 2371dcbf507ec6f6b6e165b9d740d95087f0f2ea Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Thu, 30 May 2024 14:00:37 +0200 Subject: [PATCH 1/5] Refactor used attributes --- jscomp/frontend/ast_attributes.ml | 10 +++++----- jscomp/frontend/ast_config.ml | 4 ++-- jscomp/frontend/bs_ast_invariant.ml | 22 ++-------------------- jscomp/frontend/bs_ast_invariant.mli | 2 -- jscomp/frontend/bs_builtin_ppx.ml | 2 +- jscomp/ml/used_attributes.ml | 19 +++++++++++++++++++ jscomp/ml/used_attributes.mli | 3 +++ 7 files changed, 32 insertions(+), 30 deletions(-) create mode 100644 jscomp/ml/used_attributes.ml create mode 100644 jscomp/ml/used_attributes.mli diff --git a/jscomp/frontend/ast_attributes.ml b/jscomp/frontend/ast_attributes.ml index 6cffbed669..a609a60655 100644 --- a/jscomp/frontend/ast_attributes.ml +++ b/jscomp/frontend/ast_attributes.ml @@ -212,7 +212,7 @@ let iter_process_bs_string_int_unwrap_uncurry (attrs : t) = let st = ref `Nothing in let assign v (({loc; _}, _) as attr : attr) = if !st = `Nothing then ( - Bs_ast_invariant.mark_used_bs_attribute attr; + Used_attributes.mark_used_attribute attr; st := v) else Bs_syntaxerr.err loc Conflict_attributes in @@ -235,7 +235,7 @@ let iter_process_bs_string_as (attrs : t) : string option = match Ast_payload.is_single_string payload with | None -> Bs_syntaxerr.err loc Expect_string_literal | Some (v, _dec) -> - Bs_ast_invariant.mark_used_bs_attribute attr; + Used_attributes.mark_used_attribute attr; st := Some v) else raise (Ast_untagged_variants.Error (loc, Duplicated_bs_as)) | _ -> ()); @@ -244,7 +244,7 @@ let has_bs_optional (attrs : t) : bool = Ext_list.exists attrs (fun (({txt}, _) as attr) -> match txt with | "optional" -> - Bs_ast_invariant.mark_used_bs_attribute attr; + Used_attributes.mark_used_attribute attr; true | _ -> false) @@ -257,7 +257,7 @@ let iter_process_bs_int_as (attrs : t) = match Ast_payload.is_single_int payload with | None -> Bs_syntaxerr.err loc Expect_int_literal | Some _ as v -> - Bs_ast_invariant.mark_used_bs_attribute attr; + Used_attributes.mark_used_attribute attr; st := v) else raise (Ast_untagged_variants.Error (loc, Duplicated_bs_as)) | _ -> ()); @@ -271,7 +271,7 @@ let iter_process_bs_string_or_int_as (attrs : Parsetree.attributes) = match txt with | "as" -> if !st = None then ( - Bs_ast_invariant.mark_used_bs_attribute attr; + Used_attributes.mark_used_attribute attr; match Ast_payload.is_single_int payload with | None -> ( match payload with diff --git a/jscomp/frontend/ast_config.ml b/jscomp/frontend/ast_config.ml index c45c04c367..c35fb6b988 100644 --- a/jscomp/frontend/ast_config.ml +++ b/jscomp/frontend/ast_config.ml @@ -59,7 +59,7 @@ let rec iter_on_bs_config_str (x : Parsetree.structure) = | [] -> () | {pstr_desc = Pstr_attribute (({txt = "config"; loc}, payload) as attr)} :: _ -> - Bs_ast_invariant.mark_used_bs_attribute attr; + Used_attributes.mark_used_attribute attr; Ext_list.iter (Ast_payload.ident_or_record_as_config loc payload) (Ast_payload.table_dispatch !structural_config_table) @@ -75,7 +75,7 @@ let rec iter_on_bs_config_sig (x : Parsetree.signature) = | [] -> () | {psig_desc = Psig_attribute (({txt = "config"; loc}, payload) as attr)} :: _ -> - Bs_ast_invariant.mark_used_bs_attribute attr; + Used_attributes.mark_used_attribute attr; Ext_list.iter (Ast_payload.ident_or_record_as_config loc payload) (Ast_payload.table_dispatch !signature_config_table) diff --git a/jscomp/frontend/bs_ast_invariant.ml b/jscomp/frontend/bs_ast_invariant.ml index 067d5e54a9..ae75a5e1b4 100644 --- a/jscomp/frontend/bs_ast_invariant.ml +++ b/jscomp/frontend/bs_ast_invariant.ml @@ -33,28 +33,10 @@ let is_bs_attribute txt = true | _ -> false -let used_attributes : string Asttypes.loc Hash_set_poly.t = - Hash_set_poly.create 16 - -(* - let dump_attribute fmt = (fun ( (sloc : string Asttypes.loc),payload) -> - Format.fprintf fmt "@[%s %a@]" sloc.txt (Printast.payload 0 ) payload - ) - -let dump_used_attributes fmt = - Format.fprintf fmt "Used attributes Listing Start:@."; - Hash_set_poly.iter used_attributes (fun attr -> dump_attribute fmt attr) ; - Format.fprintf fmt "Used attributes Listing End:@." - *) - -(* only mark non-ghost used bs attribute *) -let mark_used_bs_attribute ((x, _) : Parsetree.attribute) = - if not x.loc.loc_ghost then Hash_set_poly.add used_attributes x - let warn_unused_attribute ((({txt; loc} as sloc), _) : Parsetree.attribute) = if is_bs_attribute txt && (not loc.loc_ghost) - && not (Hash_set_poly.mem used_attributes sloc) + && not (Used_attributes.is_used_attribute sloc) then (* dump_used_attributes Format.err_formatter; @@ -126,7 +108,7 @@ let emit_external_warnings : iterator = (fun self lbl -> Ext_list.iter lbl.pld_attributes (fun attr -> match attr with - | {txt = "as"}, _ -> mark_used_bs_attribute attr + | {txt = "as"}, _ -> Used_attributes.mark_used_attribute attr | _ -> ()); super.label_declaration self lbl); constructor_declaration = diff --git a/jscomp/frontend/bs_ast_invariant.mli b/jscomp/frontend/bs_ast_invariant.mli index dcff1c30af..b3b580ebfa 100644 --- a/jscomp/frontend/bs_ast_invariant.mli +++ b/jscomp/frontend/bs_ast_invariant.mli @@ -24,8 +24,6 @@ type iterator = Ast_iterator.iterator -val mark_used_bs_attribute : Parsetree.attribute -> unit - (** [warn_discarded_unused_attributes discarded] warn if [discarded] has unused bs attribute *) diff --git a/jscomp/frontend/bs_builtin_ppx.ml b/jscomp/frontend/bs_builtin_ppx.ml index f014d1a523..d0507dbae7 100644 --- a/jscomp/frontend/bs_builtin_ppx.ml +++ b/jscomp/frontend/bs_builtin_ppx.ml @@ -55,7 +55,7 @@ let succeed attr attrs = match attrs with | [_] -> () | _ -> - Bs_ast_invariant.mark_used_bs_attribute attr; + Used_attributes.mark_used_attribute attr; Bs_ast_invariant.warn_discarded_unused_attributes attrs type mapper = Bs_ast_mapper.mapper diff --git a/jscomp/ml/used_attributes.ml b/jscomp/ml/used_attributes.ml new file mode 100644 index 0000000000..4903002e04 --- /dev/null +++ b/jscomp/ml/used_attributes.ml @@ -0,0 +1,19 @@ +let used_attributes : string Asttypes.loc Hash_set_poly.t = + Hash_set_poly.create 16 + + +(* let dump_attribute fmt = (fun ( (sloc : string Asttypes.loc)) -> + Format.fprintf fmt "@[%s@]" sloc.txt + ) + +let dump_used_attributes fmt = + Format.fprintf fmt "Used attributes Listing Start:@."; + Hash_set_poly.iter used_attributes (fun attr -> dump_attribute fmt attr) ; + Format.fprintf fmt "Used attributes Listing End:@." *) + + +(* only mark non-ghost used bs attribute *) +let mark_used_attribute ((x, _) : Parsetree.attribute) = + if not x.loc.loc_ghost then Hash_set_poly.add used_attributes x + +let is_used_attribute (sloc : string Asttypes.loc) = Hash_set_poly.mem used_attributes sloc \ No newline at end of file diff --git a/jscomp/ml/used_attributes.mli b/jscomp/ml/used_attributes.mli new file mode 100644 index 0000000000..fe80737586 --- /dev/null +++ b/jscomp/ml/used_attributes.mli @@ -0,0 +1,3 @@ +val mark_used_attribute : Parsetree.attribute -> unit + +val is_used_attribute : string Asttypes.loc -> bool \ No newline at end of file From f0020505831b4404900dae6fab5928a367ff7ef0 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Thu, 30 May 2024 14:01:48 +0200 Subject: [PATCH 2/5] Mark "as" attribute as used when processed in untagged variants. --- jscomp/ml/ast_untagged_variants.ml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/jscomp/ml/ast_untagged_variants.ml b/jscomp/ml/ast_untagged_variants.ml index 21944c6a1d..0ac0d54d1c 100644 --- a/jscomp/ml/ast_untagged_variants.ml +++ b/jscomp/ml/ast_untagged_variants.ml @@ -111,7 +111,7 @@ let expand_head: (Env.t -> Types.type_expr -> Types.type_expr) ref = ref (Obj.ma let process_tag_type (attrs : Parsetree.attributes) = let st : tag_type option ref = ref None in - Ext_list.iter attrs (fun ({txt; loc}, payload) -> + Ext_list.iter attrs (fun (({txt; loc}, payload) as attr) -> match txt with | "as" -> if !st = None then ( @@ -135,7 +135,8 @@ let process_tag_type (attrs : Parsetree.attributes) = | Some (Lident "null") -> st := Some Null | Some (Lident "undefined") -> st := Some Undefined | Some _ -> raise (Error (loc, InvalidVariantAsAnnotation))); - if !st = None then raise (Error (loc, InvalidVariantAsAnnotation))) + if !st = None then raise (Error (loc, InvalidVariantAsAnnotation)) + else Used_attributes.mark_used_attribute attr) else raise (Error (loc, Duplicated_bs_as)) | _ -> ()); !st From 7efcd44fc41e5ac162b2e5188ffdf5a7c42dccad Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Fri, 31 May 2024 14:46:06 +0200 Subject: [PATCH 3/5] Mark @as used early in constructor declarations, and turn on unused check. A couple of test files were mis-using `@as`. --- jscomp/frontend/bs_ast_invariant.ml | 8 ++++++-- jscomp/test/AsInUncurriedExternals.res | 2 +- jscomp/test/polyvar_convert.res | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/jscomp/frontend/bs_ast_invariant.ml b/jscomp/frontend/bs_ast_invariant.ml index ae75a5e1b4..6c806a84da 100644 --- a/jscomp/frontend/bs_ast_invariant.ml +++ b/jscomp/frontend/bs_ast_invariant.ml @@ -28,8 +28,8 @@ *) let is_bs_attribute txt = match txt with - (* TODO #6636: | "as "| "int" *) - | "bs" | "config" | "ignore" | "optional" | "string" | "uncurry" | "unwrap" -> + (* TODO #6636: "int" *) + | "as" | "bs" | "config" | "ignore" | "optional" | "string" | "uncurry" | "unwrap" -> true | _ -> false @@ -113,6 +113,10 @@ let emit_external_warnings : iterator = super.label_declaration self lbl); constructor_declaration = (fun self ({pcd_name = {txt; loc}} as ctr) -> + let _ = + Ast_untagged_variants.process_tag_type + ctr.pcd_attributes (* mark @as used in variant cases *) + in (match txt with | "false" | "true" | "()" -> Location.raise_errorf ~loc "%s can not be redefined " txt diff --git a/jscomp/test/AsInUncurriedExternals.res b/jscomp/test/AsInUncurriedExternals.res index e04cdc670f..d50cad8fef 100644 --- a/jscomp/test/AsInUncurriedExternals.res +++ b/jscomp/test/AsInUncurriedExternals.res @@ -12,5 +12,5 @@ let mo = makeOptions let options = mo(~name="foo", ()) -let shouldNotFail: (~objectMode: @as(json`false`) _, ~name: string) => int = (~objectMode, ~name) => +let shouldNotFail: (~objectMode: _, ~name: string) => int = (~objectMode, ~name) => 3 diff --git a/jscomp/test/polyvar_convert.res b/jscomp/test/polyvar_convert.res index 4101046456..ab1d1245cb 100644 --- a/jscomp/test/polyvar_convert.res +++ b/jscomp/test/polyvar_convert.res @@ -1,5 +1,5 @@ type t = [ - | @as("x") #a + | #a | #b ] From 2893bd3fb29265cbbe29489fa074919a49a74105 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Fri, 31 May 2024 17:35:57 +0200 Subject: [PATCH 4/5] Fix format --- jscomp/frontend/bs_ast_invariant.ml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jscomp/frontend/bs_ast_invariant.ml b/jscomp/frontend/bs_ast_invariant.ml index 6c806a84da..136c7b3135 100644 --- a/jscomp/frontend/bs_ast_invariant.ml +++ b/jscomp/frontend/bs_ast_invariant.ml @@ -29,7 +29,8 @@ let is_bs_attribute txt = match txt with (* TODO #6636: "int" *) - | "as" | "bs" | "config" | "ignore" | "optional" | "string" | "uncurry" | "unwrap" -> + | "as" | "bs" | "config" | "ignore" | "optional" | "string" | "uncurry" + | "unwrap" -> true | _ -> false From 6ba4e590eff77bb336681c17d8113a15c73df62a Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Fri, 31 May 2024 17:41:55 +0200 Subject: [PATCH 5/5] CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1574a11ecb..c3901ceb2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ - Fix unhandled cases for exotic idents (allow to use exotic PascalCased identifiers for types). https://github.com/rescript-lang/rescript-compiler/pull/6777 - PPX v4: mark props type in externals as `@live` to avoid dead code warnings for prop fields in the editor tooling. https://github.com/rescript-lang/rescript-compiler/pull/6796 +- Fix unused attribute check for `@as`. https://github.com/rescript-lang/rescript-compiler/pull/6795 #### :house: Internal