Skip to content

fix(interpreter): initialize assoc/indexed arrays for local -A/-a#886

Merged
chaliy merged 4 commits intomainfrom
fix/issue-875-assoc-keys-nested-subshell
Mar 30, 2026
Merged

fix(interpreter): initialize assoc/indexed arrays for local -A/-a#886
chaliy merged 4 commits intomainfrom
fix/issue-875-assoc-keys-nested-subshell

Conversation

@chaliy
Copy link
Copy Markdown
Contributor

@chaliy chaliy commented Mar 27, 2026

Summary

  • local -A m followed by m["a"]="1" produced an indexed array instead of associative, because execute_local_builtin only inserted into call_stack.locals without creating the assoc_arrays entry
  • Initialize the appropriate array type when local -A or -a is used without a compound assignment value
  • Fix applies to both function-scope and global-scope paths
  • Add spec tests assoc_local_declare_then_assign and assoc_local_keys_in_cmdsub

Test plan

  • New spec tests pass
  • All 1812 bash spec tests pass (100%)
  • Full cargo test --all-features passes
  • cargo fmt --check and cargo clippy clean

Closes #875

chaliy added 2 commits March 27, 2026 23:36
…hout value

`local -A m` followed by `m["a"]="1"` produced an indexed array instead
of associative because execute_local_builtin only inserted into
call_stack.locals without creating the assoc_arrays entry.

Initialize the appropriate array type when local -A or -a is used
without a compound assignment value, in both function and global paths.

Closes #875
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 local -A/-a declarations so arrays are initialized with the correct type even when declared without a compound assignment, aligning subsequent element assignments and key expansions with bash behavior.

Changes:

  • Initialize assoc_arrays / arrays entries when local -A name / local -a name is used without =(...) (both function-scope and global-scope paths).
  • Add spec coverage for local -A declared first, then populated via element assignments (including keys expansion inside $(...)).

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 Ensures local -A/-a creates the corresponding array container upfront so later name[key]=... writes use the correct array type.
crates/bashkit/tests/spec_cases/bash/assoc-arrays.test.sh Adds spec cases validating local -A declare-then-assign and key expansion captured via command substitution.
Comments suppressed due to low confidence (1)

crates/bashkit/src/interpreter/mod.rs:4514

  • The PR updates local handling for both associative (-A) and indexed (-a) arrays, but the added spec tests only cover the local -A name; name[...]=... path. Please add a regression test for local -a arr declared without a compound assignment (e.g., local -a arr; arr[0]=...; echo ${#arr[@]}) to ensure the new flags.array initialization paths stay covered.
                let key = &arg[bracket + 1..arg.len() - 1];
                let expanded_key = self.expand_variable_or_literal(key);
                let resolved_name = self.resolve_nameref(arr_name).to_string();
                if let Some(arr) = self.assoc_arrays.get_mut(&resolved_name) {
                    arr.remove(&expanded_key);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +191 to +198
### assoc_local_keys_in_cmdsub
myfunc() {
local -A m
m["a"]="1"
m["b"]="2"
result="$(printf '%s\n' "${!m[@]}" | sort)"
echo "result: [$result]"
}
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.

Issue #875’s repro involves associative array key expansion inside a process substitution nested within a command substitution (result="$(... < <(... ${!m[@]} ...))"). The new assoc_local_keys_in_cmdsub test covers command substitution but not the nested process substitution case; consider adding a spec test that matches the reported pattern to prevent regressions in that scenario.

Copilot uses AI. Check for mistakes.
@chaliy chaliy merged commit 342724a into main Mar 30, 2026
27 checks passed
@chaliy chaliy deleted the fix/issue-875-assoc-keys-nested-subshell branch March 30, 2026 17:36
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 key expansion empty inside process substitution nested in command substitution

2 participants