Merge pull request #192 from mads-hartmann/word-at-point

Improve completion handler and wordAtPoint

Author: skovhus

.eslintignore

@@ -1,3 +1,4 @@
 **/out
 **/node_modules
 !.eslintrc.js
+coverage

server/CHANGELOG.md

@@ -1,5 +1,9 @@
 # Bash Language Server
 
+## 1.10.0
+
+* Improved completion handler and support  auto-completion and documentation for [bash reserved words](https://www.gnu.org/software/bash/manual/html_node/Reserved-Word-Index.html) (https://github.com/mads-hartmann/bash-language-server/pull/192)
+
 ## 1.9.0
 
 * Skip analyzing files with a non-bash shebang

server/package.json

@@ -3,7 +3,7 @@
   "description": "A language server for Bash",
   "author": "Mads Hartmann",
   "license": "MIT",
-  "version": "1.9.0",
+  "version": "1.10.0",
   "publisher": "mads-hartmann",
   "main": "./out/server.js",
   "typings": "./out/server.d.ts",

server/src/__tests__/__snapshots__/analyzer.test.ts.snap

@@ -131,127 +131,127 @@ Array [
   Object {
     "data": Object {
       "name": "ret",
-      "type": "function",
+      "type": 3,
     },
     "kind": 6,
     "label": "ret",
   },
   Object {
     "data": Object {
       "name": "configures",
-      "type": "function",
+      "type": 3,
     },
     "kind": 6,
     "label": "configures",
   },
   Object {
     "data": Object {
       "name": "npm_config_loglevel",
-      "type": "function",
+      "type": 3,
     },
     "kind": 6,
     "label": "npm_config_loglevel",
   },
   Object {
     "data": Object {
       "name": "node",
-      "type": "function",
+      "type": 3,
     },
     "kind": 6,
     "label": "node",
   },
   Object {
     "data": Object {
       "name": "TMP",
-      "type": "function",
+      "type": 3,
     },
     "kind": 6,
     "label": "TMP",
   },
   Object {
     "data": Object {
       "name": "BACK",
-      "type": "function",
+      "type": 3,
     },
     "kind": 6,
     "label": "BACK",
   },
   Object {
     "data": Object {
       "name": "tar",
-      "type": "function",
+      "type": 3,
     },
     "kind": 6,
     "label": "tar",
   },
   Object {
     "data": Object {
       "name": "MAKE",
-      "type": "function",
+      "type": 3,
     },
     "kind": 6,
     "label": "MAKE",
   },
   Object {
     "data": Object {
       "name": "make",
-      "type": "function",
+      "type": 3,
     },
     "kind": 6,
     "label": "make",
   },
   Object {
     "data": Object {
       "name": "clean",
-      "type": "function",
+      "type": 3,
     },
     "kind": 6,
     "label": "clean",
   },
   Object {
     "data": Object {
       "name": "node_version",
-      "type": "function",
+      "type": 3,
     },
     "kind": 6,
     "label": "node_version",
   },
   Object {
     "data": Object {
       "name": "t",
-      "type": "function",
+      "type": 3,
     },
     "kind": 6,
     "label": "t",
   },
   Object {
     "data": Object {
       "name": "url",
-      "type": "function",
+      "type": 3,
     },
     "kind": 6,
     "label": "url",
   },
   Object {
     "data": Object {
       "name": "ver",
-      "type": "function",
+      "type": 3,
     },
     "kind": 6,
     "label": "ver",
   },
   Object {
     "data": Object {
       "name": "isnpm10",
-      "type": "function",
+      "type": 3,
     },
     "kind": 6,
     "label": "isnpm10",
   },
   Object {
     "data": Object {
       "name": "NODE",
-      "type": "function",
+      "type": 3,
     },
     "kind": 6,
     "label": "NODE",

server/src/__tests__/analyzer.test.ts

@@ -86,8 +86,17 @@ describe('wordAtPoint', () => {
   it('returns current word at a given point', () => {
     analyzer.analyze(CURRENT_URI, FIXTURES.INSTALL)
     expect(analyzer.wordAtPoint(CURRENT_URI, 25, 5)).toEqual('rm')
-    // FIXME: seems like there is an issue here:
-    // expect(analyzer.wordAtPoint(CURRENT_URI, 24, 4)).toEqual('else')
+
+    // FIXME: grammar issue: else is not found
+    // expect(analyzer.wordAtPoint(CURRENT_URI, 24, 5)).toEqual('else')
+
+    expect(analyzer.wordAtPoint(CURRENT_URI, 30, 1)).toEqual(null)
+
+    expect(analyzer.wordAtPoint(CURRENT_URI, 30, 3)).toEqual('ret')
+    expect(analyzer.wordAtPoint(CURRENT_URI, 30, 4)).toEqual('ret')
+    expect(analyzer.wordAtPoint(CURRENT_URI, 30, 5)).toEqual('ret')
+
+    expect(analyzer.wordAtPoint(CURRENT_URI, 38, 5)).toEqual('configures')
   })
 })
 

server/src/__tests__/server.test.ts

@@ -2,6 +2,7 @@ import * as lsp from 'vscode-languageserver'
 
 import { FIXTURE_FOLDER, FIXTURE_URI } from '../../../testing/fixtures'
 import LspServer from '../server'
+import { CompletionItemDataType } from '../types'
 
 async function initializeServer() {
   const diagnostics: Array<lsp.PublishDiagnosticsParams | undefined> = undefined
@@ -130,7 +131,7 @@ describe('server', () => {
     })
   })
 
-  it('responds to onCompletion when word is found', async () => {
+  it('responds to onCompletion with filtered list when word is found', async () => {
     const { connection, server } = await initializeServer()
     server.register(connection)
 
@@ -142,18 +143,55 @@ describe('server', () => {
           uri: FIXTURE_URI.INSTALL,
         },
         position: {
+          // rm
           line: 25,
           character: 5,
         },
       },
       {} as any,
     )
 
+    // Limited set
+    expect('length' in result && result.length < 5).toBe(true)
+    expect(result).toEqual(
+      expect.arrayContaining([
+        {
+          data: {
+            name: 'rm',
+            type: CompletionItemDataType.Executable,
+          },
+          kind: expect.any(Number),
+          label: 'rm',
+        },
+      ]),
+    )
+  })
+
+  it('responds to onCompletion with entire list when no word is found', async () => {
+    const { connection, server } = await initializeServer()
+    server.register(connection)
+
+    const onCompletion = connection.onCompletion.mock.calls[0][0]
+
+    const result = await onCompletion(
+      {
+        textDocument: {
+          uri: FIXTURE_URI.INSTALL,
+        },
+        position: {
+          // else
+          line: 24,
+          character: 5,
+        },
+      },
+      {} as any,
+    )
+
     // Entire list
-    expect('length' in result && result.length > 50)
+    expect('length' in result && result.length > 50).toBe(true)
   })
 
-  it('responds to onCompletion when no word is found', async () => {
+  it('responds to onCompletion with empty list when word is a comment', async () => {
     const { connection, server } = await initializeServer()
     server.register(connection)
 
@@ -165,14 +203,14 @@ describe('server', () => {
           uri: FIXTURE_URI.INSTALL,
         },
         position: {
+          // inside comment
           line: 2,
           character: 1,
         },
       },
       {} as any,
     )
 
-    // Entire list
-    expect('length' in result && result.length > 50)
+    expect(result).toEqual([])
   })
 })

server/src/analyser.ts

@@ -5,6 +5,7 @@ import * as LSP from 'vscode-languageserver'
 import * as Parser from 'web-tree-sitter'
 
 import { getGlobPattern } from './config'
+import { BashCompletionItem, CompletionItemDataType } from './types'
 import { uniqueBasedOnHash } from './util/array'
 import { flattenArray, flattenObjectValues } from './util/flatten'
 import { getFilePaths } from './util/fs'
@@ -118,17 +119,17 @@ export default class Analyzer {
   }
 
   public async getExplainshellDocumentation({
-    pos,
+    params,
     endpoint,
   }: {
-    pos: LSP.TextDocumentPositionParams
+    params: LSP.TextDocumentPositionParams
     endpoint: string
   }): Promise<any> {
     const leafNode = this.uriToTreeSitterTrees[
-      pos.textDocument.uri
+      params.textDocument.uri
     ].rootNode.descendantForPosition({
-      row: pos.position.line,
-      column: pos.position.character,
+      row: params.position.line,
+      column: params.position.character,
     })
 
     // explainshell needs the whole command, not just the "word" (tree-sitter
@@ -138,7 +139,7 @@ export default class Analyzer {
     // encounters newlines.
     const interestingNode = leafNode.type === 'word' ? leafNode.parent : leafNode
 
-    const cmd = this.uriToFileContent[pos.textDocument.uri].slice(
+    const cmd = this.uriToFileContent[params.textDocument.uri].slice(
       interestingNode.startIndex,
       interestingNode.endIndex,
     )
@@ -162,7 +163,7 @@ export default class Analyzer {
       return { ...response, status: 'error' }
     } else {
       const offsetOfMousePointerInCommand =
-        this.uriToTextDocument[pos.textDocument.uri].offsetAt(pos.position) -
+        this.uriToTextDocument[params.textDocument.uri].offsetAt(params.position) -
         interestingNode.startIndex
 
       const match = explainshellResponse.matches.find(
@@ -232,7 +233,7 @@ export default class Analyzer {
   /**
    * Find unique symbol completions for the given file.
    */
-  public findSymbolCompletions(uri: string): LSP.CompletionItem[] {
+  public findSymbolCompletions(uri: string): BashCompletionItem[] {
     const hashFunction = ({ name, kind }: LSP.SymbolInformation) => `${name}${kind}`
 
     return uniqueBasedOnHash(this.findSymbols(uri), hashFunction).map(
@@ -241,7 +242,7 @@ export default class Analyzer {
         kind: this.symbolKindToCompletionKind(symbol.kind),
         data: {
           name: symbol.name,
-          type: 'function',
+          type: CompletionItemDataType.Symbol,
         },
       }),
     )
@@ -328,13 +329,25 @@ export default class Analyzer {
     const document = this.uriToTreeSitterTrees[uri]
     const contents = this.uriToFileContent[uri]
 
-    const node = document.rootNode.namedDescendantForPosition({ row: line, column })
+    if (!document.rootNode) {
+      // Check for lacking rootNode (due to failed parse?)
+      return null
+    }
+
+    const point = { row: line, column }
+
+    const node = TreeSitterUtil.namedLeafDescendantForPosition(point, document.rootNode)
+
+    if (!node) {
+      return null
+    }
 
     const start = node.startIndex
     const end = node.endIndex
     const name = contents.slice(start, end)
 
     // Hack. Might be a problem with the grammar.
+    // TODO: Document this with a test case
     if (name.endsWith('=')) {
       return name.slice(0, name.length - 1)
     }

server/src/builtins.ts

@@ -63,8 +63,10 @@ export const LIST = [
   'wait',
 ]
 
+const SET = new Set(LIST)
+
 export function isBuiltin(word: string): boolean {
-  return LIST.find(builtin => builtin === word) !== undefined
+  return SET.has(word)
 }
 
 export async function documentation(builtin: string): Promise<string> {