From 4cae5246faca4ffb7372ca0e7cb8db5364167000 Mon Sep 17 00:00:00 2001 From: Adam Obuchowicz Date: Wed, 29 Jan 2025 14:15:20 +0100 Subject: [PATCH] Expect failure when parsing external Qualified Names (#12172) Fixes #12168 Change `parseProjectPath` family of functions, so they return `Result`. Handle all translators from LS to handle cases when qualified names are not actual qualified name. Fixed WildgetSelection, so it accepts qualified names which are not absolute. --- .../__tests__/filtering.test.ts | 6 +- .../components/DocumentationPanel.vue | 16 +- .../GraphEditor/GraphNodeMessage.vue | 11 +- .../__tests__/widgetFunctionCallInfo.test.ts | 3 +- .../GraphEditor/widgets/WidgetSelection.vue | 16 +- .../__tests__/tableInputArgument.test.ts | 5 +- .../stores/graph/__tests__/imports.test.ts | 6 +- .../stores/graph/graphDatabase.ts | 31 ++-- .../src/project-view/stores/graph/imports.ts | 23 +-- .../src/project-view/stores/graph/index.ts | 4 +- .../stores/project/computedValueRegistry.ts | 41 +++-- .../src/project-view/stores/projectNames.ts | 22 ++- .../__tests__/documentation.test.ts | 8 +- .../__tests__/lsUpdate.test.ts | 4 +- .../stores/suggestionDatabase/lsUpdate.ts | 166 +++++++++++------- .../util/__tests__/callTree.test.ts | 2 +- .../util/__tests__/projectPath.test.ts | 3 +- .../util/__tests__/qualifiedName.test.ts | 2 +- .../src/project-view/util/methodPointer.ts | 15 +- app/gui/src/project-view/util/projectPath.ts | 23 ++- 20 files changed, 259 insertions(+), 148 deletions(-) diff --git a/app/gui/src/project-view/components/ComponentBrowser/__tests__/filtering.test.ts b/app/gui/src/project-view/components/ComponentBrowser/__tests__/filtering.test.ts index 01bed515c68a..9d934423b935 100644 --- a/app/gui/src/project-view/components/ComponentBrowser/__tests__/filtering.test.ts +++ b/app/gui/src/project-view/components/ComponentBrowser/__tests__/filtering.test.ts @@ -10,8 +10,8 @@ import { makeStaticMethod, } from '@/stores/suggestionDatabase/mockSuggestion' import { assert } from '@/util/assert' -import { parseAbsoluteProjectPath } from '@/util/projectPath' -import { qnLastSegment, tryQualifiedName } from '@/util/qualifiedName' +import { parseAbsoluteProjectPathRaw } from '@/util/projectPath' +import { qnLastSegment } from '@/util/qualifiedName' import { expect, test } from 'vitest' import { type Opt } from 'ydoc-shared/util/data/opt' import { unwrap } from 'ydoc-shared/util/data/result' @@ -42,7 +42,7 @@ test.each([ function stdPath(path: string) { assert(path.startsWith('Standard.')) - return parseAbsoluteProjectPath(unwrap(tryQualifiedName(path))) + return unwrap(parseAbsoluteProjectPathRaw(path)) } test('An Instance method is shown when self arg matches', () => { diff --git a/app/gui/src/project-view/components/DocumentationPanel.vue b/app/gui/src/project-view/components/DocumentationPanel.vue index f4e2db88e443..005a84f52045 100644 --- a/app/gui/src/project-view/components/DocumentationPanel.vue +++ b/app/gui/src/project-view/components/DocumentationPanel.vue @@ -17,8 +17,9 @@ import type { SuggestionId } from '@/stores/suggestionDatabase/entry' import { suggestionDocumentationUrl } from '@/stores/suggestionDatabase/entry' import { tryGetIndex } from '@/util/data/array' import { type Opt } from '@/util/data/opt' +import { Ok } from '@/util/data/result' import type { Icon as IconName } from '@/util/iconMetadata/iconName' -import type { QualifiedName } from '@/util/qualifiedName' +import { ProjectPath } from '@/util/projectPath' import { qnSegments, qnSlice } from '@/util/qualifiedName' import { computed, watch } from 'vue' @@ -58,9 +59,9 @@ const isPlaceholder = computed(() => documentation.value.kind === 'Placeholder') const projectNames = injectProjectNames() -const name = computed>(() => { +const name = computed>(() => { const docs = documentation.value - return docs.kind === 'Placeholder' ? null : projectNames.printProjectPath(docs.name) + return docs.kind === 'Placeholder' ? null : docs.name }) // === Breadcrumbs === @@ -98,7 +99,7 @@ watch(historyStack.current, (current) => { const breadcrumbs = computed(() => { if (name.value) { - const segments = qnSegments(name.value) + const segments = [...qnSegments(projectNames.printProjectPath(name.value))] return segments.slice(1).map((s) => ({ label: s.toLowerCase() })) } else { return [] @@ -107,9 +108,10 @@ const breadcrumbs = computed(() => { function handleBreadcrumbClick(index: number) { if (name.value) { - const qName = qnSlice(name.value, 0, index + 2) - if (qName.ok) { - const id = db.entries.findByProjectPath(projectNames.parseProjectPath(qName.value)) + const pathSlice = name.value.path ? qnSlice(name.value.path, 0, index) : Ok(undefined) + if (pathSlice.ok) { + const projectPathSlice = name.value.withPath(pathSlice.value) + const id = db.entries.findByProjectPath(projectPathSlice) if (id != null) { historyStack.record(id) } diff --git a/app/gui/src/project-view/components/GraphEditor/GraphNodeMessage.vue b/app/gui/src/project-view/components/GraphEditor/GraphNodeMessage.vue index 55e31bd3f983..6049acdfb063 100644 --- a/app/gui/src/project-view/components/GraphEditor/GraphNodeMessage.vue +++ b/app/gui/src/project-view/components/GraphEditor/GraphNodeMessage.vue @@ -5,7 +5,7 @@ import { useGraphStore } from '@/stores/graph' import { QualifiedImport } from '@/stores/graph/imports' import { injectProjectNames } from '@/stores/projectNames' import type { Icon } from '@/util/iconMetadata/iconName' -import { QualifiedName } from '@/util/qualifiedName' +import { ProjectPath } from '@/util/projectPath' const graph = useGraphStore() const projectNames = injectProjectNames() @@ -15,12 +15,15 @@ const props = defineProps<{ type: MessageType }>() -function containsLibraryName(): string | null { +function containsLibraryName(): ProjectPath | null { const prefix = 'Compile error: Fully qualified name references a library ' if (props.message.startsWith(prefix)) { const rest = props.message.substring(prefix.length) const libName = rest.split(' ') - return libName[0] ? libName[0] : null + if (!libName[0]) return null + const path = projectNames.parseProjectPathRaw(libName[0]) + if (!path.ok) return null + return path.value } else { return null } @@ -33,7 +36,7 @@ function fixImport() { if (libName) { const theImport = { kind: 'Qualified', - module: projectNames.parseProjectPath(libName as QualifiedName), + module: libName, } satisfies QualifiedImport graph.edit((edit) => graph.addMissingImports(edit, [theImport])) } diff --git a/app/gui/src/project-view/components/GraphEditor/widgets/WidgetFunction/__tests__/widgetFunctionCallInfo.test.ts b/app/gui/src/project-view/components/GraphEditor/widgets/WidgetFunction/__tests__/widgetFunctionCallInfo.test.ts index 581e17a9a4a8..6b50394e9539 100644 --- a/app/gui/src/project-view/components/GraphEditor/widgets/WidgetFunction/__tests__/widgetFunctionCallInfo.test.ts +++ b/app/gui/src/project-view/components/GraphEditor/widgets/WidgetFunction/__tests__/widgetFunctionCallInfo.test.ts @@ -17,6 +17,7 @@ import { } from '@/stores/suggestionDatabase/mockSuggestion' import { assert } from '@/util/assert' import { Ast } from '@/util/ast' +import { unwrap } from '@/util/data/result' import { expect, test } from 'vitest' import { ref, type Ref } from 'vue' import { type Opt } from 'ydoc-shared/util/data/opt' @@ -92,7 +93,7 @@ test.each` getExpressionInfo(astId) { if (subjectSpan != null && astId === id('subject')) { return { - typename: projectNames.parseProjectPath(subjectType), + typename: unwrap(projectNames.parseProjectPath(subjectType)), rawTypename: subjectType, methodCall: undefined, payload: { type: 'Value' }, diff --git a/app/gui/src/project-view/components/GraphEditor/widgets/WidgetSelection.vue b/app/gui/src/project-view/components/GraphEditor/widgets/WidgetSelection.vue index 7b9e8214d4e1..50b88ed39446 100644 --- a/app/gui/src/project-view/components/GraphEditor/widgets/WidgetSelection.vue +++ b/app/gui/src/project-view/components/GraphEditor/widgets/WidgetSelection.vue @@ -30,6 +30,7 @@ import { targetIsOutside } from '@/util/autoBlur' import { ArgumentInfoKey } from '@/util/callTree' import { arrayEquals } from '@/util/data/array' import type { Opt } from '@/util/data/opt' +import { ProjectPath } from '@/util/projectPath' import { qnLastSegment, tryQualifiedName } from '@/util/qualifiedName' import { autoUpdate, offset, shift, size, useFloating } from '@floating-ui/vue' import type { Ref, RendererNode, VNode } from 'vue' @@ -110,15 +111,22 @@ class ExpressionTag { public parameters?: ArgumentWidgetConfiguration[], ) {} - static FromQualifiedName(qn: Ast.QualifiedName, label?: Opt): ExpressionTag { - const entry = suggestions.entries.getEntryByProjectPath(projectNames.parseProjectPath(qn)) + static FromProjectPath(path: ProjectPath, label?: Opt): ExpressionTag | null { + const entry = suggestions.entries.getEntryByProjectPath(path) if (entry) return ExpressionTag.FromEntry(entry, label) - return new ExpressionTag(qn, label ?? qnLastSegment(qn)) + else return null } static FromExpression(expression: string, label?: Opt): ExpressionTag { const qn = tryQualifiedName(expression) - if (qn.ok) return ExpressionTag.FromQualifiedName(qn.value, label) + if (qn.ok) { + const projectPath = projectNames.parseProjectPath(qn.value) + if (projectPath.ok) { + const fromProjPath = ExpressionTag.FromProjectPath(projectPath.value, label) + if (fromProjPath) return fromProjPath + } + return new ExpressionTag(qn.value, label ?? qnLastSegment(qn.value)) + } return new ExpressionTag(expression, label) } diff --git a/app/gui/src/project-view/components/GraphEditor/widgets/WidgetTableEditor/__tests__/tableInputArgument.test.ts b/app/gui/src/project-view/components/GraphEditor/widgets/WidgetTableEditor/__tests__/tableInputArgument.test.ts index b9ad6110412f..247399ad889d 100644 --- a/app/gui/src/project-view/components/GraphEditor/widgets/WidgetTableEditor/__tests__/tableInputArgument.test.ts +++ b/app/gui/src/project-view/components/GraphEditor/widgets/WidgetTableEditor/__tests__/tableInputArgument.test.ts @@ -15,8 +15,7 @@ import { makeType } from '@/stores/suggestionDatabase/mockSuggestion' import { assert } from '@/util/assert' import { Ast } from '@/util/ast' import { type Identifier } from '@/util/ast/abstract' -import { parseAbsoluteProjectPath } from '@/util/projectPath' -import { tryQualifiedName } from '@/util/qualifiedName' +import { parseAbsoluteProjectPathRaw } from '@/util/projectPath' import { GetContextMenuItems, GetMainMenuItems } from 'ag-grid-enterprise' import { expect, test, vi } from 'vitest' import { assertDefined } from 'ydoc-shared/util/assert' @@ -43,7 +42,7 @@ assert(CELLS_LIMIT_SQRT === Math.floor(CELLS_LIMIT_SQRT)) function stdPath(path: string) { assert(path.startsWith('Standard.')) - return parseAbsoluteProjectPath(unwrap(tryQualifiedName(path))) + return unwrap(parseAbsoluteProjectPathRaw(path)) } test.each([ diff --git a/app/gui/src/project-view/stores/graph/__tests__/imports.test.ts b/app/gui/src/project-view/stores/graph/__tests__/imports.test.ts index 271c982f1dcd..74689a55418d 100644 --- a/app/gui/src/project-view/stores/graph/__tests__/imports.test.ts +++ b/app/gui/src/project-view/stores/graph/__tests__/imports.test.ts @@ -29,7 +29,7 @@ const qn = (s: string) => unwrap(tryQualifiedName(s)) const projectNames = mockProjectNameStore('local', 'Project') function projectPath(path: string) { - return projectNames.parseProjectPath(unwrap(tryQualifiedName(path))) + return unwrap(projectNames.parseProjectPath(unwrap(tryQualifiedName(path)))) } interface CoverCase { @@ -142,7 +142,7 @@ test.each([ expect( covers( { - from: projectNames.parseProjectPath(existing.from), + from: unwrap(projectNames.parseProjectPath(existing.from)), imported: existing.imported, }, required, @@ -222,7 +222,7 @@ test.each([ const conflicts = detectImportConflicts( db, alreadyImported.map(({ from, imported }) => ({ - from: projectNames.parseProjectPath(from), + from: unwrap(projectNames.parseProjectPath(from)), imported, })), importing, diff --git a/app/gui/src/project-view/stores/graph/graphDatabase.ts b/app/gui/src/project-view/stores/graph/graphDatabase.ts index 15b514802e17..1fd8d39c9b2c 100644 --- a/app/gui/src/project-view/stores/graph/graphDatabase.ts +++ b/app/gui/src/project-view/stores/graph/graphDatabase.ts @@ -1,5 +1,9 @@ import { computeNodeColor } from '@/composables/nodeColors' -import { ComputedValueRegistry, type ExpressionInfo } from '@/stores/project/computedValueRegistry' +import { + ComputedValueRegistry, + translateMethodCall, + type ExpressionInfo, +} from '@/stores/project/computedValueRegistry' import { mockProjectNameStore, type ProjectNameStore } from '@/stores/projectNames' import { SuggestionDb, type Group } from '@/stores/suggestionDatabase' import { type CallableSuggestionEntry } from '@/stores/suggestionDatabase/entry' @@ -15,7 +19,6 @@ import { Vec2 } from '@/util/data/vec2' import { ReactiveDb, ReactiveIndex, ReactiveMapping } from '@/util/database/reactiveDb' import { methodPointerEquals, - parseMethodPointer, type MethodCall, type MethodPointer, type StackItem, @@ -38,7 +41,6 @@ import { type WatchStopHandle, } from 'vue' import { type SourceDocument } from 'ydoc-shared/ast/sourceDocument' -import type { MethodCall as LSMethodCall } from 'ydoc-shared/languageServerTypes' import type { Opt } from 'ydoc-shared/util/data/opt' import type { ExternalId, VisualizationMetadata } from 'ydoc-shared/yjsModel' import { isUuid, visMetadataEquals } from 'ydoc-shared/yjsModel' @@ -210,12 +212,16 @@ export class GraphDb { getMethodCall(id: AstId): MethodCall | undefined { const info = this.getExpressionInfo(id) if (info == null) return - return ( - info.methodCall ?? - (info.payload.type === 'Value' && info.payload.functionSchema ? - translateMethodCall(info.payload.functionSchema, this.projectNames) - : undefined) - ) + if (info.methodCall) return info.methodCall + if (info.payload.type === 'Value' && info.payload.functionSchema) { + const translated = translateMethodCall(info.payload.functionSchema, this.projectNames) + if (translated.ok) return translated.value + else + translated.error.log( + "Ignoring MethodCall value in functionSchema, because it' ill formatted", + ) + } + return } /** TODO: Add docs */ @@ -532,13 +538,6 @@ export class GraphDb { } } -function translateMethodCall(ls: LSMethodCall, projectNames: ProjectNameStore): MethodCall { - return { - methodPointer: parseMethodPointer(ls.methodPointer, projectNames), - notAppliedArguments: ls.notAppliedArguments, - } -} - /** Source code data of the specific node. */ interface NodeSource { /** The outer AST of the node (see {@link NodeDataFromAst.outerAst}). */ diff --git a/app/gui/src/project-view/stores/graph/imports.ts b/app/gui/src/project-view/stores/graph/imports.ts index 7186eff51dee..009fd47d9e2a 100644 --- a/app/gui/src/project-view/stores/graph/imports.ts +++ b/app/gui/src/project-view/stores/graph/imports.ts @@ -21,15 +21,13 @@ import { // ======================== /** Read imports from given module block */ -export function readImports(ast: Ast.BodyBlock): RawImport[] { - const imports: RawImport[] = [] +export function* readImports(ast: Ast.BodyBlock): Generator { for (const stmt of ast.statements()) { if (stmt instanceof Ast.Import) { const recognized = recognizeImport(stmt) - if (recognized) imports.push(recognized) + if (recognized) yield recognized } } - return imports } /** Parse import statement. */ @@ -62,14 +60,19 @@ export function recognizeImport(ast: Ast.Import): RawImport | null { } /** Read the imports and transform their literal project paths to logical project paths. */ -export function analyzeImports( +export function* analyzeImports( ast: Ast.BodyBlock, projectNames: ProjectNameStore, -): AbstractImport[] { - return readImports(ast).map(({ from, imported }) => ({ - from: projectNames.parseProjectPath(normalizeQualifiedName(from)), - imported, - })) +): Generator { + for (const { from, imported } of readImports(ast)) { + const parsed = projectNames.parseProjectPath(normalizeQualifiedName(from)) + if (parsed.ok) { + yield { + from: parsed.value, + imported, + } + } + } } /** Information about parsed import statement. */ diff --git a/app/gui/src/project-view/stores/graph/index.ts b/app/gui/src/project-view/stores/graph/index.ts index 1f27b71a4995..30a536b0a0d8 100644 --- a/app/gui/src/project-view/stores/graph/index.ts +++ b/app/gui/src/project-view/stores/graph/index.ts @@ -276,7 +276,7 @@ export const [provideGraphStore, useGraphStore] = createContextStore( return } const topLevel = edit.getVersion(moduleRoot.value) - const existingImports = analyzeImports(topLevel, projectNames) + const existingImports = [...analyzeImports(topLevel, projectNames)] const conflicts = [] const nonConflictingImports = [] @@ -306,7 +306,7 @@ export const [provideGraphStore, useGraphStore] = createContextStore( return } const topLevel = edit.getVersion(moduleRoot.value) - const existingImports_ = existingImports ?? analyzeImports(topLevel, projectNames) + const existingImports_ = existingImports ?? [...analyzeImports(topLevel, projectNames)] const importsToAdd = filterOutRedundantImports(existingImports_, imports) if (!importsToAdd.length) return diff --git a/app/gui/src/project-view/stores/project/computedValueRegistry.ts b/app/gui/src/project-view/stores/project/computedValueRegistry.ts index 6c18dd84e2d0..de345d580647 100644 --- a/app/gui/src/project-view/stores/project/computedValueRegistry.ts +++ b/app/gui/src/project-view/stores/project/computedValueRegistry.ts @@ -1,10 +1,10 @@ import type { ExecutionContext } from '@/stores/project/executionContext' import { mockProjectNameStore, type ProjectNameStore } from '@/stores/projectNames' +import { Ok, Result, unwrapOr } from '@/util/data/result' import { ReactiveDb, ReactiveIndex } from '@/util/database/reactiveDb' import { ANY_TYPE_QN } from '@/util/ensoTypes' import { parseMethodPointer, type MethodCall } from '@/util/methodPointer' import { type ProjectPath } from '@/util/projectPath' -import { isQualifiedName } from '@/util/qualifiedName' import { markRaw } from 'vue' import type { ExpressionId, @@ -84,11 +84,21 @@ function updateInfo( if (newInfo.profilingInfo !== info.profilingInfo) info.profilingInfo = update.profilingInfo } -function translateMethodCall(ls: LSMethodCall, projectNames: ProjectNameStore): MethodCall { - return { - methodPointer: parseMethodPointer(ls.methodPointer, projectNames), +/** + * Translate the MethodCall retrieved from language server to our structure. + * + * The qualified names are validated and stored as {@link ProjectPath}s. + */ +export function translateMethodCall( + ls: LSMethodCall, + projectNames: ProjectNameStore, +): Result { + const methodPointer = parseMethodPointer(ls.methodPointer, projectNames) + if (!methodPointer.ok) return methodPointer + return Ok({ + methodPointer: methodPointer.value, notAppliedArguments: ls.notAppliedArguments, - } + }) } function combineInfo( @@ -99,14 +109,25 @@ function combineInfo( const isPending = update.payload.type === 'Pending' const updateSingleValueType = update.type.at(0) // TODO: support multi-value (aka intersection) types const rawTypename = updateSingleValueType ?? (isPending ? info?.rawTypename : undefined) + // As all objects descend from Any, we can treat Any as implicit. This reduces the depth of all type + // hierarchies that have to be stored and have to be walked when filtering. + const typename = + rawTypename && rawTypename !== ANY_TYPE_QN ? + projectNames.parseProjectPathRaw(rawTypename) + : undefined + if (typename && !typename.ok) { + typename.error.log('Discarding invalid type in expression update') + } + const newMethodCall = + update.methodCall ? translateMethodCall(update.methodCall, projectNames) : undefined + if (newMethodCall && !newMethodCall.ok) { + newMethodCall.error.log('Discarding invalid methodCall in expression update') + } return { - typename: - rawTypename && isQualifiedName(rawTypename) && rawTypename !== ANY_TYPE_QN ? - projectNames.parseProjectPath(rawTypename) - : undefined, + typename: typename ? unwrapOr(typename, undefined) : undefined, rawTypename, methodCall: - update.methodCall ? translateMethodCall(update.methodCall, projectNames) + newMethodCall?.ok ? newMethodCall.value : isPending ? info?.methodCall : undefined, payload: update.payload, diff --git a/app/gui/src/project-view/stores/projectNames.ts b/app/gui/src/project-view/stores/projectNames.ts index 1f2e26415702..25622ac377fa 100644 --- a/app/gui/src/project-view/stores/projectNames.ts +++ b/app/gui/src/project-view/stores/projectNames.ts @@ -1,6 +1,7 @@ import { createContextStore } from '@/providers' +import { Ok, Result } from '@/util/data/result' import { parseAbsoluteProjectPath, ProjectPath } from '@/util/projectPath' -import { normalizeQualifiedName, qnJoin } from '@/util/qualifiedName' +import { normalizeQualifiedName, qnJoin, tryQualifiedName } from '@/util/qualifiedName' import { type ToValue } from '@/util/reactivity' import { computed, readonly, ref, toRef, toValue } from 'vue' import { type Identifier, type QualifiedName } from 'ydoc-shared/ast' @@ -36,13 +37,25 @@ function useProjectNameStore( * To ensure that QNs are interpreted correctly during and after project renames, this should be applied to data * from the backend as it is received. */ - function parseProjectPath(path: QualifiedName): ProjectPath { + function parseProjectPath(path: QualifiedName): Result { const parsed = parseAbsoluteProjectPath(path) - return parsed.project === inboundProject.value ? - ProjectPath.create(undefined, parsed.path) + if (!parsed.ok) return parsed + return parsed.value.project === inboundProject.value ? + Ok(ProjectPath.create(undefined, parsed.value.path)) : parsed } + /** + * Interpret a string as a project path. + * + * Same as {@link parseProjectPath}, but the path is also checked for being an actual Qualified Name. + */ + function parseProjectPathRaw(path: string): Result { + const qn = tryQualifiedName(path) + if (!qn.ok) return qn + return parseProjectPath(qn.value) + } + /** * Serialize the path, with any project's `Main` segment elided. This is appropriate for values that will be displayed * to the user or written into source code. @@ -66,6 +79,7 @@ function useProjectNameStore( return { parseProjectPath, + parseProjectPathRaw, printProjectPath, serializeProjectPathForBackend, onProjectRenameRequested: (newName: Identifier) => { diff --git a/app/gui/src/project-view/stores/suggestionDatabase/__tests__/documentation.test.ts b/app/gui/src/project-view/stores/suggestionDatabase/__tests__/documentation.test.ts index 392e5403e5e8..6670ca4d8e05 100644 --- a/app/gui/src/project-view/stores/suggestionDatabase/__tests__/documentation.test.ts +++ b/app/gui/src/project-view/stores/suggestionDatabase/__tests__/documentation.test.ts @@ -2,8 +2,8 @@ import { mockProjectNameStore } from '@/stores/projectNames' import { getGroupIndex, tagValue } from '@/stores/suggestionDatabase/documentation' import { unwrap } from '@/util/data/result' import { parseDocs } from '@/util/docParser' -import { parseAbsoluteProjectPath } from '@/util/projectPath' -import { tryQualifiedName, type QualifiedName } from '@/util/qualifiedName' +import { parseAbsoluteProjectPathRaw } from '@/util/projectPath' +import { type QualifiedName } from '@/util/qualifiedName' import { expect, test } from 'vitest' test.each([ @@ -24,7 +24,7 @@ const groups = [ { name: 'Another', project: 'local.Project' }, ].map(({ name, project }) => ({ name, - project: parseAbsoluteProjectPath(unwrap(tryQualifiedName(project))).project as QualifiedName, + project: unwrap(parseAbsoluteProjectPathRaw(project)).project as QualifiedName, })) test.each([ ['From Base', 'local.Project.Main', undefined], @@ -37,7 +37,7 @@ test.each([ ['Not Existing', 'local.Project.Main', undefined], ])('Get group index case %#.', (name, definedIn, expected) => { const definedInQn = - projectNames.parseProjectPath(unwrap(tryQualifiedName(definedIn))).project ?? + unwrap(projectNames.parseProjectPathRaw(definedIn)).project ?? ('local.Project' as QualifiedName) expect(getGroupIndex(name, definedInQn, groups)).toBe(expected) }) diff --git a/app/gui/src/project-view/stores/suggestionDatabase/__tests__/lsUpdate.test.ts b/app/gui/src/project-view/stores/suggestionDatabase/__tests__/lsUpdate.test.ts index 1111ca0a6e9d..55cb6755072f 100644 --- a/app/gui/src/project-view/stores/suggestionDatabase/__tests__/lsUpdate.test.ts +++ b/app/gui/src/project-view/stores/suggestionDatabase/__tests__/lsUpdate.test.ts @@ -5,7 +5,7 @@ import { SuggestionUpdateProcessor } from '@/stores/suggestionDatabase/lsUpdate' import { assert, assertDefined } from '@/util/assert' import { unwrap } from '@/util/data/result' import { parseDocs } from '@/util/docParser' -import { parseAbsoluteProjectPath, ProjectPath } from '@/util/projectPath' +import { parseAbsoluteProjectPathRaw, ProjectPath } from '@/util/projectPath' import { tryIdentifier, tryQualifiedName, @@ -18,7 +18,7 @@ import { type SuggestionsDatabaseUpdate } from 'ydoc-shared/languageServerTypes/ function stdPath(path: string) { assert(path.startsWith('Standard.')) - return parseAbsoluteProjectPath(unwrap(tryQualifiedName(path))) + return unwrap(parseAbsoluteProjectPathRaw(path)) } const projectNames = mockProjectNameStore() diff --git a/app/gui/src/project-view/stores/suggestionDatabase/lsUpdate.ts b/app/gui/src/project-view/stores/suggestionDatabase/lsUpdate.ts index 949bf1baac6c..ae57de1c80d3 100644 --- a/app/gui/src/project-view/stores/suggestionDatabase/lsUpdate.ts +++ b/app/gui/src/project-view/stores/suggestionDatabase/lsUpdate.ts @@ -26,11 +26,8 @@ import { Identifier, type IdentifierOrOperatorIdentifier, isIdentifierOrOperatorIdentifier, - isQualifiedName, qnJoin, qnLastSegment, - type QualifiedName, - tryQualifiedName, } from '@/util/qualifiedName' import { type ToValue } from '@/util/reactivity' import { type DeepReadonly, toValue } from 'vue' @@ -40,11 +37,6 @@ import { SuggestionsDatabaseUpdate, } from 'ydoc-shared/languageServerTypes/suggestions' -function isOptQN(optQn: string | undefined): optQn is QualifiedName | undefined { - if (optQn == null) return true - return isQualifiedName(optQn) -} - interface UpdateContext { groups: DeepReadonly projectNames: ProjectNameStore @@ -92,14 +84,14 @@ abstract class BaseSuggestionEntry implements SuggestionEntryCommon { setLsModule(lsModule: ProjectPath) { this.definedIn = lsModule } - setLsReturnType(_returnType: Typename, _projectNames: ProjectNameStore) { - console.warn(`Cannot modify \`returnType\` of entry type ${this.kind}.`) + setLsReturnType(_returnType: Typename, _projectNames: ProjectNameStore): Result { + return Err(`Cannot modify \`returnType\` of entry type ${this.kind}.`) } - setLsReexported(_reexported: ProjectPath | undefined) { - console.warn(`Cannot modify \`reexported\` of entry type ${this.kind}.`) + setLsReexported(_reexported: ProjectPath | undefined): Result { + return Err(`Cannot modify \`reexported\` of entry type ${this.kind}.`) } - setLsScope(_scope: lsTypes.SuggestionEntryScope | undefined) { - console.warn(`Cannot modify \`scope\` of entry type ${this.kind}.`) + setLsScope(_scope: lsTypes.SuggestionEntryScope | undefined): Result { + return Err(`Cannot modify \`scope\` of entry type ${this.kind}.`) } } @@ -129,13 +121,14 @@ class FunctionSuggestionEntryImpl extends BaseSuggestionEntry implements Functio context: UpdateContext, ): Result { if (!isIdentifierOrOperatorIdentifier(lsEntry.name)) return Err('Invalid name') - if (!isQualifiedName(lsEntry.module)) return Err('Invalid module name') + const module = parseProjectPath(lsEntry, 'module', context) + if (!module.ok) return module return Ok( new FunctionSuggestionEntryImpl( lsEntry.name, lsEntry.scope, lsEntry.arguments, - context.projectNames.parseProjectPath(lsEntry.module), + module.value, lsEntry.returnType, lsEntry.documentation, context, @@ -145,9 +138,11 @@ class FunctionSuggestionEntryImpl extends BaseSuggestionEntry implements Functio override setLsReturnType(returnType: Typename) { this.lsReturnType = returnType + return Ok() } override setLsScope(scope: lsTypes.SuggestionEntryScope | undefined) { this.scope = scope + return Ok() } } @@ -181,20 +176,18 @@ class ModuleSuggestionEntryImpl extends BaseSuggestionEntry implements ModuleSug lsEntry: lsTypes.SuggestionEntry.Module, context: UpdateContext, ): Result { - if (!isQualifiedName(lsEntry.module)) return Err('Invalid module name') - if (!isOptQN(lsEntry.reexport)) return Err('Invalid reexport') + const module = parseProjectPath(lsEntry, 'module', context) + if (!module.ok) return module + const reexport = parseProjectPath(lsEntry, 'reexport', context) + if (!reexport.ok) return reexport return Ok( - new ModuleSuggestionEntryImpl( - context.projectNames.parseProjectPath(lsEntry.module), - lsEntry.reexport && context.projectNames.parseProjectPath(lsEntry.reexport), - lsEntry.documentation, - context, - ), + new ModuleSuggestionEntryImpl(module.value, reexport.value, lsEntry.documentation, context), ) } override setLsReexported(reexported: ProjectPath | undefined) { this.reexportedIn = reexported + return Ok() } } @@ -224,17 +217,19 @@ class TypeSuggestionEntryImpl extends BaseSuggestionEntry implements TypeSuggest context: UpdateContext, ): Result { if (!isIdentifierOrOperatorIdentifier(lsEntry.name)) return Err('Invalid name') - if (!isQualifiedName(lsEntry.module)) return Err('Invalid module name') - if (!isOptQN(lsEntry.reexport)) return Err('Invalid reexport') - if (!isOptQN(lsEntry.parentType)) return Err('Invalid parent type') - const parentTypeQn = lsEntry.parentType === ANY_TYPE_QN ? undefined : lsEntry.parentType + const module = parseProjectPath(lsEntry, 'module', context) + if (!module.ok) return module + const reexport = parseProjectPath(lsEntry, 'reexport', context) + if (!reexport.ok) return reexport + const parentType = parseProjectPath(lsEntry, 'parentType', context) + if (!parentType.ok) return parentType return Ok( new TypeSuggestionEntryImpl( lsEntry.name, lsEntry.params, - parentTypeQn && context.projectNames.parseProjectPath(parentTypeQn), - context.projectNames.parseProjectPath(lsEntry.module), - lsEntry.reexport && context.projectNames.parseProjectPath(lsEntry.reexport), + lsEntry.parentType !== ANY_TYPE_QN ? parentType.value : undefined, + module.value, + reexport.value, lsEntry.documentation, context, ), @@ -243,6 +238,7 @@ class TypeSuggestionEntryImpl extends BaseSuggestionEntry implements TypeSuggest override setLsReexported(reexported: ProjectPath | undefined) { this.reexportedIn = reexported + return Ok() } } @@ -279,17 +275,20 @@ class ConstructorSuggestionEntryImpl context: UpdateContext, ): Result { if (!isIdentifierOrOperatorIdentifier(lsEntry.name)) return Err('Invalid name') - if (!isQualifiedName(lsEntry.module)) return Err('Invalid module name') - if (!isOptQN(lsEntry.reexport)) return Err('Invalid reexport') - if (!isQualifiedName(lsEntry.returnType)) return Err('Invalid constructor return type') + const module = parseProjectPath(lsEntry, 'module', context) + if (!module.ok) return module + const reexport = parseProjectPath(lsEntry, 'reexport', context) + if (!reexport.ok) return reexport + const returnType = parseProjectPath(lsEntry, 'returnType', context) + if (!returnType.ok) return returnType return Ok( new ConstructorSuggestionEntryImpl( lsEntry.name, lsEntry.arguments, - lsEntry.reexport && context.projectNames.parseProjectPath(lsEntry.reexport), + reexport.value, lsEntry.annotations, - context.projectNames.parseProjectPath(lsEntry.module), - context.projectNames.parseProjectPath(lsEntry.returnType), + module.value, + returnType.value, lsEntry.documentation, context, ), @@ -297,11 +296,14 @@ class ConstructorSuggestionEntryImpl } override setLsReturnType(returnType: Typename, projectNames: ProjectNameStore) { - if (!isQualifiedName(returnType)) return Err('Invalid constructor return type') - this.memberOf = projectNames.parseProjectPath(returnType) + const parsed = projectNames.parseProjectPathRaw(returnType) + if (!parsed.ok) return parsed + this.memberOf = parsed.value + return Ok() } override setLsReexported(reexported: ProjectPath | undefined) { this.reexportedIn = reexported + return Ok() } } @@ -340,18 +342,21 @@ class MethodSuggestionEntryImpl extends BaseSuggestionEntry implements MethodSug context: UpdateContext, ): Result { if (!isIdentifierOrOperatorIdentifier(lsEntry.name)) return Err('Invalid name') - if (!isQualifiedName(lsEntry.module)) return Err('Invalid module name') - if (!isOptQN(lsEntry.reexport)) return Err('Invalid reexport') - if (!isQualifiedName(lsEntry.selfType)) return Err('Invalid module name') + const module = parseProjectPath(lsEntry, 'module', context) + if (!module.ok) return module + const reexport = parseProjectPath(lsEntry, 'reexport', context) + if (!reexport.ok) return reexport + const selfType = parseProjectPath(lsEntry, 'selfType', context) + if (!selfType.ok) return selfType return Ok( new MethodSuggestionEntryImpl( lsEntry.name, lsEntry.arguments, - lsEntry.reexport && context.projectNames.parseProjectPath(lsEntry.reexport), + reexport.value, lsEntry.annotations, lsEntry.isStatic, - context.projectNames.parseProjectPath(lsEntry.selfType), - context.projectNames.parseProjectPath(lsEntry.module), + selfType.value, + module.value, lsEntry.returnType, lsEntry.documentation, context, @@ -361,9 +366,11 @@ class MethodSuggestionEntryImpl extends BaseSuggestionEntry implements MethodSug override setLsReturnType(returnType: Typename) { this.lsReturnType = returnType + return Ok() } override setLsReexported(reexported: ProjectPath | undefined) { this.reexportedIn = reexported + return Ok() } setLsSelfType(selfType: ProjectPath) { this.memberOf = selfType @@ -393,12 +400,13 @@ class LocalSuggestionEntryImpl extends BaseSuggestionEntry implements LocalSugge context: UpdateContext, ): Result { if (!isIdentifierOrOperatorIdentifier(lsEntry.name)) return Err('Invalid name') - if (!isQualifiedName(lsEntry.module)) return Err('Invalid module name') + const module = parseProjectPath(lsEntry, 'module', context) + if (!module.ok) return module return Ok( new LocalSuggestionEntryImpl( lsEntry.name, lsEntry.scope, - context.projectNames.parseProjectPath(lsEntry.module), + module.value, lsEntry.returnType, lsEntry.documentation, context, @@ -408,16 +416,18 @@ class LocalSuggestionEntryImpl extends BaseSuggestionEntry implements LocalSugge override setLsReturnType(returnType: Typename) { this.lsReturnType = returnType + return Ok() } override setLsScope(scope: lsTypes.SuggestionEntryScope | undefined) { this.scope = scope + return Ok() } } function applyFieldUpdate( name: K, update: { [P in K]?: lsTypes.FieldUpdate }, - updater: (newValue: T) => R, + updater: (newValue: T) => Result, ): Result> { const field = update[name] if (field == null) return Ok(null) @@ -427,7 +437,7 @@ function applyFieldUpdate( switch (field.tag) { case 'Set': if (field.value != null) { - return Ok(updater(field.value)) + return updater(field.value) } else { return Err('Received "Set" update with no value') } @@ -447,6 +457,7 @@ function applyPropertyUpdate( ): Result { const apply = applyFieldUpdate(name, update, (newValue) => { obj[name] = newValue + return Ok() }) if (!apply.ok) return apply return Ok() @@ -502,6 +513,7 @@ function modifyArgument( if (!nameUpdate.ok) return nameUpdate const typeUpdate = applyFieldUpdate('reprType', update, (type) => { arg.reprType = type + return Ok() }) if (!typeUpdate.ok) return typeUpdate const isSuspendedUpdate = applyPropertyUpdate('isSuspended', arg, update) @@ -600,23 +612,25 @@ export class SuggestionUpdateProcessor { } const moduleUpdate = applyFieldUpdate('module', update, (module) => { - const qn = tryQualifiedName(module) - if (!qn.ok) return false - entry.setLsModule(this.projectNames.parseProjectPath(qn.value)) - return true + const pp = this.projectNames.parseProjectPathRaw(module) + if (!pp.ok) return pp + entry.setLsModule(pp.value) + return Ok() }) if (!moduleUpdate.ok) return moduleUpdate - if (moduleUpdate.value === false) return Err('Invalid module name') const selfTypeUpdate = applyFieldUpdate('selfType', update, (selfType) => { - if (!(entry instanceof MethodSuggestionEntryImpl)) return false - if (!isQualifiedName(selfType)) return false - entry.setLsSelfType(this.projectNames.parseProjectPath(selfType)) + if (!(entry instanceof MethodSuggestionEntryImpl)) + return Err('Tried to update selfType in non-method entry') + const pp = this.projectNames.parseProjectPathRaw(selfType) + if (!pp.ok) return pp + entry.setLsSelfType(pp.value) + return Ok() }) if (!selfTypeUpdate.ok) return selfTypeUpdate const returnTypeUpdate = applyFieldUpdate('returnType', update, (returnType) => { - entry.setLsReturnType(returnType, this.projectNames) + return entry.setLsReturnType(returnType, this.projectNames) }) if (!returnTypeUpdate.ok) return returnTypeUpdate @@ -626,10 +640,16 @@ export class SuggestionUpdateProcessor { if (update.scope) entry.setLsScope(update.scope.value) if (update.reexport) { - if (!isOptQN(update.reexport.value)) return Err('Invalid reexport') - entry.setLsReexported( - update.reexport.value && this.projectNames.parseProjectPath(update.reexport.value), + const reexport = withContext( + () => 'When parsing reexport field', + () => + update.reexport?.value ? + this.projectNames.parseProjectPathRaw(update.reexport.value) + : Ok(undefined), ) + + if (!reexport.ok) return reexport + entry.setLsReexported(reexport.value) } return Ok() @@ -649,3 +669,27 @@ export class SuggestionUpdateProcessor { } } } + +function parseProjectPath( + lsEntry: { [P in K]: string }, + field: K, + context: UpdateContext, +): Result +function parseProjectPath( + lsEntry: { [P in K]?: string }, + field: K, + context: UpdateContext, +): Result +function parseProjectPath( + lsEntry: { [P in K]?: string }, + field: K, + context: UpdateContext, +) { + return withContext( + () => `Parsing ${field}`, + () => + lsEntry[field] != null ? + context.projectNames.parseProjectPathRaw(lsEntry[field]) + : Ok(undefined), + ) +} diff --git a/app/gui/src/project-view/util/__tests__/callTree.test.ts b/app/gui/src/project-view/util/__tests__/callTree.test.ts index 4c120a5b10f1..33c4bd71c2df 100644 --- a/app/gui/src/project-view/util/__tests__/callTree.test.ts +++ b/app/gui/src/project-view/util/__tests__/callTree.test.ts @@ -389,7 +389,7 @@ test.each([ function stdPath(path: string) { assert(path.startsWith('Standard.')) - return parseAbsoluteProjectPath(unwrap(tryQualifiedName(path))) + return unwrap(parseAbsoluteProjectPath(unwrap(tryQualifiedName(path)))) } function prepareMocksForGetMethodCallTest(): { diff --git a/app/gui/src/project-view/util/__tests__/projectPath.test.ts b/app/gui/src/project-view/util/__tests__/projectPath.test.ts index d965e9a42c86..3e0e06d695ab 100644 --- a/app/gui/src/project-view/util/__tests__/projectPath.test.ts +++ b/app/gui/src/project-view/util/__tests__/projectPath.test.ts @@ -1,4 +1,5 @@ import { mockProjectNameStore } from '@/stores/projectNames' +import { unwrap } from '@/util/data/result' import { ProjectPath } from '@/util/projectPath' import { type QualifiedName } from '@/util/qualifiedName' import { expect, test } from 'vitest' @@ -29,7 +30,7 @@ const cases = [ const projectNames = mockProjectNameStore() test.each(cases)('Parse', ({ qn, path }) => { - expect(projectNames.parseProjectPath(qn).toJSON()).toStrictEqual(path.toJSON()) + expect(unwrap(projectNames.parseProjectPath(qn)).toJSON()).toStrictEqual(path.toJSON()) }) test.each(cases)('Normalize', ({ path, normalized }) => { diff --git a/app/gui/src/project-view/util/__tests__/qualifiedName.test.ts b/app/gui/src/project-view/util/__tests__/qualifiedName.test.ts index 4f792899fdda..e1ed38d0f438 100644 --- a/app/gui/src/project-view/util/__tests__/qualifiedName.test.ts +++ b/app/gui/src/project-view/util/__tests__/qualifiedName.test.ts @@ -81,7 +81,7 @@ test.each([ ['local.Project.elem', true], ['local.Project.Module.elem', false], ])('pathIsTopElement(%s) returns %s', (name, result) => { - expect(projectNames.parseProjectPath(unwrap(tryQualifiedName(name))).isTopElement()).toBe(result) + expect(unwrap(projectNames.parseProjectPathRaw(name)).isTopElement()).toBe(result) }) test.each([ diff --git a/app/gui/src/project-view/util/methodPointer.ts b/app/gui/src/project-view/util/methodPointer.ts index 8e75b6d28aaf..9ee99a79ddf5 100644 --- a/app/gui/src/project-view/util/methodPointer.ts +++ b/app/gui/src/project-view/util/methodPointer.ts @@ -1,4 +1,5 @@ import { type ProjectNameStore } from '@/stores/projectNames' +import { Ok, Result } from '@/util/data/result' import { type ProjectPath } from '@/util/projectPath' import { type QualifiedName } from '@/util/qualifiedName' import * as encoding from 'lib0/encoding' @@ -40,13 +41,17 @@ export function methodPointerEquals(left: MethodPointer, right: MethodPointer): export function parseMethodPointer( ptr: LSMethodPointer, projectNames: ProjectNameStore, -): MethodPointer { +): Result { const { module, definedOnType, name } = ptr - return { - module: projectNames.parseProjectPath(module as QualifiedName), - definedOnType: projectNames.parseProjectPath(definedOnType as QualifiedName), + const parsedModule = projectNames.parseProjectPath(module as QualifiedName) + if (!parsedModule.ok) return parsedModule + const parsedDefinedOnType = projectNames.parseProjectPath(definedOnType as QualifiedName) + if (!parsedDefinedOnType.ok) return parsedDefinedOnType + return Ok({ + module: parsedModule.value, + definedOnType: parsedDefinedOnType.value, name: name as Identifier, - } + }) } export type StackItem = ExplicitCall | LocalCall diff --git a/app/gui/src/project-view/util/projectPath.ts b/app/gui/src/project-view/util/projectPath.ts index 3a3f07937b26..6ef9dede3862 100644 --- a/app/gui/src/project-view/util/projectPath.ts +++ b/app/gui/src/project-view/util/projectPath.ts @@ -1,24 +1,35 @@ +import { Err, Ok, Result } from '@/util/data/result' import { qnJoin, qnSplit, + tryQualifiedName, type IdentifierOrOperatorIdentifier, type QualifiedName, } from '@/util/qualifiedName' -import { assert, assertDefined } from 'ydoc-shared/util/assert' +import { assertDefined } from 'ydoc-shared/util/assert' export type ProjectName = QualifiedName /** Parses the qualified name as a literal project path. */ -export function parseAbsoluteProjectPath(path: QualifiedName): ProjectPath { +export function parseAbsoluteProjectPath(path: QualifiedName): Result { const parts = /^([^.]+\.[^.]+)(?:\.(.+))?$/.exec(path) - assert(parts != null) + if (parts == null) return Err(`${path} is not absolute project path`) assertDefined(parts[1]) - return ProjectPath.create( - parts[1] as QualifiedName, - parts[2] ? (parts[2] as QualifiedName) : undefined, + return Ok( + ProjectPath.create( + parts[1] as QualifiedName, + parts[2] ? (parts[2] as QualifiedName) : undefined, + ), ) } +/** Parses the string as a literal project path. */ +export function parseAbsoluteProjectPathRaw(path: string): Result { + const qn = tryQualifiedName(path) + if (!qn.ok) return qn + return parseAbsoluteProjectPath(qn.value) +} + /** Prints a literal project path. */ export function printAbsoluteProjectPath(path: AbsoluteProjectPath): QualifiedName { return path.path ? qnJoin(path.project, path.path) : path.project