Fix AST spacing (#9366)

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.
This commit is contained in:
Adam Obuchowicz 2024-03-12 02:57:29 +01:00 committed by GitHub
parent c401694fa9
commit a01aeab3a4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 240 additions and 214 deletions

View File

@ -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()

View File

@ -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 {

View File

@ -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
@ -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),
}
}
@ -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 {
@ -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> {

View File

@ -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 {
@ -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)

View File

@ -3,7 +3,7 @@ import { Ast } from '@/util/ast'
import { tryQualifiedName } from '@/util/qualifiedName'
import { initializeFFI } from 'shared/ast/ffi'
import { unwrap } from 'shared/util/data/result'
import { expect, test } from 'vitest'
import { describe, expect, test } from 'vitest'
import { MutableModule, substituteQualifiedName, type Identifier } from '../abstract'
import { findExpressions, testCase, tryFindExpressions } from './testCase'
@ -454,6 +454,24 @@ test('Replace subexpression', () => {
expect(printed).toEqual("main =\n text1 = 'bar'\n")
})
test('Modify subexpression - setting a vector', () => {
// A case where the #9357 bug was visible.
const code = 'main =\n text1 = foo\n'
const root = Ast.parseBlock(code)
const main = Ast.functionBlock(root, 'main')!
expect(main).not.toBeNull()
const assignment: Ast.Assignment = main.statements().next().value
expect(assignment).toBeInstanceOf(Ast.Assignment)
const edit = root.module.edit()
const transientModule = MutableModule.Transient()
const newValue = Ast.Vector.new(transientModule, [Ast.parse('bar')])
expect(newValue.code()).toBe('[bar]')
edit.replaceValue(assignment.expression.id, newValue)
const printed = edit.getVersion(root).code()
expect(printed).toEqual('main =\n text1 = [bar]\n')
})
test('Change ID of node', () => {
const { root, assignment } = simpleModule()
expect(assignment.expression).not.toBeNull()
@ -568,213 +586,215 @@ test('Tree repair: Non-canonical block line attribution', () => {
expect(afterRepair['main ='].id).toBe(before['main ='].id)
})
test('Code edit: Change argument type', () => {
const beforeRoot = Ast.parse('func arg1 arg2')
beforeRoot.module.replaceRoot(beforeRoot)
const before = findExpressions(beforeRoot, {
func: Ast.Ident,
arg1: Ast.Ident,
arg2: Ast.Ident,
'func arg1': Ast.App,
'func arg1 arg2': Ast.App,
describe('Code edit', () => {
test('Change argument type', () => {
const beforeRoot = Ast.parse('func arg1 arg2')
beforeRoot.module.replaceRoot(beforeRoot)
const before = findExpressions(beforeRoot, {
func: Ast.Ident,
arg1: Ast.Ident,
arg2: Ast.Ident,
'func arg1': Ast.App,
'func arg1 arg2': Ast.App,
})
const edit = beforeRoot.module.edit()
const newCode = 'func 123 arg2'
edit.getVersion(beforeRoot).syncToCode(newCode)
// Ensure the change was made.
expect(edit.root()?.code()).toBe(newCode)
// Ensure the identities of all the original nodes were maintained.
const after = findExpressions(edit.root()!, {
func: Ast.Ident,
'123': Ast.NumericLiteral,
arg2: Ast.Ident,
'func 123': Ast.App,
'func 123 arg2': Ast.App,
})
expect(after.func.id).toBe(before.func.id)
expect(after.arg2.id).toBe(before.arg2.id)
expect(after['func 123'].id).toBe(before['func arg1'].id)
expect(after['func 123 arg2'].id).toBe(before['func arg1 arg2'].id)
})
const edit = beforeRoot.module.edit()
const newCode = 'func 123 arg2'
edit.getVersion(beforeRoot).syncToCode(newCode)
// Ensure the change was made.
expect(edit.root()?.code()).toBe(newCode)
// Ensure the identities of all the original nodes were maintained.
const after = findExpressions(edit.root()!, {
func: Ast.Ident,
'123': Ast.NumericLiteral,
arg2: Ast.Ident,
'func 123': Ast.App,
'func 123 arg2': Ast.App,
test('Insert argument names', () => {
const beforeRoot = Ast.parse('func arg1 arg2')
beforeRoot.module.replaceRoot(beforeRoot)
const before = findExpressions(beforeRoot, {
func: Ast.Ident,
arg1: Ast.Ident,
arg2: Ast.Ident,
'func arg1': Ast.App,
'func arg1 arg2': Ast.App,
})
const edit = beforeRoot.module.edit()
const newCode = 'func name1=arg1 name2=arg2'
edit.getVersion(beforeRoot).syncToCode(newCode)
// Ensure the change was made.
expect(edit.root()?.code()).toBe(newCode)
// Ensure the identities of all the original nodes were maintained.
const after = findExpressions(edit.root()!, {
func: Ast.Ident,
arg1: Ast.Ident,
arg2: Ast.Ident,
'func name1=arg1': Ast.App,
'func name1=arg1 name2=arg2': Ast.App,
})
expect(after.func.id).toBe(before.func.id)
expect(after.arg1.id).toBe(before.arg1.id)
expect(after.arg2.id).toBe(before.arg2.id)
expect(after['func name1=arg1'].id).toBe(before['func arg1'].id)
expect(after['func name1=arg1 name2=arg2'].id).toBe(before['func arg1 arg2'].id)
})
expect(after.func.id).toBe(before.func.id)
expect(after.arg2.id).toBe(before.arg2.id)
expect(after['func 123'].id).toBe(before['func arg1'].id)
expect(after['func 123 arg2'].id).toBe(before['func arg1 arg2'].id)
})
test('Code edit: Insert argument names', () => {
const beforeRoot = Ast.parse('func arg1 arg2')
beforeRoot.module.replaceRoot(beforeRoot)
const before = findExpressions(beforeRoot, {
func: Ast.Ident,
arg1: Ast.Ident,
arg2: Ast.Ident,
'func arg1': Ast.App,
'func arg1 arg2': Ast.App,
test('Remove argument names', () => {
const beforeRoot = Ast.parse('func name1=arg1 name2=arg2')
beforeRoot.module.replaceRoot(beforeRoot)
const before = findExpressions(beforeRoot, {
func: Ast.Ident,
arg1: Ast.Ident,
arg2: Ast.Ident,
'func name1=arg1': Ast.App,
'func name1=arg1 name2=arg2': Ast.App,
})
const edit = beforeRoot.module.edit()
const newCode = 'func arg1 arg2'
edit.getVersion(beforeRoot).syncToCode(newCode)
// Ensure the change was made.
expect(edit.root()?.code()).toBe(newCode)
// Ensure the identities of all the original nodes were maintained.
const after = findExpressions(edit.root()!, {
func: Ast.Ident,
arg1: Ast.Ident,
arg2: Ast.Ident,
'func arg1': Ast.App,
'func arg1 arg2': Ast.App,
})
expect(after.func.id).toBe(before.func.id)
expect(after.arg1.id).toBe(before.arg1.id)
expect(after.arg2.id).toBe(before.arg2.id)
expect(after['func arg1'].id).toBe(before['func name1=arg1'].id)
expect(after['func arg1 arg2'].id).toBe(before['func name1=arg1 name2=arg2'].id)
})
const edit = beforeRoot.module.edit()
const newCode = 'func name1=arg1 name2=arg2'
edit.getVersion(beforeRoot).syncToCode(newCode)
// Ensure the change was made.
expect(edit.root()?.code()).toBe(newCode)
// Ensure the identities of all the original nodes were maintained.
const after = findExpressions(edit.root()!, {
func: Ast.Ident,
arg1: Ast.Ident,
arg2: Ast.Ident,
'func name1=arg1': Ast.App,
'func name1=arg1 name2=arg2': Ast.App,
test('Rearrange block', () => {
const beforeCase = testCase({
'main =': Ast.Function,
' call_result = func sum 12': Ast.Assignment,
' sum = value + 23': Ast.Assignment,
' value = 42': Ast.Assignment,
})
const before = beforeCase.statements
const edit = beforeCase.module.edit()
const newCode = [
'main =',
'\n value = 42',
'\n sum = value + 23',
'\n call_result = func sum 12',
].join('')
edit.root()!.syncToCode(newCode)
// Ensure the change was made.
expect(edit.root()?.code()).toBe(newCode)
// Ensure the identities of all the original nodes were maintained.
const after = tryFindExpressions(edit.root()!, {
'main =': Ast.Function,
'call_result = func sum 12': Ast.Assignment,
'sum = value + 23': Ast.Assignment,
'value = 42': Ast.Assignment,
})
expect(after['call_result = func sum 12']?.id).toBe(before[' call_result = func sum 12'].id)
expect(after['sum = value + 23']?.id).toBe(before[' sum = value + 23'].id)
expect(after['value = 42']?.id).toBe(before[' value = 42'].id)
})
expect(after.func.id).toBe(before.func.id)
expect(after.arg1.id).toBe(before.arg1.id)
expect(after.arg2.id).toBe(before.arg2.id)
expect(after['func name1=arg1'].id).toBe(before['func arg1'].id)
expect(after['func name1=arg1 name2=arg2'].id).toBe(before['func arg1 arg2'].id)
})
test('Code edit: Remove argument names', () => {
const beforeRoot = Ast.parse('func name1=arg1 name2=arg2')
beforeRoot.module.replaceRoot(beforeRoot)
const before = findExpressions(beforeRoot, {
func: Ast.Ident,
arg1: Ast.Ident,
arg2: Ast.Ident,
'func name1=arg1': Ast.App,
'func name1=arg1 name2=arg2': Ast.App,
test('Inline expression change', () => {
const beforeRoot = Ast.parse('func name1=arg1 name2=arg2')
beforeRoot.module.replaceRoot(beforeRoot)
const before = findExpressions(beforeRoot, {
func: Ast.Ident,
arg1: Ast.Ident,
arg2: Ast.Ident,
'func name1=arg1': Ast.App,
'func name1=arg1 name2=arg2': Ast.App,
})
const edit = beforeRoot.module.edit()
const newArg1Code = 'arg1+1'
edit.getVersion(before['arg1']).syncToCode(newArg1Code)
// Ensure the change was made.
expect(edit.root()?.code()).toBe('func name1=arg1+1 name2=arg2')
// Ensure the identities of all the original nodes were maintained.
const after = findExpressions(edit.root()!, {
func: Ast.Ident,
arg1: Ast.Ident,
arg2: Ast.Ident,
'arg1+1': Ast.OprApp,
'func name1=arg1+1': Ast.App,
'func name1=arg1+1 name2=arg2': Ast.App,
})
expect(after.func.id).toBe(before.func.id)
expect(after.arg1.id).toBe(before.arg1.id)
expect(after.arg2.id).toBe(before.arg2.id)
expect(after['func name1=arg1+1'].id).toBe(before['func name1=arg1'].id)
expect(after['func name1=arg1+1 name2=arg2'].id).toBe(before['func name1=arg1 name2=arg2'].id)
})
const edit = beforeRoot.module.edit()
const newCode = 'func arg1 arg2'
edit.getVersion(beforeRoot).syncToCode(newCode)
// Ensure the change was made.
expect(edit.root()?.code()).toBe(newCode)
// Ensure the identities of all the original nodes were maintained.
const after = findExpressions(edit.root()!, {
func: Ast.Ident,
arg1: Ast.Ident,
arg2: Ast.Ident,
'func arg1': Ast.App,
'func arg1 arg2': Ast.App,
test('No-op inline expression change', () => {
const code = 'a = 1'
const expression = Ast.parse(code)
const module = expression.module
module.replaceRoot(expression)
expression.syncToCode(code)
expect(module.root()?.code()).toBe(code)
})
expect(after.func.id).toBe(before.func.id)
expect(after.arg1.id).toBe(before.arg1.id)
expect(after.arg2.id).toBe(before.arg2.id)
expect(after['func arg1'].id).toBe(before['func name1=arg1'].id)
expect(after['func arg1 arg2'].id).toBe(before['func name1=arg1 name2=arg2'].id)
})
test('Code edit: Rearrange block', () => {
const beforeCase = testCase({
'main =': Ast.Function,
' call_result = func sum 12': Ast.Assignment,
' sum = value + 23': Ast.Assignment,
' value = 42': Ast.Assignment,
test('No-op block change', () => {
const code = 'a = 1\nb = 2\n'
const block = Ast.parseBlock(code)
const module = block.module
module.replaceRoot(block)
block.syncToCode(code)
expect(module.root()?.code()).toBe(code)
})
const before = beforeCase.statements
const edit = beforeCase.module.edit()
const newCode = [
'main =',
'\n value = 42',
'\n sum = value + 23',
'\n call_result = func sum 12',
].join('')
edit.root()!.syncToCode(newCode)
// Ensure the change was made.
expect(edit.root()?.code()).toBe(newCode)
// Ensure the identities of all the original nodes were maintained.
const after = tryFindExpressions(edit.root()!, {
'main =': Ast.Function,
'call_result = func sum 12': Ast.Assignment,
'sum = value + 23': Ast.Assignment,
'value = 42': Ast.Assignment,
test('Shifting whitespace ownership', () => {
const beforeRoot = Ast.parseBlock('value = 1 +\n')
beforeRoot.module.replaceRoot(beforeRoot)
const before = findExpressions(beforeRoot, {
value: Ast.Ident,
'1': Ast.NumericLiteral,
'value = 1 +': Ast.Assignment,
})
const edit = beforeRoot.module.edit()
const newCode = 'value = 1 \n'
edit.getVersion(beforeRoot).syncToCode(newCode)
// Ensure the change was made.
expect(edit.root()?.code()).toBe(newCode)
// Ensure the identities of all the original nodes were maintained.
const after = findExpressions(edit.root()!, {
value: Ast.Ident,
'1': Ast.NumericLiteral,
'value = 1': Ast.Assignment,
})
expect(after.value.id).toBe(before.value.id)
expect(after['1'].id).toBe(before['1'].id)
expect(after['value = 1'].id).toBe(before['value = 1 +'].id)
})
expect(after['call_result = func sum 12']?.id).toBe(before[' call_result = func sum 12'].id)
expect(after['sum = value + 23']?.id).toBe(before[' sum = value + 23'].id)
expect(after['value = 42']?.id).toBe(before[' value = 42'].id)
})
test('Code edit: Inline expression change', () => {
const beforeRoot = Ast.parse('func name1=arg1 name2=arg2')
beforeRoot.module.replaceRoot(beforeRoot)
const before = findExpressions(beforeRoot, {
func: Ast.Ident,
arg1: Ast.Ident,
arg2: Ast.Ident,
'func name1=arg1': Ast.App,
'func name1=arg1 name2=arg2': Ast.App,
test('merging', () => {
const block = Ast.parseBlock('a = 1\nb = 2')
const module = block.module
module.replaceRoot(block)
const editA = module.edit()
editA.getVersion(block).syncToCode('a = 10\nb = 2')
const editB = module.edit()
editB.getVersion(block).syncToCode('a = 1\nb = 20')
module.applyEdit(editA)
module.applyEdit(editB)
expect(module.root()?.code()).toBe('a = 10\nb = 20')
})
const edit = beforeRoot.module.edit()
const newArg1Code = 'arg1+1'
edit.getVersion(before['arg1']).syncToCode(newArg1Code)
// Ensure the change was made.
expect(edit.root()?.code()).toBe('func name1=arg1+1 name2=arg2')
// Ensure the identities of all the original nodes were maintained.
const after = findExpressions(edit.root()!, {
func: Ast.Ident,
arg1: Ast.Ident,
arg2: Ast.Ident,
'arg1+1': Ast.OprApp,
'func name1=arg1+1': Ast.App,
'func name1=arg1+1 name2=arg2': Ast.App,
})
expect(after.func.id).toBe(before.func.id)
expect(after.arg1.id).toBe(before.arg1.id)
expect(after.arg2.id).toBe(before.arg2.id)
expect(after['func name1=arg1+1'].id).toBe(before['func name1=arg1'].id)
expect(after['func name1=arg1+1 name2=arg2'].id).toBe(before['func name1=arg1 name2=arg2'].id)
})
test('Code edit: No-op inline expression change', () => {
const code = 'a = 1'
const expression = Ast.parse(code)
const module = expression.module
module.replaceRoot(expression)
expression.syncToCode(code)
expect(module.root()?.code()).toBe(code)
})
test('Code edit: No-op block change', () => {
const code = 'a = 1\nb = 2\n'
const block = Ast.parseBlock(code)
const module = block.module
module.replaceRoot(block)
block.syncToCode(code)
expect(module.root()?.code()).toBe(code)
})
test('Code edit: Shifting whitespace ownership', () => {
const beforeRoot = Ast.parseBlock('value = 1 +\n')
beforeRoot.module.replaceRoot(beforeRoot)
const before = findExpressions(beforeRoot, {
value: Ast.Ident,
'1': Ast.NumericLiteral,
'value = 1 +': Ast.Assignment,
})
const edit = beforeRoot.module.edit()
const newCode = 'value = 1 \n'
edit.getVersion(beforeRoot).syncToCode(newCode)
// Ensure the change was made.
expect(edit.root()?.code()).toBe(newCode)
// Ensure the identities of all the original nodes were maintained.
const after = findExpressions(edit.root()!, {
value: Ast.Ident,
'1': Ast.NumericLiteral,
'value = 1': Ast.Assignment,
})
expect(after.value.id).toBe(before.value.id)
expect(after['1'].id).toBe(before['1'].id)
expect(after['value = 1'].id).toBe(before['value = 1 +'].id)
})
test('Code edit merging', () => {
const block = Ast.parseBlock('a = 1\nb = 2')
const module = block.module
module.replaceRoot(block)
const editA = module.edit()
editA.getVersion(block).syncToCode('a = 10\nb = 2')
const editB = module.edit()
editB.getVersion(block).syncToCode('a = 1\nb = 20')
module.applyEdit(editA)
module.applyEdit(editB)
expect(module.root()?.code()).toBe('a = 10\nb = 20')
})
test('Analyze app-like', () => {

View File

@ -2,6 +2,7 @@
import * as fs from 'node:fs'
import * as path from 'node:path'
import * as url from 'node:url'
import * as v from 'vitest'
@ -28,7 +29,8 @@ function testSchema(json: unknown, fileName: string): void {
}
// We need to go up from `app/ide-desktop/lib/dashboard/` to the root of the repo
const REPO_ROOT = '../../../../'
const DIR_DEPTH = 7
const REPO_ROOT = url.fileURLToPath(new URL('../'.repeat(DIR_DEPTH), import.meta.url))
const BASE_DATA_LINKS_ROOT = path.resolve(REPO_ROOT, 'test/Base_Tests/data/datalinks/')
const S3_DATA_LINKS_ROOT = path.resolve(REPO_ROOT, 'test/AWS_Tests/data/')
const TABLE_DATA_LINKS_ROOT = path.resolve(REPO_ROOT, 'test/Table_Tests/data/datalinks/')

1
vitest.workspace.ts Normal file
View File

@ -0,0 +1 @@
export default ['app/gui2/vitest.config.ts', 'app/ide-desktop/lib/dashboard/vitest.config.ts']