Imports conflict resolution

app/gui2/src/components/ComponentBrowser.vue

@@ -16,7 +16,7 @@ import { useGraphStore } from '@/stores/graph'
 import type { RequiredImport } from '@/stores/graph/imports'
 import { useProjectStore } from '@/stores/project'
 import { groupColorStyle, useSuggestionDbStore } from '@/stores/suggestionDatabase'
-import { SuggestionKind, type SuggestionEntry } from '@/stores/suggestionDatabase/entry'
+import { SuggestionKind } from '@/stores/suggestionDatabase/entry'
 import type { VisualizationDataSource } from '@/stores/visualization'
 import { targetIsOutside } from '@/util/autoBlur'
 import { tryGetIndex } from '@/util/data/array'
@@ -343,18 +343,18 @@ watch(selectedSuggestionId, (id) => {
 
 // === Accepting Entry ===
 
-function applySuggestion(component: Opt<Component> = null): SuggestionEntry | null {
+function applySuggestion(component: Opt<Component> = null) {
+  const suggestionId = component?.suggestionId ?? selectedSuggestionId.value
+  if (suggestionId == null) return
+  input.applySuggestion(suggestionId)
+}
+
+function acceptSuggestion(component: Opt<Component> = null) {
+  applySuggestion(component)
   const providedSuggestion =
     component != null ? suggestionDbStore.entries.get(component.suggestionId) : null
   const suggestion = providedSuggestion ?? selectedSuggestion.value
-  if (suggestion == null) return null
-  input.applySuggestion(suggestion)
-  return suggestion
-}
-
-function acceptSuggestion(index: Opt<Component> = null) {
-  const applied = applySuggestion(index)
-  const shouldFinish = applied != null && applied.kind !== SuggestionKind.Module
+  const shouldFinish = suggestion != null && suggestion.kind !== SuggestionKind.Module
   if (shouldFinish) acceptInput()
 }
 

app/gui2/src/components/ComponentBrowser/__tests__/input.test.ts

@@ -287,11 +287,14 @@ test.each([
   ({ code, cursorPos, suggestion, expected, expectedCursorPos }) => {
     cursorPos = cursorPos ?? code.length
     expectedCursorPos = expectedCursorPos ?? expected.length
+    const db = new SuggestionDb()
+    const dummyId = 1
+    db.set(dummyId, suggestion)
     const graphMock = GraphDb.Mock()
-    const input = useComponentBrowserInput(graphMock, new SuggestionDb())
+    const input = useComponentBrowserInput(graphMock, db)
     input.code.value = code
     input.selection.value = { start: cursorPos, end: cursorPos }
-    input.applySuggestion(suggestion)
+    input.applySuggestion(dummyId)
     expect(input.code.value).toEqual(expected)
     expect(input.selection.value).toStrictEqual({
       start: expectedCursorPos,
@@ -355,8 +358,7 @@ test.each([
     const input = useComponentBrowserInput(graphMock, db)
     input.code.value = initialCode
     input.selection.value = { start: initialCode.length, end: initialCode.length }
-    const suggestion = db.get(suggestionId)!
-    input.applySuggestion(suggestion)
+    input.applySuggestion(suggestionId)
     if (manuallyEditedCode != null) {
       input.code.value = manuallyEditedCode
     }

app/gui2/src/components/ComponentBrowser/input.ts

@@ -7,6 +7,7 @@ import {
   SuggestionKind,
   entryQn,
   type SuggestionEntry,
+  type SuggestionId,
   type Typename,
 } from '@/stores/suggestionDatabase/entry'
 import { RawAst, RawAstExtended, astContainingChar } from '@/util/ast'
@@ -261,14 +262,16 @@ export function useComponentBrowserInput(
   }
 
   /** Apply given suggested entry to the input. */
-  function applySuggestion(entry: SuggestionEntry) {
+  function applySuggestion(id: SuggestionId) {
+    const entry = suggestionDb.get(id)
+    if (!entry) return
     const { newCode, newCursorPos, requiredImport } = inputAfterApplyingSuggestion(entry)
     code.value = newCode
     selection.value = { start: newCursorPos, end: newCursorPos }
     if (requiredImport) {
-      const [id] = suggestionDb.nameToId.lookup(requiredImport)
-      if (id) {
-        const requiredEntry = suggestionDb.get(id)
+      const [importId] = suggestionDb.nameToId.lookup(requiredImport)
+      if (importId) {
+        const requiredEntry = suggestionDb.get(importId)
         if (requiredEntry) {
           imports.value = imports.value.concat(requiredImports(suggestionDb, requiredEntry))
         }

app/gui2/src/components/GraphEditor/GraphNodeComment.vue

@@ -4,7 +4,7 @@ import { injectInteractionHandler } from '@/providers/interactionHandler'
 import { defineKeybinds } from '@/util/shortcuts'
 import * as random from 'lib0/random'
 import { textChangeToEdits } from 'shared/util/data/text'
-import { computed, ref, watch, watchEffect } from 'vue'
+import { computed, ref, watchEffect } from 'vue'
 
 const { minimalSetup, EditorState, EditorView, textEditToChangeSpec } = await import(
   '@/components/CodeEditor/codemirror'

app/gui2/src/components/GraphEditor/widgets/WidgetSelection.vue

@@ -145,14 +145,20 @@ function onClick(index: number, keepOpen: boolean) {
 // When the selected index changes, we update the expression content.
 watch(selectedIndex, (_index) => {
   let edit: Ast.MutableModule | undefined
+  // Unless import conflict resolution is needed, we use the selected expression as is.
+  let value = selectedTag.value?.expression
   if (selectedTag.value?.requiredImports) {
     edit = graph.startEdit()
-    graph.addMissingImports(edit, selectedTag.value.requiredImports)
+    const conflicts = graph.addMissingImports(edit, selectedTag.value.requiredImports)
+    if (conflicts != null && conflicts.length > 0) {
+      // Is there is a conflict, it would be a single one, because we only ask about a single entry.
+      value = conflicts[0]?.fullyQualified
+    }
   }
   props.onUpdate({
     edit,
     portUpdate: {
-      value: selectedTag.value?.expression,
+      value,
       origin: asNot<TokenId>(props.input.portId),
     },
   })

app/gui2/src/stores/graph/imports.ts

@@ -9,11 +9,10 @@ import {
   type SuggestionEntry,
 } from '@/stores/suggestionDatabase/entry'
 import { Ast } from '@/util/ast'
-import { MutableModule } from '@/util/ast/abstract'
+import { MutableModule, parseIdent, parseIdents, parseQualifiedName } from '@/util/ast/abstract'
 import { unwrap } from '@/util/data/result'
 import {
-  normalizeQualifiedName,
-  qnFromSegments,
+  qnLastSegment,
   qnSegments,
   qnSplit,
   tryIdentifier,
@@ -26,63 +25,6 @@ import {
 // === Imports analysis ===
 // ========================
 
-/** If the input is a chain of applications of the given left-associative operator, and all the leaves of the
- *  operator-application tree are identifier expressions, return the identifiers from left to right.
- *  This is analogous to `ast.code().split(operator)`, but type-enforcing.
- */
-function unrollOprChain(
-  ast: Ast.Ast,
-  leftAssociativeOperator: string,
-): IdentifierOrOperatorIdentifier[] | null {
-  const idents: IdentifierOrOperatorIdentifier[] = []
-  let ast_: Ast.Ast | undefined = ast
-  while (
-    ast_ instanceof Ast.OprApp &&
-    ast_.operator.ok &&
-    ast_.operator.value.code() === leftAssociativeOperator
-  ) {
-    if (!(ast_.rhs instanceof Ast.Ident)) return null
-    idents.unshift(ast_.rhs.code())
-    ast_ = ast_.lhs
-  }
-  if (!(ast_ instanceof Ast.Ident)) return null
-  idents.unshift(ast_.code())
-  return idents
-}
-
-/** If the input is a chain of property accesses (uses of the `.` operator with a syntactic identifier on the RHS), and
- *  the value at the beginning of the sequence is an identifier expression, return all the identifiers from left to
- *  right. This is analogous to `ast.code().split('.')`, but type-enforcing.
- */
-function unrollPropertyAccess(ast: Ast.Ast): IdentifierOrOperatorIdentifier[] | null {
-  const idents: IdentifierOrOperatorIdentifier[] = []
-  let ast_: Ast.Ast | undefined = ast
-  while (ast_ instanceof Ast.PropertyAccess) {
-    idents.unshift(ast_.rhs.code())
-    ast_ = ast_.lhs
-  }
-  if (!(ast_ instanceof Ast.Ident)) return null
-  idents.unshift(ast_.code())
-  return idents
-}
-
-function parseIdent(ast: Ast.Ast): IdentifierOrOperatorIdentifier | null {
-  if (ast instanceof Ast.Ident) {
-    return ast.code()
-  } else {
-    return null
-  }
-}
-
-function parseIdents(ast: Ast.Ast): IdentifierOrOperatorIdentifier[] | null {
-  return unrollOprChain(ast, ',')
-}
-
-function parseQualifiedName(ast: Ast.Ast): QualifiedName | null {
-  const idents = unrollPropertyAccess(ast)
-  return idents && normalizeQualifiedName(qnFromSegments(idents))
-}
-
 /** Parse import statement. */
 export function recognizeImport(ast: Ast.Import): Import | null {
   const from = ast.from
@@ -296,6 +238,14 @@ function definedInEntry(db: SuggestionDb, entry: SuggestionEntry): SuggestionEnt
   return db.getEntryByQualifiedName(entry.definedIn)
 }
 
+function entryFQNFromRequiredImport(importStatement: RequiredImport): QualifiedName {
+  if (importStatement.kind === 'Qualified') {
+    return importStatement.module
+  } else {
+    return unwrap(tryQualifiedName(`${importStatement.from}.${importStatement.import}`))
+  }
+}
+
 export function requiredImportEquals(left: RequiredImport, right: RequiredImport): boolean {
   if (left.kind != right.kind) return false
   switch (left.kind) {
@@ -341,6 +291,47 @@ export function filterOutRedundantImports(
   return required.filter((info) => !existing.some((existing) => covers(existing, info)))
 }
 
+/* Information about detected conflict import, and advisory on resolution. */
+export interface DetectedConflict {
+  /* Always true, for more expressive API usage. */
+  detected: boolean
+  /* Advisory to replace the following name (qualified name or single ident)… */
+  pattern: QualifiedName | IdentifierOrOperatorIdentifier
+  /* … with this fully qualified name. */
+  fullyQualified: QualifiedName
+}
+
+export type ConflictInfo = DetectedConflict | undefined
+
+/* Detect possible name clash when adding `importsForEntry` with `existingImports` present. */
+export function detectImportConflicts(
+  suggestionDb: SuggestionDb,
+  existingImports: Import[],
+  importToCheck: RequiredImport,
+): ConflictInfo {
+  const entryFQN = entryFQNFromRequiredImport(importToCheck)
+  const [entryId] = suggestionDb.nameToId.lookup(entryFQN)
+  if (entryId == null) return
+  const name = qnLastSegment(entryFQN)
+  const conflictingIds = suggestionDb.conflictingNames.lookup(name)
+  // Obviously, the entry doesn’t conflict with itself.
+  conflictingIds.delete(entryId)
+
+  for (const id of conflictingIds) {
+    const e = suggestionDb.get(id)
+    const required = e ? requiredImports(suggestionDb, e) : []
+    for (const req of required) {
+      if (existingImports.some((existing) => covers(existing, req))) {
+        return {
+          detected: true,
+          pattern: name,
+          fullyQualified: entryFQN,
+        }
+      }
+    }
+  }
+}
+
 if (import.meta.vitest) {
   const { test, expect } = import.meta.vitest
   const { initializeFFI } = await import('shared/ast/ffi')
@@ -468,10 +459,54 @@ if (import.meta.vitest) {
     db.set(7, makeMethod('Standard.Base.Type.method'))
     db.set(8, makeType('Standard.Network.URI'))
     db.set(9, extensionMethod)
+    db.set(10, makeType('Standard.Base.Vector'))
+    db.set(11, makeStaticMethod('Standard.Base.Vector.new'))
+    db.set(12, makeModule('Project.Foo'))
+    db.set(13, makeType('Project.Foo.Vector'))
+    db.set(14, makeStaticMethod('Project.Foo.Vector.new'))
+    db.set(15, makeModule('Project.Foo.Base'))
 
     return db
   }
 
+  const qn = (s: string) => unwrap(tryQualifiedName(s))
+  test.each([
+    {
+      description: 'Conflicting Vector',
+      importing: {
+        kind: 'Unqualified',
+        from: qn('Project.Foo'),
+        import: 'Vector',
+      } as RequiredImport,
+      alreadyImported: [
+        { from: qn('Standard.Base'), imported: { kind: 'List', names: ['Vector'] } } as Import,
+      ],
+      expected: { name: 'Vector', fullyQualified: 'Project.Foo.Vector' },
+    },
+    {
+      description: 'Conflicting Vector (2)',
+      importing: {
+        kind: 'Unqualified',
+        from: qn('Project.Foo'),
+        import: 'Vector',
+      } as RequiredImport,
+      alreadyImported: [
+        { from: qn('Standard.Base'), imported: { kind: 'All', except: [] } } as Import,
+      ],
+      expected: { name: 'Vector', fullyQualified: 'Project.Foo.Vector' },
+    },
+  ])('Conflicting imports: $description', ({ importing, alreadyImported, expected }) => {
+    const db = mockDb()
+
+    const existingImports: Import[] = alreadyImported
+    const conflicts = detectImportConflicts(db, existingImports, importing)
+    expect(conflicts).toEqual({
+      detected: true,
+      pattern: expected.name,
+      fullyQualified: expected.fullyQualified,
+    } as ConflictInfo)
+  })
+
   test.each([
     {
       id: 1,