fix: reject fractional indices and step in std.slice (#797)#884
Open
He-Pin wants to merge 1 commit into
Open
Conversation
Motivation: std.slice and the [a:b:c] slice operator silently truncated fractional index/end/step arguments (e.g. std.slice([0,1,2], 0.9, 2.9, 1) returned [0, 1]). Official Jsonnet 0.22.0 rejects fractional array indices at manifestation; the maintainer decision on databricks#797 is to apply strict integer semantics to both array and string slicing rather than truncate. Modification: - Util.slice now accepts raw Option[Double] and validates integrality at a single enforcement point, erroring on NaN / Infinity / non-whole values. - The std.slice builtin and the visitSlice operator pass raw doubles through instead of pre-truncating to Int. - Add regression tests: slice_strict_integer (integers, negatives, null, and whole doubles like 2.0 still work) and error.slice_fractional_index (fractional index rejected with a clear message). Result: Fractional slice arguments now fail with "slice <index|end|step> must be an integer, got: <n>". All existing slice tests and the full JVM (Scala 3) test suite pass. References: databricks#797
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
std.sliceand the[a:b:c]slice operator silently truncated fractionalindex/end/steparguments instead of rejecting them:Official Jsonnet 0.22.0 rejects fractional array indices at manifestation (
array index was not integer: 0.9). Per the maintainer decision in #797 ("go with the strict approach — if the function expects an int, reject anything else"), this applies strict integer semantics to both array and string slicing rather than truncating.Modification
Util.slicenow accepts rawOption[Double]and validates integrality at a single enforcement point, erroring onNaN/Infinity/ non-whole values.std.slicebuiltin (SetModule) and thevisitSliceoperator (Evaluator) pass raw doubles through instead of pre-truncating toInt(which hid the fractional part before validation could see it).slice_strict_integer.jsonnet— integers, negatives,null, and whole doubles (2.0) still work, for bothstd.sliceand the[a:b:c]operator.error.slice_fractional_index.jsonnet— fractional index rejected.Result
Full JVM (Scala 3) test suite passes; formatting verified with
checkFormat.Addresses #797.