Add jsx_tag_name (#7760)

* Add jsx_tag_name

* Clean up parser

* Some better union case names

* jsx_tag_name loc

* Use info from jsx_tag_name to determine upper or lower.

* Clean up

* Parser clean up

* Refactor read_jsx_tag_name

* Add value to JsxInvalid

* Update snapshot

* Move helper out of loop

* Add changelog entry

Author: nojaf

CHANGELOG.md

@@ -26,6 +26,8 @@
 
 #### :house: Internal
 
+- AST: Use jsx_tag_name instead of Longindent.t to store jsx tag name. https://github.com/rescript-lang/rescript/pull/7760
+
 # 12.0.0-beta.5
 
 #### :bug: Bug fix

analysis/src/CompletionFrontEnd.ml

@@ -1341,8 +1341,13 @@ let completionWithParser1 ~currentFile ~debug ~offset ~path ~posCursor
                 {
                   jsx_container_element_tag_name_start = compName;
                   jsx_container_element_props = props;
-                } ) ->
+                } ) -> (
           inJsxContext := true;
+          let is_valid_tag_for_props =
+            match compName.txt with
+            | Parsetree.JsxTagInvalid _ -> false
+            | _ -> true
+          in
           let children =
             match expr.pexp_desc with
             | Pexp_jsx_element
@@ -1351,43 +1356,81 @@ let completionWithParser1 ~currentFile ~debug ~offset ~path ~posCursor
               children
             | _ -> JSXChildrenItems []
           in
-          let jsxProps =
-            CompletionJsx.extractJsxProps ~compName ~props ~children
+          let compName_loc = compName.loc in
+          let compName_lid =
+            Ast_helper.Jsx.longident_of_jsx_tag_name compName.txt
           in
-          let compNamePath = flattenLidCheckDot ~jsx:true compName in
-          if debug then
-            Printf.printf "JSX <%s:%s %s> _children:%s\n"
-              (compNamePath |> String.concat ".")
-              (Loc.toString compName.loc)
-              (jsxProps.props
-              |> List.map
-                   (fun ({name; posStart; posEnd; exp} : CompletionJsx.prop) ->
-                     Printf.sprintf "%s[%s->%s]=...%s" name
-                       (Pos.toString posStart) (Pos.toString posEnd)
-                       (Loc.toString exp.pexp_loc))
-              |> String.concat " ")
-              (match jsxProps.childrenStart with
-              | None -> "None"
-              | Some childrenPosStart -> Pos.toString childrenPosStart);
+          let jsxPropsOpt =
+            if is_valid_tag_for_props then
+              Some
+                (CompletionJsx.extractJsxProps
+                   ~compName:(Location.mkloc compName_lid compName_loc)
+                   ~props ~children)
+            else None
+          in
+          let compNamePath =
+            flattenLidCheckDot ~jsx:true
+              {txt = compName_lid; loc = compName_loc}
+          in
+          (if debug then
+             match jsxPropsOpt with
+             | Some jsxProps ->
+               Printf.printf "JSX <%s:%s %s> _children:%s\n"
+                 (compNamePath |> String.concat ".")
+                 (Loc.toString compName_loc)
+                 (jsxProps.props
+                 |> List.map
+                      (fun
+                        ({name; posStart; posEnd; exp} : CompletionJsx.prop) ->
+                        Printf.sprintf "%s[%s->%s]=...%s" name
+                          (Pos.toString posStart) (Pos.toString posEnd)
+                          (Loc.toString exp.pexp_loc))
+                 |> String.concat " ")
+                 (match jsxProps.childrenStart with
+                 | None -> "None"
+                 | Some childrenPosStart -> Pos.toString childrenPosStart)
+             | None ->
+               Printf.printf "JSX <%s:%s > _children:None\n"
+                 (compNamePath |> String.concat ".")
+                 (Loc.toString compName_loc));
+          (* If the tag name is an uppercase path and the cursor is right after a dot (e.g., <O.|),
+             prefer module member completion over JSX prop suggestions. *)
+          (match compName.txt with
+          | Parsetree.JsxUpperTag _ when blankAfterCursor = Some '.' ->
+            setResult
+              (Cpath
+                 (CPId
+                    {
+                      loc = compName_loc;
+                      path = compNamePath;
+                      completionContext = Module;
+                    }))
+          | _ -> ());
           let jsxCompletable =
-            match expr.pexp_desc with
-            | Pexp_jsx_element
-                (Jsx_container_element
-                   {
-                     jsx_container_element_closing_tag = None;
-                     jsx_container_element_children =
-                       JSXChildrenSpreading _ | JSXChildrenItems (_ :: _);
-                   }) ->
-              (* This is a weird edge case where there is no closing tag but there are children *)
+            match (jsxPropsOpt, expr.pexp_desc) with
+            | ( Some _,
+                Pexp_jsx_element
+                  (Jsx_container_element
+                     {
+                       jsx_container_element_closing_tag = None;
+                       jsx_container_element_children =
+                         JSXChildrenSpreading _ | JSXChildrenItems (_ :: _);
+                     }) ) ->
               None
-            | _ ->
+            | Some jsxProps, _ ->
               CompletionJsx.findJsxPropsCompletable ~jsxProps
                 ~endPos:(Loc.end_ expr.pexp_loc) ~posBeforeCursor
-                ~posAfterCompName:(Loc.end_ compName.loc)
+                ~posAfterCompName:(Loc.end_ compName_loc)
                 ~firstCharBeforeCursorNoWhite ~charAtCursor
+            | None, _ -> None
           in
-          if jsxCompletable <> None then setResultOpt jsxCompletable
-          else if compName.loc |> Loc.hasPos ~pos:posBeforeCursor then
+          (match jsxCompletable with
+          | Some _ as res -> setResultOpt res
+          | None -> ());
+          if
+            jsxCompletable = None
+            && compName_loc |> Loc.hasPos ~pos:posBeforeCursor
+          then
             setResult
               (match compNamePath with
               | [prefix] when Char.lowercase_ascii prefix.[0] = prefix.[0] ->
@@ -1396,11 +1439,14 @@ let completionWithParser1 ~currentFile ~debug ~offset ~path ~posCursor
                 Cpath
                   (CPId
                      {
-                       loc = compName.loc;
+                       loc = compName_loc;
                        path = compNamePath;
                        completionContext = Module;
                      }))
-          else iterateJsxProps ~iterator jsxProps
+          else
+            match jsxPropsOpt with
+            | Some jsxProps -> iterateJsxProps ~iterator jsxProps
+            | None -> ())
         | Pexp_apply
             {
               funct = {pexp_desc = Pexp_ident {txt = Lident "->"}};

analysis/src/SemanticTokens.ml

@@ -265,7 +265,9 @@ let command ~debug ~emitter ~path =
       *)
       emitter (* --> <div... *)
       |> emitJsxTag ~debug ~name:"<" ~pos:(Loc.start e.pexp_loc);
