diff --git a/server/src/__tests__/analyzer.test.ts b/server/src/__tests__/analyzer.test.ts index 51c47a7ac..3eeb753f7 100644 --- a/server/src/__tests__/analyzer.test.ts +++ b/server/src/__tests__/analyzer.test.ts @@ -123,6 +123,28 @@ describe('analyze', () => { "severity": 3, "source": "bash-language-server", }, + { + "message": "Source command could not be analyzed: failed to resolve path. + + Consider adding a ShellCheck directive above this line to fix or ignore this: + # shellcheck source=/my-file.sh # specify the file to source + # shellcheck source-path=my_script_folder # specify the folder to search in + # shellcheck source=/dev/null # to ignore the error + + Disable this message by changing the configuration option "enableSourceErrorDiagnostics"", + "range": { + "end": { + "character": 49, + "line": 26, + }, + "start": { + "character": 0, + "line": 26, + }, + }, + "severity": 3, + "source": "bash-language-server", + }, ] `) @@ -847,6 +869,7 @@ describe('initiateBackgroundAnalysis', () => { [expect.stringContaining('parse-problems.sh: syntax error')], [expect.stringContaining('sourcing.sh line 16: failed to resolve path')], [expect.stringContaining('sourcing.sh line 21: non-constant source not supported')], + [expect.stringContaining('sourcing.sh line 26: failed to resolve path')], ]) // Intro, stats on glob, one file skipped due to shebang, and outro diff --git a/server/src/util/__tests__/__snapshots__/sourcing.test.ts.snap b/server/src/util/__tests__/__snapshots__/sourcing.test.ts.snap index b567a14ce..68a3686ff 100644 --- a/server/src/util/__tests__/__snapshots__/sourcing.test.ts.snap +++ b/server/src/util/__tests__/__snapshots__/sourcing.test.ts.snap @@ -86,16 +86,30 @@ exports[`getSourcedUris returns a set of sourced files (but ignores some unhandl }, "uri": null, }, + { + "error": null, + "range": { + "end": { + "character": 39, + "line": 15, + }, + "start": { + "character": 6, + "line": 15, + }, + }, + "uri": "file:///Users/bash/issue-926.sh", + }, { "error": "non-constant source not supported", "range": { "end": { "character": 66, - "line": 16, + "line": 18, }, "start": { "character": 49, - "line": 16, + "line": 18, }, }, "uri": null, @@ -105,11 +119,11 @@ exports[`getSourcedUris returns a set of sourced files (but ignores some unhandl "range": { "end": { "character": 66, - "line": 18, + "line": 20, }, "start": { "character": 6, - "line": 18, + "line": 20, }, }, "uri": null, @@ -119,11 +133,11 @@ exports[`getSourcedUris returns a set of sourced files (but ignores some unhandl "range": { "end": { "character": 30, - "line": 25, + "line": 27, }, "start": { "character": 8, - "line": 25, + "line": 27, }, }, "uri": "file:///Users/bash/issue206.sh", @@ -133,53 +147,53 @@ exports[`getSourcedUris returns a set of sourced files (but ignores some unhandl "range": { "end": { "character": 22, - "line": 47, + "line": 49, }, "start": { "character": 8, - "line": 47, + "line": 49, }, }, "uri": null, }, { - "error": "non-constant source not supported", + "error": null, "range": { "end": { "character": 39, - "line": 59, + "line": 61, }, "start": { "character": 8, - "line": 59, + "line": 61, }, }, - "uri": null, + "uri": "file:///Users/bash/staging.sh", }, { - "error": "non-constant source not supported", + "error": null, "range": { "end": { "character": 42, - "line": 62, + "line": 64, }, "start": { "character": 8, - "line": 62, + "line": 64, }, }, - "uri": null, + "uri": "file:///Users/bash/production.sh", }, { "error": "non-constant source not supported", "range": { "end": { "character": 67, - "line": 89, + "line": 91, }, "start": { "character": 10, - "line": 89, + "line": 91, }, }, "uri": null, diff --git a/server/src/util/__tests__/sourcing.test.ts b/server/src/util/__tests__/sourcing.test.ts index 765197fa5..2fa765a42 100644 --- a/server/src/util/__tests__/sourcing.test.ts +++ b/server/src/util/__tests__/sourcing.test.ts @@ -46,6 +46,8 @@ describe('getSourcedUris', () => { source "$LIBPATH" # dynamic imports not supported + source "$SCRIPT_DIR"/issue-926.sh # remove leading dynamic segment + # conditional is currently not supported if [[ -z $__COMPLETION_LIB_LOADED ]]; then source "$LIBPATH" ; fi @@ -144,7 +146,10 @@ describe('getSourcedUris', () => { "file:///Users/bash/x", "file:///Users/scripts/release-client.sh", "file:///Users/bash-user/myscript", + "file:///Users/bash/issue-926.sh", "file:///Users/bash/issue206.sh", + "file:///Users/bash/staging.sh", + "file:///Users/bash/production.sh", } `) diff --git a/server/src/util/sourcing.ts b/server/src/util/sourcing.ts index d378c7c20..b897800f0 100644 --- a/server/src/util/sourcing.ts +++ b/server/src/util/sourcing.ts @@ -102,20 +102,32 @@ function getSourcedPathInfoFromNode({ } } - if (argumentNode.type === 'word') { + const strValue = TreeSitterUtil.resolveStaticString(argumentNode) + if (strValue !== null) { return { - sourcedPath: argumentNode.text, + sourcedPath: strValue, } } - if (argumentNode.type === 'string' || argumentNode.type === 'raw_string') { - const children = argumentNode.namedChildren - if ( - children.length === 0 || - (children.length === 1 && children[0].type === 'string_content') - ) { + // Strip one leading dynamic section. + if (argumentNode.type === 'string' && argumentNode.namedChildren.length === 1) { + const [variableNode] = argumentNode.namedChildren + if (TreeSitterUtil.isExpansion(variableNode)) { + const stringContents = argumentNode.text.slice(1, -1) + if (stringContents.startsWith(`${variableNode.text}/`)) { + return { + sourcedPath: `.${stringContents.slice(variableNode.text.length)}`, + } + } + } + } + + if (argumentNode.type === 'concatenation') { + // Strip one leading dynamic section from a concatenation node. + const sourcedPath = resolveSourceFromConcatenation(argumentNode) + if (sourcedPath) { return { - sourcedPath: argumentNode.text.slice(1, -1), + sourcedPath, } } } @@ -171,3 +183,49 @@ function resolveSourcedUri({ return null } + +/* + * Resolves the source path from a concatenation node, stripping a leading dynamic directory segment. + * Returns null if the source path can't be statically determined after stripping a segment. + * Note: If a non-concatenation node is passed, null will be returned. This is likely a programmer error. + */ +function resolveSourceFromConcatenation(node: Parser.SyntaxNode): string | null { + if (node.type !== 'concatenation') return null + const stringValue = TreeSitterUtil.resolveStaticString(node) + if (stringValue !== null) return stringValue // This string is fully static. + + const values: string[] = [] + // Since the string must begin with the variable, the variable must be in the first child. + const [firstNode, ...rest] = node.namedChildren + // The first child is static, this means one of the other children is not! + if (TreeSitterUtil.resolveStaticString(firstNode) !== null) return null + + // if the string is unquoted, the first child is the variable, so there's no more text in it. + if (!TreeSitterUtil.isExpansion(firstNode)) { + if (firstNode.namedChildCount > 1) return null // Only one variable is allowed. + // Since the string must begin with the variable, the variable must be first child. + const variableNode = firstNode.namedChildren[0] // Get the variable (quoted case) + // This is command substitution! + if (!TreeSitterUtil.isExpansion(variableNode)) return null + const stringContents = firstNode.text.slice(1, -1) + // The string doesn't start with the variable! + if (!stringContents.startsWith(variableNode.text)) return null + // Get the remaining static portion the string + values.push(stringContents.slice(variableNode.text.length)) + } + + for (const child of rest) { + const value = TreeSitterUtil.resolveStaticString(child) + // The other values weren't statically determinable! + if (value === null) return null + values.push(value) + } + + // Join all our found static values together. + const staticResult = values.join('') + // The path starts with slash, so trim the leading variable and replace with a dot + if (staticResult.startsWith('/')) return `.${staticResult}` + // The path doesn't start with a slash, so it's invalid + // PERF: can we fail earlier than this? + return null +} diff --git a/server/src/util/tree-sitter.ts b/server/src/util/tree-sitter.ts index 41fe915ac..613a15576 100644 --- a/server/src/util/tree-sitter.ts +++ b/server/src/util/tree-sitter.ts @@ -57,6 +57,16 @@ export function isVariableInReadCommand(n: SyntaxNode): boolean { return false } +export function isExpansion(n: SyntaxNode): boolean { + switch (n.type) { + case 'expansion': + case 'simple_expansion': + return true + default: + return false + } +} + export function findParent( start: SyntaxNode, predicate: (n: SyntaxNode) => boolean, @@ -78,3 +88,29 @@ export function findParentOfType(start: SyntaxNode, type: string | string[]) { return findParent(start, (n) => type.includes(n.type)) } + +/** + * Resolves the full string value of a node + * Returns null if the value can't be statically determined (ie, it contains a variable or command substition). + * Supports: word, string, raw_string, and concatenation + */ +export function resolveStaticString(node: SyntaxNode): string | null { + if (node.type === 'concatenation') { + const values = [] + for (const child of node.namedChildren) { + const value = resolveStaticString(child) + if (value === null) return null + values.push(value) + } + return values.join('') + } + if (node.type === 'word') return node.text + if (node.type === 'string' || node.type === 'raw_string') { + if (node.namedChildCount === 0) return node.text.slice(1, -1) + const children = node.namedChildren + if (children.length === 1 && children[0].type === 'string_content') + return children[0].text + return null + } + return null +} diff --git a/testing/fixtures/sourcing.sh b/testing/fixtures/sourcing.sh index 35227490f..9a4653934 100644 --- a/testing/fixtures/sourcing.sh +++ b/testing/fixtures/sourcing.sh @@ -23,3 +23,5 @@ loadlib () { } loadlib "issue206" + +source $SCRIPT_DIR/tag-release.inc with arguments