Skip to content

fix: std.removeAt errors on invalid index instead of silently returning original array#946

Merged
stephenamar-db merged 2 commits into
databricks:masterfrom
He-Pin:fix/removeAt-bounds-check
Jun 18, 2026
Merged

fix: std.removeAt errors on invalid index instead of silently returning original array#946
stephenamar-db merged 2 commits into
databricks:masterfrom
He-Pin:fix/removeAt-bounds-check

Conversation

@He-Pin

@He-Pin He-Pin commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Motivation

std.removeAt silently returned the original array when given an invalid index (negative, out-of-bounds, or non-integer), instead of producing a clear error. go-jsonnet crashes with an internal error for negative indices; jrsonnet errors on non-integer indices but silently ignores negative ones.

Modification

  • Added bounds checking: negative and out-of-range indices now error with "idx out of bounds"
  • Added integrality check: non-integer indices error with "idx must be an integer"
  • Added directional tests for valid and invalid indices

Result

std.removeAt([1,2,3], -1) now produces a clear "idx out of bounds" error instead of silently returning the original array.

References

Expression go-jsonnet v0.22.0 jrsonnet v0.5.0-pre98 sjsonnet (before) sjsonnet (after)
std.removeAt([1,2,3], 1) [1,3] [1,3] [1,3] [1,3]
std.removeAt([1,2,3], -1) CRASH (internal error) [1,2,3] (wrong) [1,2,3] (wrong) ❌ ERROR: idx out of bounds ✅
std.removeAt([1,2,3], 1.5) ERROR: Expected an integer ERROR: cannot convert [1,2,3] (wrong) ❌ ERROR: idx must be an integer ✅
std.removeAt([1,2,3], 10) CRASH (internal error) [1,2,3] (wrong) [1,2,3] (wrong) ❌ ERROR: idx out of bounds ✅

@stephenamar-db

Copy link
Copy Markdown
Collaborator

rebase

@He-Pin He-Pin force-pushed the fix/removeAt-bounds-check branch from 1bb3882 to 24efc79 Compare June 17, 2026 19:17
…ng original array

Motivation:
std.removeAt silently returned the original array when given an
out-of-bounds or negative index, masking bugs. go-jsonnet errors
on invalid indices, which is the correct behavior.

Modification:
- Validate that idx is a number, an integer, and within bounds
- Error with clear messages: "idx must be a number", "idx must be
  an integer", or "idx out of bounds"
- Remove the silent fallback to return the original array

Result:
std.removeAt now errors on invalid indices, matching go-jsonnet
behavior and helping users catch bugs earlier.
@He-Pin He-Pin force-pushed the fix/removeAt-bounds-check branch from 24efc79 to d1481d9 Compare June 17, 2026 23:56
Motivation:
The error formatting infrastructure already prepends [std.removeAt]
when errors originate from the removeAt builtin. The explicit
"std.removeAt:" prefix in error messages produced doubled prefixes:
"[std.removeAt] std.removeAt: idx out of bounds".

Modification:
Remove the "std.removeAt:" prefix from all three error messages in
the removeAt builtin. Update golden files accordingly.

Result:
Error messages now read "[std.removeAt] idx out of bounds" instead
of "[std.removeAt] std.removeAt: idx out of bounds", matching the
style used by other stdlib functions.
@stephenamar-db stephenamar-db merged commit adce175 into databricks:master Jun 18, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants