Refactor array bracket access to dedicated AST node #8168

nojaf · 2026-02-23T08:04:48Z

Was this existing code?
Why did this change?

This is to match the expected output of the gentype. I mentioned the reason here, #8168 (comment).

Essentially, arr[3] is transformed into Array.get(arr, 3) and depending on what module is in scope, the behavior of the gentype can be changed.

Since arr[3] no longer transformed to Array.get(arr, 3) internally, to get the same gentype behavior you need to be explicit.

Hmm, not sure if updating the test to avoid the problem is the best thing to do here. I have never used gen type so I have no idea what the implications are but this seems quite user impacting? If so, is this the right play then?

I mentioned before that this is going to impact gentype. I don't use gentype/typescript though, maybe @cometkim can chime in? Anyway, this is going to be in v13, we can expect to have some breaking changes.

There's also this note I found in the codebase that this needs to be fixed. This PR essentially fixes this issue.

(* FIXME: Do not implicitly rely on specific module name, even primitive one This can be abused like module Array = MyModule Find better mechanism to support it *)

nojaf · 2026-02-23T08:05:32Z

Thanks for adding this!

-Original file line number
+Diff line change
@@ -1,7 +1,6 @@
     @genType
     let testImmutableArrayGet = arr => {
-      open ImmutableArray
-      arr[3]
+      ImmutableArray.Array.get(arr, 3)
     }
     /*
@@ Expand All / @@ -10,11 +9,9 @@ let testImmutableArrayGet = arr => { @@
      */
     let testBeltArrayGet = arr => {
-      open Belt
-      arr[3]
+      Belt.Array.get(arr, 3)
     }
     let testBeltArraySet = arr => {
-      open Belt
-      arr[3] = 4
+      Belt.Array.set(arr, 3, 4)
     }

-Original file line number
+Diff line change
@@ Expand Up @@
               ~kind:
                 (Completion.ExtractedType (Toption (env, ExtractedType typ), `Type));
           ])
+      | CPIndex cp -> (
+        if Debug.verbose () then print_endline "[ctx_path]--> CPIndex";
+        match
+          cp
+          |> getCompletionsForContextPath ~debug ~full ~opens ~rawOpens ~pos ~env
+               ~exact:true ~scope
+          |> completionsGetCompletionType ~full
+        with
+        | Some (Tarray (env, TypeExpr elementType), _) ->
+          (* Return Value so completionsGetTypeEnv2 (used by CPPipe) can resolve it,
+             enabling pipe completion on arr[n]-> *)
+          [
+            Completion.create "dummy" ~env
+              ~kind:(Completion.Value (Predef.type_option elementType));
+          ]
+        | Some (Tarray (env, elementType), _) ->
+          (* Fallback when element type is itself an extracted type *)
+          [
+            Completion.create "dummy" ~env
+              ~kind:(Completion.ExtractedType (Toption (env, elementType), `Type));
+          ]
+        | _ -> [])
       | CPAwait cp -> (
         if Debug.verbose () then print_endline "[ctx_path]--> CPAwait";
         match
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up @@
         match exprToContextPath ~inJsxContext e1 with
         | None -> None
         | Some contexPath -> Some (CPObj (contexPath, txt)))
+      | Pexp_index (e1, _) -> (
+        match exprToContextPath ~inJsxContext e1 with
+        | None -> None
+        | Some contextPath -> Some (CPIndex contextPath))
       | Pexp_apply
           {
             funct =
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -60,7 +60,7 @@ let inlay ~path ~pos ~maxLength ~debug = @@
                ( Pexp_constant _ | Pexp_tuple _ | Pexp_record _ | Pexp_variant _
                | Pexp_apply _ | Pexp_match _ | Pexp_construct _ | Pexp_ifthenelse _
                | Pexp_array _ | Pexp_ident _ | Pexp_try _ | Pexp_send _
-               | Pexp_field _ | Pexp_open _
+               | Pexp_index _ | Pexp_field _ | Pexp_open _
                | Pexp_fun {arity = Some _} );
            };
         } ->
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -632,6 +632,7 @@ module Completable = struct @@
                 (** Whether this field access was found in a JSX context. *)
           }
         | CPObj of contextPath * string
