-
Notifications
You must be signed in to change notification settings - Fork 108
fix: #436 improve SQL keyword suggestions #485
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
base: next
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | |||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -26,6 +26,7 @@ import { | ||||||||||
| CaretPosition, | |||||||||||
| LOCALE_TYPE, | |||||||||||
| SemanticCollectOptions, | |||||||||||
| SuggestionOptions, | |||||||||||
| Suggestions, | |||||||||||
| SyntaxSuggestion, | |||||||||||
| } from './types'; | |||||||||||
|
|
@@ -48,6 +49,7 @@ export abstract class BasicSQL< | ||||||||||
| protected _parseTree: PRC | null; | |||||||||||
| protected _parsedInput: string; | |||||||||||
| protected _parseErrors: ParseError[] = []; | |||||||||||
| private _statementStartTokenTypes: Set<number> | null = null; | |||||||||||
| /** members for cache end */ | |||||||||||
|
|
|||||||||||
| private _errorListener: ErrorListener = (error) => { | |||||||||||
|
|
@@ -555,15 +557,98 @@ export abstract class BasicSQL< | ||||||||||
| }; | |||||||||||
| } | |||||||||||
|
|
|||||||||||
| /** | |||||||||||
| * Get the minimum statement tree for collecting completion candidates | |||||||||||
| * Each supported grammar exposes one top-level statement per direct program child | |||||||||||
| * Keep the lookup shallow to avoid selecting nested statements or subqueries | |||||||||||
| */ | |||||||||||
| private getSuggestionParseTree( | |||||||||||
| parseTree: ParserRuleContext, | |||||||||||
| caretTokenIndex: number | |||||||||||
| ): ParserRuleContext { | |||||||||||
| const children = parseTree.children; | |||||||||||
| if (!children?.length) return parseTree; | |||||||||||
|
|
|||||||||||
| for (let index = children.length - 1; index >= 0; index--) { | |||||||||||
| const child = children[index]; | |||||||||||
| if (!(child instanceof ParserRuleContext)) continue; | |||||||||||
|
|
|||||||||||
| const startTokenIndex = child.start?.tokenIndex; | |||||||||||
| const stopTokenIndex = child.stop?.tokenIndex; | |||||||||||
| if ( | |||||||||||
| startTokenIndex === undefined || | |||||||||||
| stopTokenIndex === undefined || | |||||||||||
| startTokenIndex > caretTokenIndex | |||||||||||
| ) | |||||||||||
| continue; | |||||||||||
|
|
|||||||||||
| // Use the current statement tree when the caret is inside it | |||||||||||
| if (stopTokenIndex >= caretTokenIndex) return child; | |||||||||||
|
|
|||||||||||
| // Keep using the current statement until it ends with a semicolon | |||||||||||
| return child.stop?.text === SQL_SPLIT_SYMBOL_TEXT ? parseTree : child; | |||||||||||
| } | |||||||||||
|
|
|||||||||||
| return parseTree; | |||||||||||
| } | |||||||||||
|
|
|||||||||||
| /** | |||||||||||
| * Collect candidates for the current statement and remove new-statement-only keywords | |||||||||||
| */ | |||||||||||
| private collectSuggestionCandidates( | |||||||||||
| parser: Parser, | |||||||||||
| parseTree: ParserRuleContext, | |||||||||||
| caretTokenIndex: number | |||||||||||
| ): CandidatesCollection { | |||||||||||
| const core = new CodeCompletionCore(parser); | |||||||||||
| core.preferredRules = this.preferredRules; | |||||||||||
| const candidates = core.collectCandidates(caretTokenIndex, parseTree); | |||||||||||
| const suggestionParseTree = this.getSuggestionParseTree(parseTree, caretTokenIndex); | |||||||||||
|
|
|||||||||||
| if (suggestionParseTree === parseTree) return candidates; | |||||||||||
|
|
|||||||||||
| // Keep the program candidates for outer rule paths and use statement candidates for isolation | |||||||||||
| const statementCore = new CodeCompletionCore(parser); | |||||||||||
| statementCore.preferredRules = this.preferredRules; | |||||||||||
| const statementCandidates = statementCore.collectCandidates( | |||||||||||
| caretTokenIndex, | |||||||||||
| suggestionParseTree | |||||||||||
| ); | |||||||||||
|
|
|||||||||||
| if (this._statementStartTokenTypes === null) { | |||||||||||
| const statementStartCore = new CodeCompletionCore(parser); | |||||||||||
| statementStartCore.preferredRules = this.preferredRules; | |||||||||||
| const statementStartCandidates = statementStartCore.collectCandidates(0, parseTree); | |||||||||||
| this._statementStartTokenTypes = new Set(statementStartCandidates.tokens.keys()); | |||||||||||
| } | |||||||||||
|
|
|||||||||||
| const tokens = new Map(candidates.tokens); | |||||||||||
| for (const tokenType of this._statementStartTokenTypes) { | |||||||||||
| if (!statementCandidates.tokens.has(tokenType)) { | |||||||||||
| tokens.delete(tokenType); | |||||||||||
| } else if (tokens.has(tokenType)) { | |||||||||||
| // Use the current statement follow-list to preserve valid combined keywords | |||||||||||
| tokens.set(tokenType, statementCandidates.tokens.get(tokenType)!); | |||||||||||
| } | |||||||||||
| } | |||||||||||
|
|
|||||||||||
| return { | |||||||||||
| rules: candidates.rules, | |||||||||||
| tokens, | |||||||||||
| }; | |||||||||||
| } | |||||||||||
|
|
|||||||||||
| /** | |||||||||||
| * Get suggestions of syntax and token at caretPosition | |||||||||||
| * @param input source string | |||||||||||
| * @param caretPosition caret position, such as cursor position | |||||||||||
| * @param options suggestion options | |||||||||||
| * @returns suggestion | |||||||||||
| */ | |||||||||||
| public getSuggestionAtCaretPosition( | |||||||||||
| input: string, | |||||||||||
| caretPosition: CaretPosition | |||||||||||
| caretPosition: CaretPosition, | |||||||||||
| options?: SuggestionOptions | |||||||||||
| ): Suggestions | null { | |||||||||||
| this.parseWithCache(input); | |||||||||||
| if (!this._parseTree) return null; | |||||||||||
|
|
@@ -614,12 +699,11 @@ export abstract class BasicSQL< | ||||||||||
| parseTree = sqlParserIns.program(); | |||||||||||
| } | |||||||||||
|
|
|||||||||||
| const core = new CodeCompletionCore(sqlParserIns); | |||||||||||
| core.preferredRules = this.preferredRules; | |||||||||||
| // core.showRuleStack = true; | |||||||||||
| // core.showResult = true; | |||||||||||
|
|
|||||||||||
| const candidates = core.collectCandidates(caretTokenIndex, parseTree); | |||||||||||
| const candidates = this.collectSuggestionCandidates( | |||||||||||
| sqlParserIns, | |||||||||||
| parseTree, | |||||||||||
| caretTokenIndex | |||||||||||
| ); | |||||||||||
|
Comment on lines
+702
to
+706
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. collectSuggestionCandidates 在最坏情况下会跑 3 次 collectCandidates(整棵树 + 语句树 + 仅在首次缓存的语句起始 token 集)。常见未切片场景是 2 次。对大 SQL 输入这是可感知的额外开销。建议补充一个针对大输入的 benchmark,确认回归在可接受范围。
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 三次
调用次数:首次最多 3 次,后续 2 次,分号后或无需隔离 1 次。 不能砍掉 statement 那次,否则退化回 Issue 436 之前的错误(复核确认会丢 MySQL 性能 A/B 对比(热启动 50 次取中位数):
额外成本就是一次等量 statement C3,毫秒级,误报的 SELECT/INSERT/CREATE 全部消失(93→28),可接受。 benchmark 方面: 结论:接受首次 3 次、后续 2 次的实现。 |
|||||||||||
| const originalSuggestions = this.processCandidates(candidates, allTokens, caretTokenIndex); | |||||||||||
|
|
|||||||||||
| const syntaxSuggestions: SyntaxSuggestion<WordRange>[] = originalSuggestions.syntax.map( | |||||||||||
|
|
@@ -633,9 +717,14 @@ export abstract class BasicSQL< | ||||||||||
| }; | |||||||||||
| } | |||||||||||
| ); | |||||||||||
| const keywordFilter = options?.keywordFilter; | |||||||||||
| const keywords = keywordFilter | |||||||||||
| ? originalSuggestions.keywords.filter((keyword) => keywordFilter(keyword)) | |||||||||||
| : originalSuggestions.keywords; | |||||||||||
|
|
|||||||||||
| return { | |||||||||||
| syntax: syntaxSuggestions, | |||||||||||
| keywords: originalSuggestions.keywords, | |||||||||||
| keywords, | |||||||||||
| }; | |||||||||||
| } | |||||||||||
|
|
|||||||||||
|
|
|||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
你这里是只向下看一层,它假设 program 的直接子节点就是语句。一旦文法把语句包了一层中间规则(如 program → batch → statement,或 list 规则),收窄会静默回退到整棵树,修复悄悄失效且无报错,确认下这个链路在这种情况下是否有问题?是否需要加一条断言/测试守护该假设?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
已经加了守护测试:
suggestion.test.ts的用例遍历全部方言,断言SELECT * FROM t; SELECT * FROM u解析出的 program 直接子节点恰好是两个 statement。所有方言 grammar 都是
program: (statement SEMI?)*扁平结构,直接子节点即语句;只向下看一层是刻意的,避免误选嵌套语句/子查询(见getSuggestionParseTree上方注释)。若未来引入中间规则,这条测试会立即失败暴露,不会静默回退。