Replace complex tree sitter helper

The helper was originally introduced to make the detection of a word at a given point
more accurate (this is used for completion, hovering, etc) and fixes some issues with the tree sitter grammar.

It turns out this is really not needed and introduces a bunch of issues.

Related to #192

Author: skovhus

server/src/__tests__/analyzer.test.ts

@@ -87,16 +87,28 @@ describe('findSymbolsForFile', () => {
 describe('wordAtPoint', () => {
   it('returns current word at a given point', () => {
     analyzer.analyze(CURRENT_URI, FIXTURES.INSTALL)
+    // expect(analyzer.wordAtPoint(CURRENT_URI, 25, 0)).toEqual(null)
+    expect(analyzer.wordAtPoint(CURRENT_URI, 25, 0)).toEqual(null)
+    expect(analyzer.wordAtPoint(CURRENT_URI, 25, 1)).toEqual(null)
+    expect(analyzer.wordAtPoint(CURRENT_URI, 25, 2)).toEqual(null)
+    expect(analyzer.wordAtPoint(CURRENT_URI, 25, 3)).toEqual(null)
+    expect(analyzer.wordAtPoint(CURRENT_URI, 25, 4)).toEqual('rm')
     expect(analyzer.wordAtPoint(CURRENT_URI, 25, 5)).toEqual('rm')
+    expect(analyzer.wordAtPoint(CURRENT_URI, 25, 6)).toEqual(null)
 
-    // FIXME: grammar issue: else is not found
-    // expect(analyzer.wordAtPoint(CURRENT_URI, 24, 5)).toEqual('else')
+    // expect(analyzer.wordAtPoint(CURRENT_URI, 25, 7)).toEqual('?')
+
+    expect(analyzer.wordAtPoint(CURRENT_URI, 24, 2)).toEqual('else')
+    expect(analyzer.wordAtPoint(CURRENT_URI, 24, 3)).toEqual('else')
+    expect(analyzer.wordAtPoint(CURRENT_URI, 24, 5)).toEqual('else')
+    expect(analyzer.wordAtPoint(CURRENT_URI, 24, 7)).toEqual(null)
 
     expect(analyzer.wordAtPoint(CURRENT_URI, 30, 1)).toEqual(null)
 
+    expect(analyzer.wordAtPoint(CURRENT_URI, 30, 2)).toEqual('ret')
     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, 30, 5)).toEqual('=')
 
     expect(analyzer.wordAtPoint(CURRENT_URI, 38, 5)).toEqual('configures')
   })

server/src/__tests__/server.test.ts

@@ -181,7 +181,7 @@ describe('server', () => {
       {} as any,
     )
 
-    expect(result2).toMatchInlineSnapshot(`null`)
+    expect(result2).toMatchInlineSnapshot(`Array []`)
   })
 
   it('responds to onWorkspaceSymbol', async () => {
@@ -279,17 +279,17 @@ describe('server', () => {
           uri: FIXTURE_URI.INSTALL,
         },
         position: {
-          // else
-          line: 24,
-          character: 5,
+          // empty space
+          line: 26,
+          character: 0,
         },
       },
       {} as any,
       {} as any,
     )
 
     // Entire list
-    expect(result && 'length' in result && result.length > 50).toBe(true)
+    expect(result && 'length' in result && result.length).toBeGreaterThanOrEqual(50)
   })
 
   it('responds to onCompletion with empty list when word is a comment', async () => {

server/src/analyser.ts

@@ -371,32 +371,19 @@ export default class Analyzer {
    */
   public wordAtPoint(uri: string, line: number, column: number): string | null {
     const document = this.uriToTreeSitterTrees[uri]
-    const contents = this.uriToFileContent[uri]
 
     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)
+    const node = document.rootNode.descendantForPosition({ row: line, column })
 
-    if (!node) {
+    if (!node || node.childCount > 0 || node.text.trim() === '') {
       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)
-    }
-
-    return name
+    return node.text.trim()
   }
 
   /**

server/src/server.ts

@@ -273,9 +273,11 @@ export default class BashServer {
   ): LSP.DocumentHighlight[] | null {
     const word = this.getWordAtPoint(params)
     this.logRequest({ request: 'onDocumentHighlight', params, word })
+
     if (!word) {
-      return null
+      return []
     }
+
     return this.analyzer
       .findOccurrences(params.textDocument.uri, word)
       .map(n => ({ range: n.range }))
@@ -291,7 +293,14 @@ export default class BashServer {
   }
 
   private onCompletion(params: LSP.TextDocumentPositionParams): BashCompletionItem[] {
-    const word = this.getWordAtPoint(params)
+    const word = this.getWordAtPoint({
+      ...params,
+      position: {
+        line: params.position.line,
+        // Go one character back to get completion on the current word
+        character: Math.max(params.position.character - 1, 0),
+      },
+    })
     this.logRequest({ request: 'onCompletion', params, word })
 
     const currentUri = params.textDocument.uri

server/src/util/tree-sitter.ts

@@ -1,5 +1,5 @@
 import { Range } from 'vscode-languageserver/lib/main'
-import { Point, SyntaxNode } from 'web-tree-sitter'
+import { SyntaxNode } from 'web-tree-sitter'
 
 export function forEach(node: SyntaxNode, cb: (n: SyntaxNode) => void) {
   cb(node)
@@ -52,57 +52,3 @@ 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 = 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
-}