Fix jsx completion for abstract Jsx.component

CHANGELOG.md

@@ -22,6 +22,7 @@
 
 #### :bug: Bug fix
 
+- Fix JSX completions in editor analysis for abstract component props and empty child expressions. https://github.com/rescript-lang/rescript/pull/8390
 - Preserve multibyte characters when wrapping long source lines in compiler code frames. https://github.com/rescript-lang/rescript/pull/8520
 
 #### :memo: Documentation

analysis/src/completion_back_end.ml

@@ -1641,27 +1641,36 @@ let rec complete_typed_value ?(type_arg_context : type_arg_context option)
   | TtypeT {env; path} when mode = Expression ->
     if Debug.verbose () then
       print_endline "[complete_typed_value]--> TtypeT (Expression)";
-    (* Find all values in the module with type t *)
+    let target_path = path in
+    let target_type_name = Path.last target_path in
+    let is_target_type_path path =
+      Path.same path target_path || Path.last path = target_type_name
+    in
+    (* Find all values in the module with the target type. *)
     let value_with_type_t t =
       match t.Types.desc with
-      | Tconstr (Pident {name = "t"}, [], _) -> true
+      | Tconstr (path, [], _) when is_target_type_path path -> true
       | _ -> false
     in
-    (* Find all functions in the module that returns type t *)
+    (* Find all functions in the module that return the target type. *)
     let rec fn_returns_type_t t =
       match t.Types.desc with
       | Tlink t1 | Tsubst t1 | Tpoly (t1, []) -> fn_returns_type_t t1
       | Tarrow _ -> (
         match
           Type_utils.extract_function_type ~env ~state ~package:full.package t
         with
-        | ( (Nolabel, {desc = Tconstr (Path.Pident {name = "t"}, _, _)}) :: _,
-            {desc = Tconstr (Path.Pident {name = "t"}, _, _)} ) ->
-          (* Filter out functions that take type t first. These are often
+        | ( (Nolabel, {desc = Tconstr (arg_path, _, _)}) :: _,
+            {desc = Tconstr (return_path, _, _)} )
+          when is_target_type_path arg_path && is_target_type_path return_path
+          ->
+          (* Filter out functions that take the target type first. These are often
              @send style functions that we don't want to have here because
-             they usually aren't meant to create a type t from scratch. *)
+             they usually aren't meant to create a value from scratch. *)
           false
-        | _args, {desc = Tconstr (Path.Pident {name = "t"}, _, _)} -> true
+        | _args, {desc = Tconstr (return_path, _, _)}
+          when is_target_type_path return_path ->
+          true
         | _ -> false)
       | _ -> false
     in
@@ -2443,10 +2452,91 @@ let rec process_completable ~state ~debug ~full ~scope ~env ~pos ~for_hover
             | CJsxPropValue _ -> true
             | _ -> false
         in
+        let complete_named_type_from_module_members typ =
+          match (typ, context_path) with
+          | ( TtypeT {path = target_path},
+              CPId {path = cp_path; completion_context = Type} ) -> (
+            let target_type_name = Path.last target_path in
+            let is_target_type_path path =
+              Path.same path target_path || Path.last path = target_type_name
+            in
+            let value_has_target_type type_expr =
+              match type_expr.Types.desc with
+              | Tconstr (path, [], _) when is_target_type_path path -> true
+              | _ -> false
+            in
+            let rec fn_returns_target_type ~env type_expr =
+              match type_expr.Types.desc with
+              | Tlink t1 | Tsubst t1 | Tpoly (t1, []) ->
+                fn_returns_target_type ~env t1
+              | Tarrow _ -> (
+                match
+                  Type_utils.extract_function_type ~env ~state
+                    ~package:full.package type_expr
+                with
+                | ( (Nolabel, {desc = Tconstr (arg_path, _, _)}) :: _,
+                    {desc = Tconstr (return_path, _, _)} )
+                  when is_target_type_path arg_path
+                       && is_target_type_path return_path ->
+                  false
+                | _args, {desc = Tconstr (return_path, _, _)}
+                  when is_target_type_path return_path ->
+                  true
+                | _ -> false)
+              | _ -> false
+            in
+            match List.rev cp_path with
+            | _type_name :: (_ :: _ as reversed_module_path) ->
+              let module_path = List.rev reversed_module_path in
+              let module_name = module_path |> String.concat "." in
+              get_completions_for_path ~state ~debug:false
+                ~completion_context:ValueOrField ~exact:false ~opens ~full ~pos
+                ~env ~scope (module_path @ [""])
+              |> List.filter_map (fun (c : Completion.t) ->
+                     match c.kind with
+                     | Value type_expr
+                       when value_has_target_type type_expr
+                            || fn_returns_target_type ~env:c.env type_expr ->
+                       let is_function =
+                         fn_returns_target_type ~env:c.env type_expr
+                       in
+                       let qualified_name = module_name ^ "." ^ c.name in
+                       if
+                         prefix <> ""
+                         && not
+                              (Utils.check_name qualified_name ~prefix
+                                 ~exact:false)
+                       then None
+                       else
+                         Some
+                           {
+                             c with
+                             name =
+                               (if is_function then qualified_name ^ "()"
+                                else qualified_name);
+                             sort_text = Some ("A " ^ qualified_name);
+                             insert_text =
+                               Some
+                                 (if is_function then qualified_name ^ "($0)"
+                                  else qualified_name);
+                             insert_text_format =
+                               (if is_function then
+                                  Some Lsp.Types.InsertTextFormat.Snippet
+                                else c.insert_text_format);
+                           }
+                     | _ -> None)
+            | _ -> [])
+          | _ -> []
+        in
         let items =
-          typ
-          |> complete_typed_value ?type_arg_context ~raw_opens ~mode:Expression
-               ~full ~prefix ~completion_context ~state
+          let items =
+            typ
+            |> complete_typed_value ?type_arg_context ~raw_opens
+                 ~mode:Expression ~full ~prefix ~completion_context ~state
+          in
+          (match items with
+          | [] -> complete_named_type_from_module_members typ
+          | _ -> items)
           |> List.map (fun (c : Completion.t) ->
                  if wrap_insert_text_in_braces then
                    {

analysis/src/completion_front_end.ml

@@ -438,6 +438,7 @@ let completion_with_parser1 ~debug ~offset ~pos_cursor ~kind_file
         result := Some (x, !scope)
   in
   let in_jsx_context = ref false in
+  let in_jsx_child_context = ref false in
   let set_result x = set_result_opt (Some x) in
   let scope_value_description (vd : Parsetree.value_description) =
     scope :=
@@ -1076,6 +1077,11 @@ let completion_with_parser1 ~debug ~offset ~pos_cursor ~kind_file
                 });
            expr iterator prop.exp;
            reset_current_ctx_path previous_ctx_path)
+  and iterate_jsx_children ~(iterator : Ast_iterator.iterator) children =
+    let old_in_jsx_child_context = !in_jsx_child_context in
+    in_jsx_child_context := true;
+    children |> List.iter (fun child -> iterator.expr iterator child);
+    in_jsx_child_context := old_in_jsx_child_context
   and expr (iterator : Ast_iterator.iterator) (expr : Parsetree.expression) =
     let old_in_jsx_context = !in_jsx_context in
     let processed = ref false in
@@ -1123,6 +1129,21 @@ let completion_with_parser1 ~debug ~offset ~pos_cursor ~kind_file
                 }));
         true
     in
