Skip to content

feat: support leading dynamic section when sourcing #1086

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 13, 2025
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
23 changes: 23 additions & 0 deletions server/src/__tests__/analyzer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
]
`)

Expand Down Expand Up @@ -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
Expand Down
50 changes: 32 additions & 18 deletions server/src/util/__tests__/__snapshots__/sourcing.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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",
Expand All @@ -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,
Expand Down
5 changes: 5 additions & 0 deletions server/src/util/__tests__/sourcing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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",
}
`)

Expand Down
76 changes: 67 additions & 9 deletions server/src/util/sourcing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
}
Expand Down Expand Up @@ -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
}
36 changes: 36 additions & 0 deletions server/src/util/tree-sitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
}
2 changes: 2 additions & 0 deletions testing/fixtures/sourcing.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,5 @@ loadlib () {
}

loadlib "issue206"

source $SCRIPT_DIR/tag-release.inc with arguments