allow local masking of global variables

Author: mbostock

src/javascript/assignments.test.ts

@@ -1,10 +1,10 @@
 import {assert, test} from "vitest";
-import {checkAssignments} from "./assignments.js";
 import {parseJavaScript} from "./parse.js";
+import {findReferences} from "./references.js";
 
 function check(input: string) {
   const cell = parseJavaScript(input);
-  checkAssignments(cell.body, cell.references, input);
+  findReferences(cell.body, {input});
 }
 
 test("allows non-external assignments", () => {
@@ -35,5 +35,20 @@ test("does not allow external assignments via for…of or for…in", () => {
 });
 
 test("does not allow global assignments", () => {
-  assert.throws(() => check("window = 1;"), /global 'window'/);
+  assert.throws(() => check("window = 1;"), /Assignment to global 'window'/);
+  assert.throws(() => check("const foo = (window = 1);"), /Assignment to global 'window'/);
+});
+
+test("does not allow conflicting top-level variables", () => {
+  assert.throws(() => check("const window = 1;"), /Global 'window' cannot be redefined/);
+  assert.throws(() => check("const foo = 1, window = 2;"), /Global 'window' cannot be redefined/);
+  assert.throws(() => check("const {window} = {};"), /Global 'window' cannot be redefined/);
+  assert.throws(() => check("const {window = 1} = {};"), /Global 'window' cannot be redefined/);
+});
+
+test("allows conflicting non-top-level variables", () => {
+  assert.doesNotThrow(() => check("{ const window = 1; }"));
+  assert.doesNotThrow(() => check("{ let window; window = 2; }"));
+  assert.doesNotThrow(() => check("{ const {window} = {}; }"));
+  assert.doesNotThrow(() => check("{ const {window = 1} = {}; }"));
 });

src/javascript/assignments.ts

@@ -1,37 +1,59 @@
 import type {Expression, Identifier, Node, Pattern, VariableDeclaration} from "acorn";
 import {defaultGlobals} from "./globals.js";
 import {syntaxError} from "./syntaxError.js";
-import {simple} from "./walk.js";
+import {ancestor} from "./walk.js";
 
 type Assignable = Expression | Pattern | VariableDeclaration;
 
-export function checkAssignments(node: Node, references: Identifier[], input: string): void {
-  function checkConst(node: Assignable) {
+export function checkAssignments(
+  node: Node,
+  {
+    input,
+    locals,
+    references,
+    globals = defaultGlobals
+  }: {
+    input: string;
+    locals: Map<Node, Set<string>>;
+    globals?: Set<string>;
+    references: Identifier[];
+  }
+): void {
+  function isLocal({name}: Identifier, parents: Node[]): boolean {
+    for (const p of parents) if (locals.get(p)?.has(name)) return true;
+    return false;
+  }
+
+  function checkConst(node: Assignable, parents: Node[]) {
     switch (node.type) {
       case "Identifier":
+        if (isLocal(node, parents)) break;
         if (references.includes(node))
           throw syntaxError(`Assignment to external variable '${node.name}'`, node, input);
-        if (defaultGlobals.has(node.name))
+        if (globals.has(node.name))
           throw syntaxError(`Assignment to global '${node.name}'`, node, input);
         break;
-      case "ObjectPattern":
-        node.properties.forEach((node) => checkConst(node.type === "Property" ? node.value : node));
-        break;
       case "ArrayPattern":
-        node.elements.forEach((node) => node && checkConst(node));
+        for (const e of node.elements) if (e) checkConst(e, parents);
+        break;
+      case "ObjectPattern":
+        for (const p of node.properties) checkConst(p.type === "Property" ? p.value : p, parents);
         break;
       case "RestElement":
-        checkConst(node.argument);
+        checkConst(node.argument, parents);
         break;
     }
   }
-  function checkConstLeft({left}: {left: Assignable}) {
-    checkConst(left);
+
+  function checkConstArgument({argument}: {argument: Assignable}, parents: Node[]) {
+    checkConst(argument, parents);
   }
-  function checkConstArgument({argument}: {argument: Assignable}) {
-    checkConst(argument);
+
+  function checkConstLeft({left}: {left: Assignable}, parents: Node[]) {
+    checkConst(left, parents);
   }
-  simple(node, {
+
+  ancestor(node, {
     AssignmentExpression: checkConstLeft,
     AssignmentPattern: checkConstLeft,
     UpdateExpression: checkConstArgument,

src/javascript/imports.ts

@@ -14,7 +14,7 @@ type NamedImportSpecifier = ImportSpecifier | ImportDefaultSpecifier;
 type AnyImportSpecifier = ImportSpecifier | ImportDefaultSpecifier | ImportNamespaceSpecifier;
 
 /** Throws a syntax error if any export declarations are found. */
-export function checkExports(body: Node, input: string): void {
+export function checkExports(body: Node, {input}: {input: string}): void {
   function checkExport(child: Node) {
     throw syntaxError("Unexpected token 'export'", child, input);
   }
@@ -164,10 +164,12 @@ function renderImport(source: string, node: ImportDeclaration, input: string): s
       : ""
   })${
     names.length > 0
-      ? `.then((module) => {${names.map(
-          (name) => `
+      ? `.then((module) => {${names
+          .map(
+            (name) => `
   if (!("${name}" in module)) throw new SyntaxError(\`export '${name}' not found\`);`
-        ).join("")}
+          )
+          .join("")}
   return module;
 })`
       : ""

src/javascript/parse.ts

@@ -1,8 +1,6 @@
 import {Parser, tokTypes} from "acorn";
 import type {Expression, Identifier, Options, Program} from "acorn";
-import {checkExports} from "./imports.js";
 import {findReferences} from "./references.js";
-import {checkAssignments} from "./assignments.js";
 import {findDeclarations} from "./declarations.js";
 import {findAwaits} from "./awaits.js";
 
@@ -20,9 +18,7 @@ export interface JavaScriptCell {
   async: boolean; // does this use top-level await?
 }
 
-export function maybeParseJavaScript(
-  input: string
-): JavaScriptCell | undefined {
+export function maybeParseJavaScript(input: string): JavaScriptCell | undefined {
   try {
     return parseJavaScript(input);
   } catch (error) {
@@ -36,13 +32,10 @@ export function parseJavaScript(input: string): JavaScriptCell {
   if (expression?.type === "ClassExpression" && expression.id) expression = null; // treat named class as program
   if (expression?.type === "FunctionExpression" && expression.id) expression = null; // treat named function as program
   const body = expression ?? parseProgram(input); // otherwise parse as a program
-  checkExports(body, input);
-  const references = findReferences(body);
-  checkAssignments(body, references, input);
   return {
     body,
     declarations: expression ? null : findDeclarations(body as Program, input),
-    references,
+    references: findReferences(body, {input}),
     expression: !!expression,
     async: findAwaits(body).length > 0
   };

src/javascript/references.ts

@@ -3,7 +3,9 @@ import type {BlockStatement, CatchClause, Class} from "acorn";
 import type {ForInStatement, ForOfStatement, ForStatement} from "acorn";
 import type {FunctionDeclaration, FunctionExpression} from "acorn";
 import type {Identifier, Node, Pattern, Program} from "acorn";
+import {checkAssignments} from "./assignments.js";
 import {defaultGlobals} from "./globals.js";
+import {checkExports} from "./imports.js";
 import {ancestor} from "./walk.js";
 
 // Based on https://github.com/ForbesLindesay/acorn-globals
@@ -40,10 +42,12 @@ function isBlockScope(node: Node): node is FunctionNode | Program | BlockStateme
 export function findReferences(
   node: Node,
   {
+    input,
     globals = defaultGlobals,
     filterReference = (identifier: Identifier) => !globals.has(identifier.name),
     filterDeclaration = () => true
   }: {
+    input?: string;
     globals?: Set<string>;
     filterReference?: (identifier: Identifier) => boolean;
     filterDeclaration?: (identifier: {name: string}) => boolean;
@@ -53,8 +57,7 @@ export function findReferences(
   const references: Identifier[] = [];
 
   function hasLocal(node: Node, name: string): boolean {
-    const l = locals.get(node);
-    return l ? l.has(name) : false;
+    return locals.get(node)?.has(name) ?? false;
   }
 
   function declareLocal(node: Node, id: {name: string}): void {
@@ -160,5 +163,10 @@ export function findReferences(
     Identifier: identifier
   });
 
+  if (input !== undefined) {
+    checkAssignments(node, {locals, references, globals, input});
+    checkExports(node, {input});
+  }
+
   return references;
 }