Fix substituteQualifiedName

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<void>,
+    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 }) => {

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

app/gui/src/project-view/util/ast/__tests__/abstract.test.ts

@@ -4,18 +4,17 @@ import {
   escapeTextLiteral,
   findModuleMethod,
   substituteIdentifier,
-  substituteQualifiedName,
   substituteQualifiedNameByPattern,
   subtrees,
   tryEnsoToNumber,
   tryNumberToEnso,
   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)
   },
 )
 

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,45 +187,45 @@ 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)
     }
-  }
+  })
 }
 
 /**
  * Substitute some qualified names in `expr`.
  * @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<QualifiedName>,
-): 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
     }
   })
 }