Refactor array bracket access to dedicated AST node

[shulhi]

Refactors array bracket access from Pexp_apply(Array.get/set, ...) to a dedicated Pexp_index AST node. This is an internal refactoring with no user-facing changes.

Changes:

• arr[0] now represented as Pexp_index(arr, 0, None) in AST
• arr[0] = val now represented as Pexp_index(arr, 0, Some(val)) in AST
• Same type checking, same JavaScript output, same behavior

Why:

• Cleaner AST representation
• Sets up infrastructure for future dict support (Dict : Accessing items with bracket, like an array or object #8163)

[nojaf]

Hmm, any reason you went with one new node to represent two different things?
If feel like the optional expression is not the cleanest way to model this.
Would it not make more sense to just split these?
Is this to remain consistent with something?

[pkg-pr-new]

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript-lang/rescript@8168

@rescript/darwin-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-arm64@8168

@rescript/darwin-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-x64@8168

@rescript/linux-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-arm64@8168

@rescript/linux-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-x64@8168

@rescript/runtime

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/runtime@8168

@rescript/win32-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/win32-x64@8168

commit: 0d89561

[shulhi]

Hmm, any reason you went with one new node to represent two different things? If feel like the optional expression is not the cleanest way to model this. Would it not make more sense to just split these? Is this to remain consistent with something?

I had it split initially, but thought it would be better to represent this as a single node to be consistent with record update.

  | Pexp_record of expression record_element list * expression option
    (* { l1=P1; ...; ln=Pn }     (None)
       { E0 with l1=P1; ...; ln=Pn }   (Some E0) *)

but, you could also make a counter argument with Pexp_field vs Pexp_setfield.

Splitting Pexp_index into two shouldn't be too much work if that is more desired.

[cristianoc]

This is not currently changing the parser. Guess wip. Just, it means it's not been tested yet.

[nojaf]

if that is more desired.

It was just my initial unfiltered response. With records it feels different somehow: { myRec with X } the with X is a clear child node. But for assignment in a[0] = x, it is tagged on and thus not quite an optional extension.

[shulhi]

Okay this is probably closer to record construction vs mutable record field assignment (two different nodes) rather than record update.

This is still in draft though, it is still subject to change and I don't mind changing it to two different nodes.

[zth]

I also think 2 separate nodes are cleaner for this.

Great work btw, this will be a good cleanup/enhancement.

[shulhi]

TLDR

This PR introduces Pexp_index and Pexp_setindex AST nodes for bracket access (arr[i] and arr[i] = v), replacing the previous approach of desugaring to Array.get/Array.set function calls at parse time.

Bracket access now has fixed semantics as a language primitive:

• arr[i] returns option<'a> — matching JS behavior where out-of-bounds returns undefined
• arr[i] = v returns unit

This also removes the implicit reliance on module name. The parser previously had a FIXME about this:

(* 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
*)

Even the built-in Array module could be shadowed by module Array = MyModule, making arr[3] call an arbitrary user function. The new AST node makes bracket access independent of any module.

Details

Previous behavior

The parser desugared arr[3] into Array.get(arr, 3) — a function call. The semantics depended on which Array.get was in scope:

Scope	arr[3] resolves to	Return type
Default	Array.get (Stdlib_Array.get)	option<'a>
open Belt	Belt.Array.get	option<'a>
open ImmutableArray	ImmutableArray.Array.get	option<'a>

The return type is the same as before (option<'a>), but bracket access is now a language primitive rather than a module-dependent function call.

GenType impact

Since bracket access is now a language primitive, arr[3] always types the container as array<'a> — it does not use module-specific types even if ImmutableArray or Belt is in scope. For example:

open ImmutableArray
arr[3]

On master, this resolved to ImmutableArray.Array.get(arr, 3), so the type checker saw arr as ImmutableArray.t<'a> and gentype exported it as ReadonlyArray<T1>. With the new AST, arr[3] always types arr as array<'a>, so gentype exports it as T1[] instead.

If ImmutableArray.t or Belt.Array.t types are desired (e.g. for gentype exports), users need to use explicit function calls:

// arr[3] — always array<'a>, gentype sees T1[]
let result = arr[3]

// Explicit call — preserves ImmutableArray.t, gentype sees ReadonlyArray<T1>
let result = ImmutableArray.get(arr, 3)

// Explicit call — preserves Belt types
let result = Belt.Array.get(arr, 3)