Skip to content

fix(interpreter): expand command substitutions in assoc array keys#883

Merged
chaliy merged 1 commit intomainfrom
fix/issue-872-assoc-key-cmdsub
Mar 30, 2026
Merged

fix(interpreter): expand command substitutions in assoc array keys#883
chaliy merged 1 commit intomainfrom
fix/issue-872-assoc-key-cmdsub

Conversation

@chaliy
Copy link
Copy Markdown
Contributor

@chaliy chaliy commented Mar 27, 2026

Summary

  • 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
  • Add spec test assoc_key_command_substitution

Test plan

  • New spec test assoc_key_command_substitution passes
  • All 1811 bash spec tests pass (100%)
  • Full cargo test --all-features passes
  • cargo fmt --check clean
  • cargo clippy -- -D warnings clean

Closes #872

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +7795 to +7797
/// 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('`') {
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
/// 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("$(") {

Copilot uses AI. Check for mistakes.
@chaliy chaliy force-pushed the fix/issue-872-assoc-key-cmdsub branch from ccb0bbd to 87158ea Compare March 30, 2026 17:00
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
@chaliy chaliy force-pushed the fix/issue-872-assoc-key-cmdsub branch from 144f4bd to f18c976 Compare March 30, 2026 17:37
@chaliy chaliy merged commit b2132b8 into main Mar 30, 2026
27 checks passed
@chaliy chaliy deleted the fix/issue-872-assoc-key-cmdsub branch March 30, 2026 17:44
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.

Associative array keys with command substitutions expand to empty string

2 participants