Constrain return expression; do not use outer loc

Author: cknitt

compiler/syntax/src/jsx_v4.ml

@@ -49,34 +49,30 @@ let ref_type 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 =
+let jsx_element_constraint expr =
+  Exp.constraint_ expr (jsx_element_type ~loc:expr.pexp_loc)
+
+(* Traverse the component body and force every reachable return expression to
+   be annotated as `Jsx.element`. This walks through the wrapper constructs the
+   PPX introduces (fun/newtype/let/sequence) so that the constraint ends up on
+   the real return position even after we rewrite the function. *)
+let rec constrain_jsx_return 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};
-    }
+    {expr with pexp_desc = Pexp_fun {desc with rhs = constrain_jsx_return rhs}}
   | Pexp_newtype (param, inner) ->
-    {
-      expr with
-      pexp_desc = Pexp_newtype (param, constrain_jsx_return ~loc inner);
-    }
+    {expr with pexp_desc = Pexp_newtype (param, constrain_jsx_return inner)}
   | Pexp_constraint (inner, _) ->
-    jsx_element_constraint ~loc (constrain_jsx_return ~loc inner)
+    let constrained_inner = constrain_jsx_return inner in
+    jsx_element_constraint constrained_inner
   | Pexp_let (rec_flag, bindings, body) ->
     {
       expr with
-      pexp_desc = Pexp_let (rec_flag, bindings, constrain_jsx_return ~loc body);
+      pexp_desc = Pexp_let (rec_flag, bindings, constrain_jsx_return body);
     }
   | Pexp_sequence (first, second) ->
-    {
-      expr with
-      pexp_desc = Pexp_sequence (first, constrain_jsx_return ~loc second);
-    }
-  | _ -> jsx_element_constraint ~loc expr
+    {expr with pexp_desc = Pexp_sequence (first, constrain_jsx_return second)}
+  | _ -> jsx_element_constraint expr
 
 (* Helper method to filter out any attribute that isn't [@react.component] *)
 let other_attrs_pure (loc, _) =
@@ -743,10 +739,7 @@ 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
+    let expression = constrain_jsx_return expression in
     (* (ref) => expr *)
     let expression =
       List.fold_left
@@ -890,9 +883,7 @@ let map_binding ~config ~empty_loc ~pstr_loc ~file_name ~rec_flag binding =
       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 applied_expression = constrain_jsx_return applied_expression in
       let wrapper_expr =
         Exp.fun_ ~arity:None Nolabel None props_pattern
           ~attrs:binding.pvb_expr.pexp_attributes applied_expression
@@ -929,7 +920,7 @@ let map_binding ~config ~empty_loc ~pstr_loc ~file_name ~rec_flag binding =
       {
         binding with
         pvb_attributes = binding.pvb_attributes |> List.filter other_attrs_pure;
-        pvb_expr = binding_expr |> constrain_jsx_return ~loc:binding_loc;
+        pvb_expr = binding_expr |> constrain_jsx_return;
       },
       new_binding )
   else (None, binding, None)

tests/analysis_tests/tests/src/expected/CodeLens.res.txt