+    let set_empty_jsx_child_expr_result () =
+      set_result
+        (Cexpression
+           {
+             context_path =
+               CPId
+                 {
+                   loc = expr.pexp_loc;
+                   path = ["React"; "element"];
+                   completion_context = Type;
+                 };
+             nested = [];
+             prefix = "";
+           })
+    in
     typed_completion_expr expr;
     match expr.pexp_desc with
     | Pexp_match (expr, cases)
@@ -1256,6 +1277,10 @@ let completion_with_parser1 ~debug ~offset ~pos_cursor ~kind_file
                          then ValueOrField
                          else Value);
                     }))
+        | Pexp_construct ({txt = Lident "()"}, _) when !in_jsx_child_context ->
+          set_empty_jsx_child_expr_result ()
+        | Pexp_record ([], _) when !in_jsx_child_context ->
+          set_empty_jsx_child_expr_result ()
         | Pexp_construct (lid, e_opt) -> (
           let lid_path = flatten_lid_check_dot lid in
           if debug then
@@ -1345,6 +1370,10 @@ let completion_with_parser1 ~debug ~offset ~pos_cursor ~kind_file
                         in_jsx = !in_jsx_context;
                       }))
             | None -> ())
+        | Pexp_jsx_element (Jsx_fragment {jsx_fragment_children = children}) ->
+          in_jsx_context := true;
+          iterate_jsx_children ~iterator children;
+          processed := true
         | Pexp_jsx_element
             ( Jsx_unary_element
                 {
@@ -1355,7 +1384,7 @@ let completion_with_parser1 ~debug ~offset ~pos_cursor ~kind_file
                 {
                   jsx_container_element_tag_name_start = comp_name;
                   jsx_container_element_props = props;
-                } ) -> (
+                } ) ->
           in_jsx_context := true;
           let is_valid_tag_for_props =
             match comp_name.txt with
