enso-org/enso
1
1
Fork 0
mirror of https://github.com/enso-org/enso.git synced 2024-11-22 21:51:05 +03:00
Code Issues Projects Releases Wiki Activity

Use alias analysis to handle internal variables in CB input. (#8268)

Browse Source 
Fixes #7926

See the new test cases to see what is the improvement.

The self still will be not ideal, but it's because it uses same logic as edges discovery. [When it will be improved](https://github.com/enso-org/enso/pull/8252) this should be improved as well.

https://github.com/enso-org/enso/assets/3919101/25efd56d-5aaa-4f10-957d-8ed6a40f3d9b

# Important Notes
It contains part of #8250. That PR may be reviewed, or here both PRs may be reviewed at once.
This commit is contained in:
Adam Obuchowicz 2023-11-13 13:59:15 +01:00 committed by GitHub
parent 6cf75daea9
commit c01ba83a3a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 188 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
app/gui2/package.json
View File

@ -39,6 +39,7 @@
"@lezer/highlight": "^1.1.6",
"@noble/hashes": "^1.3.2",
"@open-rpc/client-js": "^1.8.1",
"@pinia/testing": "^0.1.3",
"@vueuse/core": "^10.4.1",
"codemirror": "^6.0.1",
"culori": "^3.2.0",

20
app/gui2/src/components/ComponentBrowser/__tests__/filtering.test.ts
View File

@ -127,14 +127,22 @@ test.each([
expect(filtering.filter(entry)).toBeNull()
})
test('An Instance method is shown when self type matches', () => {
const entry = makeMethod('Standard.Base.Data.Vector.Vector.get')
test('An Instance method is shown when self arg matches', () => {
const entry1 = makeMethod('Standard.Base.Data.Vector.Vector.get')
const entry2 = makeMethod('Standard.Base.Data.Table.get')
const filteringWithSelfType = new Filtering({
selfType: 'Standard.Base.Data.Vector.Vector' as QualifiedName,
selfArg: { type: 'known', typename: 'Standard.Base.Data.Vector.Vector' },
})
expect(filteringWithSelfType.filter(entry)).not.toBeNull()
expect(filteringWithSelfType.filter(entry1)).not.toBeNull()
expect(filteringWithSelfType.filter(entry2)).toBeNull()
const filteringWithAnySelfType = new Filtering({
selfArg: { type: 'unknown' },
})
expect(filteringWithAnySelfType.filter(entry1)).not.toBeNull()
expect(filteringWithAnySelfType.filter(entry2)).not.toBeNull()
const filteringWithoutSelfType = new Filtering({ pattern: 'get' })
expect(filteringWithoutSelfType.filter(entry)).toBeNull()
expect(filteringWithoutSelfType.filter(entry1)).toBeNull()
expect(filteringWithoutSelfType.filter(entry2)).toBeNull()
})
test.each([
@ -147,7 +155,7 @@ test.each([
makeMethod('Standard.Base.Data.Vector.Vector2.get'),
])('$name is filtered out when Vector self type is specified', (entry) => {
const filtering = new Filtering({
selfType: 'Standard.Base.Data.Vector.Vector' as QualifiedName,
selfArg: { type: 'known', typename: 'Standard.Base.Data.Vector.Vector' },
})
expect(filtering.filter(entry)).toBeNull()
})

92
app/gui2/src/components/ComponentBrowser/__tests__/input.test.ts
View File

@ -8,6 +8,7 @@ import {
type SuggestionEntry,
} from '@/stores/suggestionDatabase/entry'
import { readAstSpan } from '@/util/ast'
import type { ExprId } from 'shared/yjsModel'
import { expect, test } from 'vitest'
import { useComponentBrowserInput } from '../input'
@ -40,20 +41,37 @@ test.each([
],
['2 +', 3, { type: 'insert', position: 3, oprApp: ['2', '+', null] }, {}],
['2 + 3', 5, { type: 'changeLiteral', literal: '3' }, { pattern: '3' }],
// TODO[ao] test cases for #7926
// [
// 'operator1.',
// 10,
// { type: 'insert', position: 10, oprApp: ['operator1', '.', null] },
// { selfType: 'Standard.Base.Number' },
// ],
// [
// 'operator2.',
// 10,
// { type: 'insert', position: 10, oprApp: ['operator2', '.', null] },
// // No self type, as the operator2 local is from another module
// { qualifiedNamePattern: 'operator2' },
// ],
[
'operator1.',
10,
{ type: 'insert', position: 10, oprApp: ['operator1', '.', null] },
{ selfArg: { type: 'known', typename: 'Standard.Base.Number' } },
],
[
'operator2.',
10,
{ type: 'insert', position: 10, oprApp: ['operator2', '.', null] },
{ selfArg: { type: 'unknown' } },
],
[
'operator3.',
10,
{ type: 'insert', position: 10, oprApp: ['operator3', '.', null] },
// No self type, as there is no operator3 node in current graph
{ qualifiedNamePattern: 'operator3' },
],
[
'operator1 -> operator1.',
23,
{ type: 'insert', position: 23, oprApp: ['operator1', '.', null] },
{ selfArg: { type: 'unknown' } },
],
[
'operator2 -> operator1.',
23,
{ type: 'insert', position: 23, oprApp: ['operator1', '.', null] },
{ selfArg: { type: 'known', typename: 'Standard.Base.Number' } },
],
])(
"Input context and filtering, when content is '%s' and cursor at %i",
(
@ -66,16 +84,32 @@ test.each([
identifier?: string
literal?: string
},
expFiltering: { pattern?: string; qualifiedNamePattern?: string; selfType?: string },
expFiltering: {
pattern?: string
qualifiedNamePattern?: string
selfArg?: { type: string; typename?: string }
},
) => {
// TODO[ao] See above commented cases for #7926
// const db = SuggestionDb.mock([
// makeLocal('local.Project', 'operator1', 'Standard.Base.Number'),
// makeLocal('local.Project.Another_Module', 'operator2', 'Standard.Base.Text'),
// makeType('local.Project.operator1'),
// makeLocal('local.Project', 'operator3', 'Standard.Base.Text'),
// ])
const input = useComponentBrowserInput()
const operator1Id: ExprId = '3d0e9b96-3ca0-4c35-a820-7d3a1649de55' as ExprId
const operator2Id: ExprId = '5eb16101-dd2b-4034-a6e2-476e8bfa1f2b' as ExprId
const graphStoreMock = {
identDefinitions: new Map([
['operator1', operator1Id],
['operator2', operator2Id],
]),
}
const computedValueRegistryMock = {
getExpressionInfo(id: ExprId) {
if (id === operator1Id)
return {
typename: 'Standard.Base.Number',
methodCall: undefined,
payload: { type: 'Value' },
profilingInfo: [],
}
},
}
const input = useComponentBrowserInput(graphStoreMock, computedValueRegistryMock)
input.code.value = code
input.selection.value = { start: cursorPos, end: cursorPos }
const context = input.context.value
@ -99,7 +133,7 @@ test.each([
}
expect(filter.pattern).toStrictEqual(expFiltering.pattern)
expect(filter.qualifiedNamePattern).toStrictEqual(expFiltering.qualifiedNamePattern)
expect(filter.selfType).toStrictEqual(expFiltering.selfType)
expect(filter.selfArg).toStrictEqual(expFiltering.selfArg)
},
)
@ -177,6 +211,11 @@ const baseCases: ApplySuggestionCase[] = [
suggestion: makeModule('local.Project.Module'),
expected: 'Project.Module.',
},
{
code: 'a -> a.',
suggestion: makeMethod('Standard.Base.Data.Vector.get'),
expected: 'a -> a.get ',
},
]
function makeComplexCase(prefix: string, suffix: string): ApplySuggestionCase[] {
@ -217,7 +256,10 @@ test.each([
({ code, cursorPos, suggestion, expected, expectedCursorPos }) => {
cursorPos = cursorPos ?? code.length
expectedCursorPos = expectedCursorPos ?? expected.length
const input = useComponentBrowserInput()
const input = useComponentBrowserInput(
{ identDefinitions: new Map() },
{ getExpressionInfo: (_id) => undefined },
)
input.code.value = code
input.selection.value = { start: cursorPos, end: cursorPos }
input.applySuggestion(suggestion)

35
app/gui2/src/components/ComponentBrowser/filtering.ts
View File

@ -1,11 +1,22 @@
import { SuggestionKind, type SuggestionEntry } from '@/stores/suggestionDatabase/entry'
import {
SuggestionKind,
type SuggestionEntry,
type Typename,
} from '@/stores/suggestionDatabase/entry'
import type { Opt } from '@/util/opt'
import { qnIsTopElement, qnParent, type QualifiedName } from '@/util/qualifiedName'
import { Range } from '@/util/range'
export type SelfArg =
| {
type: 'known'
typename: Typename
}
| { type: 'unknown' }
export interface Filter {
pattern?: string
selfType?: QualifiedName
selfArg?: SelfArg
qualifiedNamePattern?: string
showUnstable?: boolean
showLocal?: boolean
@ -228,8 +239,8 @@ class FilteringQualifiedName {
*
* - The private entries never matches.
*
* - If `selfType` is specified, only entries of methods taking a value of this type as self
* argument are accepted. Static methods, and methods of other types are filtered out.
* - If `selfArg` is specified, only entries of methods taking a value of this type as self
* argument are accepted (or any non-static method if the type of self argument is unknown).
*
* - If `qualifiedNamePattern` is specified, only entries being a content of a module or type
* matching the pattern are accepted. If `pattern` is also specified (see below), the content
@ -259,7 +270,7 @@ class FilteringQualifiedName {
*/
export class Filtering {
pattern?: FilteringWithPattern
selfType?: QualifiedName | undefined
selfArg?: SelfArg
qualifiedName?: FilteringQualifiedName
fullPattern: string | undefined
/** The first and last match are the parts of the string that are outside of the match.
@ -275,11 +286,11 @@ export class Filtering {
currentModule?: QualifiedName
constructor(filter: Filter, currentModule: Opt<QualifiedName> = undefined) {
const { pattern, selfType, qualifiedNamePattern, showUnstable, showLocal } = filter
const { pattern, selfArg, qualifiedNamePattern, showUnstable, showLocal } = filter
if (pattern) {
this.pattern = new FilteringWithPattern(pattern)
}
this.selfType = selfType
if (selfArg != null) this.selfArg = selfArg
if (qualifiedNamePattern) {
this.qualifiedName = new FilteringQualifiedName(qualifiedNamePattern)
this.fullPattern = pattern ? `${qualifiedNamePattern}.${pattern}` : qualifiedNamePattern
@ -303,11 +314,9 @@ export class Filtering {
}
private selfTypeMatches(entry: SuggestionEntry): boolean {
if (this.selfType == null) {
return entry.selfType == null
} else {
return entry.selfType === this.selfType
}
if (this.selfArg == null) return entry.selfType == null
else if (this.selfArg.type == 'known') return entry.selfType === this.selfArg.typename
else return entry.selfType != null
}
private qualifiedNameMatches(entry: SuggestionEntry): MatchedParts | null {
@ -317,7 +326,7 @@ export class Filtering {
isMainView() {
return (
this.pattern == null && this.selfType == null && this.qualifiedName == null && !this.showLocal
this.pattern == null && this.selfArg == null && this.qualifiedName == null && !this.showLocal
)
}

66
app/gui2/src/components/ComponentBrowser/input.ts
View File

@ -1,5 +1,11 @@
import type { Filter } from '@/components/ComponentBrowser/filtering'
import { SuggestionKind, type SuggestionEntry } from '@/stores/suggestionDatabase/entry'
import { useGraphStore } from '@/stores/graph'
import { useProjectStore } from '@/stores/project'
import {
SuggestionKind,
type SuggestionEntry,
type Typename,
} from '@/stores/suggestionDatabase/entry'
import {
Ast,
astContainingChar,
@ -8,15 +14,18 @@ import {
readAstSpan,
readTokenSpan,
} from '@/util/ast'
import { AliasAnalyzer } from '@/util/ast/aliasAnalysis'
import { GeneralOprApp } from '@/util/ast/opr'
import type { ExpressionInfo } from '@/util/computedValueRegistry'
import { MappedSet } from '@/util/containers'
import {
qnLastSegment,
qnParent,
qnSplit,
tryIdentifier,
tryQualifiedName,
type QualifiedName,
} from '@/util/qualifiedName'
import { IdMap, type ExprId } from 'shared/yjsModel'
import { computed, ref, type ComputedRef } from 'vue'
/** Input's editing context.
@ -44,16 +53,21 @@ export type EditingContext =
| { type: 'changeLiteral'; literal: Ast.Tree.TextLiteral | Ast.Tree.Number }
/** Component Browser Input Data */
export function useComponentBrowserInput() {
export function useComponentBrowserInput(
graphStore: { identDefinitions: Map<string, ExprId> } = useGraphStore(),
computedValueRegistry: {
getExpressionInfo(id: ExprId): ExpressionInfo | undefined
} = useProjectStore().computedValueRegistry,
) {
const code = ref('')
const selection = ref({ start: 0, end: 0 })
const ast = computed(() => parseEnso(code.value))
const context: ComputedRef<EditingContext> = computed(() => {
const input = code.value
const cursorPosition = selection.value.start
if (cursorPosition === 0) return { type: 'insert', position: 0 }
const editedPart = cursorPosition - 1
const inputAst = parseEnso(input)
const inputAst = ast.value
const editedAst = astContainingChar(editedPart, inputAst).values()
const leaf = editedAst.next()
if (leaf.done) return { type: 'insert', position: cursorPosition }
@ -76,6 +90,17 @@ export function useComponentBrowserInput() {
}
})
const internalUsages = computed(() => {
const analyzer = new AliasAnalyzer(code.value, ast.value)
analyzer.process()
function* internalUsages() {
for (const [_definition, usages] of analyzer.aliases) {
yield* usages
}
}
return new MappedSet(IdMap.keyForRange, internalUsages())
})
// Filter deduced from the access (`.` operator) chain written by user.
const accessChainFilter: ComputedRef<Filter> = computed(() => {
const ctx = context.value
@ -84,9 +109,9 @@ export function useComponentBrowserInput() {
const opr = ctx.oprApp.lastOpr()
const input = code.value
if (opr == null || !opr.ok || readTokenSpan(opr.value, input) !== '.') return {}
const selfType = pathAsSelfType(ctx.oprApp, input)
if (selfType != null) return { selfType }
const qn = pathAsQualifiedName(ctx.oprApp, input)
const selfArg = pathAsSelfArgument(ctx.oprApp)
if (selfArg != null) return { selfArg: selfArg }
const qn = pathAsQualifiedName(ctx.oprApp)
if (qn != null) return { qualifiedNamePattern: qn }
return {}
})
@ -132,18 +157,23 @@ export function useComponentBrowserInput() {
}
}
function pathAsSelfType(accessOpr: GeneralOprApp, inputCode: string): QualifiedName | null {
function pathAsSelfArgument(
accessOpr: GeneralOprApp,
): { type: 'known'; typename: Typename } | { type: 'unknown' } | null {
if (accessOpr.lhs == null) return null
if (accessOpr.lhs.type !== Ast.Tree.Type.Ident) return null
if (accessOpr.apps.length > 1) return null
const _ident = tryIdentifier(readAstSpan(accessOpr.lhs, inputCode))
// TODO[ao]: #7926 add implementation here
return null
if (internalUsages.value.has(parsedTreeRange(accessOpr.lhs))) return { type: 'unknown' }
const ident = readAstSpan(accessOpr.lhs, code.value)
const definition = graphStore.identDefinitions.get(ident)
if (definition == null) return null
const typename = computedValueRegistry.getExpressionInfo(definition)?.typename
return typename != null ? { type: 'known', typename } : { type: 'unknown' }
}
function pathAsQualifiedName(accessOpr: GeneralOprApp, inputCode: string): QualifiedName | null {
const operandsAsIdents = qnIdentifiers(accessOpr, inputCode)
const segments = operandsAsIdents.map((ident) => readAstSpan(ident, inputCode))
function pathAsQualifiedName(accessOpr: GeneralOprApp): QualifiedName | null {
const operandsAsIdents = qnIdentifiers(accessOpr)
const segments = operandsAsIdents.map((ident) => readAstSpan(ident, code.value))
const rawQn = segments.join('.')
const qn = tryQualifiedName(rawQn)
return qn.ok ? qn.value : null
@ -156,9 +186,9 @@ export function useComponentBrowserInput() {
* @param code The code from which `opr` was generated.
* @returns If all path segments are identifiers, return them
*/
function qnIdentifiers(opr: GeneralOprApp, inputCode: string): Ast.Tree.Ident[] {
function qnIdentifiers(opr: GeneralOprApp): Ast.Tree.Ident[] {
const operandsAsIdents = Array.from(
opr.operandsOfLeftAssocOprChain(inputCode, '.'),
opr.operandsOfLeftAssocOprChain(code.value, '.'),
(operand) =>
operand?.type === 'ast' && operand.ast.type === Ast.Tree.Type.Ident ? operand.ast : null,
).slice(0, -1)
@ -255,7 +285,7 @@ export function useComponentBrowserInput() {
if (entry.kind === SuggestionKind.Local || entry.kind === SuggestionKind.Function) return []
if (context.value.type === 'changeLiteral') return []
if (context.value.oprApp == null) return []
const writtenQn = qnIdentifiers(context.value.oprApp, code.value).reverse()
const writtenQn = qnIdentifiers(context.value.oprApp).reverse()
let containingQn =
entry.kind === SuggestionKind.Module

31
app/gui2/src/util/containers.ts
View File

@ -1,11 +1,12 @@
import type { NonEmptyArray } from '@/util/array.ts'
import { assertDefined, assertEqual } from '@/util/assert'
import { expect, test } from 'vitest'
import { mapIterator } from 'lib0/iterator'
/**
* Map that supports Object-based keys.
*
* Internally keys are converted to strings using the provided {@link keyMapper} function and then compared.
* Internally keys are converted to strings using the provided {@link keyMapper} function and
* then compared.
*
* @template Key The type of the keys.
* @template Value The type of the values.
@ -16,9 +17,9 @@ export class MappedKeyMap<Key, Value> {
/** Construct a new map with a custom key mapper.
*
* @param keyMapper The function that maps the user-facing keys to internal keys. It can be some sort of hash function or custom to-string converter.
* The function should return values that are `===`-equal for keys that should be considered equal.
*
* @param keyMapper The function that maps the user-facing keys to internal keys. It can be some
* sort of hash function or custom to-string converter. The function should return values that
* are `===`-equal for keys that should be considered equal.
*/
constructor(private readonly keyMapper: (key: Key) => any) {}
@ -67,7 +68,8 @@ export class MappedKeyMap<Key, Value> {
/**
* Set that uses a provided function to map the values to keys.
*
* It is useful e.g. when the values are objects, and we want to use different equality semantics than the default.
* It is useful e.g. when the values are objects, and we want to use different equality semantics
* than the default.
*
* @template T The type of the values.
*/
@ -76,11 +78,18 @@ export class MappedSet<T extends Object> {
private readonly set = new Map<any, T>()
/** Construct a new set, optionally setting a custom value mapper.
* @param valueMapper The function that maps the keys to strings. It can be some sort of a hash function or custom to-string converter.
* The function should return values that are `===`-equal for values that should be considered equal.
*
* @param valueMapper The function that maps the user-facing values to internal keys. It can be
* some sort of hash function or custom to-string converter. The function should return values
* that are `===`-equal for keys that should be considered equal.
*/
constructor(private readonly valueMapper: (key: T) => any) {}
constructor(
private readonly valueMapper: (key: T) => any,
elements: Iterable<T> = [],
) {
this.set = new Map(
mapIterator(elements[Symbol.iterator](), (elem) => [valueMapper(elem), elem]),
)
}
/** Add the given value to the set. */
add(value: T): this {
@ -167,6 +176,8 @@ export class NonEmptyStack<T> {
}
if (import.meta.vitest) {
const { test, expect } = import.meta.vitest
test('MyMap with number[] keys', () => {
const map = new MappedKeyMap((key: number[]) => key.join(','))

15
package-lock.json generated
View File

@ -36,6 +36,7 @@
"@lezer/highlight": "^1.1.6",
"@noble/hashes": "^1.3.2",
"@open-rpc/client-js": "^1.8.1",
"@pinia/testing": "^0.1.3",
"@vueuse/core": "^10.4.1",
"codemirror": "^6.0.1",
"culori": "^3.2.0",
@ -3127,6 +3128,20 @@
}
}
},
"node_modules/@pinia/testing": {
"version": "0.1.3",
"resolved": "https://registry.npmjs.org/@pinia/testing/-/testing-0.1.3.tgz",
"integrity": "sha512-D2Ds2s69kKFaRf2KCcP1NhNZEg5+we59aRyQalwRm7ygWfLM25nDH66267U3hNvRUOTx8ofL24GzodZkOmB5xw==",
"dependencies": {
"vue-demi": ">=0.14.5"
},
"funding": {
"url": "https://github.com/sponsors/posva"
},
"peerDependencies": {
"pinia": ">=2.1.5"
}
},
"node_modules/@pkgr/utils": {
"version": "2.4.2",
"dev": true,