diff --git a/app/gui/integration-test/project-view/componentBrowser.spec.ts b/app/gui/integration-test/project-view/componentBrowser.spec.ts index 5d22feb6e034..fb1201ba7e7c 100644 --- a/app/gui/integration-test/project-view/componentBrowser.spec.ts +++ b/app/gui/integration-test/project-view/componentBrowser.spec.ts @@ -4,11 +4,6 @@ import * as locate from './locate' const ACCEPT_INPUT_SHORTCUT = `ControlOrMeta+Enter` -async function deselectAllNodes(page: Page) { - await page.keyboard.press('Escape') - await expect(locate.selectedNodes(page)).toHaveCount(0) -} - async function expectAndCancelBrowser( page: Page, expectedText: string, @@ -157,49 +152,40 @@ test('Graph Editor pans to Component Browser', async ({ editorPage, page }) => { await expectAndCancelBrowser(page, '', null) }) -test('Accepting suggestion', async ({ editorPage, page }) => { - // Clicking entry - await editorPage - await locate.addNewNodeButton(page).click() - let nodeCount = await locate.graphNode(page).count() - await locate.componentBrowserEntry(page).nth(1).click() - await expect(locate.componentBrowser(page)).toBeHidden() - await expect(locate.graphNode(page)).toHaveCount(nodeCount + 1) - await expect(locate.graphNode(page).last().locator('.WidgetToken')).toHaveText([ - 'Data', - '.', - 'read_many', - ]) - await expect(locate.graphNode(page).last()).toBeSelected() - - // Clicking at highlighted entry - nodeCount = await locate.graphNode(page).count() - await deselectAllNodes(page) - await locate.addNewNodeButton(page).click() - await locate.componentBrowserSelectedEntry(page).first().click() - await expect(locate.componentBrowser(page)).toBeHidden() - await expect(locate.graphNode(page)).toHaveCount(nodeCount + 1) - await expect(locate.graphNode(page).last().locator('.WidgetToken')).toHaveText([ - 'Data', - '.', - 'read', - ]) - await expect(locate.graphNode(page).last()).toBeSelected() - - // Accepting with Enter - nodeCount = await locate.graphNode(page).count() - await deselectAllNodes(page) - await locate.addNewNodeButton(page).click() - await expect(locate.componentBrowserInput(page)).toBeFocused() - await page.keyboard.press('Enter') - await expect(locate.componentBrowser(page)).toBeHidden() - await expect(locate.graphNode(page)).toHaveCount(nodeCount + 1) - await expect(locate.graphNode(page).last().locator('.WidgetToken')).toHaveText([ - 'Data', - '.', - 'read', - ]) - await expect(locate.graphNode(page).last()).toBeSelected() +test.describe('Accepting suggestion', () => { + async function checkAcceptSuggestion( + page: Page, + acceptSuggestion: () => Promise, + expected: string[], + ) { + await locate.addNewNodeButton(page).click() + const nodeCount = await locate.graphNode(page).count() + await acceptSuggestion() + await expect(locate.componentBrowser(page)).toBeHidden() + await expect(locate.graphNode(page)).toHaveCount(nodeCount + 1) + await expect(locate.graphNode(page).last().locator('.WidgetToken')).toHaveText(expected) + await expect(locate.graphNode(page).last()).toBeSelected() + } + test('Accept suggestion by clicking entry', async ({ editorPage, page }) => { + await editorPage + await checkAcceptSuggestion(page, () => locate.componentBrowserEntry(page).nth(1).click(), [ + 'Data', + '.', + 'read_many', + ]) + }) + test('Accept suggestion by clicking highlighted entry', async ({ editorPage, page }) => { + await editorPage + await checkAcceptSuggestion( + page, + () => locate.componentBrowserSelectedEntry(page).first().click(), + ['Data', '.', 'read'], + ) + }) + test('Accept suggestion with Enter', async ({ editorPage, page }) => { + await editorPage + await checkAcceptSuggestion(page, () => page.keyboard.press('Enter'), ['Data', '.', 'read']) + }) }) test('Accepting any written input', async ({ editorPage, page }) => { diff --git a/app/gui/src/project-view/components/ComponentBrowser/ComponentList.vue b/app/gui/src/project-view/components/ComponentBrowser/ComponentList.vue index d39eb2f8b9fc..bc7137949222 100644 --- a/app/gui/src/project-view/components/ComponentBrowser/ComponentList.vue +++ b/app/gui/src/project-view/components/ComponentBrowser/ComponentList.vue @@ -11,9 +11,8 @@ import SvgIcon from '@/components/SvgIcon.vue' import VirtualizedList from '@/components/VirtualizedList.vue' import { groupColorStyle } from '@/composables/nodeColors' import { Ast } from '@/util/ast' -import { substituteQualifiedName } from '@/util/ast/abstract' +import { unqualifyQualifiedNames } from '@/util/ast/abstract' import { tryGetIndex } from '@/util/data/array' -import { qnLastSegment } from '@/util/qualifiedName' import * as map from 'lib0/map' import { computed, ref, watch } from 'vue' import type { ComponentExposed } from 'vue-component-type-helpers' @@ -115,10 +114,12 @@ const selectedSuggestionReturnType = computed(() => { if (selectedSuggestion.value == null) return undefined const typename = selectedSuggestion.value.returnType(projectNames) - const parsedType = parseExpression(typename) - if (parsedType == null) return typename - const substituted = substituteQualifiedName(parsedType, (qn) => qnLastSegment(qn)) - return substituted.code() + const parsedTypeExpr = parseExpression(typename) + if (parsedTypeExpr == null) return typename + const parsedType = parsedTypeExpr.module + parsedType.setRoot(parsedTypeExpr as any) + unqualifyQualifiedNames(parsedTypeExpr) + return parsedType.root()!.code() }) watch(selectedComponent, (component) => emit('update:selectedComponent', component), { 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 66df439c40a2..ef9fc072a085 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 @@ -4,7 +4,6 @@ import { escapeTextLiteral, findModuleMethod, substituteIdentifier, - substituteQualifiedName, substituteQualifiedNameByPattern, subtrees, tryEnsoToNumber, @@ -12,10 +11,10 @@ import { unescapeTextLiteral, type Identifier, } from '@/util/ast/abstract' -import { qnLastSegment } from '@/util/qualifiedName' import { fc, test } from '@fast-check/vitest' import { expect } from 'vitest' import { BodyBlock } from 'ydoc-shared/ast' +import { unqualifyQualifiedNames } from '../abstract' import { findExpressions, testCase } from './testCase' function functionBlock(topLevel: BodyBlock, name: string) { @@ -280,17 +279,25 @@ test.each([ substitution: 'ShouldNotWork', expected: 'Data.Table.new', }, + { + original: 'Should.Not.MatchPrefixOfName', + pattern: 'Should.Not.Match', + substitution: 'Unexpected', + expected: 'Should.Not.MatchPrefixOfName', + }, ])( 'Substitute qualified name $pattern inside $original', ({ original, pattern, substitution, expected }) => { const expression = Ast.parseExpression(original) ?? Ast.parseBlockStatement(original) assertDefined(expression) - const result = substituteQualifiedNameByPattern( + const mod = expression.module + mod.setRoot(expression as any) + substituteQualifiedNameByPattern( expression, pattern as Ast.Identifier, substitution as Ast.Identifier, ) - expect(result.code()).toEqual(expected) + expect(mod.root()?.code()).toEqual(expected) }, ) @@ -308,8 +315,10 @@ test.each([ ({ original, expected }) => { const expression = Ast.parseExpression(original) assertDefined(expression) - const result = substituteQualifiedName(expression, (qn) => qnLastSegment(qn)) - expect(result.code()).toEqual(expected) + const mod = expression.module + mod.setRoot(expression as any) + unqualifyQualifiedNames(expression) + expect(mod.root()?.code()).toEqual(expected) }, ) diff --git a/app/gui/src/project-view/util/ast/abstract.ts b/app/gui/src/project-view/util/ast/abstract.ts index 28a6d3a4a8b6..13af2e3ffe2d 100644 --- a/app/gui/src/project-view/util/ast/abstract.ts +++ b/app/gui/src/project-view/util/ast/abstract.ts @@ -28,16 +28,17 @@ import { NumericLiteral, OprApp, PropertyAccess, - Token, Wildcard, abstract, isTokenId, parseExpression, rawParseModule, setExternalIds, + visitRecursive, } from 'ydoc-shared/ast' import { spanMapToIdMap, spanMapToSpanGetter } from 'ydoc-shared/ast/idMap' import { IdMap } from 'ydoc-shared/yjsModel' +import { isQualifiedName } from '../qualifiedName' export * from 'ydoc-shared/ast' @@ -186,17 +187,23 @@ export function substituteIdentifier( pattern: IdentifierOrOperatorIdentifier, to: IdentifierOrOperatorIdentifier, ) { - if (expr instanceof MutableIdent && expr.code() === pattern) { - expr.setToken(to) - } else { - for (const child of expr.children()) { - if (child instanceof Token) { - continue + visitRecursive(expr, (expr) => { + if (expr instanceof MutableIdent && expr.code() === pattern) { + expr.setToken(to) + } + }) +} + +/** Replace all qualified names in the input with their last segment. */ +export function unqualifyQualifiedNames(expr: MutableAst) { + visitRecursive(expr, (expr) => { + if (expr instanceof MutablePropertyAccess) { + if (isQualifiedName(expr.code())) { + expr.replaceValue(Ident.newAllowingOperators(expr.module, expr.rhs)) + return false } - const mutableChild = expr.module.getVersion(child) - substituteIdentifier(mutableChild, pattern, to) } - } + }) } /** @@ -204,27 +211,21 @@ export function substituteIdentifier( * @param substitution is called on every qualified name in `expr`, and if non-nullish value * is returned, it replaces this qualified name. */ -export function substituteQualifiedName( +function substituteQualifiedName( expr: MutableAst, substitution: (from: QualifiedName) => Opt, -): Ast { - if (expr instanceof MutablePropertyAccess || expr instanceof MutableIdent) { - const qn = astToQualifiedName(expr) - if (!qn) return expr - const replacement = substitution(qn) - if (replacement != null) { - return expr.updateValue(() => parseExpression(replacement, expr.module)!) - } - } else { - for (const child of expr.children()) { - if (child instanceof Token) { - continue +) { + visitRecursive(expr, (expr) => { + if (expr instanceof MutablePropertyAccess || expr instanceof MutableIdent) { + const qn = astToQualifiedName(expr) + if (!qn) return + const replacement = substitution(qn) + if (replacement != null) { + const newQn = parseExpression(replacement, expr.module)! + expr.replaceValue(newQn) } - const mutableChild = expr.module.getVersion(child) - substituteQualifiedName(mutableChild, substitution) } - } - return expr + }) } /** @@ -236,12 +237,9 @@ export function substituteQualifiedNameByPattern( pattern: QualifiedName | IdentifierOrOperatorIdentifier, to: QualifiedName, ) { - return substituteQualifiedName(expr, (qn) => { + substituteQualifiedName(expr, (qn) => { if (qn === pattern) { return to - } else if (qn && qn.startsWith(pattern)) { - const withoutPattern = qn.replace(pattern, '') - return (to + withoutPattern) as QualifiedName } }) }