Unify Runner Execution (#1672)

* Refactor more runner logic into SourceRunner

* Allow source to run from the desktop

* Run format

* Made it so that all runners need to go through sourceFilesRunner

* Use fetch for docs loading exclusively

* Run format

* Add new tests to make sure that runners load modules

* Make repl work with local imports and add tests

* Fix potential issue where repl could return an error

* bumping version

---------

Co-authored-by: Martin Henz <[email protected]>

package.json

@@ -1,6 +1,6 @@
 {
   "name": "js-slang",
-  "version": "1.0.68",
+  "version": "1.0.69",
   "license": "Apache-2.0",
   "description": "Javascript-based implementations of Source, written in Typescript",
   "keywords": [
@@ -28,10 +28,11 @@
     "dist"
   ],
   "bin": {
-    "js-slang": "dist/repl/repl.js"
+    "js-slang": "dist/repl/index.js"
   },
   "dependencies": {
     "@babel/parser": "^7.19.4",
+    "@commander-js/extra-typings": "^12.0.1",
     "@joeychenofficial/alt-ergo-modified": "^2.4.0",
     "@ts-morph/bootstrap": "^0.18.0",
     "@types/estree": "0.0.52",
@@ -40,10 +41,10 @@
     "acorn-loose": "^8.0.0",
     "acorn-walk": "^8.0.0",
     "astring": "^1.4.3",
+    "commander": "^12.0.0",
     "gpu.js": "^2.16.0",
     "js-base64": "^3.7.5",
     "lodash": "^4.17.21",
-    "node-getopt": "^0.3.2",
     "source-map": "0.7.3"
   },
   "resolutions": {

src/cse-machine/__tests__/cse-machine-heap.ts

@@ -1,10 +1,9 @@
 import { mockClosure, mockContext } from '../../mocks/context'
-import { parse } from '../../parser/parser'
+import { runCodeInSource } from '../../runner'
 import { Chapter } from '../../types'
 import { stripIndent } from '../../utils/formatters'
-import { sourceRunner } from '../../runner'
 import Heap from '../heap'
-import { EnvArray } from '../types'
+import type { EnvArray } from '../types'
 
 test('Heap works correctly', () => {
   const heap1 = new Heap()
@@ -50,11 +49,11 @@ test('Heap works correctly', () => {
 const expectEnvTreeFrom = (code: string, hasPrelude = true) => {
   const context = mockContext(Chapter.SOURCE_4)
   if (!hasPrelude) context.prelude = null
-  const parsed = parse(code, context)
+
   return expect(
-    sourceRunner(parsed!, context, false, { executionMethod: 'cse-machine' }).then(
-      () => context.runtime.environmentTree
-    )
+    runCodeInSource(code, context, {
+      executionMethod: 'cse-machine'
+    }).then(() => context.runtime.environmentTree)
   ).resolves
 }
 

src/cse-machine/__tests__/cse-machine-runtime-context.ts

@@ -1,13 +1,11 @@
 import { mockContext } from '../../mocks/context'
-import { parse } from '../../parser/parser'
+import { runCodeInSource } from '../../runner'
 import { Chapter } from '../../types'
 import { stripIndent } from '../../utils/formatters'
-import { sourceRunner } from '../../runner'
 
 const getContextFrom = async (code: string) => {
   const context = mockContext(Chapter.SOURCE_4)
-  const parsed = parse(code, context)
-  await sourceRunner(parsed!, context, false, { executionMethod: 'cse-machine' })
+  await runCodeInSource(code, context, { executionMethod: 'cse-machine' })
   return context
 }
 

src/cse-machine/__tests__/cse-machine-unique-id.ts

@@ -1,14 +1,15 @@
 import { mockContext } from '../../mocks/context'
-import { parse } from '../../parser/parser'
-import { Chapter, Context, Environment } from '../../types'
+import { Chapter, type Context, type Environment } from '../../types'
 import { stripIndent } from '../../utils/formatters'
-import { sourceRunner } from '../../runner'
+import { runCodeInSource } from '../../runner'
 import { createProgramEnvironment } from '../utils'
 
 const getContextFrom = async (code: string, envSteps?: number) => {
   const context = mockContext(Chapter.SOURCE_4)
-  const parsed = parse(code, context)
-  await sourceRunner(parsed!, context, false, { envSteps, executionMethod: 'cse-machine' })
+  await runCodeInSource(code, context, {
+    envSteps,
+    executionMethod: 'cse-machine'
+  })
   return context
 }
 
@@ -87,21 +88,18 @@ const getProgramEnv = (context: Context) => {
 }
 
 test('Program environment id stays the same regardless of amount of steps', async () => {
-  const parsed = parse(
-    stripIndent`
+  const code = stripIndent`
         let x = 0;
         for (let i = 0; i < 10; i = i + 1) {
           x = [x];
         }
-      `,
-    mockContext(Chapter.SOURCE_4)
-  )
+      `
+
   let programEnvId = '47'
   // The above program has a total of 335 steps
   // Start from steps = 1 so that the program environment always exists
   for (let steps = 1; steps < 336; steps++) {
-    const context = mockContext(Chapter.SOURCE_4)
-    await sourceRunner(parsed!, context, false, { envSteps: steps, executionMethod: 'cse-machine' })
+    const context = await getContextFrom(code, steps)
     const programEnv = getProgramEnv(context)!
     if (programEnv.id !== programEnvId) {
       programEnvId = programEnv.id

src/cse-machine/interpreter.ts

@@ -214,7 +214,7 @@ function evaluateImports(program: es.Program, context: Context) {
     const [importNodeMap] = filterImportDeclarations(program)
 
     const environment = currentEnvironment(context)
-    for (const [moduleName, nodes] of Object.entries(importNodeMap)) {
+    for (const [moduleName, nodes] of importNodeMap) {
       const functions = context.nativeStorage.loadedModules[moduleName]
       for (const node of nodes) {
         for (const spec of node.specifiers) {

src/index.ts

@@ -27,20 +27,11 @@ export { SourceDocumentation } from './editors/ace/docTooltip'
 
 import { CSEResultPromise, resumeEvaluate } from './cse-machine/interpreter'
 import { ModuleNotFoundError } from './modules/errors'
-import type { ImportOptions, SourceFiles } from './modules/moduleTypes'
+import type { ImportOptions } from './modules/moduleTypes'
 import preprocessFileImports from './modules/preprocessor'
 import { validateFilePath } from './modules/preprocessor/filePaths'
-import { mergeImportOptions } from './modules/utils'
 import { getKeywords, getProgramNames, NameDeclaration } from './name-extractor'
-import { parse } from './parser/parser'
-import {
-  fullJSRunner,
-  hasVerboseErrors,
-  htmlRunner,
-  resolvedErrorPromise,
-  sourceFilesRunner
-} from './runner'
-import { mapResult } from './alt-langs/mapper'
+import { htmlRunner, resolvedErrorPromise, sourceFilesRunner } from './runner'
 
 export interface IOptions {
   scheduler: 'preemptive' | 'async'
@@ -225,56 +216,38 @@ export async function runFilesInContext(
   context: Context,
   options: RecursivePartial<IOptions> = {}
 ): Promise<Result> {
-  async function runFilesInContextHelper(
-    files: Partial<Record<string, string>>,
-    entrypointFilePath: string,
-    context: Context,
-    options: RecursivePartial<IOptions> = {}
-  ): Promise<Result> {
-    for (const filePath in files) {
-      const filePathError = validateFilePath(filePath)
-      if (filePathError !== null) {
-        context.errors.push(filePathError)
-        return resolvedErrorPromise
-      }
+  for (const filePath in files) {
+    const filePathError = validateFilePath(filePath)
+    if (filePathError !== null) {
+      context.errors.push(filePathError)
+      return resolvedErrorPromise
     }
+  }
 
+  let result: Result
+  if (context.chapter === Chapter.HTML) {
     const code = files[entrypointFilePath]
     if (code === undefined) {
       context.errors.push(new ModuleNotFoundError(entrypointFilePath))
       return resolvedErrorPromise
     }
-
-    if (
-      context.chapter === Chapter.FULL_JS ||
-      context.chapter === Chapter.FULL_TS ||
-      context.chapter === Chapter.PYTHON_1
-    ) {
-      const program = parse(code, context)
-      if (program === null) {
-        return resolvedErrorPromise
-      }
-
-      const fullImportOptions = mergeImportOptions(options.importOptions)
-      return fullJSRunner(program, context, fullImportOptions)
-    }
-
-    if (context.chapter === Chapter.HTML) {
-      return htmlRunner(code, context, options)
-    }
-
+    result = await htmlRunner(code, context, options)
+  } else {
     // FIXME: Clean up state management so that the `parseError` function is pure.
     //        This is not a huge priority, but it would be good not to make use of
     //        global state.
-    verboseErrors = hasVerboseErrors(code)
-
-    // the sourceFilesRunner
-    return sourceFilesRunner(files, entrypointFilePath, context, options)
+    ;({ result, verboseErrors } = await sourceFilesRunner(
+      p => Promise.resolve(files[p]),
+      entrypointFilePath,
+      context,
+      {
+        ...options,
+        shouldAddFileName: options.shouldAddFileName ?? Object.keys(files).length > 1
+      }
+    ))
   }
 
-  return runFilesInContextHelper(files, entrypointFilePath, context, options).then(
-    mapResult(context)
-  )
+  return result
 }
 
 export function resume(result: Result): Finished | ResultError | Promise<Result> {
@@ -320,23 +293,19 @@ export async function compileFiles(
     }
   }
 
-  const entrypointCode = files[entrypointFilePath]
-  if (entrypointCode === undefined) {
-    context.errors.push(new ModuleNotFoundError(entrypointFilePath))
-    return undefined
-  }
-
-  const preprocessedProgram = await preprocessFileImports(
-    files as SourceFiles,
+  const preprocessResult = await preprocessFileImports(
+    p => Promise.resolve(files[p]),
     entrypointFilePath,
-    context
+    context,
+    { shouldAddFileName: Object.keys(files).length > 1 }
   )
-  if (!preprocessedProgram) {
+
+  if (!preprocessResult.ok) {
     return undefined
   }
 
   try {
-    return compileToIns(preprocessedProgram, undefined, vmInternalFunctions)
+    return compileToIns(preprocessResult.program, undefined, vmInternalFunctions)
   } catch (error) {
     context.errors.push(error)
     return undefined

src/infiniteLoops/instrument.ts

@@ -6,7 +6,6 @@ import type { Node } from '../types'
 import * as create from '../utils/ast/astCreator'
 import { recursive, simple, WalkerCallback } from '../utils/walkers'
 import { getIdsFromDeclaration } from '../utils/ast/helpers'
-import assert from '../utils/assert'
 // transforms AST of program
 
 const globalIds = {
@@ -587,10 +586,7 @@ function handleImports(programs: es.Program[]): string[] {
     program.body = [...importsToAdd, ...otherNodes]
     return importsToAdd.flatMap(decl => {
       const ids = getIdsFromDeclaration(decl)
-      return ids.map(id => {
-        assert(id !== null, 'Encountered a null identifier')
-        return id.name
-      })
+      return ids.map(id => id.name)
     })
   })
 

src/modules/moduleTypes.ts

@@ -44,5 +44,5 @@ export type ImportOptions = {
 } & ImportAnalysisOptions &
   LinkerOptions
 
-export type SourceFiles = Record<string, string>
+export type SourceFiles = Partial<Record<string, string>>
 export type FileGetter = (p: string) => Promise<string | undefined>

src/modules/preprocessor/__tests__/analyzer.ts

@@ -63,7 +63,7 @@ describe('Test throwing import validation errors', () => {
     )
 
     // Return 'undefined' if there are errors while parsing.
-    if (context.errors.length !== 0 || !importGraphResult) {
+    if (context.errors.length !== 0 || !importGraphResult.ok) {
       throw context.errors[0]
     }
 

src/modules/preprocessor/__tests__/linker.ts

@@ -29,7 +29,7 @@ async function testCode<T extends SourceFiles>(files: T, entrypointFilePath: key
 
 async function expectError<T extends SourceFiles>(files: T, entrypointFilePath: keyof T) {
   const [context, result] = await testCode(files, entrypointFilePath)
-  expect(result).toBeUndefined()
+  expect(result.ok).toEqual(false)
   expect(context.errors.length).toBeGreaterThanOrEqual(1)
   return context.errors
 }
@@ -130,8 +130,13 @@ test('Linker does tree-shaking', async () => {
     '/a.js'
   )
 
+  // Wrap to appease typescript
+  function expectWrapper(cond: boolean): asserts cond {
+    expect(cond).toEqual(true)
+  }
+
   expect(errors.length).toEqual(0)
-  expect(result).toBeDefined()
+  expectWrapper(result.ok)
   expect(resolver.default).not.toHaveBeenCalledWith('./b.js')
-  expect(Object.keys(result!.programs)).not.toContain('/b.js')
+  expect(Object.keys(result.programs)).not.toContain('/b.js')
 })