+        | CPIndex of contextPath
         | CPAwait of contextPath
         | CPPipe of {
             synthetic: bool;  (** Whether this pipe completion is synthetic. *)
@@ Expand Down Expand Up / @@ -718,6 +719,7 @@ module Completable = struct @@
         | CPField {contextPath = cp; fieldName = s} ->
           contextPathToString cp ^ "." ^ str s
         | CPObj (cp, s) -> contextPathToString cp ^ "[\"" ^ s ^ "\"]"
+        | CPIndex cp -> contextPathToString cp ^ "[]"
         | CPPipe {contextPath; id; inJsx} ->
           contextPathToString contextPath
           ^ "->" ^ id
@@ Expand Down @@

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor array bracket access to dedicated AST node #8168

Diff view

Diff view

There are no files selected for viewing

nojaf Feb 23, 2026

Uh oh!

shulhi Feb 23, 2026

Uh oh!

nojaf Feb 23, 2026

Uh oh!

shulhi Feb 23, 2026

Uh oh!

nojaf Feb 23, 2026

Uh oh!

Uh oh!

Uh oh!

-Original file line number
+Diff line change
@@ Expand Up / @@ -95,6 +95,8 @@ let identifyPexp pexp = @@
       | Pexp_record _ -> "Pexp_record"
       | Pexp_field _ -> "Pexp_field"
       | Pexp_setfield _ -> "Pexp_setfield"
+      | Pexp_index _ -> "Pexp_index"
+      | Pexp_setindex _ -> "Pexp_setindex"
       | Pexp_array _ -> "Pexp_array"
       | Pexp_ifthenelse _ -> "Pexp_ifthenelse"
       | Pexp_sequence _ -> "Pexp_sequence"
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -350,6 +350,10 @@ module E = struct @@
           field ~loc ~attrs (sub.expr sub e) (map_loc sub lid)
         | Pexp_setfield (e1, lid, e2) ->
           setfield ~loc ~attrs (sub.expr sub e1) (map_loc sub lid) (sub.expr sub e2)
+        | Pexp_index (e1, e2) ->
+          index ~loc ~attrs (sub.expr sub e1) (sub.expr sub e2)
+        | Pexp_setindex (e1, e2, e3) ->
+          setindex ~loc ~attrs (sub.expr sub e1) (sub.expr sub e2) (sub.expr sub e3)
         | Pexp_array el -> array ~loc ~attrs (List.map (sub.expr sub) el)
         | Pexp_ifthenelse (e1, e2, e3) ->
           ifthenelse ~loc ~attrs (sub.expr sub e1) (sub.expr sub e2)
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -174,6 +174,8 @@ module Exp = struct @@
       let record ?loc ?attrs a b = mk ?loc ?attrs (Pexp_record (a, b))
       let field ?loc ?attrs a b = mk ?loc ?attrs (Pexp_field (a, b))
       let setfield ?loc ?attrs a b c = mk ?loc ?attrs (Pexp_setfield (a, b, c))
+      let index ?loc ?attrs a b = mk ?loc ?attrs (Pexp_index (a, b))
+      let setindex ?loc ?attrs a b c = mk ?loc ?attrs (Pexp_setindex (a, b, c))
       let array ?loc ?attrs a = mk ?loc ?attrs (Pexp_array a)
       let ifthenelse ?loc ?attrs a b c = mk ?loc ?attrs (Pexp_ifthenelse (a, b, c))
       let sequence ?loc ?attrs a b = mk ?loc ?attrs (Pexp_sequence (a, b))
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -163,6 +163,14 @@ module Exp : sig @@
       val field : ?loc:loc -> ?attrs:attrs -> expression -> lid -> expression
       val setfield :
         ?loc:loc -> ?attrs:attrs -> expression -> lid -> expression -> expression
+      val index : ?loc:loc -> ?attrs:attrs -> expression -> expression -> expression
+      val setindex :
+        ?loc:loc ->
+        ?attrs:attrs ->
+        expression ->
+        expression ->
+        expression ->
+        expression
       val array : ?loc:loc -> ?attrs:attrs -> expression list -> expression
       val ifthenelse :
         ?loc:loc ->
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -439,6 +439,24 @@ module E = struct @@
           field ~loc ~attrs (sub.expr sub e) (map_loc sub lid)
         | Pexp_setfield (e1, lid, e2) ->
           setfield ~loc ~attrs (sub.expr sub e1) (map_loc sub lid) (sub.expr sub e2)
+        | Pexp_index (e1, e2) ->
+          (* Map back to Array.get for parsetree0 compatibility *)
+          let container = sub.expr sub e1 in
+          let index = sub.expr sub e2 in
+          let array_get =
+            ident ~loc (mknoloc (Longident.Ldot (Longident.Lident "Array", "get")))
+          in
+          apply ~loc ~attrs array_get [(Nolabel, container); (Nolabel, index)]
+        | Pexp_setindex (e1, e2, e3) ->
+          (* Map back to Array.set for parsetree0 compatibility *)
+          let container = sub.expr sub e1 in
+          let index = sub.expr sub e2 in
+          let value_expr = sub.expr sub e3 in
+          let array_set =
+            ident ~loc (mknoloc (Longident.Ldot (Longident.Lident "Array", "set")))
+          in
+          apply ~loc ~attrs array_set
+            [(Nolabel, container); (Nolabel, index); (Nolabel, value_expr)]
         | Pexp_array el -> array ~loc ~attrs (List.map (sub.expr sub) el)
         | Pexp_ifthenelse (e1, e2, e3) ->
           ifthenelse ~loc ~attrs (sub.expr sub e1) (sub.expr sub e2)
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -248,6 +248,13 @@ let rec add_expr bv exp = @@
         add_expr bv e1;
         add bv fld;
         add_expr bv e2
+      | Pexp_index (e1, e2) ->
+        add_expr bv e1;
+        add_expr bv e2
+      | Pexp_setindex (e1, e2, e3) ->
+        add_expr bv e1;
+        add_expr bv e2;
+        add_expr bv e3
       | Pexp_array el -> List.iter (add_expr bv) el
       | Pexp_ifthenelse (e1, e2, opte3) ->
         add_expr bv e1;
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -279,6 +279,8 @@ and expression_desc = @@
         *)
       | Pexp_field of expression * Longident.t loc (* E.l *)
       | Pexp_setfield of expression * Longident.t loc * expression (* E1.l <- E2 *)
+      | Pexp_index of expression * expression (* E1[E2] *)
+      | Pexp_setindex of expression * expression * expression (* E1[E2] = E3 *)
       | Pexp_array of expression list (* [| E1; ...; En |] *)
       | Pexp_ifthenelse of expression * expression * expression option
         (* if E1 then E2 else E3 *)
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -691,6 +691,11 @@ and expression ctxt f x = @@
         | Pexp_setfield (e1, li, e2) ->
           pp f "@[<2>%a.%a@ <-@ %a@]" (simple_expr ctxt) e1 longident_loc li
             (simple_expr ctxt) e2
+        | Pexp_index (e1, e2) ->
+          pp f "%a.(%a)" (simple_expr ctxt) e1 (expression ctxt) e2
+        | Pexp_setindex (e1, e2, e3) ->
+          pp f "%a.(%a)@ <-@ %a" (simple_expr ctxt) e1 (expression ctxt) e2
+            (simple_expr ctxt) e3
         | Pexp_ifthenelse (e1, e2, eo) ->
           (* @;@[<2>else@ %a@]@] *)
           let fmt : (_, _, _) format =
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -294,6 +294,18 @@ and expression i ppf x = @@
         expression i ppf e1;
         longident_loc i ppf li;
         expression i ppf e2
+      | Pexp_index (e1, e2) ->
+        line i ppf "Pexp_index\n";
+        expression i ppf e1;
+        line i ppf "index:\n";
+        expression i ppf e2
+      | Pexp_setindex (e1, e2, e3) ->
+        line i ppf "Pexp_setindex\n";
+        expression i ppf e1;
+        line i ppf "index:\n";
+        expression i ppf e2;
+        line i ppf "value:\n";
+        expression i ppf e3
       | Pexp_array l ->
         line i ppf "Pexp_array\n";
         list i expression ppf l
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -196,8 +196,8 @@ let rec classify_expression : Typedtree.expression -> sd = @@
         classify_expression e
       | Texp_ident _ | Texp_for _ | Texp_constant _ | Texp_tuple _ | Texp_array _
       | Texp_construct _ | Texp_variant _ | Texp_record _ | Texp_setfield _
-      | Texp_while _ | Texp_pack _ | Texp_function _ | Texp_extension_constructor _
-        ->
+      | Texp_index _ | Texp_setindex _ | Texp_while _ | Texp_pack _
+      | Texp_function _ | Texp_extension_constructor _ ->
         Static
       | Texp_apply {funct = {exp_desc = Texp_ident (_, _, vd)}} when is_ref vd ->
         Static
@@ Expand Down Expand Up @@
             (join (expression env ifso) (option expression env ifnot)))
       | Texp_setfield (e1, _, _, e2) ->
         Use.(join (inspect (expression env e1)) (inspect (expression env e2)))
+      | Texp_index (e1, e2) ->
+        Use.(join (inspect (expression env e1)) (inspect (expression env e2)))
+      | Texp_setindex (e1, e2, e3) ->
+        Use.(
+          join
+            (join (inspect (expression env e1)) (inspect (expression env e2)))
+            (inspect (expression env e3)))
       | Texp_sequence (e1, e2) ->
         Use.(join (discard (expression env e1)) (expression env e2))
       | Texp_while (e1, e2) ->
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -176,6 +176,13 @@ let expr sub {exp_extra; exp_desc; exp_env; _} = @@
       | Texp_setfield (exp1, _, _, exp2) ->
         sub.expr sub exp1;
         sub.expr sub exp2
+      | Texp_index (exp1, exp2) ->
+        sub.expr sub exp1;
+        sub.expr sub exp2
+      | Texp_setindex (exp1, exp2, exp3) ->
+        sub.expr sub exp1;
+        sub.expr sub exp2;
+        sub.expr sub exp3
       | Texp_array list -> List.iter (sub.expr sub) list
       | Texp_ifthenelse (exp1, exp2, expo) ->
         sub.expr sub exp1;
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -232,6 +232,16 @@ let expr sub x = @@
         | Texp_field (exp, lid, ld) -> Texp_field (sub.expr sub exp, lid, ld)
         | Texp_setfield (exp1, lid, ld, exp2) ->
           Texp_setfield (sub.expr sub exp1, lid, ld, sub.expr sub exp2)
+        | Texp_index (exp1, exp2) ->
+          (* Use let bindings to ensure left-to-right evaluation order *)
+          let exp1 = sub.expr sub exp1 in
+          let exp2 = sub.expr sub exp2 in
+          Texp_index (exp1, exp2)
+        | Texp_setindex (exp1, exp2, exp3) ->
+          let exp1 = sub.expr sub exp1 in
+          let exp2 = sub.expr sub exp2 in
+          let exp3 = sub.expr sub exp3 in
+          Texp_setindex (exp1, exp2, exp3)
         | Texp_array list -> Texp_array (List.map (sub.expr sub) list)
         | Texp_ifthenelse (exp1, exp2, expo) ->
           Texp_ifthenelse
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up @@
             Psetfield (lbl.lbl_pos + 1, Lambda.fld_record_extension_set lbl)
         in
         Lprim (access, [transl_exp arg; transl_exp newval], e.exp_loc)
+      | Texp_index (container, index) ->
+        (* Read: translate to Parrayrefu primitive (unsafe array get) *)
+        let container_lambda = transl_exp container in
+        let index_lambda = transl_exp index in
+        Lprim (Parrayrefu, [container_lambda; index_lambda], e.exp_loc)
+      | Texp_setindex (container, index, value) ->
+        (* Write: translate to Parraysetu primitive (unsafe array set) *)
+        let container_lambda = transl_exp container in
+        let index_lambda = transl_exp index in
+        let value_lambda = transl_exp value in
+        Lprim (Parraysetu, [container_lambda; index_lambda; value_lambda], e.exp_loc)
       | Texp_array expr_list ->
         let ll = transl_list expr_list in
         Lprim (Pmakearray Mutable, ll, e.exp_loc)
@@ Expand Down @@

Refactor array bracket access to dedicated AST node #8168

Are you sure you want to change the base?

Refactor array bracket access to dedicated AST node #8168

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

nojaf Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

shulhi Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

nojaf Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

shulhi Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

nojaf Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!