Skip to content

Improve various error messages #7500

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.

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

Merged
merged 9 commits into from
May 22, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
add dedicated JSX element type mismatch errors
  • Loading branch information
zth committed May 22, 2025
commit 991dfb616f70193f77c5aa98f3fc70b64b847696
38 changes: 21 additions & 17 deletions compiler/bsc/rescript_compiler_main.ml
Original file line number Diff line number Diff line change
Expand Up @@ -12,30 +12,33 @@

let absname = ref false

(* TODO: Maybe there's a better place to do this init. *)
module Error_message_utils_support = struct
external to_comment : Res_comment.t -> Error_message_utils.Parser.comment
= "%identity"
external from_comment : Error_message_utils.Parser.comment -> Res_comment.t
= "%identity"
end

let () =
Error_message_utils.Parser.parse_source :=
fun source ->
let res =
Res_driver.parse_implementation_from_source ~for_printer:false
~display_filename:"<none>" ~source
in
( res.parsetree,
res.comments |> List.map Error_message_utils_support.to_comment )
let setup () =
(Error_message_utils.Parser.parse_source :=
fun source ->
let res =
Res_driver.parse_implementation_from_source ~for_printer:false
~display_filename:"<none>" ~source
in
(res.parsetree, res.comments |> List.map to_comment));

let () =
Error_message_utils.Parser.reprint_source :=
fun parsetree comments ->
Res_printer.print_implementation parsetree
~comments:(comments |> List.map Error_message_utils_support.from_comment)
~width:80
(Error_message_utils.Parser.reprint_source :=
fun parsetree comments ->
Res_printer.print_implementation parsetree
~comments:(comments |> List.map from_comment)
~width:80);

Error_message_utils.configured_jsx_module :=
Some
(match !Js_config.jsx_module with
| React -> "React"
| Generic {module_name} -> module_name)
end

let set_abs_input_name sourcefile =
let sourcefile =
Expand Down Expand Up @@ -66,6 +69,7 @@ let process_file sourcefile ?kind ppf =
properly
*)
setup_outcome_printer ();
Error_message_utils_support.setup ();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think process_file is the appropriate place to run this, but maybe there's a better place, idk.

let kind =
match kind with
| None ->
Expand Down
53 changes: 53 additions & 0 deletions compiler/ml/error_message_utils.ml
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
type extract_concrete_typedecl =
Env.t -> Types.type_expr -> Path.t * Path.t * Types.type_declaration

let configured_jsx_module : string option ref = ref None

let with_configured_jsx_module s =
match !configured_jsx_module with
| None -> s
| Some module_name -> module_name ^ "." ^ s

module Parser : sig
type comment

Expand Down Expand Up @@ -276,6 +283,52 @@ let print_extra_type_clash_help ~extract_concrete_typedecl ~env loc ppf
(match suggested_rewrite with
| Some rewrite -> rewrite
| None -> "")
| ( _,
Some
( {desc = Tconstr (p, type_params, _)},
{desc = Tconstr (Pdot (Pident {name = "Jsx"}, "element", _), _, _)} )
) -> (
(* Looking for a JSX element but got something else *)
let is_jsx_element ty =
match Ctype.expand_head env ty with
| {desc = Tconstr (Pdot (Pident {name = "Jsx"}, "element", _), _, _)} ->
true
| _ -> false
in

let print_jsx_msg ?(extra = "") name target_fn =
fprintf ppf
"@,\
@,\
In JSX, all content must be JSX elements. You can convert %s to a JSX \
element with @{<info>%s@}%s.@,"
name target_fn extra
in

match type_params with
| _ when Path.same p Predef.path_int ->
print_jsx_msg "int" (with_configured_jsx_module "int")
| _ when Path.same p Predef.path_string ->
print_jsx_msg "string" (with_configured_jsx_module "string")
| _ when Path.same p Predef.path_float ->
print_jsx_msg "float" (with_configured_jsx_module "float")
| [tp] when Path.same p Predef.path_array && is_jsx_element tp ->
print_jsx_msg
~extra:
(" (for example by using a pipe: ->"
^ with_configured_jsx_module "array"
^ ".")
"array"
(with_configured_jsx_module "array")
| [_] when Path.same p Predef.path_array ->
fprintf ppf
"@,\
@,\
You need to convert each item in this array to a JSX element first, \
then use @{<info>%s@} to convert the array of JSX elements into a \
single JSX element.@,"
(with_configured_jsx_module "array")
| _ -> ())
| _ -> ()