-      emitter |> emitJsxOpen ~lid:lident.txt ~debug ~loc:lident.loc;
+      let lid = Ast_helper.Jsx.longident_of_jsx_tag_name lident.txt in
+      let loc = lident.loc in
+      emitter |> emitJsxOpen ~lid ~debug ~loc;
       let closing_line, closing_column = Loc.end_ e.pexp_loc in
       emitter (* <foo ...props /> <-- *)
       |> emitJsxTag ~debug ~name:"/>" ~pos:(closing_line, closing_column - 2)
@@ -281,7 +283,9 @@ let command ~debug ~emitter ~path =
       (* opening tag *)
       emitter (* --> <div... *)
       |> emitJsxTag ~debug ~name:"<" ~pos:(Loc.start e.pexp_loc);
-      emitter |> emitJsxOpen ~lid:lident.txt ~debug ~loc:lident.loc;
+      let lid = Ast_helper.Jsx.longident_of_jsx_tag_name lident.txt in
+      let loc = lident.loc in
+      emitter |> emitJsxOpen ~lid ~debug ~loc;
       emitter (* <foo ...props > <-- *)
       |> emitJsxTag ~debug ~name:">"
            ~pos:(Pos.ofLexing posOfGreatherthanAfterProps);
@@ -308,9 +312,11 @@ let command ~debug ~emitter ~path =
              emitter
              |> emitJsxTag ~debug ~name:"</"
                   ~pos:(Pos.ofLexing closing_less_than);
