PR comments

Author: zth

compiler/ml/error_message_utils.ml

@@ -99,7 +99,7 @@ type type_clash_context =
       operator: string;
       is_constant: string option;
     }
-  | FunctionArgument of {optional: bool}
+  | FunctionArgument of {optional: bool; name: string option}
   | Statement of type_clash_statement
   | ForLoopCondition
 
@@ -138,11 +138,17 @@ let error_type_text ppf type_clash_context =
 
 let error_expected_type_text ppf type_clash_context =
   match type_clash_context with
-  | Some (FunctionArgument {optional}) ->
-    fprintf ppf "But this%s function argument is expecting:"
+  | Some (FunctionArgument {optional; name}) ->
+    fprintf ppf "But this%s function argument"
       (match optional with
       | false -> ""
-      | true -> " optional")
+      | true -> " optional");
+
+    (match name with
+    | Some name -> fprintf ppf " @{<info>~%s@}" name
+    | None -> ());
+
+    fprintf ppf " is expecting:"
   | Some ComparisonOperator ->
     fprintf ppf "But it's being compared to something of type:"
   | Some SwitchReturn -> fprintf ppf "But this switch is expected to return:"
@@ -158,21 +164,21 @@ let error_expected_type_text ppf type_clash_context =
     fprintf ppf "But this @{<info>if@} statement is expected to return:"
   | Some ArrayValue ->
     fprintf ppf "But this array is expected to have items of type:"
-  | Some (SetRecordField _) -> fprintf ppf "But this record field is of type:"
+  | Some (SetRecordField _) -> fprintf ppf "But the record field is of type:"
   | Some
       (RecordField {field_name = "children"; jsx = Some {jsx_type = `Fragment}})
     ->
-    fprintf ppf "But children of JSX fragments is expected to have type:"
+    fprintf ppf "But children of JSX fragments are expected to have type:"
   | Some
       (RecordField
          {field_name = "children"; jsx = Some {jsx_type = `CustomComponent}}) ->
     fprintf ppf
-      "But children passed to this component is expected to have type:"
+      "But children passed to this component are expected to have type:"
   | Some (RecordField {field_name; jsx = Some _}) ->
-    fprintf ppf "But this component prop @{<info>%s@} is expected to have type:"
+    fprintf ppf "But the component prop @{<info>%s@} is expected to have type:"
       field_name
   | Some (RecordField {field_name}) ->
-    fprintf ppf "But this record field @{<info>%s@} is expected to have type:"
+    fprintf ppf "But the record field @{<info>%s@} is expected to have type:"
       field_name
   | Some (Statement FunctionCall) -> fprintf ppf "But it's expected to return:"
   | Some (MathOperator {operator}) ->
@@ -429,9 +435,8 @@ let print_extra_type_clash_help ~extract_concrete_typedecl ~env loc ppf
        @,\
        @{<info>%s@} is an optional record field, and you're passing an \
        optional value to it.@,\
-       Optional fields expect you to pass the concrete value, not an option, \
-       when passed directly.\n\
-      \             @,\
+       Optional fields expect you to pass the concrete value, not an option.\n\
+      \       @,\
        Possible solutions: @,\
        - Unwrap the option and pass a concrete value directly@,\
        - If you really do want to pass the optional value, prepend the value \
@@ -447,8 +452,8 @@ let print_extra_type_clash_help ~extract_concrete_typedecl ~env loc ppf
        @,\
        You're passing an optional value into an optional function argument.@,\
        Optional function arguments expect you to pass the concrete value, not \
-       an option, when passed directly.\n\
-      \             @,\
+       an option.\n\
+      \       @,\
        Possible solutions: @,\
        - Unwrap the option and pass a concrete value directly@,\
        - If you really do want to pass the optional value, prepend the value \
@@ -510,9 +515,9 @@ let type_clash_context_from_function sexp sfunct =
     Some (MathOperator {for_float = true; operator; is_constant})
   | Pexp_ident {txt = Lident (("/" | "*" | "+" | "-") as operator)} ->
     Some (MathOperator {for_float = false; operator; is_constant})
-  | _ -> Some (FunctionArgument {optional = false})
+  | _ -> None
 
-let type_clash_context_for_function_argument type_clash_context sarg0 =
+let type_clash_context_for_function_argument ~label type_clash_context sarg0 =
   match type_clash_context with
   | Some (MathOperator {for_float; operator}) ->
     Some
@@ -527,7 +532,16 @@ let type_clash_context_for_function_argument type_clash_context sarg0 =
                Some txt
              | _ -> None);
          })
