From 158e497d1a694579a0d66bf6be661e7daa03d42d Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sun, 15 Sep 2024 13:34:52 +0200 Subject: [PATCH 01/11] improve component prop not existing error --- jscomp/ml/error_message_utils.ml | 33 +++++++++++++++++++++++++++++++- jscomp/ml/typecore.ml | 4 ++++ jscomp/syntax/src/jsx_v4.ml | 7 ++++++- 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/jscomp/ml/error_message_utils.ml b/jscomp/ml/error_message_utils.ml index 386e36d39e..8271b51ea5 100644 --- a/jscomp/ml/error_message_utils.ml +++ b/jscomp/ml/error_message_utils.ml @@ -1,3 +1,6 @@ +type extract_concrete_typedecl = + Env.t -> Types.type_expr -> Path.t * Path.t * Types.type_declaration + type type_clash_statement = FunctionCall type type_clash_context = | SetRecordField @@ -236,4 +239,32 @@ let print_contextual_unification_error ppf t1 t2 = @[The value you're pattern matching on here is wrapped in an \ @{option@}, but you're trying to match on the actual value.@ Wrap \ the highlighted pattern in @{Some()@} to make it work.@]" - | _ -> () \ No newline at end of file + | _ -> () + +let get_jsx_component_props + ~(extract_concrete_typedecl : extract_concrete_typedecl) env ty p = + match Path.last p with + | "props" -> ( + try + match extract_concrete_typedecl env ty with + | ( _p0, + _p, + {Types.type_kind = Type_record (fields, _repr); type_attributes} ) + when type_attributes + |> List.exists (fun ({Location.txt}, _) -> + txt = "res.jsxComponentProps") -> + Some fields + | _ -> None + with _ -> None) + | _ -> None + +let print_component_wrong_prop_error ppf (p : Path.t) + (_fields : Types.label_declaration list) name = + fprintf ppf "@["; + fprintf ppf "@[<2>The prop @{%s@} does not belong to the JSX component" + name; + (match p |> Path.name |> String.split_on_char '.' |> List.rev with + | "props" :: component_name :: _ -> + fprintf ppf " @{<%s />@}" component_name + | _ -> ()); + fprintf ppf "@]@,@," \ No newline at end of file diff --git a/jscomp/ml/typecore.ml b/jscomp/ml/typecore.ml index 9167daa966..f6b12b6c91 100644 --- a/jscomp/ml/typecore.ml +++ b/jscomp/ml/typecore.ml @@ -3857,6 +3857,9 @@ let report_error env ppf = function | Label_not_mutable lid -> fprintf ppf "The record field %a is not mutable" longident lid | Wrong_name (eorp, ty, kind, p, name, valid_names) -> + (match get_jsx_component_props ~extract_concrete_typedecl env ty p with + | Some fields -> print_component_wrong_prop_error ppf p fields name; spellcheck ppf name valid_names; + | None -> (* modified *) if Path.is_constructor_typath p then begin fprintf ppf "@[The field %s is not part of the record \ @@ -3876,6 +3879,7 @@ let report_error env ppf = function fprintf ppf "@]"; end; spellcheck ppf name valid_names; + ) | Name_type_mismatch (kind, lid, tp, tpl) -> let name = label_of_kind kind in report_ambiguous_type_error ppf env tp tpl diff --git a/jscomp/syntax/src/jsx_v4.ml b/jscomp/syntax/src/jsx_v4.ml index af2a37e2a5..f421fe12bc 100644 --- a/jscomp/syntax/src/jsx_v4.ml +++ b/jscomp/syntax/src/jsx_v4.ml @@ -347,11 +347,16 @@ let make_type_decls_with_core_type props_name loc core_type typ_vars = ] let live_attr = ({txt = "live"; loc = Location.none}, PStr []) +let jsx_component_props_attr = + ({txt = "res.jsxComponentProps"; loc = Location.none}, PStr []) (* type props<'x, 'y, ...> = { x: 'x, y?: 'y, ... } *) let make_props_record_type ~core_type_of_attr ~external_ ~typ_vars_of_core_type props_name loc named_type_list = - let attrs = if external_ then [live_attr] else [] in + let attrs = + if external_ then [jsx_component_props_attr; live_attr] + else [jsx_component_props_attr] + in Str.type_ Nonrecursive (match core_type_of_attr with | None -> make_type_decls ~attrs props_name loc named_type_list From e7e88257da43b4ac6703bc8ba86fccb2cd10e63c Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sun, 15 Sep 2024 14:07:32 +0200 Subject: [PATCH 02/11] improve JSX component missing props error message --- .../component_missing_prop.res.expected | 16 ++++---- .../fixtures/component_missing_prop.res | 1 + jscomp/ml/error_message_utils.ml | 38 ++++++++++++++----- jscomp/ml/typecore.ml | 34 +++++++++-------- jscomp/ml/typecore.mli | 2 +- 5 files changed, 58 insertions(+), 33 deletions(-) diff --git a/jscomp/build_tests/super_errors/expected/component_missing_prop.res.expected b/jscomp/build_tests/super_errors/expected/component_missing_prop.res.expected index 3df4b8fc86..bd8eb3ecf7 100644 --- a/jscomp/build_tests/super_errors/expected/component_missing_prop.res.expected +++ b/jscomp/build_tests/super_errors/expected/component_missing_prop.res.expected @@ -1,12 +1,12 @@ We've found a bug for you! - /.../fixtures/component_missing_prop.res:5:34-35 + /.../fixtures/component_missing_prop.res:6:34-35 - 3 │ type props<'name> = {name: 'name} - 4 │ - 5 │ let make = (): props<'name> => {} - 6 │ } - 7 │ + 4 │ type props<'name> = {name: 'name} + 5 │ + 6 │ let make = (): props<'name> => {} + 7 │ } + 8 │ - Some required record fields are missing: - name. If this is a component, add the missing props. \ No newline at end of file + The component is missing these required props: + name \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/fixtures/component_missing_prop.res b/jscomp/build_tests/super_errors/fixtures/component_missing_prop.res index 6dbe26bb05..297b9b9e5d 100644 --- a/jscomp/build_tests/super_errors/fixtures/component_missing_prop.res +++ b/jscomp/build_tests/super_errors/fixtures/component_missing_prop.res @@ -1,5 +1,6 @@ // Since the React transform isn't active in the tests, mimic what the transform outputs. module Component = { + @res.jsxComponentProps type props<'name> = {name: 'name} let make = (): props<'name> => {} diff --git a/jscomp/ml/error_message_utils.ml b/jscomp/ml/error_message_utils.ml index 8271b51ea5..7288f5f197 100644 --- a/jscomp/ml/error_message_utils.ml +++ b/jscomp/ml/error_message_utils.ml @@ -241,6 +241,16 @@ let print_contextual_unification_error ppf t1 t2 = the highlighted pattern in @{Some()@} to make it work.@]" | _ -> () +type jsx_prop_error_info = { + fields: Types.label_declaration list; + props_record_path: Path.t; +} + +let path_to_jsx_component_name p = + match p |> Path.name |> String.split_on_char '.' |> List.rev with + | "props" :: component_name :: _ -> Some component_name + | _ -> None + let get_jsx_component_props ~(extract_concrete_typedecl : extract_concrete_typedecl) env ty p = match Path.last p with @@ -251,20 +261,30 @@ let get_jsx_component_props _p, {Types.type_kind = Type_record (fields, _repr); type_attributes} ) when type_attributes - |> List.exists (fun ({Location.txt}, _) -> + |> List.exists (fun ({Location.txt}, _) -> txt = "res.jsxComponentProps") -> - Some fields + Some {props_record_path = p; fields} | _ -> None with _ -> None) | _ -> None +let print_component_name ppf (p : Path.t) = + match path_to_jsx_component_name p with + | Some component_name -> fprintf ppf "@{<%s />@} " component_name + | None -> () + let print_component_wrong_prop_error ppf (p : Path.t) (_fields : Types.label_declaration list) name = fprintf ppf "@["; - fprintf ppf "@[<2>The prop @{%s@} does not belong to the JSX component" - name; - (match p |> Path.name |> String.split_on_char '.' |> List.rev with - | "props" :: component_name :: _ -> - fprintf ppf " @{<%s />@}" component_name - | _ -> ()); - fprintf ppf "@]@,@," \ No newline at end of file + fprintf ppf + "@[<2>The prop @{%s@} does not belong to the JSX component " name; + print_component_name ppf p; + fprintf ppf "@]@,@," + +let print_component_labels_missing_error ppf labels + (error_info : jsx_prop_error_info) = + fprintf ppf "@[The component "; + print_component_name ppf error_info.props_record_path; + fprintf ppf "is missing these required props:@\n"; + labels |> List.iter (fun lbl -> fprintf ppf "@ %s" lbl); + fprintf ppf "@]" \ No newline at end of file diff --git a/jscomp/ml/typecore.ml b/jscomp/ml/typecore.ml index f6b12b6c91..f9d375cc88 100644 --- a/jscomp/ml/typecore.ml +++ b/jscomp/ml/typecore.ml @@ -36,7 +36,7 @@ type error = | Apply_non_function of type_expr | Apply_wrong_label of arg_label * type_expr | Label_multiply_defined of string - | Labels_missing of string list * bool + | Labels_missing of {labels: string list; jsx_component_info: jsx_prop_error_info option} | Label_not_mutable of Longident.t | Wrong_name of string * type_expr * string * Path.t * string * string list | Name_type_mismatch of @@ -1974,11 +1974,6 @@ let rec lower_args env seen ty_fun = let not_function env ty = let ls, tvar = list_labels env ty in ls = [] && not tvar - -let check_might_be_component env ty_record = - match (expand_head env ty_record).desc with - | Tconstr (path, _, _) when path |> Path.last = "props" -> true - | _ -> false type lazy_args = (Asttypes.arg_label * (unit -> Typedtree.expression) option) list @@ -2304,8 +2299,12 @@ and type_expect_ ?type_clash_context ?in_function ?(recarg=Rejected) env sexp ty Some name in let labels_missing = fields |> List.filter_map filter_missing in if labels_missing <> [] then ( - let might_be_component = check_might_be_component env ty_record in - raise(Error(loc, env, Labels_missing (labels_missing, might_be_component)))); + raise(Error(loc, env, Labels_missing { + labels = labels_missing; + jsx_component_info = (match opath with + | Some (p, _) -> get_jsx_component_props ~extract_concrete_typedecl env ty_record p + | None -> None) + }))); [||], representation | [], _ -> if fields = [] && repr_opt <> None then @@ -2330,8 +2329,12 @@ and type_expect_ ?type_clash_context ?in_function ?(recarg=Rejected) env sexp ty label_descriptions in if !labels_missing <> [] then ( - let might_be_component = check_might_be_component env ty_record in - raise(Error(loc, env, Labels_missing ((List.rev !labels_missing), might_be_component)))); + raise(Error(loc, env, Labels_missing { + labels=(List.rev !labels_missing); + jsx_component_info = (match opath with + | Some (p, _) -> get_jsx_component_props ~extract_concrete_typedecl env ty_record p + | None -> None) + }))); let fields = Array.map2 (fun descr def -> descr, def) label_descriptions label_definitions @@ -3848,17 +3851,18 @@ let report_error env ppf = function type_expr ty print_label l | Label_multiply_defined s -> fprintf ppf "The record field label %s is defined several times" s - | Labels_missing (labels, might_be_component) -> + | Labels_missing {labels; jsx_component_info = Some jsx_component_info} -> + print_component_labels_missing_error ppf labels jsx_component_info + | Labels_missing {labels} -> let print_labels ppf = List.iter (fun lbl -> fprintf ppf "@ %s" ( lbl)) in - let component_text = if might_be_component then " If this is a component, add the missing props." else "" in - fprintf ppf "@[Some required record fields are missing:%a.%s@]" - print_labels labels component_text + fprintf ppf "@[Some required record fields are missing:%a.@]" + print_labels labels | Label_not_mutable lid -> fprintf ppf "The record field %a is not mutable" longident lid | Wrong_name (eorp, ty, kind, p, name, valid_names) -> (match get_jsx_component_props ~extract_concrete_typedecl env ty p with - | Some fields -> print_component_wrong_prop_error ppf p fields name; spellcheck ppf name valid_names; + | Some {fields} -> print_component_wrong_prop_error ppf p fields name; spellcheck ppf name valid_names; | None -> (* modified *) if Path.is_constructor_typath p then begin diff --git a/jscomp/ml/typecore.mli b/jscomp/ml/typecore.mli index 1eeff0d339..1e18eea61c 100644 --- a/jscomp/ml/typecore.mli +++ b/jscomp/ml/typecore.mli @@ -70,7 +70,7 @@ type error = | Apply_non_function of type_expr | Apply_wrong_label of arg_label * type_expr | Label_multiply_defined of string - | Labels_missing of string list * bool + | Labels_missing of {labels: string list; jsx_component_info: Error_message_utils.jsx_prop_error_info option} | Label_not_mutable of Longident.t | Wrong_name of string * type_expr * string * Path.t * string * string list | Name_type_mismatch of From 4e5581e2b63b9fdf4d9be427c85becc8b7ec7984 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sun, 15 Sep 2024 15:21:21 +0200 Subject: [PATCH 03/11] improve component prop passed multiple times error --- jscomp/ml/error_message_utils.ml | 8 +++++- jscomp/ml/typecore.ml | 45 ++++++++++++++++++++------------ jscomp/ml/typecore.mli | 2 +- 3 files changed, 36 insertions(+), 19 deletions(-) diff --git a/jscomp/ml/error_message_utils.ml b/jscomp/ml/error_message_utils.ml index 7288f5f197..ded8f02314 100644 --- a/jscomp/ml/error_message_utils.ml +++ b/jscomp/ml/error_message_utils.ml @@ -287,4 +287,10 @@ let print_component_labels_missing_error ppf labels print_component_name ppf error_info.props_record_path; fprintf ppf "is missing these required props:@\n"; labels |> List.iter (fun lbl -> fprintf ppf "@ %s" lbl); - fprintf ppf "@]" \ No newline at end of file + fprintf ppf "@]" + +let get_jsx_component_error_info ~extract_concrete_typedecl opath env ty_record () = + match opath with + | Some (p, _) -> + get_jsx_component_props ~extract_concrete_typedecl env ty_record p + | None -> None \ No newline at end of file diff --git a/jscomp/ml/typecore.ml b/jscomp/ml/typecore.ml index f9d375cc88..54837dbf02 100644 --- a/jscomp/ml/typecore.ml +++ b/jscomp/ml/typecore.ml @@ -35,7 +35,7 @@ type error = | Expr_type_clash of (type_expr * type_expr) list * (type_clash_context option) | Apply_non_function of type_expr | Apply_wrong_label of arg_label * type_expr - | Label_multiply_defined of string + | Label_multiply_defined of {label: string; jsx_component_info: jsx_prop_error_info option} | Labels_missing of {labels: string list; jsx_component_info: jsx_prop_error_info option} | Label_not_mutable of Longident.t | Wrong_name of string * type_expr * string * Path.t * string * string list @@ -960,7 +960,7 @@ let type_label_a_list ?labels loc closed env type_lbl_a opath lid_a_list k = (* Checks over the labels mentioned in a record pattern: no duplicate definitions (error); properly closed (warning) *) -let check_recordpat_labels loc lbl_pat_list closed = +let check_recordpat_labels ~get_jsx_component_error_info loc lbl_pat_list closed = match lbl_pat_list with | [] -> () (* should not happen *) | (_, label1, _) :: _ -> @@ -968,7 +968,10 @@ let check_recordpat_labels loc lbl_pat_list closed = let defined = Array.make (Array.length all) false in let check_defined (_, label, _) = if defined.(label.lbl_pos) - then raise(Error(loc, Env.empty, Label_multiply_defined label.lbl_name)) + then raise(Error(loc, Env.empty, Label_multiply_defined { + label = label.lbl_name; + jsx_component_info = get_jsx_component_error_info (); + })) else defined.(label.lbl_pos) <- true in List.iter check_defined lbl_pat_list; if closed = Closed @@ -1292,6 +1295,7 @@ and type_pat_aux ~constrs ~labels ~no_existentials ~mode ~explode ~env Some (p0, p), expected_ty with Not_found -> None, newvar () in + let get_jsx_component_error_info = get_jsx_component_error_info ~extract_concrete_typedecl opath !env record_ty in let process_optional_label (ld, pat) = let exp_optional_attr = check_optional_attr !env ld pat.ppat_attributes pat.ppat_loc in let is_from_pamatch = match pat.ppat_desc with @@ -1330,7 +1334,7 @@ and type_pat_aux ~constrs ~labels ~no_existentials ~mode ~explode ~env k (label_lid, label, arg)) in let k' k lbl_pat_list = - check_recordpat_labels loc lbl_pat_list closed; + check_recordpat_labels ~get_jsx_component_error_info loc lbl_pat_list closed; unify_pat_types loc !env record_ty expected_ty; rp k { pat_desc = Tpat_record (lbl_pat_list, closed); @@ -1897,11 +1901,14 @@ let duplicate_ident_types caselist env = (* type_label_a_list returns a list of labels sorted by lbl_pos *) (* note: check_duplicates would better be implemented in type_label_a_list directly *) -let rec check_duplicates loc env = function +let rec check_duplicates ~get_jsx_component_error_info loc env = function | (_, lbl1, _) :: (_, lbl2, _) :: _ when lbl1.lbl_pos = lbl2.lbl_pos -> - raise(Error(loc, env, Label_multiply_defined lbl1.lbl_name)) + raise(Error(loc, env, Label_multiply_defined { + label = lbl1.lbl_name; + jsx_component_info = get_jsx_component_error_info(); + })) | _ :: rem -> - check_duplicates loc env rem + check_duplicates ~get_jsx_component_error_info loc env rem | [] -> () (* Getting proper location of already typed expressions. @@ -2274,6 +2281,10 @@ and type_expect_ ?type_clash_context ?in_function ?(recarg=Rejected) env sexp ty | exception Not_found -> newvar (), None, [], None + in + let get_jsx_component_error_info () = (match opath with + | Some (p, _) -> get_jsx_component_props ~extract_concrete_typedecl env ty_record p + | None -> None) in let lbl_exp_list = wrap_disambiguate "This record expression is expected to have" ty_record @@ -2283,7 +2294,7 @@ and type_expect_ ?type_clash_context ?in_function ?(recarg=Rejected) env sexp ty (fun x -> x) in unify_exp_types loc env ty_record (instance env ty_expected); - check_duplicates loc env lbl_exp_list; + check_duplicates ~get_jsx_component_error_info loc env lbl_exp_list; let label_descriptions, representation = match lbl_exp_list, repr_opt with | (_, { lbl_all = label_descriptions; lbl_repres = representation}, _) :: _, _ -> label_descriptions, representation | [], Some (representation) when lid_sexp_list = [] -> @@ -2301,9 +2312,7 @@ and type_expect_ ?type_clash_context ?in_function ?(recarg=Rejected) env sexp ty if labels_missing <> [] then ( raise(Error(loc, env, Labels_missing { labels = labels_missing; - jsx_component_info = (match opath with - | Some (p, _) -> get_jsx_component_props ~extract_concrete_typedecl env ty_record p - | None -> None) + jsx_component_info = get_jsx_component_error_info (); }))); [||], representation | [], _ -> @@ -2331,9 +2340,7 @@ and type_expect_ ?type_clash_context ?in_function ?(recarg=Rejected) env sexp ty if !labels_missing <> [] then ( raise(Error(loc, env, Labels_missing { labels=(List.rev !labels_missing); - jsx_component_info = (match opath with - | Some (p, _) -> get_jsx_component_props ~extract_concrete_typedecl env ty_record p - | None -> None) + jsx_component_info = get_jsx_component_error_info (); }))); let fields = Array.map2 (fun descr def -> descr, def) @@ -2375,6 +2382,7 @@ and type_expect_ ?type_clash_context ?in_function ?(recarg=Rejected) env sexp ty end | op -> ty_expected, op in + let get_jsx_component_error_info = get_jsx_component_error_info ~extract_concrete_typedecl opath env ty_record in let closed = false in let lbl_exp_list = wrap_disambiguate "This record expression is expected to have" ty_record @@ -2384,7 +2392,7 @@ and type_expect_ ?type_clash_context ?in_function ?(recarg=Rejected) env sexp ty (fun x -> x) in unify_exp_types loc env ty_record (instance env ty_expected); - check_duplicates loc env lbl_exp_list; + check_duplicates ~get_jsx_component_error_info loc env lbl_exp_list; let opt_exp, label_definitions = let (_lid, lbl, _lbl_exp) = List.hd lbl_exp_list in let matching_label lbl = @@ -3849,8 +3857,11 @@ let report_error env ppf = function "@[@[<2>The function applied to this argument has type@ %a@]@.\ This argument cannot be applied %a@]" type_expr ty print_label l - | Label_multiply_defined s -> - fprintf ppf "The record field label %s is defined several times" s + | Label_multiply_defined {label; jsx_component_info = Some jsx_component_info} -> + fprintf ppf "The prop @{%s@} is passed several times to the component " label; + print_component_name ppf jsx_component_info.props_record_path; + | Label_multiply_defined {label} -> + fprintf ppf "The record field label %s is defined several times" label | Labels_missing {labels; jsx_component_info = Some jsx_component_info} -> print_component_labels_missing_error ppf labels jsx_component_info | Labels_missing {labels} -> diff --git a/jscomp/ml/typecore.mli b/jscomp/ml/typecore.mli index 1e18eea61c..4b1fcc5368 100644 --- a/jscomp/ml/typecore.mli +++ b/jscomp/ml/typecore.mli @@ -69,7 +69,7 @@ type error = | Expr_type_clash of (type_expr * type_expr) list * (Error_message_utils.type_clash_context option) | Apply_non_function of type_expr | Apply_wrong_label of arg_label * type_expr - | Label_multiply_defined of string + | Label_multiply_defined of {label: string; jsx_component_info: Error_message_utils.jsx_prop_error_info option} | Labels_missing of {labels: string list; jsx_component_info: Error_message_utils.jsx_prop_error_info option} | Label_not_mutable of Longident.t | Wrong_name of string * type_expr * string * Path.t * string * string list From 750acaf91f405a8a72032142bd1cbeb71366fe49 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sun, 15 Sep 2024 15:29:21 +0200 Subject: [PATCH 04/11] add attribute in interface files too --- jscomp/syntax/src/jsx_v4.ml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/jscomp/syntax/src/jsx_v4.ml b/jscomp/syntax/src/jsx_v4.ml index f421fe12bc..d79d8e0f1f 100644 --- a/jscomp/syntax/src/jsx_v4.ml +++ b/jscomp/syntax/src/jsx_v4.ml @@ -367,7 +367,10 @@ let make_props_record_type ~core_type_of_attr ~external_ ~typ_vars_of_core_type (* type props<'x, 'y, ...> = { x: 'x, y?: 'y, ... } *) let make_props_record_type_sig ~core_type_of_attr ~external_ ~typ_vars_of_core_type props_name loc named_type_list = - let attrs = if external_ then [live_attr] else [] in + let attrs = + if external_ then [jsx_component_props_attr; live_attr] + else [jsx_component_props_attr] + in Sig.type_ Nonrecursive (match core_type_of_attr with | None -> make_type_decls ~attrs props_name loc named_type_list From 5725e059fee871dad6aa0d887939556353e09934 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sun, 15 Sep 2024 15:31:04 +0200 Subject: [PATCH 05/11] make function --- jscomp/ml/error_message_utils.ml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/jscomp/ml/error_message_utils.ml b/jscomp/ml/error_message_utils.ml index ded8f02314..d178f2129f 100644 --- a/jscomp/ml/error_message_utils.ml +++ b/jscomp/ml/error_message_utils.ml @@ -246,6 +246,10 @@ type jsx_prop_error_info = { props_record_path: Path.t; } +let attributes_include_jsx_component_props (attrs : Parsetree.attributes) = + attrs + |> List.exists (fun ({Location.txt}, _) -> txt = "res.jsxComponentProps") + let path_to_jsx_component_name p = match p |> Path.name |> String.split_on_char '.' |> List.rev with | "props" :: component_name :: _ -> Some component_name @@ -260,9 +264,7 @@ let get_jsx_component_props | ( _p0, _p, {Types.type_kind = Type_record (fields, _repr); type_attributes} ) - when type_attributes - |> List.exists (fun ({Location.txt}, _) -> - txt = "res.jsxComponentProps") -> + when attributes_include_jsx_component_props type_attributes -> Some {props_record_path = p; fields} | _ -> None with _ -> None) From fce9600efc0005178f44c07b5ab60d7273292601 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sun, 15 Sep 2024 15:39:00 +0200 Subject: [PATCH 06/11] add tests --- .../expected/component_invalid_prop.res.expected | 11 +++++++++++ ...mponent_prop_passed_multiple_times.res.expected | 14 ++++++++++++++ .../fixtures/component_invalid_prop.res | 9 +++++++++ .../component_prop_passed_multiple_times.res | 10 ++++++++++ 4 files changed, 44 insertions(+) create mode 100644 jscomp/build_tests/super_errors/expected/component_invalid_prop.res.expected create mode 100644 jscomp/build_tests/super_errors/expected/component_prop_passed_multiple_times.res.expected create mode 100644 jscomp/build_tests/super_errors/fixtures/component_invalid_prop.res create mode 100644 jscomp/build_tests/super_errors/fixtures/component_prop_passed_multiple_times.res diff --git a/jscomp/build_tests/super_errors/expected/component_invalid_prop.res.expected b/jscomp/build_tests/super_errors/expected/component_invalid_prop.res.expected new file mode 100644 index 0000000000..56e3d4a24c --- /dev/null +++ b/jscomp/build_tests/super_errors/expected/component_invalid_prop.res.expected @@ -0,0 +1,11 @@ + + We've found a bug for you! + /.../fixtures/component_invalid_prop.res:7:5-8 + + 5 │ + 6 │ let make = (): props<'name> => { + 7 │ test: false, + 8 │ } + 9 │ } + + The prop test does not belong to the JSX component \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/expected/component_prop_passed_multiple_times.res.expected b/jscomp/build_tests/super_errors/expected/component_prop_passed_multiple_times.res.expected new file mode 100644 index 0000000000..692318ead5 --- /dev/null +++ b/jscomp/build_tests/super_errors/expected/component_prop_passed_multiple_times.res.expected @@ -0,0 +1,14 @@ + + We've found a bug for you! + /.../fixtures/component_prop_passed_multiple_times.res:6:34-9:3 + + 4 │ type props<'name> = {name: 'name} + 5 │ + 6 │ let make = (): props<'name> => { + 7 │  name: "hello", + 8 │  name: "world", + 9 │  } + 10 │ } + 11 │ + + The prop name is passed several times to the component \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/fixtures/component_invalid_prop.res b/jscomp/build_tests/super_errors/fixtures/component_invalid_prop.res new file mode 100644 index 0000000000..714954a968 --- /dev/null +++ b/jscomp/build_tests/super_errors/fixtures/component_invalid_prop.res @@ -0,0 +1,9 @@ +// Since the React transform isn't active in the tests, mimic what the transform outputs. +module Component = { + @res.jsxComponentProps + type props<'name> = {name: 'name} + + let make = (): props<'name> => { + test: false, + } +} diff --git a/jscomp/build_tests/super_errors/fixtures/component_prop_passed_multiple_times.res b/jscomp/build_tests/super_errors/fixtures/component_prop_passed_multiple_times.res new file mode 100644 index 0000000000..9cb8d0db88 --- /dev/null +++ b/jscomp/build_tests/super_errors/fixtures/component_prop_passed_multiple_times.res @@ -0,0 +1,10 @@ +// Since the React transform isn't active in the tests, mimic what the transform outputs. +module Component = { + @res.jsxComponentProps + type props<'name> = {name: 'name} + + let make = (): props<'name> => { + name: "hello", + name: "world", + } +} From 350fe9ffc7432d2677c90b51ff2ec649942adf9d Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sun, 15 Sep 2024 15:41:40 +0200 Subject: [PATCH 07/11] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c2597f68ee..7ade3dd8bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ - Improve error messages for pattern matching on option vs non-option, and vice versa. https://github.com/rescript-lang/rescript-compiler/pull/7035 - Improve bigint literal comparison. https://github.com/rescript-lang/rescript-compiler/pull/7029 - Improve output of `@variadic` bindings. https://github.com/rescript-lang/rescript-compiler/pull/7030 +- Improve error messages around JSX components. https://github.com/rescript-lang/rescript-compiler/pull/7038 # 12.0.0-alpha.3 From d58f998767b8d54e2400ca18bc442ee6869ab1cf Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sun, 15 Sep 2024 15:48:01 +0200 Subject: [PATCH 08/11] tweak error message --- jscomp/syntax/src/jsx_v4.ml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jscomp/syntax/src/jsx_v4.ml b/jscomp/syntax/src/jsx_v4.ml index d79d8e0f1f..0c79ff009c 100644 --- a/jscomp/syntax/src/jsx_v4.ml +++ b/jscomp/syntax/src/jsx_v4.ml @@ -310,7 +310,8 @@ let make_label_decls named_type_list = | hd :: tl -> if mem_label hd tl then let _, label, _, loc, _ = hd in - Jsx_common.raise_error ~loc "JSX: found the duplicated prop `%s`" label + Jsx_common.raise_error ~loc + "The prop `%s` is defined several times in this component." label else check_duplicated_label tl in let () = named_type_list |> List.rev |> check_duplicated_label in From fe21a5b6ba0b89ab3f18bfe9a5cfdc0b9eee8d9e Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sun, 15 Sep 2024 16:06:00 +0200 Subject: [PATCH 09/11] fix location and message of JSX component prop passed multiple times --- ...onent_prop_passed_multiple_times.res.expected | 16 +++++++--------- jscomp/ml/typecore.ml | 11 ++++++----- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/jscomp/build_tests/super_errors/expected/component_prop_passed_multiple_times.res.expected b/jscomp/build_tests/super_errors/expected/component_prop_passed_multiple_times.res.expected index 692318ead5..60dee57082 100644 --- a/jscomp/build_tests/super_errors/expected/component_prop_passed_multiple_times.res.expected +++ b/jscomp/build_tests/super_errors/expected/component_prop_passed_multiple_times.res.expected @@ -1,14 +1,12 @@ We've found a bug for you! - /.../fixtures/component_prop_passed_multiple_times.res:6:34-9:3 + /.../fixtures/component_prop_passed_multiple_times.res:8:11-17 - 4 │ type props<'name> = {name: 'name} - 5 │ - 6 │ let make = (): props<'name> => { - 7 │  name: "hello", - 8 │  name: "world", - 9 │  } + 6 │ let make = (): props<'name> => { + 7 │ name: "hello", + 8 │ name: "world", + 9 │ } 10 │ } - 11 │ - The prop name is passed several times to the component \ No newline at end of file + The prop name has already been passed to the component +You can't pass the same prop more than once. \ No newline at end of file diff --git a/jscomp/ml/typecore.ml b/jscomp/ml/typecore.ml index 54837dbf02..9c011e9a29 100644 --- a/jscomp/ml/typecore.ml +++ b/jscomp/ml/typecore.ml @@ -963,12 +963,12 @@ let type_label_a_list ?labels loc closed env type_lbl_a opath lid_a_list k = let check_recordpat_labels ~get_jsx_component_error_info loc lbl_pat_list closed = match lbl_pat_list with | [] -> () (* should not happen *) - | (_, label1, _) :: _ -> + | (_, label1, (tpat: Typedtree.pattern)) :: _ -> let all = label1.lbl_all in let defined = Array.make (Array.length all) false in let check_defined (_, label, _) = if defined.(label.lbl_pos) - then raise(Error(loc, Env.empty, Label_multiply_defined { + then raise(Error(tpat.pat_loc, Env.empty, Label_multiply_defined { label = label.lbl_name; jsx_component_info = get_jsx_component_error_info (); })) @@ -1902,8 +1902,8 @@ let duplicate_ident_types caselist env = (* note: check_duplicates would better be implemented in type_label_a_list directly *) let rec check_duplicates ~get_jsx_component_error_info loc env = function - | (_, lbl1, _) :: (_, lbl2, _) :: _ when lbl1.lbl_pos = lbl2.lbl_pos -> - raise(Error(loc, env, Label_multiply_defined { + | (_, lbl1, _) :: (_, lbl2, (texp: Typedtree.expression)) :: _ when lbl1.lbl_pos = lbl2.lbl_pos -> + raise(Error(texp.exp_loc, env, Label_multiply_defined { label = lbl1.lbl_name; jsx_component_info = get_jsx_component_error_info(); })) @@ -3858,8 +3858,9 @@ let report_error env ppf = function This argument cannot be applied %a@]" type_expr ty print_label l | Label_multiply_defined {label; jsx_component_info = Some jsx_component_info} -> - fprintf ppf "The prop @{%s@} is passed several times to the component " label; + fprintf ppf "The prop @{%s@} has already been passed to the component " label; print_component_name ppf jsx_component_info.props_record_path; + fprintf ppf "@,@,You can't pass the same prop more than once."; | Label_multiply_defined {label} -> fprintf ppf "The record field label %s is defined several times" label | Labels_missing {labels; jsx_component_info = Some jsx_component_info} -> From befe0682f4a30c6cb67929b254c9e7a95f8d6e11 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sun, 15 Sep 2024 16:07:40 +0200 Subject: [PATCH 10/11] sync syntax roundtrip tests --- .../tests/ppx/react/expected/aliasProps.res.txt | 8 ++++++++ .../tests/ppx/react/expected/asyncAwait.res.txt | 2 ++ .../tests/ppx/react/expected/commentAtTop.res.txt | 1 + .../tests/ppx/react/expected/defaultValueProp.res.txt | 4 ++++ .../ppx/react/expected/externalWithCustomName.res.txt | 4 ++-- .../tests/ppx/react/expected/externalWithRef.res.txt | 6 +++--- .../react/expected/externalWithTypeVariables.res.txt | 4 ++-- .../tests/ppx/react/expected/fileLevelConfig.res.txt | 2 ++ .../ppx/react/expected/firstClassModules.res.txt | 3 ++- .../ppx/react/expected/firstClassModules.resi.txt | 1 + .../tests/ppx/react/expected/forwardRef.res.txt | 8 ++++++++ .../tests/ppx/react/expected/forwardRef.resi.txt | 8 ++++++++ .../syntax/tests/ppx/react/expected/interface.res.txt | 2 ++ .../tests/ppx/react/expected/interface.resi.txt | 2 ++ .../tests/ppx/react/expected/interfaceWithRef.res.txt | 2 +- .../ppx/react/expected/interfaceWithRef.resi.txt | 2 +- .../tests/ppx/react/expected/mangleKeyword.res.txt | 6 ++++-- jscomp/syntax/tests/ppx/react/expected/nested.res.txt | 2 ++ .../syntax/tests/ppx/react/expected/newtype.res.txt | 7 +++++++ .../tests/ppx/react/expected/noPropsWithKey.res.txt | 8 ++++++-- .../ppx/react/expected/optimizeAutomaticMode.res.txt | 1 + .../tests/ppx/react/expected/removedKeyProp.res.txt | 3 +++ .../syntax/tests/ppx/react/expected/topLevel.res.txt | 2 ++ .../tests/ppx/react/expected/typeConstraint.res.txt | 2 ++ .../tests/ppx/react/expected/uncurriedProps.res.txt | 3 +++ jscomp/syntax/tests/ppx/react/expected/v4.res.txt | 11 +++++++++-- 26 files changed, 88 insertions(+), 16 deletions(-) diff --git a/jscomp/syntax/tests/ppx/react/expected/aliasProps.res.txt b/jscomp/syntax/tests/ppx/react/expected/aliasProps.res.txt index 959ae19821..02248a9072 100644 --- a/jscomp/syntax/tests/ppx/react/expected/aliasProps.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/aliasProps.res.txt @@ -1,6 +1,7 @@ @@jsxConfig({version: 4, mode: "automatic"}) module C0 = { + @res.jsxComponentProps type props<'priority, 'text> = {priority: 'priority, text?: 'text} let make = ({priority: _, text: ?__text, _}: props<_, _>) => { @@ -19,6 +20,7 @@ module C0 = { } module C1 = { + @res.jsxComponentProps type props<'priority, 'text> = {priority: 'priority, text?: 'text} let make = ({priority: p, text: ?__text, _}: props<_, _>) => { @@ -37,6 +39,7 @@ module C1 = { } module C2 = { + @res.jsxComponentProps type props<'foo> = {foo?: 'foo} let make = ({foo: ?__bar, _}: props<_>) => { @@ -55,6 +58,7 @@ module C2 = { } module C3 = { + @res.jsxComponentProps type props<'foo, 'a, 'b> = {foo?: 'foo, a?: 'a, b: 'b} let make = ({foo: ?__bar, a: ?__a, b, _}: props<_, _, _>) => { @@ -79,6 +83,7 @@ module C3 = { } module C4 = { + @res.jsxComponentProps type props<'a, 'x> = {a: 'a, x?: 'x} let make = ({a: b, x: ?__x, _}: props<_, _>) => { @@ -97,6 +102,7 @@ module C4 = { } module C5 = { + @res.jsxComponentProps type props<'a, 'z> = {a: 'a, z?: 'z} let make = ({a: (x, y), z: ?__z, _}: props<_, _>) => { @@ -116,10 +122,12 @@ module C5 = { module C6 = { module type Comp = { + @res.jsxComponentProps type props = {} let make: React.componentLike } + @res.jsxComponentProps type props<'comp, 'x> = {comp: 'comp, x: 'x} let make = ({comp: module(Comp: Comp), x: (a, b), _}: props<_, _>) => React.jsx(Comp.make, {}) diff --git a/jscomp/syntax/tests/ppx/react/expected/asyncAwait.res.txt b/jscomp/syntax/tests/ppx/react/expected/asyncAwait.res.txt index d8d86dccca..37df951e28 100644 --- a/jscomp/syntax/tests/ppx/react/expected/asyncAwait.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/asyncAwait.res.txt @@ -1,6 +1,7 @@ let f = a => Js.Promise.resolve(a + a) module C0 = { + @res.jsxComponentProps type props<'a> = {a: 'a} let make = async ({a, _}: props<_>) => { @@ -15,6 +16,7 @@ module C0 = { } module C1 = { + @res.jsxComponentProps type props<'status> = {status: 'status} let make = async ({status, _}: props<_>) => { diff --git a/jscomp/syntax/tests/ppx/react/expected/commentAtTop.res.txt b/jscomp/syntax/tests/ppx/react/expected/commentAtTop.res.txt index d620f691c5..3f4656ddda 100644 --- a/jscomp/syntax/tests/ppx/react/expected/commentAtTop.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/commentAtTop.res.txt @@ -1,3 +1,4 @@ +@res.jsxComponentProps type props<'msg> = {msg: 'msg} // test React JSX file let make = ({msg, _}: props<_>) => { diff --git a/jscomp/syntax/tests/ppx/react/expected/defaultValueProp.res.txt b/jscomp/syntax/tests/ppx/react/expected/defaultValueProp.res.txt index 0b4c240a41..5cb93d4681 100644 --- a/jscomp/syntax/tests/ppx/react/expected/defaultValueProp.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/defaultValueProp.res.txt @@ -1,4 +1,5 @@ module C0 = { + @res.jsxComponentProps type props<'a, 'b> = {a?: 'a, b?: 'b} let make = ({a: ?__a, b: ?__b, _}: props<_, _>) => { let a = switch __a { @@ -19,6 +20,7 @@ module C0 = { } module C1 = { + @res.jsxComponentProps type props<'a, 'b> = {a?: 'a, b: 'b} let make = ({a: ?__a, b, _}: props<_, _>) => { @@ -38,6 +40,7 @@ module C1 = { module C2 = { let a = "foo" + @res.jsxComponentProps type props<'a> = {a?: 'a} let make = ({a: ?__a, _}: props<_>) => { @@ -56,6 +59,7 @@ module C2 = { } module C3 = { + @res.jsxComponentProps type props<'disabled> = {disabled?: 'disabled} let make = ({disabled: ?__everythingDisabled, _}: props) => { diff --git a/jscomp/syntax/tests/ppx/react/expected/externalWithCustomName.res.txt b/jscomp/syntax/tests/ppx/react/expected/externalWithCustomName.res.txt index be73710b13..92e89a9738 100644 --- a/jscomp/syntax/tests/ppx/react/expected/externalWithCustomName.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/externalWithCustomName.res.txt @@ -13,7 +13,7 @@ let t = React.createElement(Foo.component, Foo.componentProps(~a=1, ~b={"1"}, () @@jsxConfig({version: 4, mode: "classic"}) module Foo = { - @live + @res.jsxComponentProps @live type props<'a, 'b> = {a: 'a, b: 'b} @module("Foo") @@ -25,7 +25,7 @@ let t = React.createElement(Foo.component, {a: 1, b: "1"}) @@jsxConfig({version: 4, mode: "automatic"}) module Foo = { - @live + @res.jsxComponentProps @live type props<'a, 'b> = {a: 'a, b: 'b} @module("Foo") diff --git a/jscomp/syntax/tests/ppx/react/expected/externalWithRef.res.txt b/jscomp/syntax/tests/ppx/react/expected/externalWithRef.res.txt index 036b7cf555..41f643cecb 100644 --- a/jscomp/syntax/tests/ppx/react/expected/externalWithRef.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/externalWithRef.res.txt @@ -18,7 +18,7 @@ module V3 = { @@jsxConfig({version: 4, mode: "classic"}) module V4C = { - @live + @res.jsxComponentProps @live type props<'x, 'ref> = { x: 'x, ref?: 'ref, @@ -30,7 +30,7 @@ module V4C = { } module type V4CType = { - @live + @res.jsxComponentProps @live type props<'x, 'y> = { x: 'x, y: 'y, @@ -43,7 +43,7 @@ module type V4CType = { @@jsxConfig({version: 4, mode: "automatic"}) module V4C = { - @live + @res.jsxComponentProps @live type props<'x, 'ref> = { x: 'x, ref?: 'ref, diff --git a/jscomp/syntax/tests/ppx/react/expected/externalWithTypeVariables.res.txt b/jscomp/syntax/tests/ppx/react/expected/externalWithTypeVariables.res.txt index 1a62c59653..454aba7f5a 100644 --- a/jscomp/syntax/tests/ppx/react/expected/externalWithTypeVariables.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/externalWithTypeVariables.res.txt @@ -16,7 +16,7 @@ module V3 = { @@jsxConfig({version: 4, mode: "classic"}) module V4C = { - @live + @res.jsxComponentProps @live type props<'x, 'children> = { x: 'x, children: 'children, @@ -29,7 +29,7 @@ module V4C = { @@jsxConfig({version: 4, mode: "automatic"}) module V4C = { - @live + @res.jsxComponentProps @live type props<'x, 'children> = { x: 'x, children: 'children, diff --git a/jscomp/syntax/tests/ppx/react/expected/fileLevelConfig.res.txt b/jscomp/syntax/tests/ppx/react/expected/fileLevelConfig.res.txt index 817461d18b..a6423491db 100644 --- a/jscomp/syntax/tests/ppx/react/expected/fileLevelConfig.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/fileLevelConfig.res.txt @@ -18,6 +18,7 @@ module V3 = { @@jsxConfig({version: 4, mode: "classic"}) module V4C = { + @res.jsxComponentProps type props<'msg> = {msg: 'msg} let make = ({msg, _}: props<_>) => { @@ -33,6 +34,7 @@ module V4C = { @@jsxConfig({version: 4, mode: "automatic"}) module V4A = { + @res.jsxComponentProps type props<'msg> = {msg: 'msg} let make = ({msg, _}: props<_>) => { diff --git a/jscomp/syntax/tests/ppx/react/expected/firstClassModules.res.txt b/jscomp/syntax/tests/ppx/react/expected/firstClassModules.res.txt index 44a9545886..7e330ec780 100644 --- a/jscomp/syntax/tests/ppx/react/expected/firstClassModules.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/firstClassModules.res.txt @@ -57,6 +57,7 @@ module Select = { type key type t } + @res.jsxComponentProps type props<'model, 'selected, 'onChange, 'items> = { model: 'model, selected: 'selected, @@ -88,7 +89,7 @@ module External = { type key type t } - @live + @res.jsxComponentProps @live type props<'model, 'selected, 'onChange, 'items> = { model: 'model, selected: 'selected, diff --git a/jscomp/syntax/tests/ppx/react/expected/firstClassModules.resi.txt b/jscomp/syntax/tests/ppx/react/expected/firstClassModules.resi.txt index 4c0e30f781..3a0e98b433 100644 --- a/jscomp/syntax/tests/ppx/react/expected/firstClassModules.resi.txt +++ b/jscomp/syntax/tests/ppx/react/expected/firstClassModules.resi.txt @@ -38,6 +38,7 @@ module Select: { type key type t } + @res.jsxComponentProps type props<'model, 'selected, 'onChange, 'items> = { model: 'model, selected: 'selected, diff --git a/jscomp/syntax/tests/ppx/react/expected/forwardRef.res.txt b/jscomp/syntax/tests/ppx/react/expected/forwardRef.res.txt index 80f843e18b..59ac6dbb17 100644 --- a/jscomp/syntax/tests/ppx/react/expected/forwardRef.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/forwardRef.res.txt @@ -69,6 +69,7 @@ module V3 = { module V4C = { module FancyInput = { + @res.jsxComponentProps type props<'className, 'children, 'ref> = { className?: 'className, children: 'children, @@ -100,6 +101,7 @@ module V4C = { \"ForwardRef$V4C$FancyInput" }) } + @res.jsxComponentProps type props = {} let make = (_: props) => { @@ -124,6 +126,7 @@ module V4C = { module V4CUncurried = { module FancyInput = { + @res.jsxComponentProps type props<'className, 'children, 'ref> = { className?: 'className, children: 'children, @@ -155,6 +158,7 @@ module V4CUncurried = { \"ForwardRef$V4CUncurried$FancyInput" }) } + @res.jsxComponentProps type props = {} let make = (_: props) => { @@ -181,6 +185,7 @@ module V4CUncurried = { module V4A = { module FancyInput = { + @res.jsxComponentProps type props<'className, 'children, 'ref> = { className?: 'className, children: 'children, @@ -210,6 +215,7 @@ module V4A = { \"ForwardRef$V4A$FancyInput" }) } + @res.jsxComponentProps type props = {} let make = (_: props) => { @@ -233,6 +239,7 @@ module V4A = { module V4AUncurried = { module FancyInput = { + @res.jsxComponentProps type props<'className, 'children, 'ref> = { className?: 'className, children: 'children, @@ -262,6 +269,7 @@ module V4AUncurried = { \"ForwardRef$V4AUncurried$FancyInput" }) } + @res.jsxComponentProps type props = {} let make = (_: props) => { diff --git a/jscomp/syntax/tests/ppx/react/expected/forwardRef.resi.txt b/jscomp/syntax/tests/ppx/react/expected/forwardRef.resi.txt index 4b8c51cd83..0c8c8d69dc 100644 --- a/jscomp/syntax/tests/ppx/react/expected/forwardRef.resi.txt +++ b/jscomp/syntax/tests/ppx/react/expected/forwardRef.resi.txt @@ -42,6 +42,7 @@ module V3: { module V4C: { module FancyInput: { + @res.jsxComponentProps type props<'className, 'children, 'ref> = { className?: 'className, children: 'children, @@ -55,6 +56,7 @@ module V4C: { } module ForwardRef: { + @res.jsxComponentProps type props<'ref> = {ref?: 'ref} let make: React.componentLike>>, React.element> @@ -63,6 +65,7 @@ module V4C: { module V4CUncurried: { module FancyInput: { + @res.jsxComponentProps type props<'className, 'children, 'ref> = { className?: 'className, children: 'children, @@ -76,6 +79,7 @@ module V4CUncurried: { } module ForwardRef: { + @res.jsxComponentProps type props<'ref> = {ref?: 'ref} let make: React.componentLike>>, React.element> @@ -86,6 +90,7 @@ module V4CUncurried: { module V4A: { module FancyInput: { + @res.jsxComponentProps type props<'className, 'children, 'ref> = { className?: 'className, children: 'children, @@ -99,6 +104,7 @@ module V4A: { } module ForwardRef: { + @res.jsxComponentProps type props<'ref> = {ref?: 'ref} let make: React.componentLike>>, React.element> @@ -107,6 +113,7 @@ module V4A: { module V4AUncurried: { module FancyInput: { + @res.jsxComponentProps type props<'className, 'children, 'ref> = { className?: 'className, children: 'children, @@ -120,6 +127,7 @@ module V4AUncurried: { } module ForwardRef: { + @res.jsxComponentProps type props<'ref> = {ref?: 'ref} let make: React.componentLike>>, React.element> diff --git a/jscomp/syntax/tests/ppx/react/expected/interface.res.txt b/jscomp/syntax/tests/ppx/react/expected/interface.res.txt index 166ed2ed90..645636451e 100644 --- a/jscomp/syntax/tests/ppx/react/expected/interface.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/interface.res.txt @@ -1,4 +1,5 @@ module A = { + @res.jsxComponentProps type props<'x> = {x: 'x} let make = ({x, _}: props<_>) => React.string(x) let make = { @@ -8,6 +9,7 @@ module A = { } module NoProps = { + @res.jsxComponentProps type props = {} let make = (_: props) => ReactDOM.jsx("div", {}) diff --git a/jscomp/syntax/tests/ppx/react/expected/interface.resi.txt b/jscomp/syntax/tests/ppx/react/expected/interface.resi.txt index f4ffd9b65c..0f045c0beb 100644 --- a/jscomp/syntax/tests/ppx/react/expected/interface.resi.txt +++ b/jscomp/syntax/tests/ppx/react/expected/interface.resi.txt @@ -1,9 +1,11 @@ module A: { + @res.jsxComponentProps type props<'x> = {x: 'x} let make: React.componentLike, React.element> } module NoProps: { + @res.jsxComponentProps type props = {} let make: React.componentLike diff --git a/jscomp/syntax/tests/ppx/react/expected/interfaceWithRef.res.txt b/jscomp/syntax/tests/ppx/react/expected/interfaceWithRef.res.txt index dbb6acfd5e..eb7616f71f 100644 --- a/jscomp/syntax/tests/ppx/react/expected/interfaceWithRef.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/interfaceWithRef.res.txt @@ -1,4 +1,4 @@ -type props<'x, 'ref> = {x: 'x, ref?: 'ref} +@res.jsxComponentProps type props<'x, 'ref> = {x: 'x, ref?: 'ref} let make = ( {x, _}: props, ref: Js.Nullable.t, diff --git a/jscomp/syntax/tests/ppx/react/expected/interfaceWithRef.resi.txt b/jscomp/syntax/tests/ppx/react/expected/interfaceWithRef.resi.txt index ee5da3ca6d..c28c75f9d9 100644 --- a/jscomp/syntax/tests/ppx/react/expected/interfaceWithRef.resi.txt +++ b/jscomp/syntax/tests/ppx/react/expected/interfaceWithRef.resi.txt @@ -1,2 +1,2 @@ -type props<'x, 'ref> = {x: 'x, ref?: 'ref} +@res.jsxComponentProps type props<'x, 'ref> = {x: 'x, ref?: 'ref} let make: React.componentLike, React.element> diff --git a/jscomp/syntax/tests/ppx/react/expected/mangleKeyword.res.txt b/jscomp/syntax/tests/ppx/react/expected/mangleKeyword.res.txt index d803631282..673d055399 100644 --- a/jscomp/syntax/tests/ppx/react/expected/mangleKeyword.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/mangleKeyword.res.txt @@ -37,6 +37,7 @@ let c3a1 = React.createElement(C3A1.make, C3A1.makeProps(~_open="x", ~_type="t", @@jsxConfig({version: 4, mode: "classic"}) module C4C0 = { + @res.jsxComponentProps type props<'T_open, 'T_type> = {@as("open") _open: 'T_open, @as("type") _type: 'T_type} let make = ({@as("open") _open, @as("type") _type, _}: props<_, string>) => React.string(_open) @@ -47,7 +48,7 @@ module C4C0 = { } } module C4C1 = { - @live + @res.jsxComponentProps @live type props<'T_open, 'T_type> = {@as("open") _open: 'T_open, @as("type") _type: 'T_type} external make: @as("open") React.componentLike, React.element> = "default" @@ -59,6 +60,7 @@ let c4c1 = React.createElement(C4C1.make, {_open: "x", _type: "t"}) @@jsxConfig({version: 4, mode: "automatic"}) module C4A0 = { + @res.jsxComponentProps type props<'T_open, 'T_type> = {@as("open") _open: 'T_open, @as("type") _type: 'T_type} let make = ({@as("open") _open, @as("type") _type, _}: props<_, string>) => React.string(_open) @@ -69,7 +71,7 @@ module C4A0 = { } } module C4A1 = { - @live + @res.jsxComponentProps @live type props<'T_open, 'T_type> = {@as("open") _open: 'T_open, @as("type") _type: 'T_type} external make: @as("open") React.componentLike, React.element> = "default" diff --git a/jscomp/syntax/tests/ppx/react/expected/nested.res.txt b/jscomp/syntax/tests/ppx/react/expected/nested.res.txt index f290915b30..0775867aa9 100644 --- a/jscomp/syntax/tests/ppx/react/expected/nested.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/nested.res.txt @@ -1,7 +1,9 @@ module Outer = { + @res.jsxComponentProps type props = {} let make = (_: props) => { module Inner = { + @res.jsxComponentProps type props = {} let make = (_: props) => ReactDOM.jsx("div", {}) diff --git a/jscomp/syntax/tests/ppx/react/expected/newtype.res.txt b/jscomp/syntax/tests/ppx/react/expected/newtype.res.txt index ca23fe5ad2..240548e08e 100644 --- a/jscomp/syntax/tests/ppx/react/expected/newtype.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/newtype.res.txt @@ -24,6 +24,7 @@ module V3 = { @@jsxConfig({version: 4, mode: "classic"}) module V4C = { + @res.jsxComponentProps type props<'a, 'b, 'c> = {a: 'a, b: 'b, c: 'c} let make = (type a, {a, b, c, _}: props>, 'a>) => @@ -38,6 +39,7 @@ module V4C = { @@jsxConfig({version: 4, mode: "automatic"}) module V4A = { + @res.jsxComponentProps type props<'a, 'b, 'c> = {a: 'a, b: 'b, c: 'c} let make = (type a, {a, b, c, _}: props>, 'a>) => @@ -50,6 +52,7 @@ module V4A = { } module V4A1 = { + @res.jsxComponentProps type props<'a, 'b, 'c> = {a: 'a, b: 'b, c: 'c} let make = (type x y, {a, b, c, _}: props, 'a>) => ReactDOM.jsx("div", {}) @@ -65,6 +68,7 @@ module type T = { } module V4A2 = { + @res.jsxComponentProps type props<'foo> = {foo: 'foo} let make = (type a, {foo: (foo: module(T with type t = a)), _}: props<_>) => { @@ -79,6 +83,7 @@ module V4A2 = { } module V4A3 = { + @res.jsxComponentProps type props<'foo> = {foo: 'foo} let make = (type a, {foo, _}: props<_>) => { @@ -91,6 +96,7 @@ module V4A3 = { \"Newtype$V4A3" } } +@res.jsxComponentProps type props<'x, 'q> = {x: 'x, q: 'q} let make = ({x, q, _}: props<('a, 'b), 'a>) => [fst(x), q] @@ -103,6 +109,7 @@ let make = { @@uncurried module Uncurried = { + @res.jsxComponentProps type props<'foo> = {foo?: 'foo} let make = (type a, {?foo, _}: props<_>) => React.null diff --git a/jscomp/syntax/tests/ppx/react/expected/noPropsWithKey.res.txt b/jscomp/syntax/tests/ppx/react/expected/noPropsWithKey.res.txt index 90b8603465..4acf55ebdb 100644 --- a/jscomp/syntax/tests/ppx/react/expected/noPropsWithKey.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/noPropsWithKey.res.txt @@ -1,6 +1,7 @@ @@jsxConfig({version: 4, mode: "classic"}) module V4CA = { + @res.jsxComponentProps type props = {} let make = (_: props) => ReactDOM.createDOMElementVariadic("div", []) @@ -12,7 +13,7 @@ module V4CA = { } module V4CB = { - @live + @res.jsxComponentProps @live type props = {} @module("c") @@ -20,6 +21,7 @@ module V4CB = { } module V4C = { + @res.jsxComponentProps type props = {} let make = (_: props) => @@ -41,6 +43,7 @@ module V4C = { @@jsxConfig({version: 4, mode: "automatic"}) module V4CA = { + @res.jsxComponentProps type props = {} let make = (_: props) => ReactDOM.jsx("div", {}) @@ -52,7 +55,7 @@ module V4CA = { } module V4CB = { - @live + @res.jsxComponentProps @live type props = {} @module("c") @@ -60,6 +63,7 @@ module V4CB = { } module V4C = { + @res.jsxComponentProps type props = {} let make = (_: props) => diff --git a/jscomp/syntax/tests/ppx/react/expected/optimizeAutomaticMode.res.txt b/jscomp/syntax/tests/ppx/react/expected/optimizeAutomaticMode.res.txt index 18a5a3198b..4dbaf2e084 100644 --- a/jscomp/syntax/tests/ppx/react/expected/optimizeAutomaticMode.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/optimizeAutomaticMode.res.txt @@ -4,6 +4,7 @@ module User = { type t = {firstName: string, lastName: string} let format = user => "Dr." ++ user.lastName + @res.jsxComponentProps type props<'doctor> = {doctor: 'doctor} let make = ({doctor, _}: props<_>) => { diff --git a/jscomp/syntax/tests/ppx/react/expected/removedKeyProp.res.txt b/jscomp/syntax/tests/ppx/react/expected/removedKeyProp.res.txt index bad43e7220..b2f3551db3 100644 --- a/jscomp/syntax/tests/ppx/react/expected/removedKeyProp.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/removedKeyProp.res.txt @@ -1,6 +1,7 @@ @@jsxConfig({version: 4, mode: "classic"}) module Foo = { + @res.jsxComponentProps type props<'x, 'y> = {x: 'x, y: 'y} let make = ({x, y, _}: props<_, _>) => React.string(x ++ y) @@ -12,6 +13,7 @@ module Foo = { } module HasChildren = { + @res.jsxComponentProps type props<'children> = {children: 'children} let make = ({children, _}: props<_>) => React.createElement(React.fragment, {children: children}) @@ -21,6 +23,7 @@ module HasChildren = { \"RemovedKeyProp$HasChildren" } } +@res.jsxComponentProps type props = {} let make = (_: props) => diff --git a/jscomp/syntax/tests/ppx/react/expected/topLevel.res.txt b/jscomp/syntax/tests/ppx/react/expected/topLevel.res.txt index 36e60aed27..7bcd208661 100644 --- a/jscomp/syntax/tests/ppx/react/expected/topLevel.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/topLevel.res.txt @@ -22,6 +22,7 @@ module V3 = { @@jsxConfig({version: 4, mode: "classic"}) module V4C = { + @res.jsxComponentProps type props<'a, 'b> = {a: 'a, b: 'b} let make = ({a, b, _}: props<_, _>) => { @@ -38,6 +39,7 @@ module V4C = { @@jsxConfig({version: 4, mode: "automatic"}) module V4A = { + @res.jsxComponentProps type props<'a, 'b> = {a: 'a, b: 'b} let make = ({a, b, _}: props<_, _>) => { diff --git a/jscomp/syntax/tests/ppx/react/expected/typeConstraint.res.txt b/jscomp/syntax/tests/ppx/react/expected/typeConstraint.res.txt index 6196ae1df3..d2e8f62c0c 100644 --- a/jscomp/syntax/tests/ppx/react/expected/typeConstraint.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/typeConstraint.res.txt @@ -16,6 +16,7 @@ module V3 = { @@jsxConfig({version: 4, mode: "classic"}) module V4C = { + @res.jsxComponentProps type props = {} let make = (type a, _: props) => (~a, ~b, _) => ReactDOM.createDOMElementVariadic("div", []) @@ -29,6 +30,7 @@ module V4C = { @@jsxConfig({version: 4, mode: "automatic"}) module V4A = { + @res.jsxComponentProps type props = {} let make = (type a, _: props) => (~a, ~b, _) => ReactDOM.jsx("div", {}) diff --git a/jscomp/syntax/tests/ppx/react/expected/uncurriedProps.res.txt b/jscomp/syntax/tests/ppx/react/expected/uncurriedProps.res.txt index 7e3dae8434..28da95c83e 100644 --- a/jscomp/syntax/tests/ppx/react/expected/uncurriedProps.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/uncurriedProps.res.txt @@ -1,4 +1,5 @@ @@jsxConfig({version: 4}) +@res.jsxComponentProps type props<'a> = {a?: 'a} let make = ({a: ?__a, _}: props unit>) => { @@ -26,6 +27,7 @@ func(~callback=(str, a, b) => { }, ()) module Foo = { + @res.jsxComponentProps type props<'callback> = {callback?: 'callback} let make = ({callback: ?__callback, _}: props<(string, bool, bool) => unit>) => { @@ -46,6 +48,7 @@ module Foo = { } module Bar = { + @res.jsxComponentProps type props = {} let make = (_: props) => React.jsx(Foo.make, {callback: (_, _, _) => ()}) diff --git a/jscomp/syntax/tests/ppx/react/expected/v4.res.txt b/jscomp/syntax/tests/ppx/react/expected/v4.res.txt index b0f8ded29c..001501391f 100644 --- a/jscomp/syntax/tests/ppx/react/expected/v4.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/v4.res.txt @@ -1,3 +1,4 @@ +@res.jsxComponentProps type props<'x, 'y> = {x: 'x, y: 'y} // Component with type constraint let make = ({x, y, _}: props) => React.string(x ++ y) let make = { @@ -6,6 +7,7 @@ let make = { } module AnotherName = { + @res.jsxComponentProps type // Component with another name than "make" props<'x> = {x: 'x} @@ -18,6 +20,7 @@ module AnotherName = { } module Uncurried = { + @res.jsxComponentProps type props<'x> = {x: 'x} let make = ({x, _}: props<_>) => React.string(x) @@ -29,26 +32,28 @@ module Uncurried = { } module type TUncurried = { + @res.jsxComponentProps type props<'x> = {x: 'x} let make: React.componentLike, React.element> } module E = { - @live + @res.jsxComponentProps @live type props<'x> = {x: 'x} external make: React.componentLike, React.element> = "default" } module EUncurried = { - @live + @res.jsxComponentProps @live type props<'x> = {x: 'x} external make: React.componentLike, React.element> = "default" } module Rec = { + @res.jsxComponentProps type props = {} let rec make = { @@ -66,6 +71,7 @@ module Rec = { } module Rec1 = { + @res.jsxComponentProps type props = {} let rec make = { @@ -83,6 +89,7 @@ module Rec1 = { } module Rec2 = { + @res.jsxComponentProps type props = {} let rec make = { From f3198b78248d08675d198bd32fabff0b1015f09c Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Sun, 15 Sep 2024 18:06:03 +0200 Subject: [PATCH 11/11] try pointing to label instead of expr for error highlight --- .../component_prop_passed_multiple_times.res.expected | 4 ++-- jscomp/ml/typecore.ml | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/jscomp/build_tests/super_errors/expected/component_prop_passed_multiple_times.res.expected b/jscomp/build_tests/super_errors/expected/component_prop_passed_multiple_times.res.expected index 60dee57082..2ceb99067c 100644 --- a/jscomp/build_tests/super_errors/expected/component_prop_passed_multiple_times.res.expected +++ b/jscomp/build_tests/super_errors/expected/component_prop_passed_multiple_times.res.expected @@ -1,10 +1,10 @@ We've found a bug for you! - /.../fixtures/component_prop_passed_multiple_times.res:8:11-17 + /.../fixtures/component_prop_passed_multiple_times.res:8:5-8 6 │ let make = (): props<'name> => { 7 │ name: "hello", - 8 │ name: "world", + 8 │ name: "world", 9 │ } 10 │ } diff --git a/jscomp/ml/typecore.ml b/jscomp/ml/typecore.ml index 9c011e9a29..564a828db4 100644 --- a/jscomp/ml/typecore.ml +++ b/jscomp/ml/typecore.ml @@ -963,12 +963,12 @@ let type_label_a_list ?labels loc closed env type_lbl_a opath lid_a_list k = let check_recordpat_labels ~get_jsx_component_error_info loc lbl_pat_list closed = match lbl_pat_list with | [] -> () (* should not happen *) - | (_, label1, (tpat: Typedtree.pattern)) :: _ -> + | ((l: Longident.t loc), label1, _) :: _ -> let all = label1.lbl_all in let defined = Array.make (Array.length all) false in let check_defined (_, label, _) = if defined.(label.lbl_pos) - then raise(Error(tpat.pat_loc, Env.empty, Label_multiply_defined { + then raise(Error(l.loc, Env.empty, Label_multiply_defined { label = label.lbl_name; jsx_component_info = get_jsx_component_error_info (); })) @@ -1902,8 +1902,8 @@ let duplicate_ident_types caselist env = (* note: check_duplicates would better be implemented in type_label_a_list directly *) let rec check_duplicates ~get_jsx_component_error_info loc env = function - | (_, lbl1, _) :: (_, lbl2, (texp: Typedtree.expression)) :: _ when lbl1.lbl_pos = lbl2.lbl_pos -> - raise(Error(texp.exp_loc, env, Label_multiply_defined { + | (_, lbl1, _) :: ((l: Longident.t loc), lbl2, _) :: _ when lbl1.lbl_pos = lbl2.lbl_pos -> + raise(Error(l.loc, env, Label_multiply_defined { label = lbl1.lbl_name; jsx_component_info = get_jsx_component_error_info(); }))