Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Literals and operators in Component Browser #12420

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 1 addition & 11 deletions app/gui/src/project-view/components/ComponentBrowser.vue
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ function applyComponent(component: Opt<Component> = null) {
input.switchToCodeEditMode()
return Ok()
}
if (component.suggestionId) {
if (component.suggestionId != null) {
return input.applySuggestion(component.suggestionId)
} else {
// Component without suggestion database entry, for example "literal" component.
Expand All @@ -297,16 +297,6 @@ function acceptComponent(component: Opt<Component> = null) {
else result.error.log('Cannot apply suggestion')
}

// function editComponent(component: Opt<Component> = null) {
// const result = applyComponentToInput(component)
// if (result.ok) acceptInput()
// else result.error.log('Cannot apply suggestion')
// const suggestionId = component?.suggestionId ?? selectedSuggestionId.value
// if (suggestionId == null) return input.switchToCodeEditMode()
// const result = input.applySuggestion(suggestionId)
// if (!result.ok) result.error.log('Cannot apply suggestion')
// }

function acceptInput() {
const appliedReturnType =
input.mode.mode === 'codeEditing' ?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const MOUSE_SELECTION_DEBOUNCE = 200

const props = defineProps<{
filter: Filter
literal?: Ast.TextLiteral | Ast.NumericLiteral | undefined
literal?: Ast.Ast | undefined
}>()
const emit = defineEmits<{
acceptSuggestion: [suggestion: Component]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { useComponentBrowserInput } from '@/components/ComponentBrowser/input'
import { GraphDb, NodeId } from '@/stores/graph/graphDatabase'
import { ComputedValueRegistry } from '@/stores/project/computedValueRegistry'
import { SuggestionDb } from '@/stores/suggestionDatabase'
import { unwrap } from '@/util/data/result'
import { parseAbsoluteProjectPathRaw } from '@/util/projectPath'
import { expect, test } from 'vitest'
import { assert, assertUnreachable } from 'ydoc-shared/util/assert'
import { Range } from 'ydoc-shared/util/data/range'

const aiMock = { query: assertUnreachable }
const operator1Id = '3d0e9b96-3ca0-4c35-a820-7d3a1649de55' as NodeId
const operator2Id = '5eb16101-dd2b-4034-a6e2-476e8bfa1f2b' as NodeId

function mockGraphDb() {
const computedValueRegistryMock = ComputedValueRegistry.Mock()
computedValueRegistryMock.db.set(operator1Id, {
typename: unwrap(parseAbsoluteProjectPathRaw('Standard.Base.Number')),
rawTypename: 'Standard.Base.Number',
methodCall: undefined,
payload: { type: 'Value' },
profilingInfo: [],
})
const db = GraphDb.Mock(computedValueRegistryMock)
db.mockNode('operator1', operator1Id, 'Data.read')
db.mockNode('operator2', operator2Id)
return db
}

test.each`
inputContent | expectedLiteral
${'read'} | ${undefined}
${'operator1'} | ${undefined}
${'12 + 14'} | ${undefined}
${'12'} | ${'12'}
${'12.6'} | ${'12.6'}
${'"text"'} | ${'"text"'}
${"'text'"} | ${"'text'"}
${"'text"} | ${"'text'"}
${"'''text"} | ${"'''text"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Negative numeric literal case?

`('Reading literal from $inputContent', ({ inputContent, expectedLiteral }) => {
const input = useComponentBrowserInput(mockGraphDb(), new SuggestionDb(), aiMock)
input.reset({ type: 'newNode' })
input.content = { text: inputContent, selection: Range.empty }
assert(input.mode.mode === 'componentBrowsing')
expect(input.mode.literal?.code()).toBe(expectedLiteral)
})

test.each`
inputContent | source | expectedCode
${'read'} | ${'operator1'} | ${'operator1.read'}
${'read'} | ${'operator2'} | ${'operator2.read'}
${'read "file"'} | ${'operator2'} | ${'operator2.read "file"'}
${'+ 2'} | ${'operator1'} | ${'operator1 + 2'}
${'+ 3'} | ${'operator2'} | ${'operator2 + 3'}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some more good cases:

  • Operators without space
  • - without space--to make sure we don't treat that as a negative literal here

`(
'Code generated by CB from $inputContent with source node $sourceNode',
({ inputContent, source, expectedCode }) => {
const db = mockGraphDb()
const input = useComponentBrowserInput(db, new SuggestionDb(), aiMock)
const sourcePort = db.getNodeFirstOutputPort(db.getIdentDefiningNode(source))
assert(sourcePort != null)
input.reset({ type: 'newNode', sourcePort })
input.content = { text: inputContent, selection: Range.empty }
expect(input.code).toBe(expectedCode)
},
)
12 changes: 6 additions & 6 deletions app/gui/src/project-view/components/ComponentBrowser/input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
type SuggestionId,
} from '@/stores/suggestionDatabase/entry'
import { Ast } from '@/util/ast'
import { startsWithIdentifier } from '@/util/ast/abstract'
import { Err, Ok, type Result } from '@/util/data/result'
import { type ProjectPath } from '@/util/projectPath'
import { qnJoin, qnLastSegment } from '@/util/qualifiedName'
Expand All @@ -32,7 +33,7 @@ export type ComponentBrowserMode =
| {
mode: 'componentBrowsing'
filter: Filter
literal?: Ast.TextLiteral | Ast.NumericLiteral
literal?: Ast.TextLiteral | Ast.NumericLiteral | Ast.NegationApp | undefined
}
| {
mode: 'codeEditing'
Expand Down Expand Up @@ -111,10 +112,10 @@ export function useComponentBrowserInput(
: {}),
}
} else {
let literal: Ast.MutableTextLiteral | Ast.NumericLiteral | undefined =
let literal: Ast.MutableTextLiteral | Ast.NumericLiteral | Ast.NegationApp | undefined =
Ast.TextLiteral.tryParse(text.value)
if (literal == null) {
literal = Ast.NumericLiteral.tryParse(text.value)
literal = Ast.NumericLiteral.tryParseWithSign(text.value)
} else {
literal.fixBoundaries()
}
Expand All @@ -124,7 +125,7 @@ export function useComponentBrowserInput(
pattern: text.value,
...(sourceNodeType.value != null ? { selfArg: sourceNodeType.value } : {}),
},
...(literal ? { literal } : {}),
literal,
}
}
})
Expand Down Expand Up @@ -284,8 +285,7 @@ export function useComponentBrowserInput(
function applySourceNode(text: string) {
return (
sourceNodeIdentifier.value ?
/^[a-zA-Z]/.test(text) ?
`${sourceNodeIdentifier.value}.${text}`
startsWithIdentifier(sourceNodeIdentifier.value) ? `${sourceNodeIdentifier.value}.${text}`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
startsWithIdentifier(sourceNodeIdentifier.value) ? `${sourceNodeIdentifier.value}.${text}`
startsWithIdentifier(text) ? `${sourceNodeIdentifier.value}.${text}`

: `${sourceNodeIdentifier.value} ${text}`
: text
)
Expand Down
14 changes: 14 additions & 0 deletions app/gui/src/project-view/util/ast/__tests__/abstract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1092,6 +1092,20 @@ test('setRawTextContent promotes single-line uninterpolated text to interpolated
expect(literal.code()).toBe(`'${escapeTextLiteral(rawText)}'`)
})

test.each`
text | fixed
${"'abc'"} | ${"'abc'"}
${'"abc"'} | ${'"abc"'}
${"'''abc\n cde"} | ${"'''abc\n cde"}
${"'abc"} | ${"'abc'"}
${'"abc'} | ${'"abc"'}
`('Fixing boundaries in $text', ({ text, fixed }) => {
const literal = Ast.TextLiteral.tryParse(text)
assert(literal != null)
literal.fixBoundaries()
expect(literal.code()).toBe(fixed)
})

test.each([
{ code: 'operator1', expected: { subject: 'operator1', accesses: [] } },
{ code: 'operator1 foo bar', expected: { subject: 'operator1 foo bar', accesses: [] } },
Expand Down
40 changes: 25 additions & 15 deletions app/rust-ffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@ pub fn parse_block(code: &str) -> Vec<u8> {
enso_parser::format::serialize(&ast).expect("Failed to serialize AST to binary format")
}

#[wasm_bindgen]
pub fn is_ident_or_operator(code: &str) -> u32 {
fn starts_with_ident_or_operator<const ONLY_ONE_TOKEN_ALLOWED: bool>(code: &str) -> u32 {
let parsed = enso_parser::lexer::run(code);
if parsed.internal_error.is_some() {
return 0;
}
let token = match &parsed.value[..] {
[token] => token,
[token, ..] if !ONLY_ONE_TOKEN_ALLOWED => token,
_ => return 0,
};
match &token.variant {
Expand All @@ -45,21 +45,15 @@ pub fn is_ident_or_operator(code: &str) -> u32 {

#[wasm_bindgen]
pub fn is_ident_or_operator(code: &str) -> u32 {
let parsed = enso_parser::lexer::run(code);
if parsed.internal_error.is_some() {
return 0;
}
let token = match &parsed.value[..] {
[token] => token,
_ => return 0,
};
match &token.variant {
enso_parser::syntax::token::Variant::Ident(_) => 1,
enso_parser::syntax::token::Variant::Operator(_) => 2,
_ => 0,
}
starts_with_ident_or_operator::<true>(code)
}

#[wasm_bindgen]
pub fn is_first_token_ident_or_operator(code: &str) -> u32 {
starts_with_ident_or_operator::<false>(code)
}


#[wasm_bindgen]
pub fn is_numeric_literal(code: &str) -> bool {
let parsed = PARSER.with(|parser| parser.parse_block(code));
Expand Down Expand Up @@ -100,4 +94,20 @@ mod tests {
assert!(!is_numeric_literal("1234!"));
assert!(!is_numeric_literal("1234e5"));
}

#[test]
fn test_checking_ident_or_operator() {
assert_eq!(is_ident_or_operator("abc"), 1);
assert_eq!(is_ident_or_operator("Abc"), 1);
assert_eq!(is_ident_or_operator("abc 14"), 0);
assert_eq!(is_ident_or_operator("+"), 2);
assert_eq!(is_ident_or_operator("+ 2"), 0);
assert_eq!(is_ident_or_operator("[]"), 0);

assert_eq!(is_first_token_ident_or_operator("abc"), 1);
assert_eq!(is_first_token_ident_or_operator("abc 14"), 1);
assert_eq!(is_first_token_ident_or_operator("+"), 2);
assert_eq!(is_first_token_ident_or_operator("+ 2"), 2);
assert_eq!(is_first_token_ident_or_operator("[]"), 0);
}
}
1 change: 1 addition & 0 deletions app/ydoc-server-polyglot/src/ffiPolyglot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

export const {
is_ident_or_operator,
is_first_token_ident_or_operator,
is_numeric_literal,
parse_doc_to_json,
parse_block,
Expand Down
1 change: 1 addition & 0 deletions app/ydoc-server-polyglot/src/polyglot.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@ declare function parse_block(code: string): Uint8Array
declare function parse_module(code: string): Uint8Array
declare function parse_doc_to_json(docs: string): string
declare function is_ident_or_operator(code: string): number
declare function is_first_token_ident_or_operator(code: string): number
declare function is_numeric_literal(code: string): boolean
declare function xxHash128(input: IDataType): string
12 changes: 10 additions & 2 deletions app/ydoc-shared/src/ast/ffi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import { createXXHash128 } from 'hash-wasm'
import type { IDataType } from 'hash-wasm/dist/lib/util'
import {
is_first_token_ident_or_operator,
is_ident_or_operator,
is_numeric_literal,
parse_block,
Expand All @@ -21,5 +22,12 @@ export function xxHash128(input: IDataType) {
return xxHasher128.digest()
}

/* eslint-disable-next-line camelcase */
export { is_ident_or_operator, is_numeric_literal, parse_block, parse_doc_to_json, parse_module }
/* eslint-disable camelcase */
export {
is_first_token_ident_or_operator,
is_ident_or_operator,
is_numeric_literal,
parse_block,
parse_doc_to_json,
parse_module,
}
6 changes: 5 additions & 1 deletion app/ydoc-shared/src/ast/token.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { assert } from '../util/assert'
import type { ExternalId } from '../yjsModel'
import { isUuid } from '../yjsModel'
import { is_ident_or_operator } from './ffi'
import { is_first_token_ident_or_operator, is_ident_or_operator } from './ffi'
import * as RawAst from './generated/ast'
import { newExternalId } from './idMap'
import type { AstId, DeepReadonly, NodeChild, Owned } from './tree'
Expand Down Expand Up @@ -135,6 +135,10 @@ export function isIdentifier(code: string): code is Identifier {
return is_ident_or_operator(code) === 1
}

export function startsWithIdentifier(code: string): boolean {
return is_first_token_ident_or_operator(code) === 1
}

/**
* Whether the given code is a type or constructor identifier.
* This is true if the code is an identifier beginning with an uppercase letter.
Expand Down
8 changes: 6 additions & 2 deletions app/ydoc-shared/src/ast/tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1890,13 +1890,17 @@ export class MutableTextLiteral extends TextLiteral implements MutableExpression

setBoundaries(code: string) {
this.fields.set('open', unspaced(Token.new(code)))
this.fields.set('close', unspaced(Token.new(code)))
if (code.length > 1) {
this.fields.set('close', undefined)
} else {
this.fields.set('close', unspaced(Token.new(code)))
}
}

fixBoundaries() {
const open = this.open
const close = this.close
if (open != null && close == null) {
if (open != null && close == null && open.code().length === 1) {
this.fields.set('close', unspaced(Token.new(open.code())))
} else if (open == null && close != null) {
this.fields.set('open', unspaced(Token.new(close.code())))
Expand Down
Loading