JSX PPX: add Jsx.element return constraint

Author: cknitt

compiler/syntax/src/jsx_v4.ml

@@ -46,6 +46,38 @@ let ref_type loc =
     {loc; txt = Ldot (Ldot (Lident "Js", "Nullable"), "t")}
     [ref_type_var loc]
 
+let jsx_element_type ~loc =
+  Typ.constr ~loc {loc; txt = Ldot (Lident "Jsx", "element")} []
+
+let jsx_element_constraint ~loc expr =
+  Exp.constraint_ ~loc expr (jsx_element_type ~loc)
+
+let rec constrain_jsx_return ~loc expr =
+  match expr.pexp_desc with
+  | Pexp_fun ({rhs} as desc) ->
+    {
+      expr with
+      pexp_desc = Pexp_fun {desc with rhs = constrain_jsx_return ~loc rhs};
+    }
+  | Pexp_newtype (param, inner) ->
+    {
+      expr with
+      pexp_desc = Pexp_newtype (param, constrain_jsx_return ~loc inner);
+    }
+  | Pexp_constraint (inner, _) ->
+    jsx_element_constraint ~loc (constrain_jsx_return ~loc inner)
+  | Pexp_let (rec_flag, bindings, body) ->
+    {
+      expr with
+      pexp_desc = Pexp_let (rec_flag, bindings, constrain_jsx_return ~loc body);
+    }
+  | Pexp_sequence (first, second) ->
+    {
+      expr with
+      pexp_desc = Pexp_sequence (first, constrain_jsx_return ~loc second);
+    }
+  | _ -> jsx_element_constraint ~loc expr
+
 let merlin_focus = ({loc = Location.none; txt = "merlin.focus"}, PStr [])
 
 (* Helper method to filter out any attribute that isn't [@react.component] *)
@@ -713,6 +745,10 @@ let map_binding ~config ~empty_loc ~pstr_loc ~file_name ~rec_flag binding =
         vb_match_expr named_arg_list expression
       else expression
     in
+    let expression =
+      Exp.constraint_ ~loc:binding_loc expression
+        (jsx_element_type ~loc:binding_loc)
+    in
     (* (ref) => expr *)
     let expression =
       List.fold_left
@@ -784,6 +820,7 @@ let map_binding ~config ~empty_loc ~pstr_loc ~file_name ~rec_flag binding =
     (Some props_record_type, binding, new_binding))
   else if Jsx_common.has_attr_on_binding Jsx_common.has_attr_with_props binding
   then
+    let binding_loc = binding.pvb_loc in
     let modified_binding =
       {
         binding with
@@ -839,21 +876,28 @@ let map_binding ~config ~empty_loc ~pstr_loc ~file_name ~rec_flag binding =
         | _ -> Pat.var {txt = "props"; loc}
       in
 
+      let applied_expression =
+        Exp.apply
+          (Exp.ident
+             {
+               txt =
+                 Lident
+                   (match rec_flag with
+                   | Recursive -> internal_fn_name
+                   | Nonrecursive -> fn_name);
+               loc;
+             })
+          [(Nolabel, Exp.ident {txt = Lident "props"; loc})]
+      in
+      let applied_expression =
+        Jsx_common.async_component ~async:is_async applied_expression
+      in
+      let applied_expression =
+        Exp.constraint_ ~loc applied_expression (jsx_element_type ~loc)
+      in
       let wrapper_expr =
         Exp.fun_ ~arity:None Nolabel None props_pattern
-          ~attrs:binding.pvb_expr.pexp_attributes
-          (Jsx_common.async_component ~async:is_async
-             (Exp.apply
-                (Exp.ident
-                   {
-                     txt =
-                       Lident
-                         (match rec_flag with
-                         | Recursive -> internal_fn_name
-                         | Nonrecursive -> fn_name);
-                     loc;
-                   })
-                [(Nolabel, Exp.ident {txt = Lident "props"; loc})]))
+          ~attrs:binding.pvb_expr.pexp_attributes applied_expression
       in
 
       let wrapper_expr = Ast_uncurried.uncurried_fun ~arity:1 wrapper_expr in
@@ -874,18 +918,20 @@ let map_binding ~config ~empty_loc ~pstr_loc ~file_name ~rec_flag binding =
       | Recursive -> None
       | Nonrecursive ->
         Some
-          (make_new_binding ~loc:empty_loc ~full_module_name modified_binding)
+          (make_new_binding ~loc:binding_loc ~full_module_name modified_binding)
     in
     ( None,
       {
         binding with
         pvb_attributes = binding.pvb_attributes |> List.filter other_attrs_pure;
         pvb_expr =
-          {
-            binding.pvb_expr with
-            (* moved to wrapper_expr *)
-            pexp_attributes = [];
-          };
+          ( binding.pvb_expr |> fun expr ->
+            {
+              expr with
+              (* moved to wrapper_expr *)
+              pexp_attributes = [];
+            }
+            |> constrain_jsx_return ~loc:binding_loc );
       },
       new_binding )
   else (None, binding, None)
@@ -934,7 +980,7 @@ let transform_structure_item ~config item =
             (arg.lbl, arg.attrs, return_value.ptyp_loc, arg.typ) :: types )
         | _ -> (full_type, types)
       in
