Preparing for AST-based synchronization

app/gui2/shared/__tests__/yjsModel.test.ts

@@ -1,7 +1,7 @@
-import { rangeEncloses, rangeIntersects, type ContentRange } from 'shared/yjsModel'
+import { rangeEncloses, rangeIntersects, type SourceRange } from 'shared/yjsModel'
 import { expect, test } from 'vitest'
 
-type RangeTest = { a: ContentRange; b: ContentRange }
+type RangeTest = { a: SourceRange; b: SourceRange }
 
 const equalRanges: RangeTest[] = [
   { a: [0, 0], b: [0, 0] },

app/gui2/shared/yjsModel.ts

@@ -168,7 +168,6 @@ export class DistributedModule {
 }
 
 export type SourceRange = readonly [start: number, end: number]
-export type RelativeRange = readonly [start: number, end: number]
 
 export class IdMap {
   private readonly rangeToExpr: Map<string, ExprId>
@@ -231,29 +230,52 @@ export class IdMap {
     }
     return true
   }
+
+  validate() {
+    const uniqueValues = new Set(this.rangeToExpr.values())
+    if (uniqueValues.size < this.rangeToExpr.size) {
+      console.warn(`Duplicate UUID in IdMap`)
+    }
+  }
+
+  clone(): IdMap {
+    return new IdMap(this.entries())
+  }
+
+  // Debugging.
+  compare(other: IdMap) {
+    console.info(`IdMap.compare -------`)
+    const allKeys = new Set<string>()
+    for (const key of this.rangeToExpr.keys()) allKeys.add(key)
+    for (const key of other.rangeToExpr.keys()) allKeys.add(key)
+    for (const key of allKeys) {
+      const mine = this.rangeToExpr.get(key)
+      const yours = other.rangeToExpr.get(key)
+      if (mine !== yours) {
+        console.info(`IdMap.compare[${key}]: ${mine} -> ${yours}`)
+      }
+    }
+  }
 }
 
 const uuidRegex = /^[0-9a-f]{8}-(?:[0-9a-f]{4}-){3}[0-9a-f]{12}$/
 export function isUuid(x: unknown): x is Uuid {
   return typeof x === 'string' && x.length === 36 && uuidRegex.test(x)
 }
 