@@ -1457,10 +1486,12 @@ let completion_with_parser1 ~debug ~offset ~pos_cursor ~kind_file
                        path = comp_name_path;
                        completion_context = Module;
                      }))
-          else
-            match jsx_props_opt with
-            | Some jsx_props -> iterate_jsx_props ~iterator jsx_props
-            | None -> ())
+          else ();
+          (match jsx_props_opt with
+          | Some jsx_props -> iterate_jsx_props ~iterator jsx_props
+          | None -> ());
+          iterate_jsx_children ~iterator children;
+          processed := true
         | Pexp_apply
             {
               funct = {pexp_desc = Pexp_ident {txt = Lident "->"}};

analysis/src/completion_jsx.ml

@@ -231,42 +231,39 @@ let get_jsx_labels ~component_path ~find_type_of_value ~package ~state =
                (name, t, env))
       | _ -> []
     in
+    let is_component_path path =
+      match Path.last path with
+      | "component" -> true
+      | _ -> false
+    in
+    let is_component_like_path path =
+      match Path.last path with
+      | "componentLike" -> true
+      | _ -> false
+    in
+    let rec get_props_type (t : Types.type_expr) =
+      match t.desc with
+      | Tlink t1 | Tsubst t1 | Tpoly (t1, []) -> get_props_type t1
+      | Tconstr (path, type_args, _) -> Some (path, type_args)
+      | _ -> None
+    in
     let rec get_labels (t : Types.type_expr) =
       match t.desc with
       | Tlink t1 | Tsubst t1 | Tpoly (t1, []) -> get_labels t1
