hotfix: fix uniqby regression tests broken on main#207
Merged
Conversation
PR #195 merged with regression_uniqby tests broken on main: 1. parse_cli_arg treated each comma-split chunk of a bracketed list as raw text, so `["apple","ant"]` became a list of strings with the literal double quotes still attached. uniqby then keyed every entry by `"` (the first char of the quoted form), kept just the first element, and rendered as `["apple"]`. Fix: strip surrounding `"..."` in parse_cli_arg and split on top-level commas only (respecting quotes and nested brackets) so list elements parse the way they read. 2. PARITY_SRC used a multi-line `par` body that the parser does not accept (bare expression after a newline triggers ILO-P001 expected declaration). Rewritten as a single-line prefix ternary, which is the canonical ilo idiom: `par n:n>t;?=(mod n 2) 0 "even" "odd"`. Example expected outputs (rsrt, sort-by-key, multiline-fn) were written around the old buggy behaviour and have been updated to reflect the correct rendering with quotes stripped. cargo test --features cranelift: all green.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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
1 participant
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.
Summary
PR #195 (uniqby) merged with
tests/regression_uniqby.rsfailing on main. Two distinct issues:CLI arg parsing kept quotes inside list literals.
parse_cli_argdid a naivesplit(',')of bracketed input, so["apple","ant"]became[Text("\"apple\""), Text("\"ant\"")], i.e. each element still had its surrounding double quotes.uniqby(usinghd sas the key) then keyed every element by", kept only the first, and rendered as["apple"]. Fix: strip surrounding"..."from string args and split on top-level commas only (quote-aware + bracket-aware) so nested lists and strings containing commas survive.PARITY_SRC used a multi-line function body the parser rejects. A bare expression on its own line after the conditional arm triggered
ILO-P001 expected declaration, got Text("odd"). Rewritten as a single-line prefix ternary, which is the canonical ilo idiom for this shape:par n:n>t;?=(mod n 2) 0 "even" "odd"Three example
.ilofiles (rsrt.ilo,sort-by-key.ilo,multiline-fn.ilo) had-- out:expectations written around the old buggy rendering (literal quotes preserved on string args). Updated to match the correct behaviour.The uniqby tree-walker arm itself in
src/interpreter/mod.rswas correct - the bug was upstream in CLI arg parsing. No interpreter changes needed.This unblocks downstream PRs.
Test plan
cargo test --features cranelift --test regression_uniqby- 6/6 passcargo test --features cranelift- full suite green (no regressions)cargo fmt --all -- --check- cleancargo clippy --all-targets --features cranelift -- -D warnings- clean