-  | None -> Some (FunctionArgument {optional = false})
+  | None ->
+    Some
+      (FunctionArgument
+         {
+           optional = false;
+           name =
+             (match label with
+             | Asttypes.Nolabel -> None
+             | Optional {txt = l} | Labelled {txt = l} -> Some l);
+         })
   | type_clash_context -> type_clash_context
 
 let type_clash_context_maybe_option ty_expected ty_res =

compiler/ml/typecore.ml

@@ -3647,12 +3647,22 @@ and type_application ~context total_app env funct (sargs : sargs) :
               (if (not optional) || is_optional_loc l' then fun () ->
                  type_argument
                    ~context:
-                     (type_clash_context_for_function_argument context sarg0)
+                     (type_clash_context_for_function_argument ~label:l' context
+                        sarg0)
                    env sarg0 ty ty0
                else fun () ->
                  option_some
                    (type_argument
-                      ~context:(Some (FunctionArgument {optional = true}))
+                      ~context:
+                        (Some
+                           (FunctionArgument
+                              {
+                                optional = true;
+                                name =
+                                  (match l' with
+                                  | Nolabel -> None
+                                  | Optional l | Labelled l -> Some l.txt);
+                              }))
                       env sarg0
                       (extract_option_type env ty)
                       (extract_option_type env ty0))) )

tests/build_tests/super_errors/expected/inline_types_record_type_params.res.expected

@@ -9,6 +9,6 @@
   15 ┆ otherExtra: Some({test: true, anotherInlined: {record: true}}),
 
   This has type: int
-  But this record field age is expected to have type: string
+  But the record field age is expected to have type: string
 
   You can convert int to string with Int.toString.

tests/build_tests/super_errors/expected/jsx_custom_component_children.res.expected

@@ -8,7 +8,7 @@
   25 │ 
 
   This has type: float
-  But children passed to this component is expected to have type:
+  But children passed to this component are expected to have type:
     React.element (defined as Jsx.element)
 
   In JSX, all content must be JSX elements. You can convert float to a JSX element with React.float.

tests/build_tests/super_errors/expected/jsx_custom_component_optional.res.expected

@@ -8,6 +8,6 @@
   32 │ 
 
   This has type: string
-  But this component prop someOpt is expected to have type: float
+  But the component prop someOpt is expected to have type: float
 
   You can convert string to float with Float.fromString.

tests/build_tests/super_errors/expected/jsx_type_mismatch_float.res.expected

@@ -8,7 +8,7 @@
   18 │ 
 
   This has type: float
-  But children of JSX fragments is expected to have type:
+  But children of JSX fragments are expected to have type:
     React.element (defined as Jsx.element)
 
   In JSX, all content must be JSX elements. You can convert float to a JSX element with React.float.

tests/build_tests/super_errors/expected/jsx_type_mismatch_int.res.expected

@@ -8,7 +8,7 @@
   18 │ 
 
   This has type: int
-  But children of JSX fragments is expected to have type:
+  But children of JSX fragments are expected to have type:
     React.element (defined as Jsx.element)
 
   In JSX, all content must be JSX elements. You can convert int to a JSX element with React.int.

tests/build_tests/super_errors/expected/jsx_type_mismatch_string.res.expected

@@ -8,7 +8,7 @@
   18 │ 
 
   This has type: string
-  But children of JSX fragments is expected to have type:
+  But children of JSX fragments are expected to have type:
     React.element (defined as Jsx.element)
 
   In JSX, all content must be JSX elements. You can convert string to a JSX element with React.string.

tests/build_tests/super_errors/expected/optional_fn_argument_pass_option.res.expected

@@ -8,11 +8,11 @@
   6 │ 
 
   This has type: option<int>
-  But this optional function argument is expecting: int
+  But this optional function argument ~x is expecting: int
 
   You're passing an optional value into an optional function argument.
-  Optional function arguments expect you to pass the concrete value, not an option, when passed directly.
-             
+  Optional function arguments expect you to pass the concrete value, not an option.
+       
   Possible solutions: 
   - Unwrap the option and pass a concrete value directly
   - If you really do want to pass the optional value, prepend the value with ? to show you want to pass the option, like: ?t

tests/build_tests/super_errors/expected/optional_record_field_pass_option.res.expected

@@ -8,11 +8,11 @@
   5 │ 
 
   This has type: option<bool>
-  But this record field test is expected to have type: bool
+  But the record field test is expected to have type: bool
 
   test is an optional record field, and you're passing an optional value to it.
-  Optional fields expect you to pass the concrete value, not an option, when passed directly.
-             
+  Optional fields expect you to pass the concrete value, not an option.
+       
   Possible solutions: 
   - Unwrap the option and pass a concrete value directly
   - If you really do want to pass the optional value, prepend the value with ? to show you want to pass the option, like: {test: ?t}