Merge pull request #1 from bash-lsp/master

brings in upstream changes

Author: nikita-skobov

server/CHANGELOG.md

@@ -1,5 +1,10 @@
 # Bash Language Server
 
+## 1.15.0
+
+* Use comments above symbols for documentation (https://github.com/bash-lsp/bash-language-server/pull/234, https://github.com/bash-lsp/bash-language-server/pull/235)
+
+
 ## 1.14.0
 
 * onHover and onCompletion documentation improvements (https://github.com/bash-lsp/bash-language-server/pull/230)

server/package.json

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

server/src/__tests__/analyzer.test.ts

@@ -192,6 +192,35 @@ describe('findSymbolCompletions', () => {
   })
 })
 
+describe('commentsAbove', () => {
+  it('returns a string of a comment block above a line', () => {
+    analyzer.analyze(CURRENT_URI, FIXTURES.COMMENT_DOC)
+    expect(analyzer.commentsAbove(CURRENT_URI, 22)).toEqual('doc for func_one')
+  })
+
+  it('handles line breaks in comments', () => {
+    analyzer.analyze(CURRENT_URI, FIXTURES.COMMENT_DOC)
+    expect(analyzer.commentsAbove(CURRENT_URI, 28)).toEqual(
+      'doc for func_two\nhas two lines',
+    )
+  })
+
+  it('only returns connected comments', () => {
+    analyzer.analyze(CURRENT_URI, FIXTURES.COMMENT_DOC)
+    expect(analyzer.commentsAbove(CURRENT_URI, 36)).toEqual('doc for func_three')
+  })
+
+  it('returns null if no comment found', () => {
+    analyzer.analyze(CURRENT_URI, FIXTURES.COMMENT_DOC)
+    expect(analyzer.commentsAbove(CURRENT_URI, 45)).toEqual(null)
+  })
+
+  it('works for variables', () => {
+    analyzer.analyze(CURRENT_URI, FIXTURES.COMMENT_DOC)
+    expect(analyzer.commentsAbove(CURRENT_URI, 42)).toEqual('works for variables')
+  })
+})
+
 describe('fromRoot', () => {
   it('initializes an analyzer from a root', async () => {
     const parser = await initializeParser()
@@ -210,7 +239,8 @@ describe('fromRoot', () => {
 
     expect(connection.window.showWarningMessage).not.toHaveBeenCalled()
 
-    const FIXTURE_FILES_MATCHING_GLOB = 10
+    // if you add a .sh file to testing/fixtures, update this value
+    const FIXTURE_FILES_MATCHING_GLOB = 11
 
     // 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

@@ -76,6 +76,33 @@ describe('server', () => {
     })
   })
 
+  it('responds to onHover with function documentation extracted from comments', async () => {
+    const { connection, server } = await initializeServer()
+    server.register(connection)
+
+    const onHover = connection.onHover.mock.calls[0][0]
+
+    const result = await onHover(
+      {
+        textDocument: {
+          uri: FIXTURE_URI.COMMENT_DOC,
+        },
+        position: {
+          line: 17,
+          character: 0,
+        },
+      },
+      {} as any,
+      {} as any,
+    )
+
+    expect(result).toBeDefined()
+    expect(result).toEqual({
+      contents:
+        'Function defined on line 8\n\nthis is a comment\ndescribing the function\nhello_world\nthis function takes two arguments',
+    })
+  })
+
   it('responds to onDocumentHighlight', async () => {
     const { connection, server } = await initializeServer()
     server.register(connection)
@@ -225,7 +252,7 @@ describe('server', () => {
     )
 
     // Limited set (not using snapshot due to different executables on CI and locally)
-    expect(result && 'length' in result && result.length < 5).toBe(true)
+    expect(result && 'length' in result && result.length < 8).toBe(true)
     expect(result).toEqual(
       expect.arrayContaining([
         {
@@ -340,18 +367,20 @@ describe('server', () => {
     )
 
     expect(resultFunction).toMatchInlineSnapshot(`
-      Array [
-        Object {
-          "data": Object {
-            "name": "add_a_user",
-            "type": 3,
-          },
-          "documentation": "Function defined in ../issue101.sh",
-          "kind": 3,
-          "label": "add_a_user",
-        },
-      ]
-    `)
+Array [
+  Object {
+    "data": Object {
+      "name": "add_a_user",
+      "type": 3,
+    },
+    "documentation": "Function defined in ../issue101.sh
+
+Helper function to add a user",
+    "kind": 3,
+    "label": "add_a_user",
+  },
+]
+`)
   })
 
   it('responds to onCompletion with local symbol when word is found in multiple files', async () => {

server/src/analyser.ts

@@ -399,6 +399,54 @@ export default class Analyzer {
     return name
   }
 
+  /**
+   * Find a block of comments above a line position
+   */
+  public commentsAbove(uri: string, line: number): string | null {
+    const doc = this.uriToTextDocument[uri]
+
+    const commentBlock = []
+
+    // start from the line above
+    let commentBlockIndex = line - 1
+
+    // will return the comment string without the comment '#'
+    // and without leading whitespace, or null if the line 'l'
+    // is not a comment line
+    const getComment = (l: string): null | string => {
+      // this regexp has to be defined within the function
+      const commentRegExp = /^\s*#\s*(.*)/g
+      const matches = commentRegExp.exec(l)
+      return matches ? matches[1].trim() : null
+    }
+
+    let currentLine = doc.getText({
+      start: { line: commentBlockIndex, character: 0 },
+      end: { line: commentBlockIndex + 1, character: 0 },
+    })
+
+    // iterate on every line above and including
+    // the current line until getComment returns null
+    let currentComment: string | null = ''
+    while ((currentComment = getComment(currentLine))) {
+      commentBlock.push(currentComment)
+      commentBlockIndex -= 1
+      currentLine = doc.getText({
+        start: { line: commentBlockIndex, character: 0 },
+        end: { line: commentBlockIndex + 1, character: 0 },
+      })
+    }
+
+    if (commentBlock.length) {
+      // since we searched from bottom up, we then reverse
+      // the lines so that it reads top down.
+      return commentBlock.reverse().join('\n')
+    }
+
+    // no comments found above line:
+    return null
+  }
+
   private getAllSymbols(): LSP.SymbolInformation[] {
     // NOTE: this could be cached, it takes < 1 ms to generate for a project with 250 bash files...
     const symbols: LSP.SymbolInformation[] = []

server/src/server.ts

@@ -135,6 +135,54 @@ export default class BashServer {
     )
   }
 
+  private getDocumentationForSymbol({
+    currentUri,
+    symbol,
+  }: {
+    symbol: LSP.SymbolInformation
+    currentUri: string
+  }): string {
+    const symbolUri = symbol.location.uri
+    const symbolStarLine = symbol.location.range.start.line
+
+    const commentAboveSymbol = this.analyzer.commentsAbove(symbolUri, symbolStarLine)
+    const symbolDocumentation = commentAboveSymbol ? `\n\n${commentAboveSymbol}` : ''
+
+    return symbolUri !== currentUri
+      ? `${symbolKindToDescription(symbol.kind)} defined in ${path.relative(
+          currentUri,
+          symbolUri,
+        )}${symbolDocumentation}`
+      : `${symbolKindToDescription(symbol.kind)} defined on line ${symbolStarLine +
+          1}${symbolDocumentation}`
+  }
+
+  private getCompletionItemsForSymbols({
+    symbols,
+    currentUri,
+  }: {
+    symbols: LSP.SymbolInformation[]
+    currentUri: string
+  }): BashCompletionItem[] {
+    return deduplicateSymbols({ symbols, currentUri }).map(
+      (symbol: LSP.SymbolInformation) => ({
+        label: symbol.name,
+        kind: symbolKindToCompletionKind(symbol.kind),
+        data: {
+          name: symbol.name,
+          type: CompletionItemDataType.Symbol,
+        },
+        documentation:
+          symbol.location.uri !== currentUri
+            ? this.getDocumentationForSymbol({
+                currentUri,
+                symbol,
+              })
+            : undefined,
+      }),
+    )
+  }
+
   private async onHover(
     params: LSP.TextDocumentPositionParams,
   ): Promise<LSP.Hover | null> {
@@ -190,13 +238,7 @@ export default class BashServer {
         // do not return hover referencing for the current line
         .filter(symbol => symbol.location.range.start.line !== params.position.line)
         .map((symbol: LSP.SymbolInformation) =>
-          symbol.location.uri !== currentUri
-            ? `${symbolKindToDescription(symbol.kind)} defined in ${path.relative(
-                currentUri,
-                symbol.location.uri,
-              )}`
-            : `${symbolKindToDescription(symbol.kind)} defined on line ${symbol.location
-                .range.start.line + 1}`,
+          this.getDocumentationForSymbol({ currentUri, symbol }),
         )
 
       if (symbolDocumentation.length === 1) {
@@ -257,7 +299,7 @@ export default class BashServer {
     const symbolCompletions =
       word === null
         ? []
-        : getCompletionItemsForSymbols({
+        : this.getCompletionItemsForSymbols({
             symbols: this.analyzer.findSymbolsMatchingWord({
               exactMatch: false,
               word,
@@ -380,32 +422,6 @@ function deduplicateSymbols({
   return uniqueBasedOnHash([...symbolsCurrentFile, ...symbolsOtherFiles], getSymbolId)
 }
 
-function getCompletionItemsForSymbols({
-  symbols,
-  currentUri,
-}: {
-  symbols: LSP.SymbolInformation[]
-  currentUri: string
-}): BashCompletionItem[] {
-  return deduplicateSymbols({ symbols, currentUri }).map(
-    (symbol: LSP.SymbolInformation) => ({
-      label: symbol.name,
-      kind: symbolKindToCompletionKind(symbol.kind),
-      data: {
-        name: symbol.name,
-        type: CompletionItemDataType.Symbol,
-      },
-      documentation:
-        symbol.location.uri !== currentUri
-          ? `${symbolKindToDescription(symbol.kind)} defined in ${path.relative(
-              currentUri,
-              symbol.location.uri,
-            )}`
-          : undefined,
-    }),
-  )
-}
-
 function symbolKindToCompletionKind(s: LSP.SymbolKind): LSP.CompletionItemKind {
   switch (s) {
     case LSP.SymbolKind.File:

testing/fixtures.ts

@@ -20,6 +20,7 @@ export const FIXTURE_URI = {
   MISSING_NODE: `file://${path.join(FIXTURE_FOLDER, 'missing-node.sh')}`,
   PARSE_PROBLEMS: `file://${path.join(FIXTURE_FOLDER, 'parse-problems.sh')}`,
   SOURCING: `file://${path.join(FIXTURE_FOLDER, 'sourcing.sh')}`,
+  COMMENT_DOC: `file://${path.join(FIXTURE_FOLDER, 'comment-doc-on-hover.sh')}`,
 }
 
 export const FIXTURE_DOCUMENT = {
@@ -28,6 +29,7 @@ export const FIXTURE_DOCUMENT = {
   MISSING_NODE: getDocument(FIXTURE_URI.MISSING_NODE),
   PARSE_PROBLEMS: getDocument(FIXTURE_URI.PARSE_PROBLEMS),
   SOURCING: getDocument(FIXTURE_URI.SOURCING),
+  COMMENT_DOC: getDocument(FIXTURE_URI.COMMENT_DOC),
 }
 
 export default FIXTURE_DOCUMENT

testing/fixtures/comment-doc-on-hover.sh

@@ -0,0 +1,47 @@
+#!/usr/bin/env bash
+
+
+# this is a comment
+# describing the function
+# hello_world
+# this function takes two arguments
+hello_world() {
+    echo "hello world to: $1 and $2"
+}
+
+
+
+# if the user hovers above the below hello_world invocation
+# they should see the comment doc string in a tooltip
+# containing the lines 4 - 7 above
+
+hello_world "bob" "sally"
+
+
+
+# doc for func_one
+func_one() {
+    echo "func_one"
+}
+
+# doc for func_two
+# has two lines
+func_two() {
+    echo "func_two"
+}
+
+
+# this is not included
+
+# doc for func_three
+func_three() {
+    echo "func_three"
+}
+
+
+# works for variables
+my_var="pizza"
+
+
+my_other_var="no comments above me :("
+

testing/fixtures/issue101.sh

@@ -1,6 +1,6 @@
 #!/bin/sh
-# A simple script with a function...
 
+# Helper function to add a user
 add_a_user()
 {
   USER=$1