From 198a65ebdab5fcb8c4e56e39339cab109c90909b Mon Sep 17 00:00:00 2001 From: Sal Date: Wed, 2 Sep 2026 23:04:33 +0100 Subject: [PATCH] docs(zsh-standard): require capturing a parameter before localizing it z-shell/zi lost a working search path twice in one function for the same reason, and nothing in the standard covered it. zi#477 removed this as dead: local +h FPATH=$PLUGINS_DIR${fpath_elements:+:...}:$FPATH local +h -a fpath fpath=( $PLUGIN_DIR $fpath_elements $fpath ) It was not. `fpath' and `FPATH' are tied, so the scalar assignment already created the local pair populated with the caller's value, and the `local +h -a fpath' that follows does not reset it because the parameter is by then already local to that scope. Remove the scalar and the array declaration creates a fresh empty array, `$fpath' on the right-hand side expands to nothing, and the assignment silently truncates itself to $PLUGIN_DIR. Immediate `autoload +X' of any function the loading plug-in did not own then stopped resolving. Two properties combine, each individually easy to misread: a second `local' on a parameter already local to the same scope is not a reset, and localizing a tied parameter starts it empty so `x=( new $x )' self-truncates. Together they make a load-bearing line look inert, and deleting it changes behaviour with no error. zsh/parameters/avoid-special-name-collisions already names the path/PATH tie, but only to warn against repurposing the name; nothing covered what the tie does to localization. The same shape applies to path, cdpath, manpath, and module_path. Add zsh/parameters/capture-before-localizing to the parameters section, with the matching object in lib/zsh-standard-policy.json and both ordered inventories in the validator. The frozen consumer-parser golden moves accordingly: the rule count goes from 64 to 65, startup-file membership from 49 to 50, and the digest is replaced as scripts/test_validate_zsh_standard_policy.py instructs. Evidence: z-shell/zi#488, fixed in z-shell/zi#491. Closes #602 --- .../zsh-scripting.instructions.md | 38 +++++++++++++++++++ lib/zsh-standard-policy.json | 15 ++++++++ scripts/test_validate_zsh_standard_policy.py | 7 ++-- scripts/validate-zsh-standard-policy.py | 2 + 4 files changed, 59 insertions(+), 3 deletions(-) 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",