-      | Tconstr (p, [props_type], _) when Path.name p = "React.component" -> (
-        let rec get_props_type (t : Types.type_expr) =
-          match t.desc with
-          | Tlink t1 | Tsubst t1 | Tpoly (t1, []) -> get_props_type t1
-          | Tconstr (path, type_args, _) when Path.last path = "props" ->
-            Some (path, type_args)
-          | _ -> None
-        in
+      | Tconstr (p, [props_type], _) when is_component_path p -> (
         match props_type |> get_props_type with
         | Some (path, type_args) -> get_fields ~path ~type_args
         | None -> [])
-      | Tarrow
-          ({lbl = Nolabel; typ = {desc = Tconstr (path, type_args, _)}}, _, _, _)
-        when Path.last path = "props" ->
-        get_fields ~path ~type_args
-      | Tconstr (cl_path, [{desc = Tconstr (path, type_args, _)}; _], _)
-        when Path.name cl_path = "React.componentLike"
-             && Path.last path = "props" ->
-        (* JSX V4 external or interface *)
-        get_fields ~path ~type_args
       | Tarrow ({lbl = Nolabel; typ}, _, _, _) -> (
-        (* Component without the JSX PPX, like a make fn taking a hand-written
-           type props. *)
-        let rec dig_to_constr typ =
-          match typ.Types.desc with
-          | Tlink t1 | Tsubst t1 | Tpoly (t1, []) -> dig_to_constr t1
-          | Tconstr (path, type_args, _) when Path.last path = "props" ->
-            Some (path, type_args)
-          | _ -> None
-        in
-        match dig_to_constr typ with
-        | None -> []
-        | Some (path, type_args) -> get_fields ~path ~type_args)
+        match typ |> get_props_type with
+        | Some (path, type_args) -> get_fields ~path ~type_args
+        | None -> [])
+      | Tconstr (cl_path, [props_type; _], _)
+        when is_component_like_path cl_path -> (
+        (* JSX V4 external or interface *)
+        match props_type |> get_props_type with
+        | Some (path, type_args) -> get_fields ~path ~type_args
+        | None -> [])
       | _ -> []
     in
     typ |> get_labels

analysis/src/type_utils.ml

@@ -576,9 +576,15 @@ let extract_type_from_resolved_type (typ : Type.t) ~env ~full ~state =
          {env; constructors; variant_name = typ.name; variant_decl = typ.decl})
   | Abstract _ | Open -> (
     match typ.decl.type_manifest with
-    | None -> None
-    | Some t ->
-      t |> extract_type ~state ~env ~package:full.package |> get_extracted_type)
+    | None -> Some (TtypeT {env; path = Pident (Ident.create typ.name)})
+    | Some t -> (
+      match
+        t
+        |> extract_type ~state ~env ~package:full.package
+        |> get_extracted_type
+      with
+      | Some extracted_type -> Some extracted_type
+      | None -> Some (TtypeT {env; path = Pident (Ident.create typ.name)})))
 
 (** The context we just came from as we resolve the nested structure. *)
 type ctx = Rfield of string  (** A record field of name *)

tests/analysis_tests/tests/src/CompletionJsx.res

@@ -28,6 +28,8 @@ module SomeComponent = {
       //      ^com
       // {someArr->a}
       //            ^com
+      // {}
+      //  ^com
       // <di
       //    ^com
     </div>
@@ -87,9 +89,20 @@ module Info = {
   }
 }
 
+module DomPropsComponent = {
+  @react.componentWithProps
+  let make = (props: JsxDOM.domProps) => {
+    ignore(props)
+    React.null
+  }
+}
+
 // <Info _type={#warning} >
 //                        ^com
 
+// <DomPropsComponent onClick=>
+//                            ^com
+
 
 // let _ = <p>{"".s}</p>
 //                 ^com

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

@@ -2384,9 +2384,6 @@ posCursor:[339:26] posNoWhite:[339:25] Found expr:[339:16->341:5]
 posCursor:[339:26] posNoWhite:[339:25] Found pattern:[339:20->341:5]
 posCursor:[339:26] posNoWhite:[339:25] Found type:[339:23->341:5]
 Ptyp_constr Res:[339:23->341:5]
-posCursor:[339:26] posNoWhite:[339:25] Found pattern:[339:20->341:5]
-posCursor:[339:26] posNoWhite:[339:25] Found type:[339:23->341:5]
-Ptyp_constr Res:[339:23->341:5]
 Completable: Cpath Type[Res]
 Raw opens: 2 Shadow.B.place holder ... Shadow.A.place holder
 Package opens Stdlib.place holder Pervasives.JsxModules.place holder