let type_clash_context_from_function sexp sfunct =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@

We've found a bug for you!
/.../fixtures/jsx_type_mismatch_array_element.res:19:13-30

17 │ }
18 │
19 │ let x = <> {[React.string("")]} </>
20 │

This has type: array<'a>
But it's expected to have type: React.element (defined as Jsx.element)

You need to convert each item in this array to a JSX element first, then use React.array to convert the array of JSX elements into a single JSX element.
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@

We've found a bug for you!
/.../fixtures/jsx_type_mismatch_array_raw.res:17:13-16

15 │ }
16 │
17 │ let x = <> {[""]} </>
18 │

This has type: array<'a>
But it's expected to have type: React.element (defined as Jsx.element)

You need to convert each item in this array to a JSX element first, then use React.array to convert the array of JSX elements into a single JSX element.
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@

We've found a bug for you!
/.../fixtures/jsx_type_mismatch_float.res:17:13-14

15 │ }
16 │
17 │ let x = <> {1.} </>
18 │

This has type: float
But it's 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.
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@

We've found a bug for you!
/.../fixtures/jsx_type_mismatch_int.res:17:13

15 │ }
16 │
17 │ let x = <> {1} </>
18 │

This has type: int
But it's 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.
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@

We've found a bug for you!
/.../fixtures/jsx_type_mismatch_string.res:17:13-14

15 │ }
16 │
17 │ let x = <> {""} </>
18 │

This has type: string
But it's 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.
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
@@config({
flags: ["-bs-jsx", "4"],
})

module React = {
type element = Jsx.element
type componentLike<'props, 'return> = 'props => 'return
type component<'props> = Jsx.component<'props>

@module("react/jsx-runtime")
external jsx: (component<'props>, 'props) => element = "jsx"

type fragmentProps = {children?: element}
@module("react/jsx-runtime") external jsxFragment: component<fragmentProps> = "Fragment"

external string: string => element = "%identity"
}

let x = <> {[React.string("")]} </>
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
@@config({
flags: ["-bs-jsx", "4"],
})

module React = {
type element = Jsx.element
type componentLike<'props, 'return> = 'props => 'return
type component<'props> = Jsx.component<'props>

@module("react/jsx-runtime")
external jsx: (component<'props>, 'props) => element = "jsx"

type fragmentProps = {children?: element}
@module("react/jsx-runtime") external jsxFragment: component<fragmentProps> = "Fragment"
}

let x = <> {[""]} </>
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
@@config({
flags: ["-bs-jsx", "4"],
})

module React = {
type element = Jsx.element
type componentLike<'props, 'return> = 'props => 'return
type component<'props> = Jsx.component<'props>

@module("react/jsx-runtime")
external jsx: (component<'props>, 'props) => element = "jsx"

type fragmentProps = {children?: element}
@module("react/jsx-runtime") external jsxFragment: component<fragmentProps> = "Fragment"
}

let x = <> {1.} </>
17 changes: 17 additions & 0 deletions tests/build_tests/super_errors/fixtures/jsx_type_mismatch_int.res
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
@@config({
flags: ["-bs-jsx", "4"],
})

module React = {
type element = Jsx.element
type componentLike<'props, 'return> = 'props => 'return
type component<'props> = Jsx.component<'props>

@module("react/jsx-runtime")
external jsx: (component<'props>, 'props) => element = "jsx"

type fragmentProps = {children?: element}
@module("react/jsx-runtime") external jsxFragment: component<fragmentProps> = "Fragment"
}

let x = <> {1} </>
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
@@config({
flags: ["-bs-jsx", "4"],
})

module React = {
type element = Jsx.element
type componentLike<'props, 'return> = 'props => 'return
type component<'props> = Jsx.component<'props>

@module("react/jsx-runtime")
external jsx: (component<'props>, 'props) => element = "jsx"

type fragmentProps = {children?: element}
@module("react/jsx-runtime") external jsxFragment: component<fragmentProps> = "Fragment"
}

let x = <> {""} </>