diff --git a/.github/instructions/zsh-scripting.instructions.md b/.github/instructions/zsh-scripting.instructions.md index 4c1f8c4bf..637299dcb 100644 --- a/.github/instructions/zsh-scripting.instructions.md +++ b/.github/instructions/zsh-scripting.instructions.md @@ -439,6 +439,44 @@ example, `status` is read-only, and `path` is tied to `PATH`; assignment can fail or change command lookup. Use purpose-specific names such as `command_status` and `candidate_paths`. +### `zsh/parameters/capture-before-localizing` + +- Level: `required` +- Profiles: `standalone-executable`, `startup-file`, `sourced-library`, `autoload-function`, `test-fixture` +- Minimum Zsh: `null` +- Basis: `language-semantics` +- Evidence: `parameters`, `functions` +- Enforcement: `lint`, `human-review` + +When a function localizes a parameter and the intended new value derives from +the caller's value, capture that value before the localizing declaration. A +localizing declaration starts the parameter empty, so an assignment that reads +the same name on its right-hand side truncates itself instead of extending. + +This matters most for tied pairs such as `fpath`/`FPATH`, `path`/`PATH`, +`cdpath`/`CDPATH`, `manpath`/`MANPATH`, and `module_path`/`MODULE_PATH`. +Assigning the scalar localizes the array as well, already populated, which +makes a following array declaration look redundant when it is not: a second +`local` on a parameter already local to the same scope is not a reset, so the +earlier value survives. Removing the scalar as dead silently empties the array. + +```zsh +# Wrong: $fpath is already empty when the assignment reads it. +local +h -a fpath +fpath=( $PLUGIN_DIR $fpath ) + +# Right: capture first, then localize. +local -a caller_fpath +caller_fpath=( $fpath ) +local +h -a fpath +fpath=( $PLUGIN_DIR $caller_fpath ) +``` + +Do not rely on a tied scalar assignment to carry the caller's value across a +later declaration, and do not rely on a second `local` to reset a parameter +already local to the scope. Capture explicitly, so that neither line can be +read as inert and deleted. + ### `zsh/parameters/account-dynamic-scope` - Level: `review` diff --git a/lib/zsh-standard-policy.json b/lib/zsh-standard-policy.json index a26d92402..1b800846b 100644 --- a/lib/zsh-standard-policy.json +++ b/lib/zsh-standard-policy.json @@ -427,6 +427,21 @@ "evidence": ["parameters"], "enforcement": ["lint", "human-review"] }, + { + "id": "zsh/parameters/capture-before-localizing", + "level": "required", + "profiles": [ + "standalone-executable", + "startup-file", + "sourced-library", + "autoload-function", + "test-fixture" + ], + "minimum_zsh": null, + "basis": "language-semantics", + "evidence": ["parameters", "functions"], + "enforcement": ["lint", "human-review"] + }, { "id": "zsh/parameters/account-dynamic-scope", "level": "review", diff --git a/scripts/test_validate_zsh_standard_policy.py b/scripts/test_validate_zsh_standard_policy.py index 173aba5a9..655d75bfe 100644 --- a/scripts/test_validate_zsh_standard_policy.py +++ b/scripts/test_validate_zsh_standard_policy.py @@ -210,6 +210,7 @@ def test_startup_profile_has_exact_rule_membership(self) -> None: "zsh/options/constrain-multios", "zsh/parameters/declare-scope", "zsh/parameters/avoid-special-name-collisions", + "zsh/parameters/capture-before-localizing", "zsh/parameters/account-dynamic-scope", "zsh/arrays/declare-kind", "zsh/arrays/native-indexing", @@ -250,7 +251,7 @@ def test_startup_profile_has_exact_rule_membership(self) -> None: [rule["id"] for rule in matching_rules], expected_rule_ids, ) - self.assertEqual(len(matching_rules), 49) + self.assertEqual(len(matching_rules), 50) for rule in matching_rules: with self.subTest(rule_id=rule["id"]): if rule["id"] == "zsh/completion/preserve-trust-boundaries": @@ -1978,7 +1979,7 @@ def test_repair_2_consumer_parser_outputs_match_frozen_golden(self) -> None: }, "parsed_rules": validator._markdown_rules(instruction), } - self.assertEqual(len(snapshot["rule_blocks"]), 64) + self.assertEqual(len(snapshot["rule_blocks"]), 65) digest = hashlib.sha256( json.dumps( snapshot, @@ -1990,7 +1991,7 @@ def test_repair_2_consumer_parser_outputs_match_frozen_golden(self) -> None: self.assertEqual( digest, - "607f548eb63159678258e5caec4ae63ced8592906d2cb542f57b9ac1239a945b", + "d841ec864632352bc399bb035780e6805200425a4f14b0b700d3e7a9e8f7c59e", msg=( "The frozen golden covers the parsed output of every path in " f"{paths}. Editing any of them changes this digest, which is " diff --git a/scripts/validate-zsh-standard-policy.py b/scripts/validate-zsh-standard-policy.py index fe8ba63e4..e80555694 100644 --- a/scripts/validate-zsh-standard-policy.py +++ b/scripts/validate-zsh-standard-policy.py @@ -153,6 +153,7 @@ "zsh/options/constrain-multios", "zsh/parameters/declare-scope", "zsh/parameters/avoid-special-name-collisions", + "zsh/parameters/capture-before-localizing", "zsh/parameters/account-dynamic-scope", "zsh/arrays/declare-kind", "zsh/arrays/native-indexing", @@ -210,6 +211,7 @@ "zsh/options/constrain-multios", "zsh/parameters/declare-scope", "zsh/parameters/avoid-special-name-collisions", + "zsh/parameters/capture-before-localizing", "zsh/parameters/account-dynamic-scope", "zsh/arrays/declare-kind", "zsh/arrays/native-indexing",