CSE Machine: implement variable declaration check (#1517)

* add validation function to CSE machine

* cleanup: moved all validation functions to src/transpiler

* add test for undefined unreached variables

* checkProgramForUndefinedVariables now considers prelude

* checkProgramForUndefinedVariables now checks for current environment

* reformatting

this is why you don't do --no-verify when pushing

* errors from the validator are now handled normally

* update snapshots

src/ec-evaluator/__tests__/__snapshots__/ec-evaluator-errors.ts.snap

@@ -1042,3 +1042,21 @@ Object {
   "visualiseListResult": Array [],
 }
 `;
+
+exports[`Undefined variables are caught even when unreached: expectParsedError 1`] = `
+Object {
+  "alertResult": Array [],
+  "code": "const a = 1;
+if (false) {
+  im_undefined;
+} else {
+  a;
+}",
+  "displayResult": Array [],
+  "numErrors": 1,
+  "parsedErrors": "Line 3: Name im_undefined not declared.",
+  "result": undefined,
+  "resultStatus": "error",
+  "visualiseListResult": Array [],
+}
+`;

src/ec-evaluator/__tests__/ec-evaluator-errors.ts

@@ -51,6 +51,21 @@ test('Undefined variable error is thrown - verbose', () => {
           `)
 })
 
+const undefinedUnreachedVariable = stripIndent`
+const a = 1;
+if (false) {
+  im_undefined;
+} else {
+  a;
+}
+`
+
+test('Undefined variables are caught even when unreached', () => {
+  return expectParsedError(undefinedUnreachedVariable, optionEC).toMatchInlineSnapshot(
+    `"Line 3: Name im_undefined not declared."`
+  )
+})
+
 test('Undefined variable error message differs from verbose version', () => {
   return expectDifferentParsedErrors(undefinedVariable, undefinedVariableVerbose, optionEC).toBe(
     undefined

src/ec-evaluator/interpreter.ts

@@ -18,6 +18,7 @@ import { UndefinedImportError } from '../modules/errors'
 import { initModuleContext, loadModuleBundle } from '../modules/moduleLoader'
 import { ImportTransformOptions } from '../modules/moduleTypes'
 import { checkEditorBreakpoints } from '../stdlib/inspector'
+import { checkProgramForUndefinedVariables } from '../transpiler/transpiler'
 import { Context, ContiguousArrayElements, Result, Value } from '../types'
 import assert from '../utils/assert'
 import { filterImportDeclarations } from '../utils/ast/helpers'
@@ -132,6 +133,13 @@ export class Stash extends Stack<Value> {
  * @returns The result of running the ECE machine.
  */
 export function evaluate(program: es.Program, context: Context, options: IOptions): Value {
+  try {
+    checkProgramForUndefinedVariables(program, context)
+  } catch (error) {
+    context.errors.push(error)
+    return new ECError(error)
+  }
+
   try {
     context.runtime.isRunning = true
     context.runtime.agenda = new Agenda(program)

src/stepper/stepper.ts

@@ -7,6 +7,7 @@ import { UndefinedImportError } from '../modules/errors'
 import { initModuleContextAsync, loadModuleBundleAsync } from '../modules/moduleLoaderAsync'
 import type { ImportTransformOptions } from '../modules/moduleTypes'
 import { parse } from '../parser/parser'
+import { checkProgramForUndefinedVariables } from '../transpiler/transpiler'
 import {
   BlockExpression,
   Context,
@@ -28,13 +29,6 @@ import {
 } from '../utils/dummyAstCreator'
 import { evaluateBinaryExpression, evaluateUnaryExpression } from '../utils/operators'
 import * as rttc from '../utils/rttc'
-import {
-  getFunctionDeclarationNamesInProgram,
-  getIdentifiersInNativeStorage,
-  getIdentifiersInProgram,
-  getUniqueId
-} from '../utils/uniqueIds'
-import { ancestor } from '../utils/walkers'
 import { nodeToValue, objectToString, valueToExpression } from './converter'
 import * as builtin from './lib'
 import {
@@ -3206,131 +3200,6 @@ async function evaluateImports(
   program.body = otherNodes
 }
 
-const globalIdNames = [
-  'native',
-  'callIfFuncAndRightArgs',
-  'boolOrErr',
-  'wrap',
-  'wrapSourceModule',
-  'unaryOp',
-  'binaryOp',
-  'throwIfTimeout',
-  'setProp',
-  'getProp',
-  'builtins'
-] as const
-type NativeIds = Record<typeof globalIdNames[number], es.Identifier>
-
-function getNativeIds(program: es.Program, usedIdentifiers: Set<string>): NativeIds {
-  const globalIds = {}
-  for (const identifier of globalIdNames) {
-    globalIds[identifier] = ast.identifier(getUniqueId(usedIdentifiers, identifier))
-  }
-  return globalIds as NativeIds
-}
-
-function checkForUndefinedVariables(program: es.Program, context: Context) {
-  const usedIdentifiers = new Set<string>([
-    ...getIdentifiersInProgram(program),
-    ...getIdentifiersInNativeStorage(context.nativeStorage)
-  ])
-  const globalIds = getNativeIds(program, usedIdentifiers)
-
-  const preludes = context.prelude
-    ? getFunctionDeclarationNamesInProgram(parse(context.prelude, context)!)
-    : new Set<String>()
-  const builtins = context.nativeStorage.builtins
-  const identifiersIntroducedByNode = new Map<es.Node, Set<string>>()
-  function processBlock(node: es.Program | es.BlockStatement) {
-    const identifiers = new Set<string>()
-    for (const statement of node.body) {
-      if (statement.type === 'VariableDeclaration') {
-        identifiers.add((statement.declarations[0].id as es.Identifier).name)
-      } else if (statement.type === 'FunctionDeclaration') {
-        if (statement.id === null) {
-          throw new Error(
-            'Encountered a FunctionDeclaration node without an identifier. This should have been caught when parsing.'
-          )
-        }
-        identifiers.add(statement.id.name)
-      } else if (statement.type === 'ImportDeclaration') {
-        for (const specifier of statement.specifiers) {
-          identifiers.add(specifier.local.name)
-        }
-      }
-    }
-    identifiersIntroducedByNode.set(node, identifiers)
-  }
-  function processFunction(
-    node: es.FunctionDeclaration | es.ArrowFunctionExpression,
-    _ancestors: es.Node[]
-  ) {
-    identifiersIntroducedByNode.set(
-      node,
-      new Set(
-        node.params.map(id =>
-          id.type === 'Identifier'
-            ? id.name
-            : ((id as es.RestElement).argument as es.Identifier).name
-        )
-      )
-    )
-  }
-  const identifiersToAncestors = new Map<es.Identifier, es.Node[]>()
-  ancestor(program, {
-    Program: processBlock,
-    BlockStatement: processBlock,
-    FunctionDeclaration: processFunction,
-    ArrowFunctionExpression: processFunction,
-    ForStatement(forStatement: es.ForStatement, ancestors: es.Node[]) {
-      const init = forStatement.init!
-      if (init.type === 'VariableDeclaration') {
-        identifiersIntroducedByNode.set(
-          forStatement,
-          new Set([(init.declarations[0].id as es.Identifier).name])
-        )
-      }
-    },
-    Identifier(identifier: es.Identifier, ancestors: es.Node[]) {
-      identifiersToAncestors.set(identifier, [...ancestors])
-    },
-    Pattern(node: es.Pattern, ancestors: es.Node[]) {
-      if (node.type === 'Identifier') {
-        identifiersToAncestors.set(node, [...ancestors])
-      } else if (node.type === 'MemberExpression') {
-        if (node.object.type === 'Identifier') {
-          identifiersToAncestors.set(node.object, [...ancestors])
-        }
-      }
-    }
-  })
-  const nativeInternalNames = new Set(Object.values(globalIds).map(({ name }) => name))
-
-  for (const [identifier, ancestors] of identifiersToAncestors) {
-    const name = identifier.name
-    const isCurrentlyDeclared = ancestors.some(a => identifiersIntroducedByNode.get(a)?.has(name))
-    if (isCurrentlyDeclared) {
-      continue
-    }
-    const isPreviouslyDeclared = context.nativeStorage.previousProgramsIdentifiers.has(name)
-    if (isPreviouslyDeclared) {
-      continue
-    }
-    const isBuiltin = builtins.has(name)
-    if (isBuiltin) {
-      continue
-    }
-    const isPrelude = preludes.has(name)
-    if (isPrelude) {
-      continue
-    }
-    const isNativeId = nativeInternalNames.has(name)
-    if (!isNativeId) {
-      throw new errors.UndefinedVariable(name, identifier)
-    }
-  }
-}
-
 // the context here is for builtins
 export async function getEvaluationSteps(
   program: es.Program,
@@ -3339,7 +3208,7 @@ export async function getEvaluationSteps(
 ): Promise<[es.Program, string[][], string][]> {
   const steps: [es.Program, string[][], string][] = []
   try {
-    checkForUndefinedVariables(program, context)
+    checkProgramForUndefinedVariables(program, context)
     const limit = stepLimit === undefined ? 1000 : stepLimit % 2 === 0 ? stepLimit : stepLimit + 1
     await evaluateImports(program, context, importOptions)
     // starts with substituting predefined constants

src/transpiler/transpiler.ts

@@ -15,6 +15,7 @@ import {
   memoizedGetModuleManifestAsync
 } from '../modules/moduleLoaderAsync'
 import type { ImportTransformOptions } from '../modules/moduleTypes'
+import { parse } from '../parser/parser'
 import {
   AllowedDeclarations,
   Chapter,
@@ -452,6 +453,114 @@ export function checkForUndefinedVariables(
   }
 }
 
+export function checkProgramForUndefinedVariables(program: es.Program, context: Context) {
+  const usedIdentifiers = new Set<string>([
+    ...getIdentifiersInProgram(program),
+    ...getIdentifiersInNativeStorage(context.nativeStorage)
+  ])
+  const globalIds = getNativeIds(program, usedIdentifiers)
+
+  const preludes = context.prelude
+    ? getFunctionDeclarationNamesInProgram(parse(context.prelude, context)!)
+    : new Set<String>()
+
+  const builtins = context.nativeStorage.builtins
+  const env = context.runtime.environments[0].head
+
+  const identifiersIntroducedByNode = new Map<es.Node, Set<string>>()
+  function processBlock(node: es.Program | es.BlockStatement) {
+    const identifiers = new Set<string>()
+    for (const statement of node.body) {
+      if (statement.type === 'VariableDeclaration') {
+        identifiers.add((statement.declarations[0].id as es.Identifier).name)
+      } else if (statement.type === 'FunctionDeclaration') {
+        if (statement.id === null) {
+          throw new Error(
+            'Encountered a FunctionDeclaration node without an identifier. This should have been caught when parsing.'
+          )
+        }
+        identifiers.add(statement.id.name)
+      } else if (statement.type === 'ImportDeclaration') {
+        for (const specifier of statement.specifiers) {
+          identifiers.add(specifier.local.name)
+        }
+      }
+    }
+    identifiersIntroducedByNode.set(node, identifiers)
+  }
+  function processFunction(
+    node: es.FunctionDeclaration | es.ArrowFunctionExpression,
+    _ancestors: es.Node[]
+  ) {
+    identifiersIntroducedByNode.set(
+      node,
+      new Set(
+        node.params.map(id =>
+          id.type === 'Identifier'
+            ? id.name
+            : ((id as es.RestElement).argument as es.Identifier).name
+        )
+      )
+    )
+  }
+  const identifiersToAncestors = new Map<es.Identifier, es.Node[]>()
+  ancestor(program, {
+    Program: processBlock,
+    BlockStatement: processBlock,
+    FunctionDeclaration: processFunction,
+    ArrowFunctionExpression: processFunction,
+    ForStatement(forStatement: es.ForStatement, ancestors: es.Node[]) {
+      const init = forStatement.init!
+      if (init.type === 'VariableDeclaration') {
+        identifiersIntroducedByNode.set(
+          forStatement,
+          new Set([(init.declarations[0].id as es.Identifier).name])
+        )
+      }
+    },
+    Identifier(identifier: es.Identifier, ancestors: es.Node[]) {
+      identifiersToAncestors.set(identifier, [...ancestors])
+    },
+    Pattern(node: es.Pattern, ancestors: es.Node[]) {
+      if (node.type === 'Identifier') {
+        identifiersToAncestors.set(node, [...ancestors])
+      } else if (node.type === 'MemberExpression') {
+        if (node.object.type === 'Identifier') {
+          identifiersToAncestors.set(node.object, [...ancestors])
+        }
+      }
+    }
+  })
+  const nativeInternalNames = new Set(Object.values(globalIds).map(({ name }) => name))
+
+  for (const [identifier, ancestors] of identifiersToAncestors) {
+    const name = identifier.name
+    const isCurrentlyDeclared = ancestors.some(a => identifiersIntroducedByNode.get(a)?.has(name))
+    if (isCurrentlyDeclared) {
+      continue
+    }
+    const isPreviouslyDeclared = context.nativeStorage.previousProgramsIdentifiers.has(name)
+    if (isPreviouslyDeclared) {
+      continue
+    }
+    const isBuiltin = builtins.has(name)
+    if (isBuiltin) {
+      continue
+    }
+    const isPrelude = preludes.has(name)
+    if (isPrelude) {
+      continue
+    }
+    const isInEnv = name in env
+    if (isInEnv) {
+      continue
+    }
+    const isNativeId = nativeInternalNames.has(name)
+    if (!isNativeId) {
+      throw new UndefinedVariable(name, identifier)
+    }
+  }
+}
 function transformSomeExpressionsToCheckIfBoolean(program: es.Program, globalIds: NativeIds) {
   function transform(
     node: