From 4036dcb15d503a373e9a27e039013ed2917a7447 Mon Sep 17 00:00:00 2001 From: Pavel Grivachev Date: Mon, 11 Apr 2022 20:56:14 +0300 Subject: [PATCH] fix signature hints, show signature on hover (#99) * show signature help for nested functions and for whole signature * show signature on hover * add e2e test * remove export * use getWordRangeAtPosition to find word * fix typo --- e2e/src/functions.spec.ts | 26 ++++++++++++++++---- server/src/DbtTextDocument.ts | 28 ++++------------------ server/src/HoverProvider.ts | 27 ++++++++++++++++----- server/src/SignatureHelpProvider.ts | 6 ++--- server/src/test/utils/TextUtils.spec.ts | 32 ++++++++++++++++++++++++- server/src/utils/TextUtils.ts | 32 +++++++++++++++++++++++++ 6 files changed, 112 insertions(+), 39 deletions(-) diff --git a/e2e/src/functions.spec.ts b/e2e/src/functions.spec.ts index 55330297..6ecca8a7 100644 --- a/e2e/src/functions.spec.ts +++ b/e2e/src/functions.spec.ts @@ -1,20 +1,21 @@ -import { assertThat, instanceOf } from 'hamjest'; +import { assertThat, hasSize, instanceOf, startsWith } from 'hamjest'; import * as vscode from 'vscode'; +import { Hover } from 'vscode'; import { activateAndWait, getCursorPosition, getDocUri, setTestContent, sleep } from './helper'; suite('Functions', () => { - const docUri = getDocUri('functions.sql'); + const DOC_URI = getDocUri('functions.sql'); test('Should show help for max function', async () => { // arrange - await activateAndWait(docUri); + await activateAndWait(DOC_URI); await setTestContent('select max('); // act const help = await vscode.commands.executeCommand( 'vscode.executeSignatureHelpProvider', - docUri, + DOC_URI, new vscode.Position(0, 11), '(', ); @@ -31,7 +32,7 @@ suite('Functions', () => { test('Should move cursor into brackets after avg function completion', async () => { // arrange - await activateAndWait(docUri); + await activateAndWait(DOC_URI); await setTestContent('select avg()'); @@ -42,4 +43,19 @@ suite('Functions', () => { await sleep(300); assertThat(getCursorPosition(), new vscode.Position(0, 11)); }); + + test('Should show signature on hover', async () => { + // arrange + await activateAndWait(DOC_URI); + + await setTestContent('select coalesce'); + + // act + const hovers = await vscode.commands.executeCommand('vscode.executeHoverProvider', DOC_URI, new vscode.Position(0, 8)); + + // assert + assertThat(hovers, hasSize(1)); + assertThat(hovers[0].contents, hasSize(1)); + assertThat((hovers[0].contents[0] as vscode.MarkdownString).value, startsWith(`\`\`\`sql\nCOALESCE(expr[, ...])\n\`\`\``)); + }); }); diff --git a/server/src/DbtTextDocument.ts b/server/src/DbtTextDocument.ts index 3b52d2d1..1119d816 100644 --- a/server/src/DbtTextDocument.ts +++ b/server/src/DbtTextDocument.ts @@ -35,6 +35,7 @@ import { ModelCompiler } from './ModelCompiler'; import { ProgressReporter } from './ProgressReporter'; import { SignatureHelpProvider } from './SignatureHelpProvider'; import { SqlRefConverter } from './SqlRefConverter'; +import { getTextRangeBeforeBracket } from './utils/TextUtils'; import { debounce, getIdentifierRangeAtPosition, getJinjaContentOffset, positionInRange } from './utils/Utils'; import { ZetaSqlAst } from './ZetaSqlAst'; @@ -51,6 +52,7 @@ export class DbtTextDocument { signatureHelpProvider = new SignatureHelpProvider(); sqlRefConverter = new SqlRefConverter(this.jinjaParser); diagnosticGenerator = new DiagnosticGenerator(); + hoverProvider = new HoverProvider(); hasDbtError = false; firstSave = true; @@ -301,7 +303,7 @@ export class DbtTextDocument { onHover(hoverParams: HoverParams): Hover | null { const range = getIdentifierRangeAtPosition(hoverParams.position, this.rawDocument.getText()); const text = this.rawDocument.getText(range); - return HoverProvider.hoverOnText(text, this.ast); + return this.hoverProvider.hoverOnText(text, this.ast); } async onCompletion(completionParams: CompletionParams, destinationDefinition: DestinationDefinition): Promise { @@ -331,8 +333,8 @@ export class DbtTextDocument { } onSignatureHelp(params: SignatureHelpParams): SignatureHelp | undefined { - const text = this.rawDocument.getText(this.getTextRangeBeforeBracket(params.position)); - return this.signatureHelpProvider.onSignatureHelp(params, text); + const text = this.rawDocument.getText(getTextRangeBeforeBracket(this.rawDocument.getText(), params.position)); + return this.signatureHelpProvider.onSignatureHelp(text); } onDefinition(definitionParams: DefinitionParams): DefinitionLink[] | undefined { @@ -345,24 +347,4 @@ export class DbtTextDocument { } return undefined; } - - getTextRangeBeforeBracket(cursorPosition: Position): Range { - const lines = this.rawDocument.getText().split('\n'); - if (lines.length === 0) { - return Range.create(cursorPosition, cursorPosition); - } - const line = Math.min(lines.length - 1, Math.max(0, cursorPosition.line)); - const lineText = lines[line]; - const textBeforeCursor = lineText.substr(0, cursorPosition.character); - const openBracketIndex = textBeforeCursor.lastIndexOf('('); - if (openBracketIndex === -1) { - return Range.create(cursorPosition, cursorPosition); - } - const closeBracketIndex = textBeforeCursor.lastIndexOf(')'); - if (closeBracketIndex > openBracketIndex) { - return Range.create(cursorPosition, cursorPosition); - } - const spaceIndex = textBeforeCursor.substr(0, openBracketIndex).lastIndexOf(' '); - return Range.create(line, spaceIndex + 1, line, openBracketIndex); - } } diff --git a/server/src/HoverProvider.ts b/server/src/HoverProvider.ts index 3fe01006..44bef971 100644 --- a/server/src/HoverProvider.ts +++ b/server/src/HoverProvider.ts @@ -1,16 +1,31 @@ import { SimpleType, TypeKind } from '@fivetrandevelopers/zetasql'; import { AnalyzeResponse } from '@fivetrandevelopers/zetasql/lib/types/zetasql/local_service/AnalyzeResponse'; -import { Hover } from 'vscode-languageserver'; +import { Hover, MarkupKind } from 'vscode-languageserver'; +import { HelpProviderWords } from './HelpProviderWords'; +import { SignatureHelpProvider } from './SignatureHelpProvider'; import { ZetaSqlAst } from './ZetaSqlAst'; export class HoverProvider { - static zetaSqlAst = new ZetaSqlAst(); + static ZETA_SQL_AST = new ZetaSqlAst(); + + signatureHelpProvider = new SignatureHelpProvider(); + + hoverOnText(text: string, ast: AnalyzeResponse | undefined): Hover | null { + const index = HelpProviderWords.findIndex(w => w.name === text); + if (index !== -1) { + const [firstSignature] = HelpProviderWords[index].signatures; + return { + contents: { + kind: MarkupKind.Markdown, + value: [`\`\`\`sql\n${firstSignature.signature}\`\`\``, firstSignature.description].join('\n---\n'), + }, + }; + } - static hoverOnText(text: string, ast: AnalyzeResponse | undefined): Hover | null { if (!ast) { return null; } - const hoverInfo = HoverProvider.zetaSqlAst.getHoverInfo(ast, text); + const hoverInfo = HoverProvider.ZETA_SQL_AST.getHoverInfo(ast, text); let hint; if (hoverInfo.outputColumn) { @@ -18,7 +33,7 @@ export class HoverProvider { if (outputColumn.column?.tableName === '$query' || outputColumn.column?.name !== outputColumn.name) { hint = `Alias: ${outputColumn.name}`; } else if (outputColumn.name) { - hint = HoverProvider.getColumnHint(outputColumn.column?.tableName, outputColumn.name, outputColumn.column?.type?.typeKind as TypeKind); + hint = this.getColumnHint(outputColumn.column?.tableName, outputColumn.name, outputColumn.column?.type?.typeKind as TypeKind); } } else if (hoverInfo.withQueryName) { hint = `Temporary table introduced in a WITH clause: ${hoverInfo.withQueryName}`; @@ -38,7 +53,7 @@ export class HoverProvider { : null; } - static getColumnHint(tableName?: string, columnName?: string, columnTypeKind?: TypeKind): string { + getColumnHint(tableName?: string, columnName?: string, columnTypeKind?: TypeKind): string { const type = columnTypeKind ? new SimpleType(columnTypeKind).getTypeName() : 'unknown'; return `Table: ${tableName}\nColumn: ${columnName}\nType: ${type}`; } diff --git a/server/src/SignatureHelpProvider.ts b/server/src/SignatureHelpProvider.ts index 42e3ad31..ef340e71 100644 --- a/server/src/SignatureHelpProvider.ts +++ b/server/src/SignatureHelpProvider.ts @@ -1,4 +1,4 @@ -import { MarkupKind, SignatureHelp, SignatureHelpParams, SignatureInformation } from 'vscode-languageserver'; +import { MarkupKind, SignatureHelp, SignatureInformation } from 'vscode-languageserver'; import { HelpProviderWords } from './HelpProviderWords'; export interface FunctionInfo { @@ -12,9 +12,7 @@ export interface SignatureInfo { } export class SignatureHelpProvider { - signatureInformations = new Map(); - - onSignatureHelp(params: SignatureHelpParams, text: string): SignatureHelp | undefined { + onSignatureHelp(text: string): SignatureHelp | undefined { const index = HelpProviderWords.findIndex(w => w.name === text); if (index !== -1) { return { diff --git a/server/src/test/utils/TextUtils.spec.ts b/server/src/test/utils/TextUtils.spec.ts index b474c780..0e3f0f87 100644 --- a/server/src/test/utils/TextUtils.spec.ts +++ b/server/src/test/utils/TextUtils.spec.ts @@ -3,7 +3,7 @@ import { Position, Range } from 'vscode-languageserver'; import { MacroDefinitionFinder } from '../../definition/MacroDefinitionFinder'; import { RefDefinitionFinder } from '../../definition/RefDefinitionFinder'; import { SourceDefinitionFinder } from '../../definition/SourceDefinitionFinder'; -import { getWordRangeAtPosition } from '../../utils/TextUtils'; +import { getTextRangeBeforeBracket, getWordRangeAtPosition } from '../../utils/TextUtils'; describe('TextUtils', () => { it('getWordRangeAtPosition should find words', () => { @@ -77,6 +77,36 @@ describe('TextUtils', () => { ); }); + it('getTextRangeBeforeBracket should return cursor position if no range found', function () { + getTextRangeBeforeBracket_shouldReturnRange('a()', Position.create(0, 0), Range.create(0, 0, 0, 0)); + getTextRangeBeforeBracket_shouldReturnRange('a()', Position.create(0, 1), Range.create(0, 1, 0, 1)); + getTextRangeBeforeBracket_shouldReturnRange('a()', Position.create(0, 3), Range.create(0, 3, 0, 3)); + getTextRangeBeforeBracket_shouldReturnRange('a(b)', Position.create(0, 4), Range.create(0, 4, 0, 4)); + getTextRangeBeforeBracket_shouldReturnRange('a)(', Position.create(0, 1), Range.create(0, 1, 0, 1)); + getTextRangeBeforeBracket_shouldReturnRange('a)(', Position.create(0, 2), Range.create(0, 2, 0, 2)); + }); + + it('getTextRangeBeforeBracket should return range', function () { + getTextRangeBeforeBracket_shouldReturnRange('a()', Position.create(0, 2), Range.create(0, 0, 0, 1)); + getTextRangeBeforeBracket_shouldReturnRange(' a()', Position.create(0, 4), Range.create(0, 2, 0, 3)); + getTextRangeBeforeBracket_shouldReturnRange('a(b)', Position.create(0, 3), Range.create(0, 0, 0, 1)); + getTextRangeBeforeBracket_shouldReturnRange('a(b())', Position.create(0, 3), Range.create(0, 0, 0, 1)); + getTextRangeBeforeBracket_shouldReturnRange('a(b())', Position.create(0, 5), Range.create(0, 0, 0, 1)); + getTextRangeBeforeBracket_shouldReturnRange('a( b())', Position.create(0, 5), Range.create(0, 0, 0, 1)); + getTextRangeBeforeBracket_shouldReturnRange('a(b())', Position.create(0, 4), Range.create(0, 2, 0, 3)); + getTextRangeBeforeBracket_shouldReturnRange('a(b(c()))', Position.create(0, 6), Range.create(0, 4, 0, 5)); + getTextRangeBeforeBracket_shouldReturnRange(' coalesce(max())', Position.create(0, 13), Range.create(0, 1, 0, 9)); + getTextRangeBeforeBracket_shouldReturnRange(' coalesce(max())', Position.create(0, 14), Range.create(0, 10, 0, 13)); + }); + + function getTextRangeBeforeBracket_shouldReturnRange(text: string, position: Position, expectedRange: Range): void { + // act + const range = getTextRangeBeforeBracket(text, position); + + // assert + assertThat(range, expectedRange); + } + function assertWordRangeAtPosition(position: Position, regex: RegExp, textLines: string[], wordRange: Range | undefined): void { const range = getWordRangeAtPosition(position, regex, textLines); assertThat(range, wordRange); diff --git a/server/src/utils/TextUtils.ts b/server/src/utils/TextUtils.ts index fd22c533..b48901a8 100644 --- a/server/src/utils/TextUtils.ts +++ b/server/src/utils/TextUtils.ts @@ -202,3 +202,35 @@ function createWordRegExp(allowInWords = ''): RegExp { source += '\\s]+)'; return new RegExp(source, 'g'); } + +export function getTextRangeBeforeBracket(text: string, cursorPosition: Position): Range { + const lines = text.split('\n'); + if (lines.length === 0) { + return Range.create(cursorPosition, cursorPosition); + } + const line = Math.min(lines.length - 1, Math.max(0, cursorPosition.line)); + const lineText = lines[line]; + const textBeforeCursor = lineText.substring(0, cursorPosition.character); + let openBracketIndex = -1; + let closedBracketCount = 0; + let index = textBeforeCursor.length - 1; + while (index > 0) { + const char = textBeforeCursor.charAt(index); + if (char === ')') { + closedBracketCount++; + } else if (char === '(') { + if (closedBracketCount === 0) { + openBracketIndex = index; + break; + } else { + closedBracketCount--; + } + } + index--; + } + if (openBracketIndex === -1) { + return Range.create(cursorPosition, cursorPosition); + } + + return getWordRangeAtPosition(Position.create(0, openBracketIndex), /\w+/, [textBeforeCursor]) ?? Range.create(cursorPosition, cursorPosition); +}