fix(interpreter): expand command substitutions in assoc array keys#883
fix(interpreter): expand command substitutions in assoc array keys#883
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a bash-compatibility bug in Bashkit’s interpreter where associative array assignments using command substitution in the subscript (e.g. m["$(cmd)"]=...) would incorrectly resolve to an empty key instead of the expanded command output.
Changes:
- Add async associative-key expansion (
expand_assoc_key) and use it for associative array assignments. - Add a new bash spec case covering command substitution in associative array keys.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
crates/bashkit/src/interpreter/mod.rs |
Expands associative array assignment keys via full word expansion when command substitution is present. |
crates/bashkit/tests/spec_cases/bash/assoc-arrays.test.sh |
Adds a spec test asserting correct key expansion for m["$(echo hello)"]="world". |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// Falls back to `expand_variable_or_literal` for keys without `$(` or backtick. | ||
| async fn expand_assoc_key(&mut self, s: &str) -> Result<String> { | ||
| if s.contains("$(") || s.contains('`') { |
There was a problem hiding this comment.
In expand_assoc_key, the s.contains('')check looks redundant/dead for assignment subscripts because the lexer rewrites backtick substitutions to$(` during tokenization. Consider dropping the backtick condition (and adjusting the docstring accordingly) to avoid suggesting that the word parser here understands raw backticks.
| /// Falls back to `expand_variable_or_literal` for keys without `$(` or backtick. | |
| async fn expand_assoc_key(&mut self, s: &str) -> Result<String> { | |
| if s.contains("$(") || s.contains('`') { | |
| /// Falls back to `expand_variable_or_literal` for keys without `$(`. | |
| async fn expand_assoc_key(&mut self, s: &str) -> Result<String> { | |
| if s.contains("$(") { |
ccb0bbd to
87158ea
Compare
Associative array assignments where the key is a command substitution (e.g. m["$(echo hello)"]="world") silently produced an empty key. Add async expand_assoc_key() that parses the subscript as a full Word and expands it with expand_word() when it contains $( or backtick. Closes #872
144f4bd to
f18c976
Compare
Summary
m["$(echo hello)"]="world") silently produced an empty keyexpand_assoc_key()that parses the subscript as a full Word and expands it withexpand_word()when it contains$(or backtickassoc_key_command_substitutionTest plan
assoc_key_command_substitutionpassescargo test --all-featurespassescargo fmt --checkcleancargo clippy -- -D warningscleanCloses #872