Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions server/src/__tests__/__snapshots__/server.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand Down Expand Up @@ -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": [],
Expand Down
34 changes: 33 additions & 1 deletion server/src/__tests__/analyzer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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 })
Expand Down
72 changes: 71 additions & 1 deletion server/src/util/__tests__/sourcing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -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([])
})
})
47 changes: 38 additions & 9 deletions server/src/util/sourcing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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),
Expand All @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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}`
}
}
}

Expand Down
2 changes: 2 additions & 0 deletions testing/fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')}`,
Expand Down
12 changes: 12 additions & 0 deletions testing/fixtures/bats/sourcing.bats
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#!/usr/bin/env bats

load test_helper

setup() {
setup_test_env
}

@test "it works" {
run true
[ "$status" -eq 0 ]
}
5 changes: 5 additions & 0 deletions testing/fixtures/bats/test_helper.bash
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#!/usr/bin/env bash

setup_test_env() {
echo "setting up"
}
Loading