diff --git a/app/gui/src/project-view/util/ast/__tests__/abstract.test.ts b/app/gui/src/project-view/util/ast/__tests__/abstract.test.ts index ec3305bc0d2b..ecc768e54336 100644 --- a/app/gui/src/project-view/util/ast/__tests__/abstract.test.ts +++ b/app/gui/src/project-view/util/ast/__tests__/abstract.test.ts @@ -673,6 +673,30 @@ describe('Code edit', () => { expect(after['func 123 arg2'].id).toBe(before['func arg1 arg2'].id) }) + test('syncToCode does not create unneeded AST nodes', () => { + const beforeRoot = Ast.parseModule('main = func 1 2\n') + beforeRoot.module.setRoot(beforeRoot) + const edit = beforeRoot.module.edit() + const newCode = 'main = func 10 2\n' + let changes: Record | undefined = undefined + edit.observe( + (update) => + (changes = { + added: update.nodesAdded.size, + deleted: update.nodesDeleted.size, + updated: update.nodesUpdated.size, + }), + ) + edit.syncToCode(newCode) + expect(edit.root()?.code()).toBe(newCode) + expect(edit.root()?.id).toBe(beforeRoot.id) + expect(changes).toEqual({ + added: 0, + deleted: 0, + updated: 1, + }) + }) + test('Insert argument names', () => { const beforeRoot = Ast.parseExpression('func arg1 arg2') assertDefined(beforeRoot) diff --git a/app/ydoc-shared/src/ast/mutableModule.ts b/app/ydoc-shared/src/ast/mutableModule.ts index cfa4ca5fdbf2..748b7e56b7ed 100644 --- a/app/ydoc-shared/src/ast/mutableModule.ts +++ b/app/ydoc-shared/src/ast/mutableModule.ts @@ -180,6 +180,19 @@ export class MutableModule implements Module { return materializeMutable(this, fields) as Owned> } + /** + * Copy the node into the module *without copying children*. The returned object, and the module containing it, may be + * in an invalid state until the callee has ensured all references are resolvable, e.g. by recursing into children. + * @internal + */ + splice(ast: T): Owned> { + assert(ast.module !== this) + const fields = ast.fields.clone() as any + this.nodes.set(ast.id, fields) + fields.set('parent', undefined) + return materializeMutable(this, fields) as Owned> + } + /** TODO: Add docs */ static Transient() { return new this(new Y.Doc()) diff --git a/app/ydoc-shared/src/ast/parse.ts b/app/ydoc-shared/src/ast/parse.ts index b25071f7cbe9..134aff9ba2cd 100644 --- a/app/ydoc-shared/src/ast/parse.ts +++ b/app/ydoc-shared/src/ast/parse.ts @@ -536,12 +536,12 @@ export function setExternalIds(edit: MutableModule, spans: SpanMap, ids: IdMap): * context. */ export function parseInSameContext( - module: MutableModule, code: string, ast: Ast, + module?: MutableModule, ): { root: Owned; spans: SpanMap } { const rawParsed = rawParseInContext(code, getParseContext(ast)) - return abstract(module, rawParsed, code) + return abstract(module ?? MutableModule.Transient(), rawParsed, code) } type ParseContext = 'module' | 'block' | 'expression' | 'statement' diff --git a/app/ydoc-shared/src/ast/syncToCode.ts b/app/ydoc-shared/src/ast/syncToCode.ts index e67afb70a3c7..77c5feb376d2 100644 --- a/app/ydoc-shared/src/ast/syncToCode.ts +++ b/app/ydoc-shared/src/ast/syncToCode.ts @@ -29,7 +29,6 @@ import { type AstId, MutableAssignment, type MutableAst, - type Owned, rewriteRefs, syncFields, syncNodeMetadata, @@ -175,38 +174,36 @@ export function applyTextEditsToAst( ) { const printed = printWithSpans(ast) const code = applyTextEdits(printed.code, textEdits) - ast.module.transact(() => { - const parsed = parseInSameContext(ast.module, code, ast) - const toSync = calculateCorrespondence( - ast, - printed.info.nodes, - parsed.root, - parsed.spans.nodes, - textEdits, - code, - ) - syncTree(ast, parsed.root, toSync, ast.module, metadataSource) - }) + const parsed = parseInSameContext(code, ast) + const toSync = calculateCorrespondence( + ast, + printed.info.nodes, + parsed.root, + parsed.spans.nodes, + textEdits, + code, + ) + ast.module.transact(() => syncTree(ast, parsed.root, toSync, ast.module, metadataSource)) } /** Replace `target` with `newContent`, reusing nodes according to the correspondence in `toSync`. */ function syncTree( target: MutableAst, - newContent: Owned, + newContent: Ast, toSync: Map, edit: MutableModule, metadataSource: Module, ) { const newIdToEquivalent = new Map() for (const [beforeId, after] of toSync) newIdToEquivalent.set(after.id, beforeId) - const childReplacerFor = (parentId: AstId) => (id: AstId) => { + const childSplicerFor = (parentId: AstId) => (id: AstId) => { const original = newIdToEquivalent.get(id) if (original) { const replacement = edit.get(original) if (replacement.parentId !== parentId) replacement.fields.set('parent', parentId) return original } else { - const child = edit.get(id) + const child = edit.splice(newContent.module.get(id)!) if (child.parentId !== parentId) child.fields.set('parent', parentId) } } @@ -215,12 +212,16 @@ function syncTree( const parent = edit.get(parentId) const targetSyncEquivalent = toSync.get(target.id) const syncRoot = targetSyncEquivalent?.id === newContent.id ? targetSyncEquivalent : undefined - if (!syncRoot) { - parent.replaceChild(target.id, newContent) - newContent.fields.set('metadata', target.fields.get('metadata').clone()) + let newRoot: MutableAst + if (syncRoot) { + newRoot = target + } else { + const spliced = edit.splice(newContent) + parent.replaceChild(target.id, spliced) + newRoot = spliced + newRoot.fields.set('metadata', target.fields.get('metadata').clone()) target.fields.get('metadata').set('externalId', newExternalId()) } - const newRoot = syncRoot ? target : newContent newRoot.visitRecursive(ast => { const syncFieldsFrom = toSync.get(ast.id) const editAst = edit.getVersion(ast) @@ -229,7 +230,7 @@ function syncTree( ast instanceof Assignment ? metadataSource.get(ast.fields.get('expression').node) : undefined - syncFields(edit.getVersion(ast), syncFieldsFrom, childReplacerFor(ast.id)) + syncFields(editAst, syncFieldsFrom, childSplicerFor(ast.id)) if (editAst instanceof MutableAssignment && originalAssignmentExpression) { if (editAst.expression.externalId !== originalAssignmentExpression.externalId) editAst.expression.setExternalId(originalAssignmentExpression.externalId) @@ -239,7 +240,7 @@ function syncTree( ) } } else { - rewriteRefs(editAst, childReplacerFor(ast.id)) + rewriteRefs(editAst, childSplicerFor(ast.id)) } return true })