Merge pull request #41 from jmespath-community/fix-scope-null-handling

fix(scope): improve/propose variable lookup and null handling in scope chain

Author: cawalch

src/Scope.ts

@@ -4,6 +4,10 @@ export class ScopeChain {
   private inner?: ScopeChain = undefined;
   private data: JSONObject = {};
 
+  get currentScopeData(): JSONObject {
+    return this.data;
+  }
+
   public withScope(data: JSONObject): ScopeChain {
     const outer: ScopeChain = new ScopeChain();
     outer.inner = this;
@@ -12,15 +16,14 @@ export class ScopeChain {
   }
 
   public getValue(identifier: string): JSONValue {
-    if (identifier in this.data) {
-      const result = this.data[identifier];
-      if (result !== null && result !== undefined) {
-        return result;
-      }
+    if (Object.prototype.hasOwnProperty.call(this.data, identifier)) {
+      return this.data[identifier];
     }
+
     if (this.inner) {
       return this.inner.getValue(identifier);
     }
+
     return null;
   }
 }

src/TreeInterpreter.ts

@@ -59,11 +59,13 @@ export class TreeInterpreter {
       }
       case 'Variable': {
         const variable = node.name;
-        const result = this._scope.getValue(variable) ?? null;
-        if (result === null || result === undefined) {
+        if (
+          !this._scope.getValue(variable) &&
+          !Object.prototype.hasOwnProperty.call(this._scope.currentScopeData, variable)
+        ) {
           throw new Error(`Error referencing undefined variable ${variable}`);
         }
-        return result;
+        return this._scope.getValue(variable);
       }
       case 'IndexExpression':
         return this.visit(node.right, this.visit(node.left, value));

test/scopes.spec.ts

@@ -1,3 +1,4 @@
+import { describe, it, expect } from 'vitest';
 import { Scope } from '../src';
 
 describe('scopes', () => {
@@ -24,4 +25,55 @@ describe('scopes', () => {
       expect(outer.getValue('foo')).toEqual('bar');
     }
   });
+  it('should not return value for non-existent identifiers', () => {
+    const scope = Scope();
+    {
+      const scoped = scope.withScope({ foo: 'bar' });
+      expect(scoped.getValue('baz')).toEqual(null);
+      expect(scoped.getValue('foo')).toEqual('bar');
+    }
+  });
+  it('should return null for identifiers even in nested scopes if absent', () => {
+    const scope = Scope();
+    {
+      const outer = scope.withScope({ foo: 'bar' });
+      {
+        const inner = outer.withScope({ bar: 'baz' });
+        expect(inner.getValue('qux')).toEqual(null);
+      }
+      expect(outer.getValue('qux')).toEqual(null);
+    }
+  });
+  it('should handle values in nested scopes differently from outer scopes', () => {
+    const scope = Scope();
+    {
+      const outer = scope.withScope({ foo: 'bar' });
+      {
+        const inner = outer.withScope({ bar: 'baz' });
+        expect(inner.getValue('foo')).toEqual('bar');
+        expect(inner.getValue('bar')).toEqual('baz');
+      }
+    }
+  });
+  it('should not fall through to outer scope when key is in current scope with null/undefined', () => {
+    const scope = Scope();
+    {
+      const outer = scope.withScope({ foo: 'bar' });
+      {
+        const inner = outer.withScope({ foo: null });
+        expect(inner.getValue('foo')).toEqual(null);
+      }
+    }
+  });
+  it('should properly differentiate between keys absent entirely and those in outer scopes', () => {
+    const scope = Scope();
+    {
+      const outer = scope.withScope({ foo: 'bar' });
+      {
+        const inner = outer.withScope({});
+        expect(inner.getValue('foo')).toEqual('bar');
+        expect(inner.getValue('baz')).toEqual(null);
+      }
+    }
+  });
 });