Improve error handling for variable references

discoveryjs · Jun 12, 2024 · 5a8a20e · 5a8a20e
1 parent bb043ac
commit 5a8a20e
Show file tree

Hide file tree

Showing 9 changed files with 137 additions and 26 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,8 @@
 ## next
 
 - Added support for `_` as numeric separator for all kinds of number, e.g. `1_000`, `0xa0b1_c2d3`
+- Improved error handling for variable references, including providing query's range of the problem reference in errors
+- Fixed acessing to global variables when a variable has no definition
 
 ## 1.0.0-beta.11 (May 19, 2024)
 

diff --git a/src/lang/compile-buildin.js b/src/lang/compile-buildin.js
@@ -2,6 +2,7 @@ import { cmp, cmpAnalytical, cmpNatural, cmpNaturalAnalytical } from '../utils/c
 import { hasOwn, addToSet, getPropertyValue, isPlainObject, isRegExp, isArrayLike, isTruthy, parseIntDefault, isArray } from '../utils/misc.js';
 
 export default Object.freeze({
+    unsafeRef,
     ensureArray,
     bool: isTruthy,
     and: (a, b) => isTruthy(a) ? b : a,
@@ -36,6 +37,19 @@ export default Object.freeze({
     slice
 });
 
+function unsafeRef(fn, name, range) {
+    try {
+        return fn();
+    } catch (e) {
+        if (name) {
+            throw Object.assign(
+                new ReferenceError(`Cannot access $${name} before initialization`),
+                { details: { name, loc: { range } } }
+            );
+        }
+    }
+}
+
 function ensureArray(value) {
     return isArray(value) ? value : [value];
 }

diff --git a/src/lang/compile.js b/src/lang/compile.js
@@ -9,6 +9,22 @@ export function compileMethod(ast, tolerant, suggestions) {
     return compileInternal(ast, 'method', tolerant, suggestions);
 }
 
+class Scope extends Set {
+    constructor(items, awaitInit = [], arg1 = false, $ref = '$') {
+        super(items);
+        this.own = new Set();
+        this.awaitInit = new Set(awaitInit);
+        this.firstCurrent = null;
+        this.captureCurrent = [];
+        this.arg1 = arg1;
+        this.$ref = $ref;
+    }
+
+    spawn(arg1, $ref) {
+        return new Scope(this, this.awaitInit, arg1, $ref);
+    }
+}
+
 function compileInternal(ast, kind, tolerant = false, suggestions = null) {
     function newStatPoint(values) {
         const spName = 's' + spNames.length;
@@ -32,11 +48,11 @@ function compileInternal(ast, kind, tolerant = false, suggestions = null) {
         let range = [start, end, JSON.stringify(type)];
 
         if (type === 'var') {
-            if (!ctx.scope.length) {
+            if (!ctx.scope.size) {
                 return;
             }
 
-            range.push(JSON.stringify(ctx.scope));
+            range.push(JSON.stringify([...ctx.scope]));
         } else {
             if (!spName) {
                 spName = newStatPoint();
@@ -58,12 +74,7 @@ function compileInternal(ast, kind, tolerant = false, suggestions = null) {
         const prevScope = ctx.scope;
         const scopeStart = buffer.length;
 
-        ctx.scope = ctx.scope.slice();
-        ctx.scope.own = [];
-        ctx.scope.firstCurrent = null;
-        ctx.scope.captureCurrent = [];
-        ctx.scope.arg1 = prevScope.arg1 || kind === 'method';
-        ctx.scope.$ref = $ref;
+        ctx.scope = prevScope.spawn(prevScope.arg1 || kind === 'method', $ref);
 
         fn();
 
@@ -187,7 +198,7 @@ function compileInternal(ast, kind, tolerant = false, suggestions = null) {
             usedBuildinMethods.add(name);
             return 'f.' + name;
         },
-        scope: [],
+        scope: new Scope(),
         createScope,
         error: (message, node) => {
             const error = new SyntaxError(message);

diff --git a/src/lang/nodes/Block.js b/src/lang/nodes/Block.js
@@ -7,6 +7,10 @@ export function compile(node, ctx) {
     if (node.definitions.length) {
         ctx.createScope(
             () => {
+                for (const definition of node.definitions) {
+                    ctx.scope.awaitInit.add(definition.declarator.name);
+                }
+
                 ctx.put('(()=>{');
                 ctx.list(node.definitions);
                 ctx.put('return ');

diff --git a/src/lang/nodes/Definition.js b/src/lang/nodes/Definition.js
@@ -15,7 +15,7 @@ export function compile(node, ctx) {
         return;
     }
 
-    if (ctx.scope.own.includes(node.declarator.name)) {
+    if (ctx.scope.own.has(node.declarator.name)) {
         ctx.error(`Identifier "$${node.declarator.name}" has already been declared`, node.declarator);
         return;
     }
@@ -31,8 +31,9 @@ export function compile(node, ctx) {
     ctx.node(node.value || GetProperty(null, Identifier(node.declarator.name)));
     ctx.put(';');
 
-    ctx.scope.push(node.declarator.name);
-    ctx.scope.own.push(node.declarator.name);
+    ctx.scope.add(node.declarator.name);
+    ctx.scope.own.add(node.declarator.name);
+    ctx.scope.awaitInit.delete(node.declarator.name);
 }
 export function walk(node, ctx) {
     ctx.node(node.declarator);

diff --git a/src/lang/nodes/Function.js b/src/lang/nodes/Function.js
@@ -5,8 +5,9 @@ export function compile(node, ctx) {
 
             ctx.scope.arg1 = true;
             ctx.scope.$ref = args[0] || '$';
+
             for (const arg of node.arguments) {
-                ctx.scope.push(arg.name);
+                ctx.scope.add(arg.name);
             }
 
             ctx.put('function(');

diff --git a/src/lang/nodes/Reference.js b/src/lang/nodes/Reference.js
@@ -4,18 +4,36 @@ export function suggest(node, ctx) {
     }
 }
 export function compile(node, ctx) {
-    if (!ctx.scope.includes(node.name.name) && ctx.tolerant) {
-        // FIXME: use ctx.error() here
-        ctx.put('(typeof $');
-        ctx.node(node.name);
-        ctx.put('!=="undefined"?$');
-        ctx.node(node.name);
-        ctx.put(':undefined)');
+    const nameNode = node.name;
+    const inScope = ctx.scope.has(nameNode.name);
+    const awaitInit = ctx.scope.awaitInit.has(nameNode.name);
+
+    if (!inScope || awaitInit) {
+        if (awaitInit) {
+            ctx.put('f.unsafeRef(()=>$');
+            ctx.node(nameNode);
+
+            if (!ctx.tolerant) {
+                ctx.put(',');
+                ctx.put(JSON.stringify(nameNode.name));
+                ctx.put(',');
+                ctx.put(JSON.stringify(nameNode.range));
+            }
+
+            ctx.put(')');
+        } else {
+            if (ctx.tolerant) {
+                ctx.put('undefined');
+            } else {
+                ctx.error(`$${nameNode.name} is not defined`, nameNode);
+            }
+        }
+
         return;
     }
 
     ctx.put('$');
-    ctx.node(node.name);
+    ctx.node(nameNode);
 }
 export function walk(node, ctx) {
     ctx.node(node.name);

diff --git a/test/lang/variables.js b/test/lang/variables.js
@@ -64,11 +64,63 @@ describe('lang/variables', () => {
         );
     });
 
-    it('should throw when access before initialization', () => {
-        assert.throws(
-            () => query('$a:$a;')(),
-            /Identifier "\$a" is not defined|\$a is not defined|Cannot access '\$a' before initialization/
-        );
+    describe('should throw when access before initialization', () => {
+        it('self reference', () => {
+            assert.throws(
+                () => query('$a:$a;')(),
+                /Cannot access \$a before initialization/
+            );
+        });
+
+        it('same scope', () => {
+            assert.throws(
+                () => query('$a:$b;$b;')(),
+                /Cannot access \$b before initialization/
+            );
+        });
+
+        it('inner scope to same scope', () => {
+            assert.throws(
+                () => query('$a:1;|$b:2;$c:$b;$d:$a;$a:3;4')(),
+                /Cannot access \$a before initialization/
+            );
+        });
+
+        it('inner scope to outer scope', () => {
+            assert.throws(
+                () => query('$a:1;$b:($c:$d;1);$d:1;')(),
+                /Cannot access \$d before initialization/
+            );
+        });
+
+        it('inner scope to outer scope on initing var', () => {
+            assert.throws(
+                () => query('$a:1;$b:($c:$b;1);$d:1;')(),
+                /Cannot access \$b before initialization/
+            );
+        });
+
+        it('should not break outer await', () => {
+            assert.throws(
+                () => query('$a:1;$b:($d:1;1);$c:$d;$d:1;')(),
+                /Cannot access \$d before initialization/
+            );
+        });
+    });
+
+    describe('should not leak to globals', () => {
+        before(() => {
+            global.$test = 123;
+        });
+        after(() => {
+            delete global.$test;
+        });
+        it('should throw when access before initialization', () => {
+            assert.throws(
+                () => query('$test')(),
+                /\$test is not defined/
+            );
+        });
     });
 
     it('should return a value when access after initialization', () => {

diff --git a/test/suggestions.js b/test/suggestions.js
@@ -1134,6 +1134,10 @@ describe('query/suggestions (tolerant mode)', () => {
             suggestion('', ['foo', 'bar'], 5),
             suggestion('', ['$var:variable', 'foo', 'bar'], 6)
         ],
+        '$var;$|v|': [
+            suggestion('$v', ['$var:variable'], 5, 7),
+            suggestion('$v', ['$var:variable'], 5, 7)
+        ],
         '$foo;$var:|;|': [
             suggestion('', ['$foo:variable', 'foo', 'bar'], 10),
             suggestion('', ['$foo:variable', '$var:variable', 'foo', 'bar'], 11)
@@ -1160,6 +1164,10 @@ describe('query/suggestions (tolerant mode)', () => {
             suggestion('$x', ['$x:variable'], 20, 22),
             suggestion('', ['qux'], 23, 23)
         ],
+        '$asd;$a:$a|;$a|': [
+            suggestion('$a', ['$asd:variable'], 8, 10),
+            suggestion('$a', ['$asd:variable', '$a:variable'], 11, 13)
+        ],
         '{x:|$|}.x.|': [
             null,
             null,