Skip to content

Commit

Permalink
Expect failure when parsing external Qualified Names (#12172)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
farmaazon authored Jan 29, 2025
1 parent 54ccd1a commit 4cae524
Show file tree
Hide file tree
Showing 20 changed files with 259 additions and 148 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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', () => {
Expand Down
16 changes: 9 additions & 7 deletions app/gui/src/project-view/components/DocumentationPanel.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -58,9 +59,9 @@ const isPlaceholder = computed(() => documentation.value.kind === 'Placeholder')
const projectNames = injectProjectNames()
const name = computed<Opt<QualifiedName>>(() => {
const name = computed<Opt<ProjectPath>>(() => {
const docs = documentation.value
return docs.kind === 'Placeholder' ? null : projectNames.printProjectPath(docs.name)
return docs.kind === 'Placeholder' ? null : docs.name
})
// === Breadcrumbs ===
Expand Down Expand Up @@ -98,7 +99,7 @@ watch(historyStack.current, (current) => {
const breadcrumbs = computed<Breadcrumb[]>(() => {
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 []
Expand All @@ -107,9 +108,10 @@ const breadcrumbs = computed<Breadcrumb[]>(() => {
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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
}
Expand All @@ -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]))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -110,15 +111,22 @@ class ExpressionTag {
public parameters?: ArgumentWidgetConfiguration[],
) {}
static FromQualifiedName(qn: Ast.QualifiedName, label?: Opt<string>): ExpressionTag {
const entry = suggestions.entries.getEntryByProjectPath(projectNames.parseProjectPath(qn))
static FromProjectPath(path: ProjectPath, label?: Opt<string>): 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<string>): 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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -142,7 +142,7 @@ test.each<CoverCase>([
expect(
covers(
{
from: projectNames.parseProjectPath(existing.from),
from: unwrap(projectNames.parseProjectPath(existing.from)),
imported: existing.imported,
},
required,
Expand Down Expand Up @@ -222,7 +222,7 @@ test.each<ConflictCase>([
const conflicts = detectImportConflicts(
db,
alreadyImported.map(({ from, imported }) => ({
from: projectNames.parseProjectPath(from),
from: unwrap(projectNames.parseProjectPath(from)),
imported,
})),
importing,
Expand Down
31 changes: 15 additions & 16 deletions app/gui/src/project-view/stores/graph/graphDatabase.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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,
Expand All @@ -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'
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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}). */
Expand Down
23 changes: 13 additions & 10 deletions app/gui/src/project-view/stores/graph/imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<RawImport> {
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. */
Expand Down Expand Up @@ -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<AbstractImport> {
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. */
Expand Down
4 changes: 2 additions & 2 deletions app/gui/src/project-view/stores/graph/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = []
Expand Down Expand Up @@ -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
Expand Down
41 changes: 31 additions & 10 deletions app/gui/src/project-view/stores/project/computedValueRegistry.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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<MethodCall> {
const methodPointer = parseMethodPointer(ls.methodPointer, projectNames)
if (!methodPointer.ok) return methodPointer
return Ok({
methodPointer: methodPointer.value,
notAppliedArguments: ls.notAppliedArguments,
}
})
}

function combineInfo(
Expand All @@ -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,
Expand Down
Loading

0 comments on commit 4cae524

Please sign in to comment.