diff --git a/.eslintignore b/.eslintignore index 6a3b6ed5..809845e2 100644 --- a/.eslintignore +++ b/.eslintignore @@ -1,3 +1,4 @@ **/out **/node_modules !.eslintrc.js +coverage diff --git a/server/CHANGELOG.md b/server/CHANGELOG.md index fed00f2d..7978b9cc 100644 --- a/server/CHANGELOG.md +++ b/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 diff --git a/server/package.json b/server/package.json index 2360c8d4..c31ec9f1 100644 --- a/server/package.json +++ b/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", diff --git a/server/src/__tests__/__snapshots__/analyzer.test.ts.snap b/server/src/__tests__/__snapshots__/analyzer.test.ts.snap index f921d88d..d0268758 100644 --- a/server/src/__tests__/__snapshots__/analyzer.test.ts.snap +++ b/server/src/__tests__/__snapshots__/analyzer.test.ts.snap @@ -131,7 +131,7 @@ Array [ Object { "data": Object { "name": "ret", - "type": "function", + "type": 3, }, "kind": 6, "label": "ret", @@ -139,7 +139,7 @@ Array [ Object { "data": Object { "name": "configures", - "type": "function", + "type": 3, }, "kind": 6, "label": "configures", @@ -147,7 +147,7 @@ Array [ Object { "data": Object { "name": "npm_config_loglevel", - "type": "function", + "type": 3, }, "kind": 6, "label": "npm_config_loglevel", @@ -155,7 +155,7 @@ Array [ Object { "data": Object { "name": "node", - "type": "function", + "type": 3, }, "kind": 6, "label": "node", @@ -163,7 +163,7 @@ Array [ Object { "data": Object { "name": "TMP", - "type": "function", + "type": 3, }, "kind": 6, "label": "TMP", @@ -171,7 +171,7 @@ Array [ Object { "data": Object { "name": "BACK", - "type": "function", + "type": 3, }, "kind": 6, "label": "BACK", @@ -179,7 +179,7 @@ Array [ Object { "data": Object { "name": "tar", - "type": "function", + "type": 3, }, "kind": 6, "label": "tar", @@ -187,7 +187,7 @@ Array [ Object { "data": Object { "name": "MAKE", - "type": "function", + "type": 3, }, "kind": 6, "label": "MAKE", @@ -195,7 +195,7 @@ Array [ Object { "data": Object { "name": "make", - "type": "function", + "type": 3, }, "kind": 6, "label": "make", @@ -203,7 +203,7 @@ Array [ Object { "data": Object { "name": "clean", - "type": "function", + "type": 3, }, "kind": 6, "label": "clean", @@ -211,7 +211,7 @@ Array [ Object { "data": Object { "name": "node_version", - "type": "function", + "type": 3, }, "kind": 6, "label": "node_version", @@ -219,7 +219,7 @@ Array [ Object { "data": Object { "name": "t", - "type": "function", + "type": 3, }, "kind": 6, "label": "t", @@ -227,7 +227,7 @@ Array [ Object { "data": Object { "name": "url", - "type": "function", + "type": 3, }, "kind": 6, "label": "url", @@ -235,7 +235,7 @@ Array [ Object { "data": Object { "name": "ver", - "type": "function", + "type": 3, }, "kind": 6, "label": "ver", @@ -243,7 +243,7 @@ Array [ Object { "data": Object { "name": "isnpm10", - "type": "function", + "type": 3, }, "kind": 6, "label": "isnpm10", @@ -251,7 +251,7 @@ Array [ Object { "data": Object { "name": "NODE", - "type": "function", + "type": 3, }, "kind": 6, "label": "NODE", diff --git a/server/src/__tests__/analyzer.test.ts b/server/src/__tests__/analyzer.test.ts index 09e3fe9a..ea539fe2 100644 --- a/server/src/__tests__/analyzer.test.ts +++ b/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') }) }) diff --git a/server/src/__tests__/server.test.ts b/server/src/__tests__/server.test.ts index 8fc74981..48098e1a 100644 --- a/server/src/__tests__/server.test.ts +++ b/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 = 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,6 +143,7 @@ describe('server', () => { uri: FIXTURE_URI.INSTALL, }, position: { + // rm line: 25, character: 5, }, @@ -149,11 +151,47 @@ describe('server', () => { {} 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,6 +203,7 @@ describe('server', () => { uri: FIXTURE_URI.INSTALL, }, position: { + // inside comment line: 2, character: 1, }, @@ -172,7 +211,6 @@ describe('server', () => { {} as any, ) - // Entire list - expect('length' in result && result.length > 50) + expect(result).toEqual([]) }) }) diff --git a/server/src/analyser.ts b/server/src/analyser.ts index 7e1146cf..8de70956 100644 --- a/server/src/analyser.ts +++ b/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 { 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) } diff --git a/server/src/builtins.ts b/server/src/builtins.ts index 70a3c7a1..7b5685b0 100644 --- a/server/src/builtins.ts +++ b/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 { diff --git a/server/src/reservedWords.ts b/server/src/reservedWords.ts new file mode 100644 index 00000000..88d4629d --- /dev/null +++ b/server/src/reservedWords.ts @@ -0,0 +1,42 @@ +import * as ShUtil from './util/sh' + +// https://www.gnu.org/software/bash/manual/html_node/Reserved-Word-Index.html + +export const LIST = [ + '!', + '[[', + ']]', + '{', + '}', + 'case', + 'do', + 'done', + 'elif', + 'else', + 'esac', + 'fi', + 'for', + 'function', + 'if', + 'in', + 'select', + 'then', + 'time', + 'until', + 'while', +] + +const SET = new Set(LIST) + +export function isReservedWord(word: string): boolean { + return SET.has(word) +} + +export async function documentation(reservedWord: string): Promise { + try { + const doc = await ShUtil.execShellScript(`help ${reservedWord}`) + return doc || '' + } catch (error) { + return '' + } +} diff --git a/server/src/server.ts b/server/src/server.ts index 2362bb09..b59ce7c6 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -6,6 +6,8 @@ import * as Builtins from './builtins' import * as config from './config' import Executables from './executables' import { initializeParser } from './parser' +import * as ReservedWords from './reservedWords' +import { BashCompletionItem, CompletionItemDataType } from './types' /** * The BashServer glues together the separate components to implement @@ -106,17 +108,31 @@ export default class BashServer { ) } - private async onHover(pos: LSP.TextDocumentPositionParams): Promise { + private logRequest({ + request, + params, + word, + }: { + request: string + params: LSP.ReferenceParams | LSP.TextDocumentPositionParams + word?: string | null + }) { + const wordLog = word ? `"${word}"` : '' this.connection.console.log( - `Hovering over ${pos.position.line}:${pos.position.character}`, + `${request} ${params.position.line}:${params.position.character} ${wordLog}`, ) + } + + private async onHover(params: LSP.TextDocumentPositionParams): Promise { + const word = this.getWordAtPoint(params) + + this.logRequest({ request: 'onHover', params, word }) - const word = this.getWordAtPoint(pos) const explainshellEndpoint = config.getExplainshellEndpoint() if (explainshellEndpoint) { this.connection.console.log(`Query ${explainshellEndpoint}`) const response = await this.analyzer.getExplainshellDocumentation({ - pos, + params, endpoint: explainshellEndpoint, }) @@ -147,6 +163,12 @@ export default class BashServer { })) } + if (ReservedWords.isReservedWord(word)) { + return ReservedWords.documentation(word).then(doc => ({ + contents: getMarkdownHoverItem(doc), + })) + } + if (this.executables.isExecutableOnPATH(word)) { return this.executables.documentation(word).then(doc => ({ contents: getMarkdownHoverItem(doc), @@ -156,37 +178,49 @@ export default class BashServer { return null } - private onDefinition(pos: LSP.TextDocumentPositionParams): LSP.Definition { - this.connection.console.log( - `Asked for definition at ${pos.position.line}:${pos.position.character}`, - ) - const word = this.getWordAtPoint(pos) + private onDefinition(params: LSP.TextDocumentPositionParams): LSP.Definition { + const word = this.getWordAtPoint(params) + this.logRequest({ request: 'onDefinition', params, word }) return this.analyzer.findDefinition(word) } private onDocumentSymbol(params: LSP.DocumentSymbolParams): LSP.SymbolInformation[] { + this.connection.console.log(`onDocumentSymbol`) return this.analyzer.findSymbols(params.textDocument.uri) } private onDocumentHighlight( - pos: LSP.TextDocumentPositionParams, + params: LSP.TextDocumentPositionParams, ): LSP.DocumentHighlight[] { - const word = this.getWordAtPoint(pos) + const word = this.getWordAtPoint(params) + this.logRequest({ request: 'onDocumentHighlight', params, word }) return this.analyzer - .findOccurrences(pos.textDocument.uri, word) + .findOccurrences(params.textDocument.uri, word) .map(n => ({ range: n.range })) } private onReferences(params: LSP.ReferenceParams): LSP.Location[] { const word = this.getWordAtPoint(params) + this.logRequest({ request: 'onReferences', params, word }) return this.analyzer.findReferences(word) } - private onCompletion(pos: LSP.TextDocumentPositionParams): LSP.CompletionItem[] { - this.connection.console.log( - `Asked for completions at ${pos.position.line}:${pos.position.character}`, - ) - const symbolCompletions = this.analyzer.findSymbolCompletions(pos.textDocument.uri) + private onCompletion(params: LSP.TextDocumentPositionParams): BashCompletionItem[] { + const word = this.getWordAtPoint(params) + this.logRequest({ request: 'onCompletion', params, word }) + + const symbolCompletions = this.analyzer.findSymbolCompletions(params.textDocument.uri) + + // TODO: we could do some caching here... + + const reservedWordsCompletions = ReservedWords.LIST.map(reservedWord => ({ + label: reservedWord, + kind: LSP.SymbolKind.Interface, // ?? + data: { + name: reservedWord, + type: CompletionItemDataType.ReservedWord, + }, + })) const programCompletions = this.executables.list().map((s: string) => { return { @@ -194,30 +228,50 @@ export default class BashServer { kind: LSP.SymbolKind.Function, data: { name: s, - type: 'executable', + type: CompletionItemDataType.Executable, }, } }) const builtinsCompletions = Builtins.LIST.map(builtin => ({ label: builtin, - kind: LSP.SymbolKind.Method, // ?? + kind: LSP.SymbolKind.Interface, // ?? data: { name: builtin, - type: 'builtin', + type: CompletionItemDataType.Builtin, }, })) - return [...symbolCompletions, ...programCompletions, ...builtinsCompletions] + // TODO: we have duplicates here (e.g. echo is both a builtin AND have a man page) + const allCompletions = [ + ...reservedWordsCompletions, + ...symbolCompletions, + ...programCompletions, + ...builtinsCompletions, + ] + + if (word) { + if (word.startsWith('#')) { + // Inside a comment block + return [] + } + + // Filter to only return suffixes of the current word + return allCompletions.filter(item => item.label.startsWith(word)) + } + + return allCompletions } private async onCompletionResolve( - item: LSP.CompletionItem, + item: BashCompletionItem, ): Promise { const { data: { name, type }, } = item + this.connection.console.log(`onCompletionResolve name=${name} type=${type}`) + const getMarkdownCompletionItem = (doc: string) => ({ ...item, // LSP.MarkupContent @@ -229,12 +283,15 @@ export default class BashServer { }) try { - if (type === 'executable') { + if (type === CompletionItemDataType.Executable) { const doc = await this.executables.documentation(name) return getMarkdownCompletionItem(doc) - } else if (type === 'builtin') { + } else if (type === CompletionItemDataType.Builtin) { const doc = await Builtins.documentation(name) return getMarkdownCompletionItem(doc) + } else if (type === CompletionItemDataType.ReservedWord) { + const doc = await ReservedWords.documentation(name) + return getMarkdownCompletionItem(doc) } else { return item } diff --git a/server/src/types.ts b/server/src/types.ts new file mode 100644 index 00000000..c2c7c225 --- /dev/null +++ b/server/src/types.ts @@ -0,0 +1,15 @@ +import * as LSP from 'vscode-languageserver' + +export enum CompletionItemDataType { + Builtin, + Executable, + ReservedWord, + Symbol, +} + +export interface BashCompletionItem extends LSP.CompletionItem { + data: { + type: CompletionItemDataType + name: string + } +} diff --git a/server/src/util/tree-sitter.ts b/server/src/util/tree-sitter.ts index 3a80b4cf..5f584048 100644 --- a/server/src/util/tree-sitter.ts +++ b/server/src/util/tree-sitter.ts @@ -1,5 +1,5 @@ import { Range } from 'vscode-languageserver/lib/main' -import { SyntaxNode } from 'web-tree-sitter' +import { Point, SyntaxNode } from 'web-tree-sitter' export function forEach(node: SyntaxNode, cb: (n: SyntaxNode) => void) { cb(node) @@ -52,3 +52,56 @@ export function findParent( } return null } + +/** + * Given a tree and a point, try to find the named leaf node that the point corresponds to. + * This is a helper for wordAtPoint, useful in cases where the point occurs at the boundary of + * a word so the normal behavior of "namedDescendantForPosition" does not find the desired leaf. + * For example, if you do + * > (new Parser()).setLanguage(bash).parse("echo 42").rootNode.descendantForIndex(4).text + * then you get 'echo 42', not the leaf node for 'echo'. + * + * TODO: the need for this function might reveal a flaw in tree-sitter-bash. + */ +export function namedLeafDescendantForPosition( + point: Point, + rootNode: SyntaxNode, +): SyntaxNode | null { + const node = rootNode.namedDescendantForPosition(point) + + if (node.childCount === 0) { + return node + } else { + // The node wasn't a leaf. Try to figure out what word we should use. + const nodeToUse = searchForLeafNode(point, node) + if (nodeToUse) { + return nodeToUse + } else { + return null + } + } +} + +function searchForLeafNode(point: Point, parent: SyntaxNode): SyntaxNode | null { + let child: SyntaxNode = parent.firstNamedChild + while (child) { + if ( + pointsEqual(child.startPosition, point) || + pointsEqual(child.endPosition, point) + ) { + if (child.childCount === 0) { + return child + } else { + return searchForLeafNode(point, child) + } + } + + child = child.nextNamedSibling + } + + return null +} + +function pointsEqual(point1: Point, point2: Point) { + return point1.row === point2.row && point1.column === point2.column +}