-             emitter
-             |> emitJsxClose ~debug ~lid:lident.txt
-                  ~pos:(Loc.start tag_name_end.loc);
+             let lid =
+               Ast_helper.Jsx.longident_of_jsx_tag_name tag_name_end.txt
+             in
+             let loc = tag_name_end.loc in
+             emitter |> emitJsxClose ~debug ~lid ~pos:(Loc.end_ loc);
              emitter (* <foo> ... </foo> <-- *)
              |> emitJsxTag ~debug ~name:">"
                   ~pos:(Pos.ofLexing final_greather_than))

compiler/ml/ast_helper.ml

@@ -428,3 +428,21 @@ module Te = struct
       pext_attributes = attrs;
     }
 end
+
+module Jsx = struct
+  let string_of_jsx_tag_name (tag_name : Parsetree.jsx_tag_name) : string =
+    match tag_name with
+    | Parsetree.JsxLowerTag name -> name
+    | Parsetree.JsxQualifiedLowerTag {path; name} ->
+      String.concat "." (Longident.flatten path) ^ "." ^ name
+    | Parsetree.JsxUpperTag path -> String.concat "." (Longident.flatten path)
+    | Parsetree.JsxTagInvalid name -> name
+
+  let longident_of_jsx_tag_name (tag_name : Parsetree.jsx_tag_name) :
+      Longident.t =
+    match tag_name with
+    | Parsetree.JsxLowerTag name -> Longident.Lident name
+    | Parsetree.JsxQualifiedLowerTag {path; name} -> Longident.Ldot (path, name)
+    | Parsetree.JsxUpperTag path -> path
+    | Parsetree.JsxTagInvalid name -> Longident.Lident name
+end

compiler/ml/ast_helper.mli

@@ -212,13 +212,13 @@ module Exp : sig
   val jsx_unary_element :
     ?loc:loc ->
     ?attrs:attrs ->
-    Longident.t Location.loc ->
+    Parsetree.jsx_tag_name Location.loc ->
     Parsetree.jsx_props ->
     expression
   val jsx_container_element :
     ?loc:loc ->
     ?attrs:attrs ->
-    Longident.t Location.loc ->
+    Parsetree.jsx_tag_name Location.loc ->
     Parsetree.jsx_props ->
     Lexing.position ->
     Parsetree.jsx_children ->
@@ -301,6 +301,11 @@ module Te : sig
   val rebind : ?loc:loc -> ?attrs:attrs -> str -> lid -> extension_constructor
 end
 
+module Jsx : sig
+  val string_of_jsx_tag_name : Parsetree.jsx_tag_name -> string
+  val longident_of_jsx_tag_name : Parsetree.jsx_tag_name -> Longident.t
+end
+
 (** {1 Module language} *)
 
 (** Module type expressions *)

compiler/ml/ast_mapper_from0.ml

