Pass existing location to children prop

Author: nojaf

compiler/syntax/src/jsx_v4.ml

@@ -1208,7 +1208,7 @@ let append_children_prop (config : Jsx_common.jsx_config) mapper
           | "react" -> Lident "ReactDOM"
           | _generic -> module_access_name config "Elements"
         in
-        Exp.apply
+        Exp.apply ~loc:child.pexp_loc
           (Exp.ident
              {txt = Ldot (element_binding, "someElement"); loc = Location.none})
           [(Nolabel, mapper.expr mapper child)]
@@ -1220,18 +1220,24 @@ let append_children_prop (config : Jsx_common.jsx_config) mapper
     in
     props
     @ [
-        JSXPropValue ({txt = "children"; loc = Location.none}, is_optional, expr);
+        JSXPropValue
+          ({txt = "children"; loc = child.pexp_loc}, is_optional, expr);
       ]
-  | JSXChildrenItems xs ->
+  | JSXChildrenItems (head :: _ as xs) ->
+    let loc =
+      match List.rev xs with
+      | [] -> head.pexp_loc
+      | lastChild :: _ ->
+        {head.pexp_loc with loc_end = lastChild.pexp_loc.loc_end}
+    in
     (* this is a hack to support react components that introspect into their children *)
     props
     @ [
         JSXPropValue
-          ( {txt = "children"; loc = Location.none},
+          ( {txt = "children"; loc},
             false,
-            Exp.apply
-              (Exp.ident
-                 {txt = module_access_name config "array"; loc = Location.none})
+            Exp.apply ~loc
+              (Exp.ident {txt = module_access_name config "array"; loc})
               [(Nolabel, Exp.array (List.map (mapper.expr mapper) xs))] );
       ]
 

tests/build_tests/super_errors/expected/missing_required_prop.res.expected

@@ -0,0 +1,11 @@
+
+  We've found a bug for you!
+  /.../fixtures/missing_required_prop.res:28:7-37
+
+  26 ┆ let make = () => {
+  27 ┆   <Wrapper>
+  28 ┆     <div> {""->React.string} </div>
+  29 ┆   </Wrapper>
+  30 ┆ }
+
+  This JSX component does not accept child elements. It has no children prop <Wrapper />

tests/build_tests/super_errors/expected/missing_required_prop_when_children.res.expected

@@ -0,0 +1,13 @@
+
+  We've found a bug for you!
+  /.../fixtures/missing_required_prop_when_children.res:31:7-32:37
+
+  29 ┆ let make = () => {
+  30 ┆   <Wrapper>
+  31 ┆     <button> {"yo"->React.string} </button>
+  32 ┆     <div> {""->React.string} </div>
+  33 ┆   </Wrapper>
+  34 ┆ }
+
+  The component <Wrapper /> is missing these required props:
+   value

tests/build_tests/super_errors/expected/missing_required_prop_when_single_child.res.expected

@@ -0,0 +1,12 @@
+
+  We've found a bug for you!
+  /.../fixtures/missing_required_prop_when_single_child.res:28:7-37
+
+  26 ┆ let make = () => {
+  27 ┆   <Wrapper>
+  28 ┆     <div> {""->React.string} </div>
+  29 ┆   </Wrapper>
+  30 ┆ }
+
+  The component <Wrapper /> is missing these required props:
+   value

tests/build_tests/super_errors/fixtures/missing_required_prop.res

@@ -0,0 +1,31 @@
+module React = {
+  type element = Jsx.element
+  @val external null: element = "null"
+  type componentLike<'props, 'return> = Jsx.componentLike<'props, 'return>
+  type component<'props> = Jsx.component<'props>
+  external component: componentLike<'props, element> => component<'props> = "%identity"
+  @module("react/jsx-runtime")
+  external jsx: (component<'props>, 'props) => element = "jsx"
+  external string: string => element = "%identity"
+}
+module ReactDOM = {
+  external someElement: React.element => option<React.element> = "%identity"
+  @module("react/jsx-runtime")
+  external jsx: (string, JsxDOM.domProps) => Jsx.element = "jsx"
+}
+
+module Wrapper = {
+  @react.component
+  let make = (~value: 'value) => {
+    <div> {React.null} </div>
+  }
+}
+
+module SomeComponent = {
+  @react.component
+  let make = () => {
+    <Wrapper>
+      <div> {""->React.string} </div>
+    </Wrapper>
+  }
+}

tests/build_tests/super_errors/fixtures/missing_required_prop_when_children.res

@@ -0,0 +1,35 @@
+module React = {
+  type element = Jsx.element
+  @val external null: element = "null"
+  type componentLike<'props, 'return> = Jsx.componentLike<'props, 'return>
+  type component<'props> = Jsx.component<'props>
+  external component: componentLike<'props, element> => component<'props> = "%identity"
+  @module("react/jsx-runtime")
+  external jsx: (component<'props>, 'props) => element = "jsx"
+  @module("react/jsx-runtime")
+  external jsxs: (component<'props>, 'props) => element = "jsxs"
+  external string: string => element = "%identity"
+  external array: array<element> => element = "%identity"
+}
+module ReactDOM = {
+  external someElement: React.element => option<React.element> = "%identity"
+  @module("react/jsx-runtime")
+  external jsx: (string, JsxDOM.domProps) => Jsx.element = "jsx"
+}
+
+module Wrapper = {
+  @react.component
+  let make = (~value: 'value, ~children: React.element) => {
+    <div> {children} </div>
+  }
+}
+
+module SomeComponent = {
+  @react.component
+  let make = () => {
+    <Wrapper>
+      <button> {"yo"->React.string} </button>
+      <div> {""->React.string} </div>
+    </Wrapper>
+  }
+}

tests/build_tests/super_errors/fixtures/missing_required_prop_when_single_child.res

@@ -0,0 +1,31 @@
+module React = {
+  type element = Jsx.element
+  @val external null: element = "null"
+  type componentLike<'props, 'return> = Jsx.componentLike<'props, 'return>
+  type component<'props> = Jsx.component<'props>
+  external component: componentLike<'props, element> => component<'props> = "%identity"
+  @module("react/jsx-runtime")
+  external jsx: (component<'props>, 'props) => element = "jsx"
+  external string: string => element = "%identity"
+}
+module ReactDOM = {
+  external someElement: React.element => option<React.element> = "%identity"
+  @module("react/jsx-runtime")
+  external jsx: (string, JsxDOM.domProps) => Jsx.element = "jsx"
+}
+
+module Wrapper = {
+  @react.component
+  let make = (~value: 'value, ~children: React.element) => {
+    <div> {children} </div>
+  }
+}
+
+module SomeComponent = {
+  @react.component
+  let make = () => {
+    <Wrapper>
+      <div> {""->React.string} </div>
+    </Wrapper>
+  }
+}