-      let inner_type, prop_types = get_prop_types [] pval_type in
+      let _, prop_types = get_prop_types [] pval_type in
       let named_type_list = List.fold_left arg_to_concrete_type [] prop_types in
       let ret_props_type =
         Typ.constr ~loc:pstr_loc
@@ -955,7 +1001,7 @@ let transform_structure_item ~config item =
       let new_external_type =
         Ptyp_constr
           ( {loc = pstr_loc; txt = module_access_name config "componentLike"},
-            [ret_props_type; inner_type] )
+            [ret_props_type; jsx_element_type ~loc:pstr_loc] )
       in
       let new_structure =
         {
@@ -1046,7 +1092,7 @@ let transform_signature_item ~config item =
             (arg.lbl, arg.attrs, return_value.ptyp_loc, arg.typ) :: types )
         | _ -> (full_type, types)
       in
-      let inner_type, prop_types = get_prop_types [] pval_type in
+      let _, prop_types = get_prop_types [] pval_type in
       let named_type_list = List.fold_left arg_to_concrete_type [] prop_types in
       let ret_props_type =
         Typ.constr
@@ -1067,7 +1113,7 @@ let transform_signature_item ~config item =
       let new_external_type =
         Ptyp_constr
           ( {loc = psig_loc; txt = module_access_name config "componentLike"},
-            [ret_props_type; inner_type] )
+            [ret_props_type; jsx_element_type ~loc:psig_loc] )
       in
       let new_structure =
         {

tests/build_tests/super_errors/expected/react_async_component_missing_jsx_element.res.expected

@@ -0,0 +1,13 @@
+
+  We've found a bug for you!
+  /.../fixtures/react_async_component_missing_jsx_element.res:4:24
+
+  2 │ 
+  3 │ @react.component
+  4 │ let make = async () => 1
+  5 │ 
+
+  This has type: int
+  But it's expected to have type: 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/react_componentWithProps_missing_jsx_element.res.expected

@@ -0,0 +1,13 @@
+
+  We've found a bug for you!
+  /.../fixtures/react_componentWithProps_missing_jsx_element.res:6:35-45
+
+  4 │ 
+  5 │ @react.componentWithProps
+  6 │ let make = (props: props<int>) => props.value
+  7 │ 
+
+  This has type: int
+  But it's expected to have type: 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/react_component_missing_jsx_element.res.expected

@@ -0,0 +1,13 @@
+
+  We've found a bug for you!
+  /.../fixtures/react_component_missing_jsx_element.res:4:18
+
+  2 │ 
+  3 │ @react.component
+  4 │ let make = () => 1
+  5 │ 
+
+  This has type: int
+  But it's expected to have type: 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/react_forwardRef_missing_jsx_element.res.expected

@@ -0,0 +1,13 @@
+
+  We've found a bug for you!
+  /.../fixtures/react_forwardRef_missing_jsx_element.res:4:42
+
+  2 │ 
+  3 │ @react.component
+  4 │ let make = React.forwardRef((_, _ref) => 1)
+  5 │ 
+
+  This has type: int
+  But it's expected to have type: 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/fixtures/react_async_component_missing_jsx_element.res

@@ -0,0 +1,4 @@
+@@jsxConfig({version: 4})
+
+@react.component
+let make = async () => 1

tests/build_tests/super_errors/fixtures/react_componentWithProps_missing_jsx_element.res

@@ -0,0 +1,6 @@
+@@jsxConfig({version: 4})
+
+type props<'a> = {value: 'a}
+
+@react.componentWithProps
+let make = (props: props<int>) => props.value

