Skip to content

Commit

Permalink
Fix AST spacing (#9366)
Browse files Browse the repository at this point in the history
Fixes #9357

The main issue was the spread operator using at the wrong place in functions overriding spacing of nodes. The bug, to be visible, required copying AST node before, because during copying `whitespace` field was explicitly set to undefined (in opposite to being unset), what in turns make spread overriding the value set by those functions.

# Important Notes
* To enable VSCode debugging, added a workspace for vitest and fix any relative path to be working-dir independent.
  • Loading branch information
farmaazon authored Mar 12, 2024
1 parent c401694 commit a01aeab
Show file tree
Hide file tree
Showing 7 changed files with 236 additions and 210 deletions.
5 changes: 4 additions & 1 deletion app/gui2/shared/ast/ffi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ export function xxHash128(input: string) {
export async function initializeFFI(path?: string | undefined) {
if (isNode) {
const fs = await import('node:fs/promises')
const buffer = fs.readFile(path ?? './rust-ffi/pkg/rust_ffi_bg.wasm')
const { fileURLToPath, URL: nodeURL } = await import('node:url')
const buffer = fs.readFile(
path ?? fileURLToPath(new nodeURL('../../rust-ffi/pkg/rust_ffi_bg.wasm', import.meta.url)),
)
await init(buffer)
} else {
await init()
Expand Down
2 changes: 1 addition & 1 deletion app/gui2/shared/ast/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export function asOwned<T>(t: T): Owned<T> {
return t as Owned<T>
}

export type NodeChild<T> = { whitespace?: string | undefined; node: T }
export type NodeChild<T> = { whitespace: string | undefined; node: T }
export type RawNodeChild = NodeChild<AstId> | NodeChild<SyncTokenId>

export function newExternalId(): ExternalId {
Expand Down
30 changes: 15 additions & 15 deletions app/gui2/shared/ast/tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -642,17 +642,20 @@ function ensureSpacedOnlyIf<T>(
}
function ensureSpaced<T>(child: NodeChild<T>, verbatim: boolean | undefined): NodeChild<T> {
if (verbatim && child.whitespace != null) return child
return child.whitespace ? child : { whitespace: ' ', ...child }
return child.whitespace ? child : { ...child, whitespace: ' ' }
}
function ensureUnspaced<T>(child: NodeChild<T>, verbatim: boolean | undefined): NodeChild<T> {
if (verbatim && child.whitespace != null) return child
return child.whitespace === '' ? child : { whitespace: '', ...child }
return child.whitespace === '' ? child : { ...child, whitespace: '' }
}
function preferSpacedIf<T>(child: NodeChild<T>, condition: boolean): NodeChild<T> {
return condition ? preferSpaced(child) : preferUnspaced(child)
}
function preferUnspaced<T>(child: NodeChild<T>): NodeChild<T> {
return child.whitespace === undefined ? { whitespace: '', ...child } : child
return child.whitespace === undefined ? { ...child, whitespace: '' } : child
}
function preferSpaced<T>(child: NodeChild<T>): NodeChild<T> {
return child.whitespace === undefined ? { whitespace: ' ', ...child } : child
return child.whitespace === undefined ? { ...child, whitespace: ' ' } : child
}
export class MutableApp extends App implements MutableAst {
declare readonly module: MutableModule
Expand Down Expand Up @@ -1031,7 +1034,7 @@ function multiSegmentAppSegment<T extends MutableAst>(
body: Owned<T> | undefined,
): MultiSegmentAppSegment<OwnedRefs> | undefined {
return {
header: { node: Token.new(header, RawAst.Token.Type.Ident) },
header: autospaced(Token.new(header, RawAst.Token.Type.Ident)),
body: spaced(body ? (body as any) : undefined),
}
}
Expand Down Expand Up @@ -1777,12 +1780,7 @@ export class Function extends Ast {
yield name
for (const def of argumentDefinitions) yield* def
yield { whitespace: equals.whitespace ?? ' ', node: this.module.getToken(equals.node) }
if (body)
yield ensureSpacedOnlyIf(
body,
!(this.module.tryGet(body.node) instanceof BodyBlock),
verbatim,
)
if (body) yield preferSpacedIf(body, this.module.tryGet(body.node) instanceof BodyBlock)
}
}
export class MutableFunction extends Function implements MutableAst {
Expand Down Expand Up @@ -2526,11 +2524,13 @@ function unspaced<T extends object | string>(node: T | undefined): NodeChild<T>
return { whitespace: '', node }
}

function autospaced<T extends object | string>(node: T): NodeChild<T>
function autospaced<T extends object | string>(node: T | undefined): NodeChild<T> | undefined
function autospaced<T extends object | string>(node: T | undefined): NodeChild<T> | undefined {
export function autospaced<T extends object | string>(node: T): NodeChild<T>
export function autospaced<T extends object | string>(node: T | undefined): NodeChild<T> | undefined
export function autospaced<T extends object | string>(
node: T | undefined,
): NodeChild<T> | undefined {
if (node === undefined) return node
return { node }
return { whitespace: undefined, node }
}

export interface Removed<T extends MutableAst> {
Expand Down
4 changes: 2 additions & 2 deletions app/gui2/src/components/GraphEditor/collapsing.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { asNodeId, GraphDb, type NodeId } from '@/stores/graph/graphDatabase'
import { assert, assertDefined } from '@/util/assert'
import { Ast } from '@/util/ast'
import { isIdentifier, moduleMethodNames, type Identifier } from '@/util/ast/abstract'
import { autospaced, isIdentifier, moduleMethodNames, type Identifier } from '@/util/ast/abstract'
import { nodeFromAst } from '@/util/ast/node'
import { unwrap } from '@/util/data/result'
import {
Expand Down Expand Up @@ -185,7 +185,7 @@ export function performCollapse(
if (astIdsToExtract.has(ast.id)) {
collapsed.push(ast)
if (ast.id === astIdToReplace) {
refactored.push({ expression: { node: refactoredAst } })
refactored.push({ expression: autospaced(refactoredAst) })
}
} else {
refactored.push(line)
Expand Down
Loading

0 comments on commit a01aeab

Please sign in to comment.