From a652f83b638be95784ee0e02289f88aa9adde58b Mon Sep 17 00:00:00 2001
From: Corey Johnson <probablycorey@gmail.com>
Date: Fri, 17 Oct 2025 10:46:52 -0700
Subject: [PATCH] refactor(parser): move pendingIdentifiers and isInParams into
 Scope class
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Replace module-level mutable state with immutable state managed within the Scope
class itself. This eliminates state leakage between parser invocations and makes
the code more functional and predictable.

Changes:
- Add pendingIdentifiers and isInParams as Scope constructor parameters
- Add helper methods: withPendingIdentifiers(), withIsInParams(), clearPending()
- Update hash() to include new state fields
- Convert all mutable state operations to return new Scope instances
- Remove module-level variables entirely

Benefits:
- No state leakage between tests or parser invocations
- Easier to reason about - state is explicit in the context
- More functional programming style with immutable updates
- Eliminates entire class of bugs related to stale module state

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
---
 src/parser/scopeTracker.ts | 66 +++++++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 29 deletions(-)

diff --git a/src/parser/scopeTracker.ts b/src/parser/scopeTracker.ts
index 3ac9921..3ba0044 100644
--- a/src/parser/scopeTracker.ts
+++ b/src/parser/scopeTracker.ts
@@ -2,7 +2,12 @@ import { ContextTracker } from '@lezer/lr'
 import * as terms from './shrimp.terms'
 
 export class Scope {
-  constructor(public parent: Scope | null, public vars: Set<string>) {}
+  constructor(
+    public parent: Scope | null,
+    public vars: Set<string>,
+    public pendingIdentifiers: string[] = [],
+    public isInParams: boolean = false
+  ) {}
 
   has(name: string): boolean {
     return this.vars.has(name) ?? this.parent?.has(name)
@@ -11,15 +16,27 @@ export class Scope {
   add(...names: string[]): Scope {
     const newVars = new Set(this.vars)
     names.forEach((name) => newVars.add(name))
-    return new Scope(this.parent, newVars)
+    return new Scope(this.parent, newVars, [], this.isInParams)
   }
 
   push(): Scope {
-    return new Scope(this, new Set())
+    return new Scope(this, new Set(), [], false)
   }
 
   pop(): Scope {
-    return this.parent ?? new Scope(null, new Set())
+    return this.parent ?? new Scope(null, new Set(), [], false)
+  }
+
+  withPendingIdentifiers(ids: string[]): Scope {
+    return new Scope(this.parent, this.vars, ids, this.isInParams)
+  }
+
+  withIsInParams(value: boolean): Scope {
+    return new Scope(this.parent, this.vars, this.pendingIdentifiers, value)
+  }
+
+  clearPending(): Scope {
+    return new Scope(this.parent, this.vars, [], this.isInParams)
   }
 
   hash(): number {
@@ -34,23 +51,21 @@ export class Scope {
       h = (h << 5) - h + this.parent.hash()
       h |= 0
     }
+    // Include pendingIdentifiers and isInParams in hash
+    h = (h << 5) - h + this.pendingIdentifiers.length
+    h = (h << 5) - h + (this.isInParams ? 1 : 0)
+    h |= 0
     return h
   }
 }
 
-// Module-level state for tracking identifiers
-let pendingIdentifiers: string[] = []
-let isInParams = false
-
 export const trackScope = new ContextTracker<Scope>({
-  start: new Scope(null, new Set()),
+  start: new Scope(null, new Set(), [], false),
 
   shift(context, term, stack, input) {
     // Track fn keyword to enter param capture mode
     if (term === terms.Fn) {
-      isInParams = true
-      pendingIdentifiers = []
-      return context
+      return context.withIsInParams(true).withPendingIdentifiers([])
     }
 
     // Capture identifiers
@@ -66,14 +81,13 @@ export const trackScope = new ContextTracker<Scope>({
         text += String.fromCharCode(ch)
       }
 
-
       // Capture ALL identifiers when in params
-      if (isInParams) {
-        pendingIdentifiers.push(text)
+      if (context.isInParams) {
+        return context.withPendingIdentifiers([...context.pendingIdentifiers, text])
       }
       // Capture FIRST identifier for assignments
-      else if (pendingIdentifiers.length === 0) {
-        pendingIdentifiers.push(text)
+      else if (context.pendingIdentifiers.length === 0) {
+        return context.withPendingIdentifiers([text])
       }
     }
 
@@ -82,23 +96,17 @@ export const trackScope = new ContextTracker<Scope>({
 
   reduce(context, term, stack, input) {
     // Add assignment variable to scope
-    if (term === terms.Assign && pendingIdentifiers.length > 0) {
-      const newContext = context.add(pendingIdentifiers[0]!)
-      pendingIdentifiers = []
-      return newContext
+    if (term === terms.Assign && context.pendingIdentifiers.length > 0) {
+      return context.add(context.pendingIdentifiers[0]!)
     }
 
     // Push new scope and add parameters
     if (term === terms.Params) {
       const newScope = context.push()
-      if (pendingIdentifiers.length > 0) {
-        const newContext = newScope.add(...pendingIdentifiers)
-        pendingIdentifiers = []
-        isInParams = false
-        return newContext
+      if (context.pendingIdentifiers.length > 0) {
+        return newScope.add(...context.pendingIdentifiers).withIsInParams(false)
       }
-      isInParams = false
-      return newScope
+      return newScope.withIsInParams(false)
     }
 
     // Pop scope when exiting function
@@ -108,7 +116,7 @@ export const trackScope = new ContextTracker<Scope>({
 
     // Clear stale identifiers after non-assignment statements
     if (term === terms.DotGet || term === terms.FunctionCallOrIdentifier || term === terms.FunctionCall) {
-      pendingIdentifiers = []
+      return context.clearPending()
     }
 
     return context