Merge pull request #1392 from kolyshkin/bats-load-sourcing

feat: resolve bats `load` as a sourcing command

Author: skovhus

server/src/__tests__/__snapshots__/server.test.ts.snap

@@ -1248,6 +1248,7 @@ exports[`server onRenameRequest Workspace-wide rename returns correct WorkspaceE
 exports[`server onRenameRequest Workspace-wide rename returns correct WorkspaceEdits for unsourced symbols when includeAllWorkspaceSymbols is true 1`] = `
 {
   "changes": {
+    "file://__REPO_ROOT_FOLDER__/testing/fixtures/bats/test_helper.bash": [],
     "file://__REPO_ROOT_FOLDER__/testing/fixtures/comment-doc-on-hover.sh": [],
     "file://__REPO_ROOT_FOLDER__/testing/fixtures/extension.inc": [],
     "file://__REPO_ROOT_FOLDER__/testing/fixtures/install.sh": [
@@ -1355,6 +1356,7 @@ exports[`server onRenameRequest Workspace-wide rename returns correct WorkspaceE
 exports[`server onRenameRequest Workspace-wide rename returns correct WorkspaceEdits for unsourced symbols when includeAllWorkspaceSymbols is true 2`] = `
 {
   "changes": {
+    "file://__REPO_ROOT_FOLDER__/testing/fixtures/bats/test_helper.bash": [],
     "file://__REPO_ROOT_FOLDER__/testing/fixtures/comment-doc-on-hover.sh": [],
     "file://__REPO_ROOT_FOLDER__/testing/fixtures/extension.inc": [],
     "file://__REPO_ROOT_FOLDER__/testing/fixtures/install.sh": [],

server/src/__tests__/analyzer.test.ts

@@ -16,7 +16,7 @@ import { Logger } from '../util/logger'
 const CURRENT_URI = 'dummy-uri.sh'
 
 // if you add a .sh file to testing/fixtures, update this value
-const FIXTURE_FILES_MATCHING_GLOB = 20
+const FIXTURE_FILES_MATCHING_GLOB = 21
 
 const defaultConfig = getDefaultConfiguration()
 
@@ -243,6 +243,38 @@ describe('findDeclarationLocations', () => {
     `)
   })
 
+  it('returns a location in a bats helper file pulled in with `load`', async () => {
+    const analyzer = await getAnalyzer({
+      runBackgroundAnalysis: true,
+      workspaceFolder: FIXTURE_FOLDER,
+    })
+    const document = FIXTURE_DOCUMENT.BATS_SOURCING
+    const { uri } = document
+    analyzer.analyze({ uri, document })
+    const result = analyzer.findDeclarationLocations({
+      uri,
+      word: 'setup_test_env',
+      position: { character: 4, line: 5 },
+    })
+    expect(updateSnapshotUris(result)).toMatchInlineSnapshot(`
+      [
+        {
+          "range": {
+            "end": {
+              "character": 1,
+              "line": 4,
+            },
+            "start": {
+              "character": 0,
+              "line": 2,
+            },
+          },
+          "uri": "file://__REPO_ROOT_FOLDER__/testing/fixtures/bats/test_helper.bash",
+        },
+      ]
+    `)
+  })
+
   it('returns a local reference if definition is found', async () => {
     const analyzer = await getAnalyzer({})
     analyzer.analyze({ uri: CURRENT_URI, document: FIXTURE_DOCUMENT.INSTALL })

server/src/util/__tests__/sourcing.test.ts

@@ -2,7 +2,7 @@ import * as fs from 'fs'
 import * as os from 'os'
 import * as Parser from 'web-tree-sitter'
 
-import { REPO_ROOT_FOLDER } from '../../../../testing/fixtures'
+import { FIXTURE_FOLDER, REPO_ROOT_FOLDER } from '../../../../testing/fixtures'
 import { initializeParser } from '../../parser'
 import { getSourceCommands } from '../sourcing'
 
@@ -220,4 +220,74 @@ describe('getSourcedUris', () => {
       ]
     `)
   })
+  it('resolves bats `load` commands in .bats files', () => {
+    jest.restoreAllMocks()
+
+    const fileContent = `
+      load test_helper # bats appends the .bash extension
+
+      load ./test_helper.bash # explicit extension
+
+      load "${FIXTURE_FOLDER}bats/test_helper" # absolute path
+
+      load ../issue101.sh # relative to the test file
+
+      load "$SOME_VARIABLE" # dynamic loads are not supported
+
+      load # not finished
+      `
+
+    const sourceCommands = getSourceCommands({
+      fileUri: `${FIXTURE_FOLDER}bats/sourcing.bats`,
+      rootPath: REPO_ROOT_FOLDER,
+      tree: parser.parse(fileContent),
+    })
+
+    const sourcedUris = new Set(
+      sourceCommands
+        .map((sourceCommand) => sourceCommand.uri)
+        .filter((uri) => uri !== null),
+    )
+
+    expect(sourcedUris).toEqual(
+      new Set([
+        `file://${FIXTURE_FOLDER}bats/test_helper.bash`,
+        `file://${FIXTURE_FOLDER}issue101.sh`,
+      ]),
+    )
+
+    expect(
+      sourceCommands
+        .filter((command) => command.error)
+        .map(({ error, range }) => ({
+          error,
+          line: range.start.line,
+        })),
+    ).toMatchInlineSnapshot(`
+      [
+        {
+          "error": "non-constant source not supported",
+          "line": 9,
+        },
+      ]
+    `)
+  })
+
+  it('does not treat `load` as a sourcing command outside of .bats files', () => {
+    jest.restoreAllMocks()
+
+    const fileContent = `
+      load test_helper
+
+      load ../issue101.sh
+      `
+
+    const sourceCommands = getSourceCommands({
+      fileUri: `${FIXTURE_FOLDER}bats/not-a-bats-file.sh`,
+      rootPath: REPO_ROOT_FOLDER,
+      tree: parser.parse(fileContent),
+    })
+
+    expect(sourceCommands).toEqual([])
+  })
 })

server/src/util/sourcing.ts

@@ -10,6 +10,14 @@ import * as TreeSitterUtil from './tree-sitter'
 
 const SOURCING_COMMANDS = ['source', '.']
 
+// Bats (https://bats-core.readthedocs.io) test files pull in helper files using
+// `load`, which behaves like `source` but resolves relative to the directory of
+// the test file and appends ".bash" if the given path does not exist. It is only
+// treated as a sourcing command in .bats files, as `load` is a common enough
+// name for an unrelated command or function elsewhere.
+const BATS_SOURCING_COMMANDS = ['load']
+const BATS_SOURCED_EXTENSION = '.bash'
+
 export type SourceCommand = {
   range: LSP.Range
   uri: string | null // resolved URIs
@@ -31,13 +39,16 @@ export function getSourceCommands({
   const sourceCommands: SourceCommand[] = []
 
   const rootPaths = [path.dirname(fileUri), rootPath].filter(Boolean) as string[]
+  const isBatsFile = fileUri.endsWith('.bats')
 
   TreeSitterUtil.forEach(tree.rootNode, (node) => {
-    const sourcedPathInfo = getSourcedPathInfoFromNode({ node })
+    const sourcedPathInfo = getSourcedPathInfoFromNode({ node, isBatsFile })
 
     if (sourcedPathInfo) {
       const { sourcedPath, parseError } = sourcedPathInfo
-      const uri = sourcedPath ? resolveSourcedUri({ rootPaths, sourcedPath }) : null
+      const uri = sourcedPath
+        ? resolveSourcedUri({ rootPaths, sourcedPath, isBatsFile })
+        : null
 
       sourceCommands.push({
         range: TreeSitterUtil.range(node),
@@ -54,9 +65,15 @@ export function getSourceCommands({
 
 function getSourcedPathInfoFromNode({
   node,
+  isBatsFile,
 }: {
   node: Parser.SyntaxNode
+  isBatsFile: boolean
 }): null | { sourcedPath?: string; parseError?: string } {
+  const sourcingCommands = isBatsFile
+    ? [...SOURCING_COMMANDS, ...BATS_SOURCING_COMMANDS]
+    : SOURCING_COMMANDS
+
   if (node.type === 'command') {
     const [commandNameNode, argumentNode] = node.namedChildren
 
@@ -66,7 +83,7 @@ function getSourcedPathInfoFromNode({
 
     if (
       commandNameNode.type === 'command_name' &&
-      SOURCING_COMMANDS.includes(commandNameNode.text)
+      sourcingCommands.includes(commandNameNode.text)
     ) {
       const previousCommentNode =
         node.previousSibling?.type === 'comment' ? node.previousSibling : null
@@ -148,6 +165,7 @@ function getSourcedPathInfoFromNode({
  * - Converts a relative paths to absolute paths
  * - Converts a tilde path to an absolute path
  * - Resolves the path
+ * - For bats files, retries with a ".bash" suffix, like bats' own `load` does
  *
  * NOTE: for future improvements:
  * "If filename does not contain a slash, file names in PATH are used to find
@@ -156,28 +174,39 @@ function getSourcedPathInfoFromNode({
 function resolveSourcedUri({
   rootPaths,
   sourcedPath,
+  isBatsFile,
 }: {
   rootPaths: string[]
   sourcedPath: string
+  isBatsFile: boolean
 }): string | null {
   if (sourcedPath.startsWith('~')) {
     sourcedPath = untildify(sourcedPath)
   }
 
+  // bats' `load` falls back to appending ".bash" when the given path is not a file
+  const sourcedPaths = isBatsFile
+    ? [sourcedPath, `${sourcedPath}${BATS_SOURCED_EXTENSION}`]
+    : [sourcedPath]
+
   if (sourcedPath.startsWith('/')) {
-    if (fs.existsSync(sourcedPath)) {
-      return `file://${sourcedPath}`
+    for (const candidate of sourcedPaths) {
+      if (fs.existsSync(candidate)) {
+        return `file://${candidate}`
+      }
     }
     return null
   }
 
   // resolve  relative path
   for (const rootPath of rootPaths) {
-    const potentialPath = path.join(rootPath.replace('file://', ''), sourcedPath)
+    for (const candidate of sourcedPaths) {
+      const potentialPath = path.join(rootPath.replace('file://', ''), candidate)
 
-    // check if path is a file
-    if (fs.existsSync(potentialPath)) {
-      return `file://${potentialPath}`
+      // check if path is a file
+      if (fs.existsSync(potentialPath)) {
+        return `file://${potentialPath}`
+      }
     }
   }
 

testing/fixtures.ts

@@ -16,6 +16,8 @@ function getDocument(uri: string) {
 type FIXTURE_KEY = keyof typeof FIXTURE_URI
 
 export const FIXTURE_URI = {
+  BATS_SOURCING: `file://${path.join(FIXTURE_FOLDER, 'bats', 'sourcing.bats')}`,
+  BATS_TEST_HELPER: `file://${path.join(FIXTURE_FOLDER, 'bats', 'test_helper.bash')}`,
   COMMENT_DOC: `file://${path.join(FIXTURE_FOLDER, 'comment-doc-on-hover.sh')}`,
   CRASH: `file://${path.join(FIXTURE_FOLDER, 'crash.zsh')}`,
   INSTALL: `file://${path.join(FIXTURE_FOLDER, 'install.sh')}`,

testing/fixtures/bats/sourcing.bats

@@ -0,0 +1,12 @@
+#!/usr/bin/env bats
+
+load test_helper
+
+setup() {
+  setup_test_env
+}
+
+@test "it works" {
+  run true
+  [ "$status" -eq 0 ]
+}

testing/fixtures/bats/test_helper.bash

@@ -0,0 +1,5 @@
+#!/usr/bin/env bash
+
+setup_test_env() {
+  echo "setting up"
+}