fix(analyzer): resolve push write targets to their root binding for constness (#673)

CHANGELOG.md

@@ -36,6 +36,19 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
   drop.
 
 ### Fixed
+- **`push` onto a constant list is now reported** (#673). `push with ROLES and
+  "guest"` on a `store new constant` list previously ran and mutated the list,
+  because `push` carries its target as an expression rather than a bare name and
+  so escaped the constness check that already covered `add`/`remove`/`clear`
+  (#671). Targets rooted at a constant through an index or member chain
+  (`push with CONFIG[0] and x`) are reported the same way, against the root
+  binding's name. **Compatibility:** a program that pushed onto a constant now
+  exits 3 at analysis instead of running — it was mutating something declared
+  immutable, which the other three statements already refused. Targets with no
+  root binding (a call result, a literal), mutable bindings, parameters and loop
+  variables are unaffected. Writes that reach a constant's contents through a
+  *different* binding — an alias or a loop variable — are still not caught;
+  constants fix the binding, not the contents.
 - **Transaction control SQL sent through `query`/`execute` is now rejected**
   (#664). `BEGIN`, `COMMIT`, `ROLLBACK`, `START TRANSACTION`, `SAVEPOINT` and
   `RELEASE` previously ran on arbitrary pooled connections, so a hand-written

Docs/03-language-basics/variables-and-types.md

@@ -305,24 +305,38 @@ multiply MAX_SIZE by 2        // ERROR: Cannot modify constant 'MAX_SIZE'
 divide MAX_SIZE by 2          // ERROR: Cannot modify constant 'MAX_SIZE'
 ```
 
-The same applies to a constant list — `add ... to`, `remove ... from`, and
-`clear` are all rejected:
+The same applies to a constant list — `add ... to`, `remove ... from`, `clear`,
+and `push with ... and ...` are all rejected:
 
 ```wfl
 store new constant ALLOWED_ROLES as ["admin" and "editor"]
 
 add "guest" to ALLOWED_ROLES  // ERROR: Cannot modify constant 'ALLOWED_ROLES'
 clear ALLOWED_ROLES           // ERROR: Cannot modify constant 'ALLOWED_ROLES'
+push with ALLOWED_ROLES and "guest"  // ERROR: Cannot modify constant 'ALLOWED_ROLES'
 ```
 
-> **Known gap:** `push with <list> and <value>` is **not** yet checked. Pushing
-> onto a constant list currently succeeds silently, at both check time and run
-> time — see [issue #673](https://github.com/WebFirstLanguage/wfl/issues/673).
-> Use `add ... to` when you want the constant to be enforced.
+A push into an element is rejected too when the target is written *through the
+constant's own name* — the report names the binding the write reaches:
+
+```wfl
+store new constant ROLE_GROUPS as [["admin"] and ["editor"]]
+
+push with ROLE_GROUPS[0] and "guest"  // ERROR: Cannot modify constant 'ROLE_GROUPS'
+```
+
+> Constants fix the *binding*, not the contents reached through it. These checks
+> are made by reading the code, so they catch writes spelled through the
+> constant's name. A write that reaches the same list through a *different*
+> name is not caught, and does change the underlying list — whether that name
+> comes from an alias (`store alias as ALLOWED_ROLES`) or from a loop variable:
 >
-> Constants also fix the *binding*, not the contents reached through it. Binding
-> a constant list to a mutable name (`store alias as ALLOWED_ROLES`) and mutating
-> the alias changes the underlying list.
+> ```wfl
+> store new constant ROLE_GROUPS as [["admin"]]
+> for each group in ROLE_GROUPS:
+>     push with group and "guest"   // runs; ROLE_GROUPS is changed
+> end for
+> ```
 
 **Best practice:** Use uppercase names for constants so a reader can see at a
 glance that a value is fixed:

Docs/06-best-practices/naming-conventions.md

@@ -208,9 +208,10 @@ end
 ## Constants
 
 Declare a real constant with `store new constant` — WFL then rejects `change`,
-`add`, `subtract`, `multiply`, `divide`, `remove`, and `clear` against it (see
+`add`, `subtract`, `multiply`, `divide`, `remove`, `clear`, and `push` against
+it (see
 [Variables and Types — Constants](../03-language-basics/variables-and-types.md#constants)
-for the one form that is not yet checked) — and **use SCREAMING_SNAKE_CASE** so
+for what a constant does and does not fix) — and **use SCREAMING_SNAKE_CASE** so
 the name itself says “do not reassign”:
 
 ```wfl

History/dev-diary/2026/2026-08-14-issue-673-push-write-target-constness.md

@@ -0,0 +1,135 @@
+# Dev Diary — 2026-08-14: Resolving `push` write targets to their root binding (#673)
+
+## Context
+
+#671 closed the analyzer's blind spot for the bare-name mutation statements
+(`add ... to`, `remove ... from`, `clear`). Its own diary entry named the piece
+it deliberately left open, and filed it as #673:
+
+```wfl
+store new constant ROLES as ["admin"]
+push with ROLES and "guest"
+display ROLES                    // [admin, guest], exit 0
+```
+
+No analyzer report, no runtime error. Nested targets escaped the same way:
+
+```wfl
+store new constant CONFIG as [["a"]]
+push with CONFIG[0] and "b"
+display CONFIG                   // [[a, b]], exit 0
+```
+
+## Root cause
+
+`add`/`remove`/`clear` each carry their target as a `list_name: String`, which
+`report_constant_mutation` resolves directly. `push` does not:
+`Statement::PushStatement` carries `list: Expression` (`src/parser/ast.rs`),
+because `parse_push_statement` parses a whole primary expression for the target.
+The analyzer's arm therefore had nothing to look up and merely walked both
+sub-expressions.
+
+There is no runtime backstop either. The interpreter mutates in place through
+the list's `Rc<RefCell<Vec<Value>>>` and never reassigns the binding, so
+`Environment::assign` — the only place constness is enforced at run time — is
+never reached. Exactly the constant-*list* hole #671 described, arriving through
+a statement shape its fix could not see.
+
+## What changed
+
+One analyzer-only change (`src/analyzer/mod.rs`). A new pure helper,
+`write_target_root_binding`, resolves a write-target expression to the binding
+the write ultimately reaches:
+
+- `Expression::Variable` → itself.
+- `IndexAccess` / `MemberAccess` / `PropertyAccess` → recurse into the
+  collection or object, so `CONFIG[0]`, `CONFIG[0][1]`, and `CONFIG.entries`
+  all resolve to `CONFIG`. Mutating an element mutates the collection the
+  binding names.
+- Anything else (a call result, a literal) → `None`. There is no binding whose
+  constness could be violated, so nothing is reported.
+
+The `PushStatement` arm feeds that root name to the existing
+`report_constant_mutation`, so `push` now emits exactly the message
+`add`/`remove`/`clear` already emit:
+
+```
+Cannot modify constant 'ROLES' - constants are immutable once defined
+```
+
+The constness test itself is unchanged — it is still `constant_bindings`
+membership, not `mutable: false`, so the immutable-but-not-constant symbols
+(action and container-method parameters, loop variables, REPL parent-scope
+bindings) stay untouched. The missing piece was only the root-binding walk.
+
+Undefined names still produce exactly one diagnostic: `analyze_expression(list)`
+reports `Variable '<name>' is not defined`, and an unresolvable name has no
+binding key, so `report_constant_mutation` stays silent.
+
+Nothing in `src/interpreter/`, `src/parser/`, or `src/typechecker/` was touched.
+
+## Explicitly out of scope
+
+The aliasing hole the issue also documents:
+
+```wfl
+store new constant ROLES as ["admin"]
+store alias as ROLES
+add "guest" to alias     // still mutates ROLES, exit 0
+```
+
+`alias` is a legitimately mutable binding, so no write-target analysis can
+reject this. What `constant` means for a reference value is a language-design
+question, and the issue says plainly it must not be folded into this fix. The
+constants documentation continues to state the limitation outright.
+
+## Compatibility
+
+Risk class **R2** — analyzer diagnostics gate whether a program runs at all
+(`wfl <file>` exits 3 on an analysis error), so this is public CLI/contract
+behavior, and pushing onto a constant list moves from "silently allowed" to
+"rejected at analysis time".
+
+That is the documented meaning of `constant`, and nothing shipped relies on the
+old behavior: sweeping every gated `TestPrograms/` and `examples/` program
+through `--analyze` with the release binary produced **zero** new
+`Cannot modify constant` reports. The only source that pushes onto a constant is
+the docs error-example written to demonstrate the diagnostic.
+
+## Testing
+
+Added to `tests/constant_mutation_analyzer_test.rs` as a **test-only Red
+commit** before the fix:
+
+| Test | Red | Green |
+|---|---|---|
+| `push_onto_a_constant_list_is_rejected` | 0 reports, expected 1 | pass |
+| `push_onto_an_indexed_constant_target_names_the_root_binding` | 0 reports, expected 1 | pass |
+| `push_targets_without_a_root_binding_are_accepted` | pass | pass |
+| `push_onto_a_mutable_list_is_accepted` | pass | pass |
+| `push_onto_a_parameter_or_loop_variable_is_accepted` | pass | pass |
+
+The last three pass in both states on purpose: they are the negative assertions
+that the new walk does not over-report, and they guard the same shapes #671's
+over-reporting bug hit. The mutable case covers both a bare and an indexed
+target; the no-root-binding case covers both a call result and a literal.
+
+Also run: `cargo test --all --no-fail-fast`, `cargo clippy --all-targets
+--all-features -- -D warnings`, `cargo fmt --all`,
+`python scripts/validate_docs_examples.py` (25/25), and the `--analyze` sweep
+described above.
+
+## Documentation
+
+`Docs/03-language-basics/variables-and-types.md` carried a **Known gap**
+callout saying `push` was not checked and telling readers to "use `add ... to`
+when you want the constant to be enforced". That claim is now false, so it is
+gone; the constant-list section lists `push` alongside `add`/`remove`/`clear`
+and adds the indexed-target case. The alias caveat below it stays — it is still
+true. `Docs/06-best-practices/naming-conventions.md` had the matching "the one
+form that is not yet checked" pointer and now names `push` as rejected.
+
+`TestPrograms/docs_examples/basic_syntax/constants_immutable_01.wfl` carried a
+comment explaining that `push` was "deliberately absent"; it now exercises both
+the bare and indexed push forms (10 reports, up from 8), with its manifest
+`doc_purpose` updated to match.

TestPrograms/docs_examples/_meta/manifest.json

@@ -219,7 +219,7 @@
       "immutability",
       "error-example"
     ],
-    "doc_purpose": "Every mutation form of a constant (change/add/subtract/multiply/divide, and add/remove/clear on a constant list) is rejected during semantic analysis (issue #671)"
+    "doc_purpose": "Every mutation form of a constant (change/add/subtract/multiply/divide, and add/remove/clear/push on a constant list, including an indexed push target) is rejected during semantic analysis (issues #671, #673)"
   },
   "docs_examples/basic_syntax/operators_01.wfl": {
     "doc_section": "Docs/03-language-basics/operators-and-expressions.md",

TestPrograms/docs_examples/basic_syntax/constants_immutable_01.wfl

@@ -1,8 +1,6 @@
 // CI-SKIP: intentional error example — must fail semantic analysis (asserted by validate_docs_examples.py)
 // Constants: the mutation forms that are rejected before the program runs
 // Source: Docs/03-language-basics/variables-and-types.md#constants
-// `push with <list> and <value>` is deliberately absent — it is not yet
-// checked against constants (issue #673).
 
 store new constant MAX_SIZE as 100
 
@@ -17,3 +15,10 @@ store new constant ALLOWED_ROLES as ["admin" and "editor"]
 add "guest" to ALLOWED_ROLES  // ERROR: Cannot modify constant 'ALLOWED_ROLES'
 remove "editor" from ALLOWED_ROLES  // ERROR: Cannot modify constant 'ALLOWED_ROLES'
 clear ALLOWED_ROLES           // ERROR: Cannot modify constant 'ALLOWED_ROLES'
+push with ALLOWED_ROLES and "guest"  // ERROR: Cannot modify constant 'ALLOWED_ROLES'
+
+// A nested push target is still a write through the constant binding, so it is
+// reported against the name the write reaches (issue #673).
+store new constant ROLE_GROUPS as [["admin"]]
+
+push with ROLE_GROUPS[0] and "guest"  // ERROR: Cannot modify constant 'ROLE_GROUPS'

src/analyzer/mod.rs

@@ -2415,9 +2415,22 @@ impl Analyzer {
                 }
             }
 
-            Statement::PushStatement { list, value, .. } => {
+            Statement::PushStatement {
+                list,
+                value,
+                line,
+                column,
+            } => {
                 self.analyze_expression(list);
                 self.analyze_expression(value);
+                // #673: unlike `add`/`remove`/`clear`, this statement carries its
+                // target as an expression rather than a bare name, so resolve the
+                // binding the write actually reaches before the constness check.
+                // A target with no root binding (a call result, a literal) has
+                // nothing to check and reports nothing.
+                if let Some(root) = Self::write_target_root_binding(list) {
+                    self.report_constant_mutation(root, *line, *column);
+                }
             }
 
             Statement::MapCreation {
@@ -4742,6 +4755,33 @@ impl Analyzer {
         }
     }
 
+    /// Resolve a write-target expression to the name of the binding the write
+    /// ultimately reaches, or `None` when it reaches no binding at all.
+    ///
+    /// A bare `ROLES` resolves to itself. Index and member chains resolve to
+    /// whatever they bottom out in — `CONFIG[0]`, `CONFIG[0][1]`, and
+    /// `CONFIG.entries` all resolve to `CONFIG` — because mutating an element
+    /// mutates the collection the binding names. Anything rooted in a computed
+    /// value (a call result, a literal) resolves to `None`: there is no binding
+    /// whose constness could be violated, so the caller reports nothing.
+    ///
+    /// Needed by `push` (#673), whose `Statement::PushStatement` carries its
+    /// target as an `Expression`; the bare-name mutation statements of #671
+    /// carry a `list_name: String` and go straight to
+    /// [`Self::report_constant_mutation`].
+    fn write_target_root_binding(expression: &Expression) -> Option<&str> {
+        match expression {
+            Expression::Variable(name, _, _) => Some(name),
+            Expression::IndexAccess { collection, .. } => {
+                Self::write_target_root_binding(collection)
+            }
+            Expression::MemberAccess { object, .. } | Expression::PropertyAccess { object, .. } => {
+                Self::write_target_root_binding(object)
+            }
+            _ => None,
+        }
+    }
+
     fn report_constant_mutation(&mut self, name: &str, line: usize, column: usize) {
         let is_constant = self
             .get_symbol_binding_key(name)