From 5a8a20e9ee9efbca8047c840acea27e49f1d93c2 Mon Sep 17 00:00:00 2001 From: Roman Dvornov Date: Wed, 12 Jun 2024 17:11:05 +0200 Subject: [PATCH] Improve error handling for variable references --- CHANGELOG.md | 2 ++ src/lang/compile-buildin.js | 14 ++++++++ src/lang/compile.js | 29 +++++++++++------ src/lang/nodes/Block.js | 4 +++ src/lang/nodes/Definition.js | 7 ++-- src/lang/nodes/Function.js | 3 +- src/lang/nodes/Reference.js | 34 +++++++++++++++----- test/lang/variables.js | 62 +++++++++++++++++++++++++++++++++--- test/suggestions.js | 8 +++++ 9 files changed, 137 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 22262fb..b40d290 100644 --- 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 index d922f81..25ce69e 100644 --- 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 index b1051a6..d210268 100644 --- 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 index 8bdfe9e..adeea51 100644 --- 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 index af9a54e..b68c33d 100644 --- 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 index 63243bc..2faac23 100644 --- 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 index 8ed392d..21772dd 100644 --- 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 index 9e7a25c..9eef07b 100644 --- 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 index 57eff21..7edd9cb 100644 --- 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,