From 61b54288b80a6c4b75185b4307c4e6852880b1c6 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 19 Aug 2026 00:19:58 -0700 Subject: [PATCH] feat: resolve bats `load` as a sourcing command Bats test files pull in helpers with `load test_helper` rather than `source`. Since `load` was not recognized as sourcing, none of the helper's symbols were reachable from a .bats file: go to definition returned nothing, and the only workaround was to turn on `includeAllWorkspaceSymbols`, which widens completion and definition across the whole workspace. Treat `load` as a sourcing command, but only in .bats files, as `load` is a common enough name for an unrelated command or function elsewhere. Path resolution follows bats: relative to the test file, retrying with a ".bash" suffix when the given path is not a file. `bats_load_library` is deliberately left out, as it resolves against BATS_LIB_PATH, which is not statically known. Fixes #181 --- .../__snapshots__/server.test.ts.snap | 2 + server/src/__tests__/analyzer.test.ts | 34 ++++++++- server/src/util/__tests__/sourcing.test.ts | 72 ++++++++++++++++++- server/src/util/sourcing.ts | 47 +++++++++--- testing/fixtures.ts | 2 + testing/fixtures/bats/sourcing.bats | 12 ++++ testing/fixtures/bats/test_helper.bash | 5 ++ 7 files changed, 163 insertions(+), 11 deletions(-) create mode 100644 testing/fixtures/bats/sourcing.bats create mode 100644 testing/fixtures/bats/test_helper.bash diff --git a/server/src/__tests__/__snapshots__/server.test.ts.snap b/server/src/__tests__/__snapshots__/server.test.ts.snap index c19f48334..a660d5473 100644 --- a/server/src/__tests__/__snapshots__/server.test.ts.snap +++ b/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": [], diff --git a/server/src/__tests__/analyzer.test.ts b/server/src/__tests__/analyzer.test.ts index 3eeb753f7..23dc676ef 100644 --- a/server/src/__tests__/analyzer.test.ts +++ b/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 }) diff --git a/server/src/util/__tests__/sourcing.test.ts b/server/src/util/__tests__/sourcing.test.ts index 2fa765a42..cb9359b5e 100644 --- a/server/src/util/__tests__/sourcing.test.ts +++ b/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([]) + }) }) diff --git a/server/src/util/sourcing.ts b/server/src/util/sourcing.ts index b897800f0..c0639491b 100644 --- a/server/src/util/sourcing.ts +++ b/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}` + } } } diff --git a/testing/fixtures.ts b/testing/fixtures.ts index c98331f77..3a52f9b82 100644 --- a/testing/fixtures.ts +++ b/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')}`, diff --git a/testing/fixtures/bats/sourcing.bats b/testing/fixtures/bats/sourcing.bats new file mode 100644 index 000000000..40b49dbbf --- /dev/null +++ b/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 ] +} diff --git a/testing/fixtures/bats/test_helper.bash b/testing/fixtures/bats/test_helper.bash new file mode 100644 index 000000000..16b96139e --- /dev/null +++ b/testing/fixtures/bats/test_helper.bash @@ -0,0 +1,5 @@ +#!/usr/bin/env bash + +setup_test_env() { + echo "setting up" +}