-/** A range represented as start and end indices. */
-export type ContentRange = [start: number, end: number]
-
-export function rangeEquals(a: ContentRange, b: ContentRange): boolean {
+export function rangeEquals(a: SourceRange, b: SourceRange): boolean {
   return a[0] == b[0] && a[1] == b[1]
 }
 
-export function rangeEncloses(a: ContentRange, b: ContentRange): boolean {
+export function rangeEncloses(a: SourceRange, b: SourceRange): boolean {
   return a[0] <= b[0] && a[1] >= b[1]
 }
 
-export function rangeIntersects(a: ContentRange, b: ContentRange): boolean {
+export function rangeIntersects(a: SourceRange, b: SourceRange): boolean {
   return a[0] <= b[1] && a[1] >= b[0]
 }
 
 /** Whether the given range is before the other range. */
-export function rangeIsBefore(a: ContentRange, b: ContentRange): boolean {
+export function rangeIsBefore(a: SourceRange, b: SourceRange): boolean {
   return a[1] <= b[0]
 }

app/gui2/src/components/CodeEditor.vue

@@ -38,8 +38,8 @@ const rootElement = ref<HTMLElement>()
 useAutoBlur(rootElement)
 
 const executionContextDiagnostics = computed(() =>
-  projectStore.module
-    ? lsDiagnosticsToCMDiagnostics(projectStore.module.doc.getCode(), projectStore.diagnostics)
+  projectStore.module && graphStore.moduleCode
+    ? lsDiagnosticsToCMDiagnostics(graphStore.moduleCode, projectStore.diagnostics)
     : [],
 )
 
@@ -54,8 +54,8 @@ const expressionUpdatesDiagnostics = computed(() => {
     if (!update) continue
     const node = nodeMap.get(id)
     if (!node) continue
-    if (!node.rootSpan.astExtended) continue
-    const [from, to] = node.rootSpan.astExtended.span()
+    if (!node.rootSpan.span) continue
+    const [from, to] = node.rootSpan.span
     switch (update.payload.type) {
       case 'Panic': {
         diagnostics.push({ from, to, message: update.payload.message, severity: 'error' })
@@ -85,9 +85,10 @@ watchEffect(() => {
   const awareness = projectStore.awareness.internal
   extensions: [yCollab(yText, awareness, { undoManager }), ...]
    */
+  if (!graphStore.moduleCode) return
   editorView.setState(
     EditorState.create({
-      doc: module.doc.getCode(),
+      doc: graphStore.moduleCode,
       extensions: [
         minimalSetup,
         syntaxHighlighting(defaultHighlightStyle as Highlighter),
@@ -101,10 +102,7 @@ watchEffect(() => {
           const astSpan = ast.span()
           let foundNode: ExprId | undefined
           for (const [id, node] of graphStore.db.nodeIdToNode.entries()) {
-            if (
-              node.rootSpan.astExtended &&
-              rangeEncloses(node.rootSpan.astExtended.span(), astSpan)
-            ) {
+            if (node.rootSpan.span && rangeEncloses(node.rootSpan.span, astSpan)) {
               foundNode = id
               break
             }

app/gui2/src/components/ComponentBrowser/input.ts

@@ -21,7 +21,7 @@ import {
   type QualifiedName,
 } from '@/util/qualifiedName'
 import { equalFlat } from 'lib0/array'
-import { IdMap, type ContentRange, type ExprId } from 'shared/yjsModel'
+import { IdMap, type ExprId, type SourceRange } from 'shared/yjsModel'
 import { computed, ref, type ComputedRef } from 'vue'
 
 /** Information how the component browser is used, needed for proper input initializing. */
@@ -60,7 +60,7 @@ export type EditingContext =
 interface Change {
   str: string
   /** Range in the original code to be replaced with `str`. */
-  range: ContentRange
+  range: SourceRange
 }
 
 /** Component Browser Input Data */
@@ -399,7 +399,7 @@ export function useComponentBrowserInput(
           result.push({ range: span, str: segment as string })
         } else {
           // The rest of qualified name needs to be added at the end.
-          const range: ContentRange = [lastEditedCharIndex, lastEditedCharIndex]
+          const range: SourceRange = [lastEditedCharIndex, lastEditedCharIndex]
           result.push({ range, str: ('.' + segment) as string })
         }
       }
@@ -423,7 +423,7 @@ export function useComponentBrowserInput(
         }
         break
       case 'editNode':
-        code.value = graphDb.nodeIdToNode.get(usage.node)?.rootSpan.repr() ?? ''
+        code.value = graphDb.nodeIdToNode.get(usage.node)?.rootSpan.code() ?? ''
         selection.value = { start: usage.cursorPos, end: usage.cursorPos }
         break
     }

app/gui2/src/components/GraphEditor.vue

@@ -426,7 +426,7 @@ function copyNodeContent() {
   const id = nodeSelection.selected.values().next().value
   const node = graphStore.db.nodeIdToNode.get(id)
   if (!node) return
-  const content = node.rootSpan.repr()
+  const content = node.rootSpan.code()
   const metadata = projectStore.module?.getNodeMetadata(id) ?? undefined
   const copiedNode: CopiedNode = { expression: content, metadata }
   const clipboardData: ClipboardData = { nodes: [copiedNode] }

app/gui2/src/components/GraphEditor/GraphNode.vue

@@ -348,7 +348,7 @@ function portGroupStyle(port: PortData) {
   >
     <div class="selection" v-on="dragPointer.events"></div>
     <div class="binding" @pointerdown.stop>
-      {{ node.pattern?.repr() ?? '' }}
+      {{ node.pattern?.code() ?? '' }}
     </div>
     <CircularMenu
       v-if="menuVisible"

app/gui2/src/components/GraphEditor/NodeWidget.vue

@@ -84,8 +84,8 @@ provideWidgetUsageInfo(
 
 const spanStart = computed(() => {
   if (!(props.input instanceof Ast.Ast)) return undefined
-  if (props.input.astExtended == null) return undefined
-  return props.input.astExtended.span()[0] - tree.nodeSpanStart
+  if (props.input.span == null) return undefined
+  return props.input.span[0] - tree.nodeSpanStart
 })
 </script>
 

app/gui2/src/components/GraphEditor/NodeWidgetTree.vue

@@ -36,7 +36,7 @@ function handleWidgetUpdates(value: unknown, origin: PortId) {
   } else if (typeof value === 'string') {
     graph.setExpressionContent(origin, value)
   } else if (value instanceof Ast.Ast) {
-    graph.setExpressionContent(origin, value.repr())
+    graph.setExpression(origin, value)
   } else if (value == null) {
     graph.setExpressionContent(origin, '_')
   } else {

app/gui2/src/components/GraphEditor/widgets/WidgetFunction.vue

@@ -101,15 +101,12 @@ const widgetConfiguration = computed(() => {
 /**
  * Process an argument value update. Takes care of inserting assigned placeholder values, as well as
  * handling deletions of arguments and rewriting the applications to named as appropriate.
- *
- * FIXME: This method has to be rewritten usign AST manipulation instead of string concatenation
- * once AST updates are implemented. Depends on #8367
  */
 function handleArgUpdate(value: unknown, origin: PortId): boolean {
   const app = application.value
   if (app instanceof ArgumentApplication) {
     // Find the updated argument by matching origin port/expression with the appropriate argument.
-    // We are insterested only in updates at the top level of the argument AST. Updates from nested
+    // We are interested only in updates at the top level of the argument AST. Updates from nested
     // widgets do not need to be processed at the function application level.
     const argApp = [...app.iterApplications()].find(
       (app) => 'portId' in app.argument && app.argument.portId === origin,
@@ -118,13 +115,17 @@ function handleArgUpdate(value: unknown, origin: PortId): boolean {
     // Perform appropriate AST update, either insertion or deletion.
     if (value != null && argApp?.argument instanceof ArgumentPlaceholder) {
       /* Case: Inserting value to a placeholder. */
-      const codeToInsert = value instanceof Ast.Ast ? value.repr() : value
-      const argCode = argApp.argument.insertAsNamed
-        ? `${argApp.argument.info.name}=${codeToInsert}`
-        : codeToInsert
-
-      // FIXME[#8367]: Create proper application AST instead of concatenating strings.
-      props.onUpdate(`${argApp.appTree.repr()} ${argCode}`, argApp.appTree.exprId)
+      const edit = argApp.appTree.module.edit()
+      let newArg: Ast.Ast | undefined
+      if (value instanceof Ast.Ast) newArg = value
+      else if (typeof value === 'string') newArg = Ast.parse(value, edit)
+      if (!newArg) {
+        console.error(`Don't know how to put this in a tree`, value)
+        return true
+      }
+      const name = argApp.argument.insertAsNamed ? argApp.argument.info.name : null
+      const ast = Ast.App.new(argApp.appTree, name, newArg, edit)
+      props.onUpdate(ast, argApp.appTree.exprId)
       return true
     } else if (value == null && argApp?.argument instanceof ArgumentAst) {
       /* Case: Removing existing argument. */
@@ -147,44 +148,48 @@ function handleArgUpdate(value: unknown, origin: PortId): boolean {
       } else if (argApp.appTree instanceof Ast.App && argApp.appTree.argumentName == null) {
         /* Case: Removing positional prefix argument. */
 
-        // Since the update of this kind can affect following arguments, it is necessary to
-        // construct the new AST for the whole application in order to update it. Because we lack
-        // the ability to do AST manipulation directly, we are forced to do it by string operations.
-        // FIXME[#8367]: Edit application AST instead of concatenating strings.
-        let newRepr = ''
+        // Since the update of this kind can affect following arguments, it may be necessary to
+        // replace the AST for multiple levels of application.
 
+        // The unmodified LHS subtree of the subtree that is being replaced.
+        let innerBound: Ast.Ast | undefined
+        // The top level of the subtree that is being replaced.
+        let outerBound = argApp.appTree
+        // The levels of the application tree to apply to `innerBound` to yield the new `outerBound` expression.
+        const newArgs: { name: string | null; value: Ast.Ast }[] = []
         // Traverse the application chain, starting from the outermost application and going
         // towards the innermost target.
         for (let innerApp of app.iterApplications()) {
           if (innerApp === argApp) {
             // Found the application with the argument to remove. Skip the argument and use the
             // application target's code. This is the final iteration of the loop.
-            newRepr = `${argApp.appTree.function.repr().trimEnd()} ${newRepr.trimStart()}`
-            // Perform the actual update, since we already have the whole new application code
-            // collected.
-            props.onUpdate(newRepr, app.appTree.exprId)
-            return true
+            innerBound = argApp.appTree.function
+            break
           } else {
             // Process an argument to the right of the removed argument.
             assert(innerApp.appTree instanceof Ast.App)
-            const argRepr = innerApp.appTree
-              .repr()
-              .substring(innerApp.appTree.function.repr().length)
-              .trim()
-            if (
-              innerApp.argument instanceof ArgumentAst &&
-              innerApp.appTree.argumentName == null &&
-              innerApp.argument.info != null
-            ) {
+            const infoName =
+              innerApp.argument instanceof ArgumentAst && innerApp.argument.info != null
+                ? innerApp.argument.info?.name ?? null
+                : null
+            if (newArgs.length || (!innerApp.appTree.argumentName && infoName)) {
               // Positional arguments following the deleted argument must all be rewritten to named.
-              newRepr = `${innerApp.argument.info.name}=${argRepr} ${newRepr.trimStart()}`
+              newArgs.unshift({
+                name: infoName,
+                value: innerApp.appTree.argument,
+              })
             } else {
-              // All other arguments are copied as-is.
-              newRepr = `${argRepr} ${newRepr.trimStart()}`
+              // We haven't reached the subtree that needs to be modified yet.
+              outerBound = innerApp.appTree
             }
           }
         }
-        assertUnreachable()
+        assert(innerBound !== undefined)
+        const edit = outerBound.module.edit()
+        let newAst = innerBound
+        for (const arg of newArgs) newAst = Ast.App.new(newAst, arg.name, arg.value, edit)
+        props.onUpdate(newAst, outerBound.exprId)
+        return true
       } else if (value == null && argApp.argument instanceof ArgumentPlaceholder) {
         /* Case: Removing placeholder value. */
         // Do nothing. The argument already doesn't exist, so there is nothing to update.

app/gui2/src/providers/widgetTree.ts

@@ -7,7 +7,7 @@ const { provideFn, injectFn } = createContextStore(
   'Widget tree',
   (astRoot: Ref<Ast.Ast>, hasActiveAnimations: Ref<boolean>) => {
     const nodeId = computed(() => astRoot.value.exprId)
-    const nodeSpanStart = computed(() => astRoot.value.astExtended!.span()[0])
+    const nodeSpanStart = computed(() => astRoot.value.span![0])
     return proxyRefs({ astRoot, nodeId, nodeSpanStart, hasActiveAnimations })
   },
 )

app/gui2/src/stores/graph/__tests__/graphDatabase.test.ts

@@ -32,7 +32,7 @@ test('Reading graph from definition', () => {
   const db = GraphDb.Mock()
   const ast = Ast.parseTransitional(code, idMap)
   assert(ast instanceof Ast.BodyBlock)
-  const expressions = Array.from(ast.expressions())
+  const expressions = Array.from(ast.statements())
   const func = expressions[0]
   assert(func instanceof Ast.Function)
   db.readFunctionAst(func, (_) => ({ x: 0.0, y: 0.0, vis: null }))

app/gui2/src/stores/graph/graphDatabase.ts

@@ -17,9 +17,9 @@ import { methodPointerEquals, type MethodCall } from 'shared/languageServerTypes
 import {
   IdMap,
   visMetadataEquals,
-  type ContentRange,
   type ExprId,
   type NodeMetadata,
+  type SourceRange,
   type VisualizationMetadata,
 } from 'shared/yjsModel'
 import { ref, type Ref } from 'vue'
@@ -90,9 +90,9 @@ export class BindingsDb {
   private static rangeMappings(
     ast: RawAstExtended,
     analyzer: AliasAnalyzer,
-  ): [MappedKeyMap<ContentRange, RawAstExtended>, Map<ExprId, ContentRange>] {
-    const bindingRangeToTree = new MappedKeyMap<ContentRange, RawAstExtended>(IdMap.keyForRange)
-    const bindingIdToRange = new Map<ExprId, ContentRange>()
+  ): [MappedKeyMap<SourceRange, RawAstExtended>, Map<ExprId, SourceRange>] {
+    const bindingRangeToTree = new MappedKeyMap<SourceRange, RawAstExtended>(IdMap.keyForRange)
+    const bindingIdToRange = new Map<ExprId, SourceRange>()
     const bindingRanges = new MappedSet(IdMap.keyForRange)
     for (const [binding, usages] of analyzer.aliases) {
       bindingRanges.add(binding)