@@ -396,10 +396,23 @@ module E = struct
       when has_jsx_attribute () -> (
       let attrs = attrs |> List.filter (fun ({txt}, _) -> txt <> "JSX") in
       let props, children = extract_props_and_children sub args in
+      let jsx_tag : Pt.jsx_tag_name =
+        match tag_name.txt with
+        | Longident.Lident s
+          when String.length s > 0 && Char.lowercase_ascii s.[0] = s.[0] ->
+          Pt.JsxLowerTag s
+        | Longident.Lident _ -> Pt.JsxUpperTag tag_name.txt
+        | Longident.Ldot (path, last)
+          when String.length last > 0
+               && Char.lowercase_ascii last.[0] = last.[0] ->
+          Pt.JsxQualifiedLowerTag {path; name = last}
+        | _ -> Pt.JsxUpperTag tag_name.txt
+      in
+      let jsx_tag_name = {txt = jsx_tag; loc = tag_name.loc} in
       match children with
-      | None -> jsx_unary_element ~loc ~attrs tag_name props
+      | None -> jsx_unary_element ~loc ~attrs jsx_tag_name props
       | Some children ->
-        jsx_container_element ~loc ~attrs tag_name props Lexing.dummy_pos
+        jsx_container_element ~loc ~attrs jsx_tag_name props Lexing.dummy_pos
           children None)
     | Pexp_apply (e, l) ->
       let e =

compiler/ml/ast_mapper_to0.ml

@@ -495,7 +495,9 @@ module E = struct
              jsx_unary_element_tag_name = tag_name;
              jsx_unary_element_props = props;
            }) ->
-      let tag_ident = map_loc sub tag_name in
+      let tag_ident : Longident.t Location.loc =
+        tag_name |> Location.map_loc Ast_helper.Jsx.longident_of_jsx_tag_name
+      in
       let props = map_jsx_props sub props in
       let children_expr =
         let loc =
@@ -525,7 +527,9 @@ module E = struct
              jsx_container_element_props = props;
              jsx_container_element_children = children;
            }) ->
-      let tag_ident = map_loc sub tag_name in
+      let tag_ident : Longident.t Location.loc =
+        tag_name |> Location.map_loc Ast_helper.Jsx.longident_of_jsx_tag_name
+      in
       let props = map_jsx_props sub props in
       let children_expr = map_jsx_children sub loc children in
       apply ~loc ~attrs:(jsx_attr sub :: attrs) (ident tag_ident)

compiler/ml/depend.ml

@@ -294,7 +294,10 @@ let rec add_expr bv exp =
       (Jsx_unary_element
          {jsx_unary_element_tag_name = name; jsx_unary_element_props = props})
     ->
-    add bv name;
+    (* Conservatively add all module path segments referenced by the tag name *)
+    (match name.txt with
+    | JsxLowerTag _ | JsxTagInvalid _ -> ()
+    | JsxQualifiedLowerTag {path; _} | JsxUpperTag path -> add_path bv path);
     and_jsx_props bv props
   | Pexp_jsx_element
       (Jsx_container_element
@@ -303,7 +306,9 @@ let rec add_expr bv exp =
            jsx_container_element_props = props;
            jsx_container_element_children = children;
          }) ->
-    add bv name;
+    (match name.txt with
+    | JsxLowerTag _ | JsxTagInvalid _ -> ()
+    | JsxQualifiedLowerTag {path; _} | JsxUpperTag path -> add_path bv path);
     and_jsx_props bv props;
     add_jsx_children bv children
 

compiler/ml/location.ml

@@ -300,3 +300,5 @@ let raise_errorf ?(loc = none) ?(sub = []) ?(if_highlight = "") =
 
 let deprecated ?(def = none) ?(use = none) loc msg =
   prerr_warning loc (Warnings.Deprecated (msg, def, use))
+
+let map_loc f {txt; loc} = {txt = f txt; loc}

compiler/ml/location.mli

@@ -131,3 +131,5 @@ val report_exception : formatter -> exn -> unit
 (** Reraise the exception if it is unknown. *)
 
 val deprecated : ?def:t -> ?use:t -> t -> string -> unit
+
+val map_loc : ('a -> 'b) -> 'a loc -> 'b loc