Fix index out of bounds exception in rescript-editor-analysis codeAction

[mediremi]

When editing switch statements in VS Code, I've sometimes been seeing error messages of the form rescript-editor-analysis codeAction MyFile.res 42 0 42 0 /tmp/xxx.res failed with Fatal error: exception Invalid_argument("index out of bounds").

(Since the temporary files generated by the extension were deleted as soon as this command failed, I could not come up with a simple repo. However, by hitting backspace quickly on the first line of switch statements, I was sometimes able to reproduce this error.)

After enabling backtraces with Printexc.record_backtrace true, I managed to track this down to line 376 of CompletionFrontEnd.completionWithParser1:

Fatal error: exception Invalid_argument("index out of bounds") Raised by primitive operation at Analysis__CompletionFrontEnd.completionWithParser1 in file "analysis/src/CompletionFrontEnd.ml", line 376, characters 40-53 Called from Analysis__Xform.extractTypeFromExpr in file "analysis/src/Xform.ml", lines 7-9

rescript/analysis/src/CompletionFrontEnd.ml

Line 376 in 6f4275c

if offset < String.length text then text.[offset] else '\n'

While we're ensuring that offset < String.length text, there was no check that offset >= 0 and so text.[-1] triggered an index out of bounds exception.

I'm not sure exactly why offset was negative, but I'm guessing it's because loc_start.pos_cnum is initialised as -1 and so Pos.positionToOffset is called with args "..." (line, -1).

rescript/analysis/src/Xform.ml

Lines 7 to 9 in 6f4275c

	expr.Parsetree.pexp_loc
	|> CompletionFrontEnd.findTypeOfExpressionAtLoc ~debug ~path ~currentFile
	~posCursor:(Pos.ofLexing expr.Parsetree.pexp_loc.loc_start)

rescript/analysis/src/Pos.ml

Lines 3 to 4 in 6f4275c

	let ofLexing {Lexing.pos_lnum; pos_cnum; pos_bol} =
	(pos_lnum - 1, pos_cnum - pos_bol)

rescript/analysis/src/Pos.ml

Lines 21 to 26 in 6f4275c

	let positionToOffset text (line, character) =
	match offsetOfLine text line with
	| None -> None
	| Some bol ->
	if bol + character <= String.length text then Some (bol + character)
	else None

rescript/analysis/src/CompletionFrontEnd.ml

Lines 1755 to 1757 in 6f4275c

	match Pos.positionToOffset text posCursor with
	| Some offset ->
	completionWithParser1 ~currentFile ~debug ~offset ~path ~posCursor text

---

In addition to adding bounds checks before using offset for charAtCursor, I've also added them for some offset-related logic further down.

I've also fixed the root of the issue by ensuring that Pos.positionToOffset does not return negative offsets.

[mediremi]

@zth can I get a review for this PR please?

[zth]

Fantastic, this one has haunted us for a while! Thank you! 🙏

[zth]

@mediremi thank you for your thorough PR descriptions as well. Makes reviewing very easy!

[pkg-pr-new]

Open in StackBlitz

rescript

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

@rescript/darwin-x64

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

@rescript/darwin-arm64

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

@rescript/linux-arm64

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

@rescript/win32-x64

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

@rescript/linux-x64

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

commit: 29f2150