Merge pull request #209 from bash-lsp/fix-invalid-document-highlights

Fix invalid document highlights

Author: skovhus

package.json

@@ -26,9 +26,9 @@
     "eslint-plugin-jest": "^22.17.0",
     "eslint-plugin-prettier": "^3.1.2",
     "eslint-plugin-simple-import-sort": "^4.0.0",
-    "jest": "25.0.0",
+    "jest": "^25.3.0",
     "prettier": "^1.19.1",
-    "ts-jest": "^25.2.1",
+    "ts-jest": "^25.3.1",
     "typescript": "^3.8.3",
     "vscode-languageserver": "^5.2.1"
   },

server/CHANGELOG.md

@@ -1,5 +1,9 @@
 # Bash Language Server
 
+## 1.11.2
+
+* Fix invalid documentHighlight response when word cannot be found (https://github.com/bash-lsp/bash-language-server/pull/209)
+
 ## 1.11.1
 
 * Workspace symbols are resolved using fuzzy search (not just starting with it)

server/package.json

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

server/src/__tests__/analyzer.test.ts

@@ -127,7 +127,7 @@ describe('fromRoot', () => {
 
     expect(newAnalyzer).toBeDefined()
 
-    const FIXTURE_FILES_MATCHING_GLOB = 8
+    const FIXTURE_FILES_MATCHING_GLOB = 9
 
     // Intro, stats on glob, one file skipped due to shebang, and outro
     const LOG_LINES = FIXTURE_FILES_MATCHING_GLOB + 4

server/src/__tests__/server.test.ts

@@ -132,6 +132,85 @@ describe('server', () => {
     })
   })
 
+  it('responds to onDocumentHighlight', async () => {
+    const { connection, server } = await initializeServer()
+    server.register(connection)
+
+    const onDocumentHighlight = connection.onDocumentHighlight.mock.calls[0][0]
+
+    const result1 = await onDocumentHighlight(
+      {
+        textDocument: {
+          uri: FIXTURE_URI.ISSUE206,
+        },
+        position: {
+          // FOO
+          line: 0,
+          character: 10,
+        },
+      },
+      {} as any,
+    )
+
+    // TODO: there is a superfluous range here on line 0:
+    expect(result1).toMatchInlineSnapshot(`
+      Array [
+        Object {
+          "range": Object {
+            "end": Object {
+              "character": 12,
+              "line": 0,
+            },
+            "start": Object {
+              "character": 9,
+              "line": 0,
+            },
+          },
+        },
+        Object {
+          "range": Object {
+            "end": Object {
+              "character": 12,
+              "line": 0,
+            },
+            "start": Object {
+              "character": 9,
+              "line": 0,
+            },
+          },
+        },
+        Object {
+          "range": Object {
+            "end": Object {
+              "character": 28,
+              "line": 1,
+            },
+            "start": Object {
+              "character": 25,
+              "line": 1,
+            },
+          },
+        },
+      ]
+    `)
+
+    const result2 = await onDocumentHighlight(
+      {
+        textDocument: {
+          uri: FIXTURE_URI.ISSUE206,
+        },
+        position: {
+          // readonly cannot be parsed as a word
+          line: 0,
+          character: 0,
+        },
+      },
+      {} as any,
+    )
+
+    expect(result2).toMatchInlineSnapshot(`Array []`)
+  })
+
   it('responds to onWorkspaceSymbol', async () => {
     const { connection, server } = await initializeServer()
     server.register(connection)

server/src/server.ts

@@ -119,9 +119,9 @@ export default class BashServer {
     params: LSP.ReferenceParams | LSP.TextDocumentPositionParams
     word?: string | null
   }) {
-    const wordLog = word ? `"${word}"` : ''
+    const wordLog = word ? `"${word}"` : 'null'
     this.connection.console.log(
-      `${request} ${params.position.line}:${params.position.character} ${wordLog}`,
+      `${request} ${params.position.line}:${params.position.character} word=${wordLog}`,
     )
   }
 
@@ -201,6 +201,11 @@ export default class BashServer {
   ): LSP.DocumentHighlight[] {
     const word = this.getWordAtPoint(params)
     this.logRequest({ request: 'onDocumentHighlight', params, word })
+
+    if (!word) {
+      return []
+    }
+
     return this.analyzer
       .findOccurrences(params.textDocument.uri, word)
       .map(n => ({ range: n.range }))

testing/fixtures.ts

@@ -16,6 +16,7 @@ function getDocument(uri: string) {
 export const FIXTURE_URI = {
   MISSING_NODE: `file://${path.join(FIXTURE_FOLDER, 'missing-node.sh')}`,
   ISSUE101: `file://${path.join(FIXTURE_FOLDER, 'issue101.sh')}`,
+  ISSUE206: `file://${path.join(FIXTURE_FOLDER, 'issue206.sh')}`,
   INSTALL: `file://${path.join(FIXTURE_FOLDER, 'install.sh')}`,
   PARSE_PROBLEMS: `file://${path.join(FIXTURE_FOLDER, 'parse-problems.sh')}`,
 }

testing/fixtures/issue206.sh

@@ -0,0 +1,2 @@
+readonly FOO=$1
+readonly BAR=$UNDEFINED/$FOO