Skip to content

Commit

Permalink
Refactor Suggestion DB (#12065)
Browse files Browse the repository at this point in the history
Allow different suggestion DB types to have different properties, instead of assigning placeholder values; move type-specific entry logic into entry definitions. The `Any` type is now implicit: It is no longer stored as a parent of every type. When filtering by self-type, `Any` is treated as unknown.

Part of #11815.
  • Loading branch information
kazcw authored Jan 17, 2025
1 parent ddda787 commit 5817dfc
Show file tree
Hide file tree
Showing 20 changed files with 951 additions and 586 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,10 @@ test.each`
}

const pattern = 'foo_bar'
const entry = {
...makeModuleMethod(`local.Project.${name}`),
aliases: aliases ?? [],
}
const entry = makeModuleMethod(`local.Project.${name}`, { aliases: aliases ?? [] })
const filtering = new Filtering({ pattern })
const componentInfo = { id: 0, entry, match: filtering.filter(entry)! }
const match = filtering.filter(entry, [])
expect(match).not.toBeNull()
const componentInfo = { id: 0, entry, match: match! }
expect(replaceMatches(makeComponent(componentInfo))).toEqual(highlighted)
})
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Filtering, type MatchResult } from '@/components/ComponentBrowser/filtering'
import { entryQn, SuggestionEntry } from '@/stores/suggestionDatabase/entry'
import { SuggestionEntry } from '@/stores/suggestionDatabase/entry'
import {
makeConstructor,
makeFunction,
Expand All @@ -14,9 +14,9 @@ import { expect, test } from 'vitest'
import { Opt } from 'ydoc-shared/util/data/opt'

test.each([
{ ...makeModuleMethod('Standard.Base.Data.read'), groupIndex: 0 },
{ ...makeModuleMethod('Standard.Base.Data.write'), groupIndex: 0 },
{ ...makeStaticMethod('Standard.Base.Data.Vector.Vector.new'), groupIndex: 1 },
makeModuleMethod('Standard.Base.Data.read', { group: 'Standard.Base.MockGroup1' }),
makeModuleMethod('Standard.Base.Data.write', { group: 'Standard.Base.MockGroup1' }),
makeStaticMethod('Standard.Base.Data.Vector.Vector.new', { group: 'Standard.Base.MockGroup2' }),
makeModuleMethod('Standard.Base.Data.read_text'),
makeStaticMethod('local.Project.Foo.new'),
makeStaticMethod('local.Project.Internalization.internalize'),
Expand All @@ -27,7 +27,7 @@ test.each([

test.each([
makeModuleMethod('Standard.Base.Data.Vector.some_method'), // not in top group
{ ...makeMethod('Standard.Base.Data.Vector.Vector.get'), groupIndex: 1 }, // not static method
makeMethod('Standard.Base.Data.Vector.Vector.get', { group: 'Standard.Base.MockGroup2' }), // not static method
makeModule('Standard.Base.Data.Vector'), // Not top module
makeModule('local.New_Project'), // Main module
makeModule('Standard.Base.Data'), // Top module
Expand Down Expand Up @@ -55,13 +55,27 @@ test('An Instance method is shown when self arg matches', () => {
expect(filteringWithoutSelfType.filter(entry2, [])).toBeNull()
})

test('Additional self types are taken into account when filtering', () => {
test('`Any` type methods taken into account when filtering', () => {
const entry1 = makeMethod('Standard.Base.Data.Vector.Vector.get')
const entry2 = makeMethod('Standard.Base.Any.Any.to_string')
const additionalSelfType = 'Standard.Base.Any.Any' as QualifiedName
const filtering = new Filtering({
selfArg: { type: 'known', typename: 'Standard.Base.Data.Vector.Vector' },
})
expect(filtering.filter(entry1, [])).not.toBeNull()
expect(filtering.filter(entry2, [])).not.toBeNull()

const filteringWithoutSelfType = new Filtering({})
expect(filteringWithoutSelfType.filter(entry1, [])).toBeNull()
expect(filteringWithoutSelfType.filter(entry2, [])).toBeNull()
})

test('Additional self types are taken into account when filtering', () => {
const entry1 = makeMethod('Standard.Base.Data.Numbers.Float.abs')
const entry2 = makeMethod('Standard.Base.Data.Numbers.Number.sqrt')
const additionalSelfType = 'Standard.Base.Data.Numbers.Number' as QualifiedName
const filtering = new Filtering({
selfArg: { type: 'known', typename: 'Standard.Base.Data.Numbers.Float' },
})
expect(filtering.filter(entry1, [additionalSelfType])).not.toBeNull()
expect(filtering.filter(entry2, [additionalSelfType])).not.toBeNull()
expect(filtering.filter(entry2, [])).toBeNull()
Expand Down Expand Up @@ -118,12 +132,12 @@ function matchedText(ownerName: string, name: string, matchResult: MatchResult)

type MatchingTestCase = {
pattern: string
matchedSorted: { module?: string; name: string; aliases: string[] }[]
notMatched: { module?: string; name: string; aliases: string[] }[]
matchedSorted: { module?: string; name: string; aliases?: string[] }[]
notMatched: { module?: string; name: string; aliases?: string[] }[]
}

// In this test, `matchedSorted` are specified in expected score ascending order.
test.each([
test.each<MatchingTestCase>([
{
pattern: 'foo',
matchedSorted: [
Expand Down Expand Up @@ -210,29 +224,34 @@ test.each([
{ module: 'local.Pr', name: 'bar' },
],
},
] as MatchingTestCase[])('Matching pattern $pattern', ({ pattern, matchedSorted, notMatched }) => {
])('Matching pattern $pattern', ({ pattern, matchedSorted, notMatched }) => {
const filtering = new Filtering({ pattern })
const matchedSortedEntries = Array.from(matchedSorted, ({ name, aliases, module }) => ({
...makeModuleMethod(`${module ?? 'local.Project'}.${name}`),
aliases: aliases ?? [],
}))
const matchedSortedEntries = Array.from(matchedSorted, ({ name, aliases, module }) =>
makeModuleMethod(`${module ?? 'local.Project'}.${name}`, { aliases: aliases ?? [] }),
)
const matchResults = Array.from(matchedSortedEntries, (entry) => filtering.filter(entry, []))
// Checking matching entries
function checkResult(entry: SuggestionEntry, result: Opt<MatchResult>) {
expect(result, `Matching entry ${entryQn(entry)}`).not.toBeNull()
expect(result, `Matching entry ${entry.definitionPath}`).not.toBeNull()
expect(
matchedText(entry.memberOf ? qnLastSegment(entry.memberOf) : '', entry.name, result!)
matchedText(
'memberOf' in entry && entry.memberOf ? qnLastSegment(entry.memberOf) : '',
entry.name,
result!,
)
.toLowerCase()
.replace(/ /g, '_'),
`Matched text of entry ${entryQn(entry)}`,
`Matched text of entry ${entry.definitionPath}`,
).toEqual(pattern.toLowerCase().replace(/ /g, '_'))
}
expect(matchedSortedEntries.length).toBeGreaterThan(0)
expect(matchedSortedEntries[0]).not.toBeNull()
checkResult(matchedSortedEntries[0]!, matchResults[0])
for (let i = 1; i < matchResults.length; i++) {
checkResult(matchedSortedEntries[i]!, matchResults[i])
expect(
matchResults[i]!.score,
`score('${entryQn(matchedSortedEntries[i]!)}') > score('${entryQn(matchedSortedEntries[i - 1]!)}')`,
`score('${matchedSortedEntries[i]!.definitionPath}') > score('${matchedSortedEntries[i - 1]!.definitionPath}')`,
).toBeGreaterThan(matchResults[i - 1]!.score)
}

Expand All @@ -242,6 +261,6 @@ test.each([
...makeModuleMethod(`${module ?? 'local.Project'}.${name}`),
aliases: aliases ?? [],
}
expect(filtering.filter(entry, []), entryQn(entry)).toBeNull()
expect(filtering.filter(entry, []), entry.definitionPath).toBeNull()
}
})
38 changes: 12 additions & 26 deletions app/gui/src/project-view/components/ComponentBrowser/component.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
import { Filtering, type MatchResult } from '@/components/ComponentBrowser/filtering'
import { SuggestionDb } from '@/stores/suggestionDatabase'
import {
entryIsStatic,
SuggestionKind,
type SuggestionEntry,
type SuggestionId,
} from '@/stores/suggestionDatabase/entry'
import { compareOpt } from '@/util/compare'
import { isSome } from '@/util/data/opt'
import { Range } from '@/util/data/range'
import { ANY_TYPE_QN } from '@/util/ensoTypes'
import { displayedIconOf } from '@/util/getIconName'
import type { Icon } from '@/util/iconMetadata/iconName'
import type { QualifiedName } from '@/util/qualifiedName'
import { qnLastSegmentIndex, tryQualifiedName } from '@/util/qualifiedName'
import { qnJoin, qnLastSegment, tryQualifiedName, type QualifiedName } from '@/util/qualifiedName'

interface ComponentLabelInfo {
label: string
Expand All @@ -34,19 +33,18 @@ export interface Component extends ComponentLabel {

/** @returns the displayed label of given suggestion entry with information of highlighted ranges. */
export function labelOfEntry(entry: SuggestionEntry, match: MatchResult): ComponentLabelInfo {
if (entry.memberOf && entry.selfType == null) {
const ownerLastSegmentStart = qnLastSegmentIndex(entry.memberOf) + 1
const ownerName = entry.memberOf.substring(ownerLastSegmentStart)
const nameOffset = ownerName.length + '.'.length
if (entryIsStatic(entry) && entry.memberOf) {
const label = qnJoin(qnLastSegment(entry.memberOf), entry.name)
if ((!match.ownerNameRanges && !match.nameRanges) || match.matchedAlias) {
return {
label: `${ownerName}.${entry.name}`,
label,
matchedAlias: match.matchedAlias,
matchedRanges: match.nameRanges,
}
}
const nameOffset = label.length - entry.name.length
return {
label: `${ownerName}.${entry.name}`,
label,
matchedAlias: match.matchedAlias,
matchedRanges: [
...(match.ownerNameRanges ?? []),
Expand Down Expand Up @@ -112,12 +110,13 @@ export function makeComponent({ id, entry, match }: ComponentInfo): Component {
/** Create {@link Component} list from filtered suggestions. */
export function makeComponentList(db: SuggestionDb, filtering: Filtering): Component[] {
function* matchSuggestions() {
// All types are descendants of `Any`, so we can safely prepopulate it here.
// This way, we will use it even when `selfArg` is not a valid qualified name.
const additionalSelfTypes: QualifiedName[] = [ANY_TYPE_QN]
const additionalSelfTypes: QualifiedName[] = []
if (filtering.selfArg?.type === 'known') {
const maybeName = tryQualifiedName(filtering.selfArg.typename)
if (maybeName.ok) populateAdditionalSelfTypes(db, additionalSelfTypes, maybeName.value)
if (maybeName.ok) {
const entry = db.getEntryByQualifiedName(maybeName.value)
if (entry) additionalSelfTypes.push(...db.ancestors(entry))
}
}

for (const [id, entry] of db.entries()) {
Expand All @@ -130,16 +129,3 @@ export function makeComponentList(db: SuggestionDb, filtering: Filtering): Compo
const matched = Array.from(matchSuggestions()).sort(compareSuggestions)
return Array.from(matched, (info) => makeComponent(info))
}

/**
* Type can inherit methods from `parentType`, and it can do that recursively.
* In practice, these hierarchies are at most two levels deep.
*/
function populateAdditionalSelfTypes(db: SuggestionDb, list: QualifiedName[], name: QualifiedName) {
let entry = db.getEntryByQualifiedName(name)
// We don’t need to add `Any` to the list, because the caller already did that.
while (entry != null && entry.parentType != null && entry.parentType !== ANY_TYPE_QN) {
list.push(entry.parentType)
entry = db.getEntryByQualifiedName(entry.parentType)
}
}
21 changes: 11 additions & 10 deletions app/gui/src/project-view/components/ComponentBrowser/filtering.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import {
SuggestionKind,
type SuggestionEntry,
SuggestionKind,
type Typename,
} from '@/stores/suggestionDatabase/entry'
import type { Opt } from '@/util/data/opt'
import { Range } from '@/util/data/range'
import { ANY_TYPE_QN } from '@/util/ensoTypes'
import { qnIsTopElement, qnLastSegment, type QualifiedName } from '@/util/qualifiedName'
import escapeStringRegexp from '@/util/regexp'

Expand Down Expand Up @@ -220,7 +221,7 @@ class FilteringWithPattern {
*
* - If `pattern` is specified with dot, the part after dot must match entry name or alias, while
* on the left side of the dot must match type/module on which the entry is specified.
* there must exists a subsequence of words in name/alias (words are separated by `_`), so each
* there must exist a subsequence of words in name/alias (words are separated by `_`), so each
* word:
* - starts with respective word in the pattern,
* - or starts with respective _letter_ in the pattern (initials match).
Expand Down Expand Up @@ -249,13 +250,14 @@ export class Filtering {
}

private selfTypeMatches(entry: SuggestionEntry, additionalSelfTypes: QualifiedName[]): boolean {
if (this.selfArg == null) return entry.selfType == null
else if (this.selfArg.type == 'known')
return (
entry.selfType === this.selfArg.typename ||
additionalSelfTypes.some((t) => entry.selfType === t)
)
else return entry.selfType != null
if (this.selfArg == null) return entry.kind !== SuggestionKind.Method || entry.selfType == null
if (entry.kind !== SuggestionKind.Method || entry.selfType == null) return false
return (
this.selfArg.type !== 'known' ||
entry.selfType === this.selfArg.typename ||
entry.selfType === ANY_TYPE_QN ||
additionalSelfTypes.some((t) => entry.selfType === t)
)
}

/** TODO: Add docs */
Expand All @@ -281,7 +283,6 @@ export class Filtering {
if (this.selfArg == null && isInternal(entry)) return null
if (!this.selfTypeMatches(entry, additionalSelfTypes)) return null
if (this.pattern) {
if (entry.memberOf == null) return null
const patternMatch = this.pattern.tryMatch(entry.name, entry.aliases, entry.memberOf)
if (!patternMatch) return null
if (this.isLocal(entry)) patternMatch.score *= 2
Expand Down
52 changes: 26 additions & 26 deletions app/gui/src/project-view/components/ComponentBrowser/input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,15 @@ import { useGraphStore, type NodeId } from '@/stores/graph'
import type { GraphDb } from '@/stores/graph/graphDatabase'
import { requiredImportEquals, requiredImports, type RequiredImport } from '@/stores/graph/imports'
import { useSuggestionDbStore, type SuggestionDb } from '@/stores/suggestionDatabase'
import { type SuggestionEntry, type SuggestionId } from '@/stores/suggestionDatabase/entry'
import {
entryIsStatic,
type SuggestionEntry,
type SuggestionId,
} from '@/stores/suggestionDatabase/entry'
import { isIdentifier, type AstId, type Identifier } from '@/util/ast/abstract'
import { Err, Ok, type Result } from '@/util/data/result'
import { qnLastSegment, type QualifiedName } from '@/util/qualifiedName'
import { ANY_TYPE_QN } from '@/util/ensoTypes'
import { qnJoin, qnLastSegment, type QualifiedName } from '@/util/qualifiedName'
import { useToast } from '@/util/toast'
import { computed, proxyRefs, readonly, ref, type ComputedRef } from 'vue'

Expand Down Expand Up @@ -122,15 +127,18 @@ export function useComponentBrowserInput(
const definition = graphDb.getIdentDefiningNode(sourceNodeIdentifier.value)
if (definition == null) return null
const typename = graphDb.getExpressionInfo(definition)?.typename
return typename != null ? { type: 'known', typename } : { type: 'unknown' }
return typename != null && typename !== ANY_TYPE_QN ?
{ type: 'known', typename }
: { type: 'unknown' }
})

/** Apply given suggested entry to the input. */
function applySuggestion(id: SuggestionId): Result {
const entry = suggestionDb.get(id)
if (!entry) return Err(`No entry with id ${id}`)
switchedToCodeMode.value = { appliedSuggestion: id }
const { newText, newCursorPos, requiredImport } = inputAfterApplyingSuggestion(entry)
const { newText, requiredImport } = inputAfterApplyingSuggestion(entry)
const newCursorPos = newText.length
text.value = newText
selection.value = { start: newCursorPos, end: newCursorPos }
if (requiredImport) {
Expand All @@ -153,28 +161,20 @@ export function useComponentBrowserInput(

function inputAfterApplyingSuggestion(entry: SuggestionEntry): {
newText: string
newCode: string
newCursorPos: number
requiredImport: QualifiedName | null
requiredImport: QualifiedName | undefined
} {
const newText =
!sourceNodeIdentifier.value && entry.memberOf ?
`${qnLastSegment(entry.memberOf)}.${entry.name} `
: `${entry.name} `
const newCode =
sourceNodeIdentifier.value ? `${sourceNodeIdentifier.value}.${entry.name} ` : `${newText} `
const newCursorPos = newText.length

return {
newText,
newCode,
newCursorPos,
requiredImport:
sourceNodeIdentifier.value ? null
: entry.memberOf ? entry.memberOf
// Perhaps we will add cases for Type/Con imports, but they are not displayed as
// suggestion ATM.
: null,
if (sourceNodeIdentifier.value) {
return {
newText: entry.name + ' ',
requiredImport: undefined,
}
} else {
// Perhaps we will add cases for Type/Con imports, but they are not displayed as suggestion ATM.
const owner = entryIsStatic(entry) ? entry.memberOf : undefined
return {
newText: (owner ? qnJoin(qnLastSegment(owner), entry.name) : entry.name) + ' ',
requiredImport: owner,
}
}
}

Expand Down Expand Up @@ -282,7 +282,7 @@ export function useComponentBrowserInput(
selfArgument: sourceNodeIdentifier,
/** The current selection (or cursor position if start is equal to end). */
selection,
/** Flag indincating that we're waiting for AI's answer for user's prompt. */
/** Flag indicating that we're waiting for AI's answer for user's prompt. */
processingAIPrompt,
/** Re-initializes the input for given usage. */
reset,
Expand Down
Loading

0 comments on commit 5817dfc

Please sign in to comment.