tests/build_tests/super_errors/fixtures/react_component_missing_jsx_element.res

@@ -0,0 +1,4 @@
+@@jsxConfig({version: 4})
+
+@react.component
+let make = () => 1

tests/build_tests/super_errors/fixtures/react_forwardRef_missing_jsx_element.res

@@ -0,0 +1,4 @@
+@@jsxConfig({version: 4})
+
+@react.component
+let make = React.forwardRef((_, _ref) => 1)

tests/syntax_tests/data/ppx/react/expected/aliasProps.res.txt

@@ -4,7 +4,7 @@ module C0 = {
   @res.jsxComponentProps
   type props<'priority, 'text> = {priority: 'priority, text?: 'text}
 
-  let make = ({priority: _, text: ?__text, _}: props<_, _>) => {
+  let make = ({priority: _, text: ?__text, _}: props<_, _>): Jsx.element => {
     let text = switch __text {
     | Some(text) => text
     | None => "Test"
@@ -23,7 +23,7 @@ module C1 = {
   @res.jsxComponentProps
   type props<'priority, 'text> = {priority: 'priority, text?: 'text}
 
-  let make = ({priority: p, text: ?__text, _}: props<_, _>) => {
+  let make = ({priority: p, text: ?__text, _}: props<_, _>): Jsx.element => {
     let text = switch __text {
     | Some(text) => text
     | None => "Test"
@@ -42,7 +42,7 @@ module C2 = {
   @res.jsxComponentProps
   type props<'foo> = {foo?: 'foo}
 
-  let make = ({foo: ?__bar, _}: props<_>) => {
+  let make = ({foo: ?__bar, _}: props<_>): Jsx.element => {
     let bar = switch __bar {
     | Some(foo) => foo
     | None => ""
@@ -61,7 +61,7 @@ module C3 = {
   @res.jsxComponentProps
   type props<'foo, 'a, 'b> = {foo?: 'foo, a?: 'a, b: 'b}
 
-  let make = ({foo: ?__bar, a: ?__a, b, _}: props<_, _, _>) => {
+  let make = ({foo: ?__bar, a: ?__a, b, _}: props<_, _, _>): Jsx.element => {
     let bar = switch __bar {
     | Some(foo) => foo
     | None => ""
@@ -86,7 +86,7 @@ module C4 = {
   @res.jsxComponentProps
   type props<'a, 'x> = {a: 'a, x?: 'x}
 
-  let make = ({a: b, x: ?__x, _}: props<_, _>) => {
+  let make = ({a: b, x: ?__x, _}: props<_, _>): Jsx.element => {
     let x = switch __x {
     | Some(x) => x
     | None => true
@@ -105,7 +105,7 @@ module C5 = {
   @res.jsxComponentProps
   type props<'a, 'z> = {a: 'a, z?: 'z}
 
-  let make = ({a: (x, y), z: ?__z, _}: props<_, _>) => {
+  let make = ({a: (x, y), z: ?__z, _}: props<_, _>): Jsx.element => {
     let z = switch __z {
     | Some(z) => z
     | None => 3
@@ -125,12 +125,13 @@ module C6 = {
     @res.jsxComponentProps
     type props = {}
 
-    let make: React.componentLike<props, React.element>
+    let make: React.componentLike<props, Jsx.element>
   }
   @res.jsxComponentProps
   type props<'comp, 'x> = {comp: 'comp, x: 'x}
 
-  let make = ({comp: module(Comp: Comp), x: (a, b), _}: props<_, _>) => React.jsx(Comp.make, {})
+  let make = ({comp: module(Comp: Comp), x: (a, b), _}: props<_, _>): Jsx.element =>
+    React.jsx(Comp.make, {})
   let make = {
     let \"AliasProps$C6" = (props: props<_>) => make(props)