From fa4d984a5c5a7c07c826bd9a3c90042898d19aa5 Mon Sep 17 00:00:00 2001 From: skovhus Date: Thu, 16 Apr 2020 23:05:52 +0200 Subject: [PATCH 1/3] Add missing tests for array utilities --- server/src/util/__tests__/array.test.ts | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 server/src/util/__tests__/array.test.ts diff --git a/server/src/util/__tests__/array.test.ts b/server/src/util/__tests__/array.test.ts new file mode 100644 index 000000000..7c9959769 --- /dev/null +++ b/server/src/util/__tests__/array.test.ts @@ -0,0 +1,20 @@ +import { uniqueBasedOnHash } from '../array' + +describe('uniqueBasedOnHash', () => { + it('returns a list of unique elements', () => { + type Item = { x: string; y: number } + + const items: Item[] = [ + { x: '1', y: 1 }, + { x: '1', y: 2 }, + { x: '2', y: 3 }, + ] + + const hashFunction = ({ x }: Item) => x + const result = uniqueBasedOnHash(items, hashFunction) + expect(result).toEqual([ + { x: '1', y: 1 }, + { x: '2', y: 3 }, + ]) + }) +}) From d26151d989da5aa8c3208baba29eb7e23f0791db Mon Sep 17 00:00:00 2001 From: skovhus Date: Thu, 16 Apr 2020 23:06:33 +0200 Subject: [PATCH 2/3] Refactor array utility --- server/src/util/array.ts | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/server/src/util/array.ts b/server/src/util/array.ts index 7a25b248d..87e847da5 100644 --- a/server/src/util/array.ts +++ b/server/src/util/array.ts @@ -15,17 +15,24 @@ export function uniq(a: A[]): A[] { /** * Removed all duplicates from the list based on the hash function. + * First element matching the hash function wins. */ -export function uniqueBasedOnHash(list: A[], elementToHash: (a: A) => string): A[] { - const hashSet = new Set() +export function uniqueBasedOnHash>( + list: A[], + elementToHash: (a: A) => string, + __result: A[] = [], +): A[] { + const result: typeof list = [] + const hashSet = new Set() - return list.reduce((accumulator, currentValue) => { - const hash = elementToHash(currentValue) + list.forEach(element => { + const hash = elementToHash(element) if (hashSet.has(hash)) { - return accumulator + return } hashSet.add(hash) + result.push(element) + }) - return [...accumulator, currentValue] - }, []) + return result } From 47b175f8788829ef32907ab43c56d4845b5d2582 Mon Sep 17 00:00:00 2001 From: skovhus Date: Thu, 16 Apr 2020 23:09:03 +0200 Subject: [PATCH 3/3] Enable TypeScript strict mode --- server/src/__tests__/executables.test.ts | 2 +- server/src/__tests__/server.test.ts | 10 ++--- server/src/analyser.ts | 43 +++++++++++++------- server/src/server.ts | 51 ++++++++++++++++-------- server/src/util/tree-sitter.ts | 3 +- server/tsconfig.json | 3 +- 6 files changed, 73 insertions(+), 39 deletions(-) diff --git a/server/src/__tests__/executables.test.ts b/server/src/__tests__/executables.test.ts index cd1ead268..0a5eaaf18 100644 --- a/server/src/__tests__/executables.test.ts +++ b/server/src/__tests__/executables.test.ts @@ -2,7 +2,7 @@ import * as path from 'path' import Executables from '../executables' -let executables: Executables = null +let executables: Executables beforeAll(async () => { executables = await Executables.fromPath( diff --git a/server/src/__tests__/server.test.ts b/server/src/__tests__/server.test.ts index f436c230d..7b1570a93 100644 --- a/server/src/__tests__/server.test.ts +++ b/server/src/__tests__/server.test.ts @@ -6,13 +6,13 @@ import LspServer from '../server' import { CompletionItemDataType } from '../types' async function initializeServer() { - const diagnostics: Array = undefined + const diagnostics: Array = [] const connection = getMockConnection() const server = await LspServer.initialize(connection, { rootPath: FIXTURE_FOLDER, - rootUri: undefined, + rootUri: null, processId: 42, capabilities: {} as any, workspaceFolders: null, @@ -154,7 +154,7 @@ describe('server', () => { {} as any, ) - expect(result2).toMatchInlineSnapshot(`Array []`) + expect(result2).toMatchInlineSnapshot(`null`) }) it('responds to onWorkspaceSymbol', async () => { @@ -225,7 +225,7 @@ describe('server', () => { ) // Limited set (not using snapshot due to different executables on CI and locally) - expect('length' in result && result.length < 5).toBe(true) + expect(result && 'length' in result && result.length < 5).toBe(true) expect(result).toEqual( expect.arrayContaining([ { @@ -262,7 +262,7 @@ describe('server', () => { ) // Entire list - expect('length' in result && result.length > 50).toBe(true) + expect(result && 'length' in result && result.length > 50).toBe(true) }) it('responds to onCompletion with empty list when word is a comment', async () => { diff --git a/server/src/analyser.ts b/server/src/analyser.ts index 94311cbe8..f3500e6ab 100644 --- a/server/src/analyser.ts +++ b/server/src/analyser.ts @@ -40,7 +40,7 @@ export default class Analyzer { parser, }: { connection: LSP.Connection - rootPath: string | null + rootPath: LSP.InitializeParams['rootPath'] parser: Parser }): Promise { const analyzer = new Analyzer(parser) @@ -154,6 +154,13 @@ export default class Analyzer { // encounters newlines. const interestingNode = leafNode.type === 'word' ? leafNode.parent : leafNode + if (!interestingNode) { + return { + status: 'error', + message: 'no interestingNode found', + } + } + const cmd = this.uriToFileContent[params.textDocument.uri].slice( interestingNode.startIndex, interestingNode.endIndex, @@ -216,21 +223,23 @@ export default class Analyzer { const locations: LSP.Location[] = [] TreeSitterUtil.forEach(tree.rootNode, n => { - let name: string = null - let rng: LSP.Range = null + let name: null | string = null + let range: null | LSP.Range = null if (TreeSitterUtil.isReference(n)) { const node = n.firstNamedChild || n name = contents.slice(node.startIndex, node.endIndex) - rng = TreeSitterUtil.range(node) + range = TreeSitterUtil.range(node) } else if (TreeSitterUtil.isDefinition(n)) { const namedNode = n.firstNamedChild - name = contents.slice(namedNode.startIndex, namedNode.endIndex) - rng = TreeSitterUtil.range(n.firstNamedChild) + if (namedNode) { + name = contents.slice(namedNode.startIndex, namedNode.endIndex) + range = TreeSitterUtil.range(namedNode) + } } - if (name === query) { - locations.push(LSP.Location.create(uri, rng)) + if (name === query && range !== null) { + locations.push(LSP.Location.create(uri, range)) } }) @@ -294,16 +303,22 @@ export default class Analyzer { return } else if (TreeSitterUtil.isDefinition(n)) { const named = n.firstNamedChild + + if (named === null) { + return + } + const name = contents.slice(named.startIndex, named.endIndex) const namedDeclarations = this.uriToDeclarations[uri][name] || [] const parent = TreeSitterUtil.findParent(n, p => p.type === 'function_definition') - const parentName = parent - ? contents.slice( - parent.firstNamedChild.startIndex, - parent.firstNamedChild.endIndex, - ) - : null + const parentName = + parent && parent.firstNamedChild + ? contents.slice( + parent.firstNamedChild.startIndex, + parent.firstNamedChild.endIndex, + ) + : '' // TODO: unsure what we should do here? namedDeclarations.push( LSP.SymbolInformation.create( diff --git a/server/src/server.ts b/server/src/server.ts index 306003ea1..5c6c8d835 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -27,8 +27,14 @@ export default class BashServer { ): Promise { const parser = await initializeParser() + const { PATH } = process.env + + if (!PATH) { + throw new Error('Expected PATH environment variable to be set') + } + return Promise.all([ - Executables.fromPath(process.env.PATH), + Executables.fromPath(PATH), Analyzer.fromRoot({ connection, rootPath, parser }), ]).then(xs => { const executables = xs[0] @@ -128,11 +134,17 @@ export default class BashServer { ) } - private async onHover(params: LSP.TextDocumentPositionParams): Promise { + private async onHover( + params: LSP.TextDocumentPositionParams, + ): Promise { const word = this.getWordAtPoint(params) this.logRequest({ request: 'onHover', params, word }) + if (!word) { + return null + } + const explainshellEndpoint = config.getExplainshellEndpoint() if (explainshellEndpoint) { this.connection.console.log(`Query ${explainshellEndpoint}`) @@ -185,9 +197,12 @@ export default class BashServer { return null } - private onDefinition(params: LSP.TextDocumentPositionParams): LSP.Definition { + private onDefinition(params: LSP.TextDocumentPositionParams): LSP.Definition | null { const word = this.getWordAtPoint(params) this.logRequest({ request: 'onDefinition', params, word }) + if (!word) { + return null + } return this.analyzer.findDefinition(word) } @@ -203,22 +218,23 @@ export default class BashServer { private onDocumentHighlight( params: LSP.TextDocumentPositionParams, - ): LSP.DocumentHighlight[] { + ): LSP.DocumentHighlight[] | null { const word = this.getWordAtPoint(params) this.logRequest({ request: 'onDocumentHighlight', params, word }) - if (!word) { - return [] + return null } - return this.analyzer .findOccurrences(params.textDocument.uri, word) .map(n => ({ range: n.range })) } - private onReferences(params: LSP.ReferenceParams): LSP.Location[] { + private onReferences(params: LSP.ReferenceParams): LSP.Location[] | null { const word = this.getWordAtPoint(params) this.logRequest({ request: 'onReferences', params, word }) + if (!word) { + return null + } return this.analyzer.findReferences(word) } @@ -228,12 +244,15 @@ export default class BashServer { const currentUri = params.textDocument.uri - const symbolCompletions = getCompletionItemsForSymbols({ - symbols: this.analyzer.findSymbolsMatchingWord({ - word, - }), - currentUri, - }) + const symbolCompletions = + word === null + ? [] + : getCompletionItemsForSymbols({ + symbols: this.analyzer.findSymbolsMatchingWord({ + word, + }), + currentUri, + }) const reservedWordsCompletions = ReservedWords.LIST.map(reservedWord => ({ label: reservedWord, @@ -288,11 +307,11 @@ export default class BashServer { } private async onCompletionResolve( - item: BashCompletionItem, + item: LSP.CompletionItem, ): Promise { const { data: { name, type }, - } = item + } = item as BashCompletionItem this.connection.console.log(`onCompletionResolve name=${name} type=${type}`) diff --git a/server/src/util/tree-sitter.ts b/server/src/util/tree-sitter.ts index 5f5840481..cdf7ee572 100644 --- a/server/src/util/tree-sitter.ts +++ b/server/src/util/tree-sitter.ts @@ -83,7 +83,8 @@ export function namedLeafDescendantForPosition( } function searchForLeafNode(point: Point, parent: SyntaxNode): SyntaxNode | null { - let child: SyntaxNode = parent.firstNamedChild + let child = parent.firstNamedChild + while (child) { if ( pointsEqual(child.startPosition, point) || diff --git a/server/tsconfig.json b/server/tsconfig.json index 5f4011026..4bb2e797a 100644 --- a/server/tsconfig.json +++ b/server/tsconfig.json @@ -7,7 +7,6 @@ "dom", // Required by @types/turndown "es2016" ], - // TODO: enable: - "strict": false, + "strict": true, } }