@@ -1,8 +1,5 @@
 Code Lens src/CodeLens.res
 [{
-        "range": {"start": {"line": 9, "character": 4}, "end": {"line": 9, "character": 8}},
-        "command": {"title": "Jsx.element", "command": ""}
-    }, {
         "range": {"start": {"line": 4, "character": 4}, "end": {"line": 4, "character": 6}},
         "command": {"title": "(~opt1: int=?, ~a: int, ~b: int, unit, ~opt2: int=?, unit, ~c: int) => int", "command": ""}
     }, {

tests/analysis_tests/tests/src/expected/Hover.res.txt

@@ -22,10 +22,10 @@ Hover src/Hover.res 33:4
 {"contents": {"kind": "markdown", "value": "```rescript\nunit => int\n```\n---\nDoc comment for functionWithTypeAnnotation"}}
 
 Hover src/Hover.res 37:13
-{"contents": {"kind": "markdown", "value": "```rescript\nJsx.element\n```\n\n---\n\n```\n \n```\n```rescript\ntype Jsx.element\n```\nGo to: [Type definition](command:rescript-vscode.go_to_location?%5B%22Jsx.res%22%2C25%2C0%5D)\n"}}
+{"contents": {"kind": "markdown", "value": "```rescript\nstring\n```"}}
 
 Hover src/Hover.res 42:15
-{"contents": {"kind": "markdown", "value": "```rescript\nJsx.element\n```\n\n---\n\n```\n \n```\n```rescript\ntype Jsx.element\n```\nGo to: [Type definition](command:rescript-vscode.go_to_location?%5B%22Jsx.res%22%2C25%2C0%5D)\n"}}
+{"contents": {"kind": "markdown", "value": "```rescript\nstring\n```"}}
 
 Hover src/Hover.res 46:10
 {"contents": {"kind": "markdown", "value": "```rescript\nint\n```"}}

tests/analysis_tests/tests/src/expected/InlayHint.res.txt

@@ -35,12 +35,6 @@ Inlay Hint src/InlayHint.res 1:34
     "kind": 1,
     "paddingLeft": true,
     "paddingRight": false
-}, {
-    "position": {"line": 14, "character": 8},
-    "label": ": Jsx.element",
-    "kind": 1,
-    "paddingLeft": true,
-    "paddingRight": false
 }, {
     "position": {"line": 10, "character": 10},
     "label": ": (~xx: int) => int",

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

@@ -4,13 +4,12 @@ module C0 = {
   @res.jsxComponentProps
   type props<'priority, 'text> = {priority: 'priority, text?: 'text}
 
-  let make = ({priority: _, text: ?__text, _}: props<_, _>): Jsx.element => {
+  let make = ({priority: _, text: ?__text, _}: props<_, _>) => {
     let text = switch __text {
     | Some(text) => text
     | None => "Test"
     }
-
-    React.string(text)
+    (React.string(text): Jsx.element)
   }
   let make = {
     let \"AliasProps$C0" = (props: props<_>) => make(props)
@@ -23,13 +22,12 @@ module C1 = {
   @res.jsxComponentProps
   type props<'priority, 'text> = {priority: 'priority, text?: 'text}
 
-  let make = ({priority: p, text: ?__text, _}: props<_, _>): Jsx.element => {
+  let make = ({priority: p, text: ?__text, _}: props<_, _>) => {
     let text = switch __text {
     | Some(text) => text
     | None => "Test"
     }
-
-    React.string(p ++ text)
+    (React.string(p ++ text): Jsx.element)
   }
   let make = {
     let \"AliasProps$C1" = (props: props<_>) => make(props)
@@ -42,13 +40,12 @@ module C2 = {
   @res.jsxComponentProps
   type props<'foo> = {foo?: 'foo}
 
-  let make = ({foo: ?__bar, _}: props<_>): Jsx.element => {
+  let make = ({foo: ?__bar, _}: props<_>) => {
     let bar = switch __bar {
     | Some(foo) => foo
     | None => ""
     }
-
-    React.string(bar)
+    (React.string(bar): Jsx.element)
   }
   let make = {
     let \"AliasProps$C2" = (props: props<_>) => make(props)
@@ -61,7 +58,7 @@ module C3 = {
   @res.jsxComponentProps
   type props<'foo, 'a, 'b> = {foo?: 'foo, a?: 'a, b: 'b}
 
-  let make = ({foo: ?__bar, a: ?__a, b, _}: props<_, _, _>): Jsx.element => {
+  let make = ({foo: ?__bar, a: ?__a, b, _}: props<_, _, _>) => {
     let bar = switch __bar {
     | Some(foo) => foo
     | None => ""
@@ -70,10 +67,11 @@ module C3 = {
     | Some(a) => a
     | None => bar
     }
-
-    {
-      React.string(bar ++ a ++ b)
-    }
+    (
+      {
+        React.string(bar ++ a ++ b)
+      }: Jsx.element
+    )
   }
   let make = {
     let \"AliasProps$C3" = (props: props<_>) => make(props)
@@ -86,13 +84,12 @@ module C4 = {
   @res.jsxComponentProps
   type props<'a, 'x> = {a: 'a, x?: 'x}
 
-  let make = ({a: b, x: ?__x, _}: props<_, _>): Jsx.element => {
+  let make = ({a: b, x: ?__x, _}: props<_, _>) => {
     let x = switch __x {
     | Some(x) => x
     | None => true
     }
-
-    ReactDOM.jsx("div", {children: ?ReactDOM.someElement(b)})
+    (ReactDOM.jsx("div", {children: ?ReactDOM.someElement(b)}): Jsx.element)
   }
   let make = {
     let \"AliasProps$C4" = (props: props<_>) => make(props)
@@ -105,13 +102,12 @@ module C5 = {
   @res.jsxComponentProps
   type props<'a, 'z> = {a: 'a, z?: 'z}
 
-  let make = ({a: (x, y), z: ?__z, _}: props<_, _>): Jsx.element => {
+  let make = ({a: (x, y), z: ?__z, _}: props<_, _>) => {
     let z = switch __z {
     | Some(z) => z
     | None => 3
     }
-
-    x + y + z
+    (x + y + z: Jsx.element)
   }
   let make = {
     let \"AliasProps$C5" = (props: props<_>) => make(props)

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

@@ -4,9 +4,9 @@ module C0 = {
   @res.jsxComponentProps
   type props<'a> = {a: 'a}
 
-  let make = async ({a, _}: props<_>): Jsx.element => {
+  let make = async ({a, _}: props<_>) => {
     let a = await f(a)
-    ReactDOM.jsx("div", {children: ?ReactDOM.someElement({React.int(a)})})
+    (ReactDOM.jsx("div", {children: ?ReactDOM.someElement({React.int(a)})}): Jsx.element)
   }
   let make = {
     let \"AsyncAwait$C0" = (props: props<_>) => Jsx.promise(make(props))

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

@@ -1,7 +1,7 @@
 module C0 = {
   @res.jsxComponentProps
   type props<'a, 'b> = {a?: 'a, b?: 'b}
-  let make = ({a: ?__a, b: ?__b, _}: props<_, _>): Jsx.element => {
+  let make = ({a: ?__a, b: ?__b, _}: props<_, _>) => {
     let a = switch __a {
     | Some(a) => a
     | None => 2
@@ -10,8 +10,7 @@ module C0 = {
     | Some(b) => b
     | None => a * 2
     }
-
-    React.int(a + b)
+    (React.int(a + b): Jsx.element)
   }
   let make = {
     let \"DefaultValueProp$C0" = (props: props<_>) => make(props)
@@ -23,13 +22,12 @@ module C1 = {
   @res.jsxComponentProps
   type props<'a, 'b> = {a?: 'a, b: 'b}
 
-  let make = ({a: ?__a, b, _}: props<_, _>): Jsx.element => {
+  let make = ({a: ?__a, b, _}: props<_, _>) => {
     let a = switch __a {
     | Some(a) => a
     | None => 2
     }
-
-    React.int(a + b)
+    (React.int(a + b): Jsx.element)
   }
   let make = {
     let \"DefaultValueProp$C1" = (props: props<_>) => make(props)
@@ -43,13 +41,12 @@ module C2 = {
   @res.jsxComponentProps
   type props<'a> = {a?: 'a}
 
-  let make = ({a: ?__a, _}: props<_>): Jsx.element => {
+  let make = ({a: ?__a, _}: props<_>) => {
     let a = switch __a {
     | Some(a) => a
     | None => a
     }
-
-    React.string(a)
+    (React.string(a): Jsx.element)
   }
   let make = {
     let \"DefaultValueProp$C2" = (props: props<_>) => make(props)
@@ -62,15 +59,16 @@ module C3 = {
   @res.jsxComponentProps
   type props<'disabled> = {disabled?: 'disabled}
 
-  let make = ({disabled: ?__everythingDisabled, _}: props<bool>): Jsx.element => {
+  let make = ({disabled: ?__everythingDisabled, _}: props<bool>) => {
     let everythingDisabled = switch __everythingDisabled {
     | Some(disabled) => disabled
     | None => false
     }
-
-    {
-      React.string(everythingDisabled ? "true" : "false")
-    }
+    (
+      {
+        React.string(everythingDisabled ? "true" : "false")
+      }: Jsx.element
+    )
   }
   let make = {
     let \"DefaultValueProp$C3" = (props: props<_>) => make(props)

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

@@ -38,16 +38,17 @@ module V4A = {
   @res.jsxComponentProps
   type props = {}
 
-  let make = (_: props): Jsx.element => {
+  let make = (_: props) => {
     let input = React.useRef(Js.Nullable.null)
-
-    ReactDOM.jsx(
-      "div",
-      {
-        children: ?ReactDOM.someElement(
-          React.jsx(FancyInput.make, {ref: input, children: {React.string("Click to focus")}}),
-        ),
-      },
+    (
+      ReactDOM.jsx(
+        "div",
+        {
+          children: ?ReactDOM.someElement(
+            React.jsx(FancyInput.make, {ref: input, children: {React.string("Click to focus")}}),
+          ),
+        },
+      ): Jsx.element
     )
   }
   let make = {
@@ -95,16 +96,17 @@ module V4AUncurried = {
   @res.jsxComponentProps
   type props = {}
 
-  let make = (_: props): Jsx.element => {
+  let make = (_: props) => {
     let input = React.useRef(Js.Nullable.null)
-
-    ReactDOM.jsx(
-      "div",
-      {
-        children: ?ReactDOM.someElement(
-          React.jsx(FancyInput.make, {ref: input, children: {React.string("Click to focus")}}),
-        ),
-      },
+    (
+      ReactDOM.jsx(
+        "div",
+        {
+          children: ?ReactDOM.someElement(
+            React.jsx(FancyInput.make, {ref: input, children: {React.string("Click to focus")}}),
+          ),
+        },
+      ): Jsx.element
     )
   }
   let make = {

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

@@ -2,9 +2,9 @@
 let make = (
   {x, _}: props<string, ReactDOM.Ref.currentDomRef>,
   ref: Js.Nullable.t<ReactDOM.Ref.currentDomRef>,
-): Jsx.element => {
+) => {
   let _ = ref->Js.Nullable.toOption->Belt.Option.map(ReactDOM.Ref.domRef)
-  React.string(x)
+  (React.string(x): Jsx.element)
 }
 let make = React.forwardRef({
   let \"InterfaceWithRef" = (props: props<_>, ref) => make(props, ref)

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

@@ -4,9 +4,9 @@ module V4A = {
   @res.jsxComponentProps
   type props<'a, 'b> = {a: 'a, b: 'b}
 
-  let make = ({a, b, _}: props<_, _>): Jsx.element => {
+  let make = ({a, b, _}: props<_, _>) => {
     Js.log("This function should be named 'TopLevel.react'")
-    ReactDOM.jsx("div", {})
+    (ReactDOM.jsx("div", {}): Jsx.element)
   }
   let make = {
     let \"TopLevel$V4A" = (props: props<_>) => make(props)