Repository navigation
fix(jq): indices/index/rindex on null or an object look the key up (#3890) - #3952
Conversation
|
Review pass on 53abf42 (fresh-context Fixed in 945ac79
Not a regression (verified against the base binary) Re-verified after the rewrite: by-value matrix vs jq 1.7.1, 630 rows, 0 differences; sweep over the eight operands is re-running on the final binary. |
CoverageTotal: 95.03% ⚪ 0 pp vs Comparing No per-file coverage changes vs 🔇 0 ignored region(s), 409 tolerated region(s)
Excluded by ignore-filename-regex: 1 file (none of them touched by this diff). Patch coveragePatch: 98.48% (65/66 new lines covered)
Uncovered new lines (1)
Indirect coverage changes🔴 0 lines lost coverage, 🟢 1 lines gained coverage on unchanged code. Indirect changes
|
CoverageTotal: 94.94% ⚪ 0 pp vs Comparing No per-file coverage changes vs 🔇 0 ignored region(s), 410 tolerated region(s)
Excluded by ignore-filename-regex: 1 file (none of them touched by this diff). Patch coveragePatch: 98.48% (65/66 new lines covered)
Uncovered new lines (1)
Indirect coverage changes🔴 0 lines lost coverage, 🟢 1 lines gained coverage on unchanged code. Indirect changes
|
…3890) jq defines `indices($i)` as `.[$i]` for every input but an array and a string, and `index`/`rindex` as that followed by `.[0]` / `.[-1:][0]`. `unsearchable_input` answered `null` for any key on `null` or an object, on the strength of a comment that called it `_strindices`'s answer; that held for the `null` and `{}` it was probed with and nothing else. `null | indices(true)` answered where jq raises `Cannot index null with boolean`, `{"a":1} | indices("a")` answered `null` where jq answers `1`, and `{"a":[5,6]} | rindex("a")` answered `null` where jq answers `6`. In `path()` the wrong `null` was the root's own, so `[path(indices(true))]` on `null` answered `[[]]` and `del`, `=` and `|=` ran through it: these are the 210 ACCEPT_WRONG rows the path-register sweep reported for the eight indices operands, now 0. `unsearchable_input` is now `index_one` (jq's `.[$i]`, lazy) followed, for `index` and `rindex`, by jq's own tail through the evaluator. A tracked array input (`path(.a | indices(1))` is `["a",[1]]`), a tracked object key and a tracked string's `index`/`rindex` still refuse, in the safe direction; #3890 stays open for them. Sweep, base vs candidate over the eight operands (106,587 rows): ACCEPT_WRONG 210 -> 0, MATCH +652, REFUSE_WRONG 1712 -> 1280, DIFF 20 -> 10; 14 rows matched jq only because the wrong `null` short-circuited an `and`, and now refuse (exit 5, no write), disclosed in docs/compliance/jq/limitations.md.
…lazy tail (#3890) - the lookup and the `.[0]` / `.[-1:][0]` tail run with JqSemantics, not the caller's: `indices` is jq surface even where yq reaches it (`--jq-extensions`, which follows jq, ADR-0018), and yq's lenient indexing answered `null` for `a: [1, 2]` | `indices(1)` (jq, and the base commit: `Cannot index object with number`) and dropped `.[0]` of a scalar to no output where jq raises - the tail is applied with `eval_single` on the looked-up document value, so a container is no longer read in full just to take one element of it, and an owned result keeps every output instead of `Ok(_) => None` silently dropping a non-single count - tests: yq-extensions rows (new), object and array patterns on `null`/an object, and the 14-row lost-match shape pinned as a residual beside its `.a[0]` contrast
…n why (#3890) Patch coverage on the PR read 86.67% (39/45): the scalar `optional` branch and the `OneCursor`/`Owned` arms of `unsearchable_input`. A probe in all three, run over the whole suite and the CLI forms (`?`, `try`, `[.[]?|...?]`, yq), fired none of them, and reading `index_one` shows why: a cursor and a computed result come only from a slice or a subarray search of an *array*, and the lookup here is on `null` or an object, which answers one document value or an error. So the arms go (a dead arm implies behaviour no test can check), the scalar branch is the existing `suppress_or_raise`, and `index_one_on_null_or_object_answers_a_value_or_an_error_3890` pins the contract over ten key kinds on `null` and an object -- with an array target and an array key as the control that makes its classification able to fail -- so a change that makes `index_one` answer a cursor fails that test instead of silently skipping the tail.
f74f361 to
bc953ee
Compare
Part of #3890 (does not close it; see "Still open").
Summary
jq defines
indices($i)as.[$i]for every input but an array and a string, andindex/rindexas that followed by.[0]/.[-1:][0].unsearchable_inputanswerednullfor any key onnullor an object, on the strength of a comment calling it_strindices's answer. That held for thenulland{}it was probed with and nothing else:null | indices(true)Cannot index null with booleannull{"a":1} | indices("a")1null{"a":[5,6]} | rindex("a")6null{"a":1} | index("a")Cannot index number with numbernullIn
path()the wrongnullwas the root's own, so[path(indices(true))]onnullanswered[[]]anddel,=and|=ran through it. Those are the 210ACCEPT_WRONGrows #3890's comments cite for theindices/index/rindexoperands. They are 0 now.unsearchable_inputis nowindex_one(jq's.[$i], lazy), followed forindex/rindexby jq's own tail through the evaluator. Under--jq-extensionsthe YAML route follows jq too (verified:a: [5, 6]givesindex("a")=5, wasnull).Still open (why this is a part, not the close)
Tracked array input (
path(.a | indices(1))is["a",[1]]), a tracked object key (["a","x"]) and a tracked string'sindex/rindexstill refuse, in the safe direction. They need the register-moving.[$i]step (thefirst/lastarm's shape plus its classification predicates), a larger change that wants its own sweep. #3890 stays open for it.Sweep (
scripts/jq-path-register-sweep.py, base vs candidate, the 8 operands, 106,587 rows)The script reports
FAIL: 14 regression(s). All 14 are one shape and are disclosed indocs/compliance/jq/limitations.md:index("a")/rindex("a")on{"a":[true]}was the wrongnull, sodel((index("a") and (.a)?) // .a)(and the.a? as $y | try (index("a") and .a) | ...shape) short-circuitedandand landed on jq's answer by another road. With jq's owntruethe right operand runs against a register the resolver does not yet move forindex, so it refuses (exit 5, nothing written). The same programs spelled.a[0]still match in base and candidate (checked directly). These close with the tracked-navigation step above.This is user-visible, so the changelog check is not waived:
changelog.d/3890.fixed.mdis included.Test plan
/usr/bin/jq: 0 differences after, thenull/object rows differed before.test_indices_family_looks_up_a_null_or_object_input_3890: 36 rows captured live from jq 1.7.1 (values, errors,?forms, path/del/assign).test_string_search_unsearchable_inputsextended and its wrong rationale corrected.cargo test --features cli,simd,regex,serde --no-fail-fast: 69 binaries, 10,226 passed, 0 failed.cargo clippy --all-targets --all-features -D warnings;cargo clippy --all-targets --features std,simd,serde,cli,regex,bench-runner,large-tests,mmap-tests -D warnings;cargo check --no-default-features;RUSTDOCFLAGS=-D warnings cargo doc;cargo fmt --check.python3 scripts/changelog.py check.