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
fivetran · Apr 11, 2022 · 4036dcb · 4036dcb
1 parent 3342d91
commit 4036dcb
Show file tree

Hide file tree

Showing 6 changed files with 112 additions and 39 deletions.
diff --git 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.SignatureHelp>(
       '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<Hover[]>('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
@@ -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<CompletionItem[] | undefined> {
@@ -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
@@ -1,24 +1,39 @@
 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) {
       const { outputColumn } = hoverInfo;
       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
@@ -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<string, SignatureInformation[]>();
-
-  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
@@ -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
@@ -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);
+}