Merge pull request bash-lsp#222 from bash-lsp/strict-tsconfig

Enable TypeScript strict mode for LSP

Author: skovhus

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(

server/src/__tests__/server.test.ts

@@ -6,13 +6,13 @@ import LspServer from '../server'
 import { CompletionItemDataType } from '../types'
 
 async function initializeServer() {
-  const diagnostics: Array<lsp.PublishDiagnosticsParams | undefined> = undefined
+  const diagnostics: Array<lsp.PublishDiagnosticsParams | undefined> = []
 
   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 () => {

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<Analyzer> {
     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(

server/src/server.ts

@@ -27,8 +27,14 @@ export default class BashServer {
   ): Promise<BashServer> {
     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<LSP.Hover> {
+  private async onHover(
+    params: LSP.TextDocumentPositionParams,
+  ): Promise<LSP.Hover | null> {
     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<LSP.CompletionItem> {
     const {
       data: { name, type },
-    } = item
+    } = item as BashCompletionItem
 
     this.connection.console.log(`onCompletionResolve name=${name} type=${type}`)
 

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 },
+    ])
+  })
+})

server/src/util/array.ts

@@ -15,17 +15,24 @@ export function uniq<A>(a: A[]): A[] {
 
 /**
  * Removed all duplicates from the list based on the hash function.
+ * First element matching the hash function wins.
  */
-export function uniqueBasedOnHash<A>(list: A[], elementToHash: (a: A) => string): A[] {
-  const hashSet = new Set()
+export function uniqueBasedOnHash<A extends Record<string, any>>(
+  list: A[],
+  elementToHash: (a: A) => string,
+  __result: A[] = [],
+): A[] {
+  const result: typeof list = []
+  const hashSet = new Set<string>()
 
-  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
 }

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) ||

server/tsconfig.json

@@ -7,7 +7,6 @@
       "dom", // Required by @types/turndown
       "es2016"
     ],
-    // TODO: enable:
-    "strict": false,
+    "strict": true,
   }
 }