Skip to content

Commit 2d41239

Browse files
committed
Add missing global variables
1 parent 8b45785 commit 2d41239

File tree

2 files changed

+83
-10
lines changed

2 files changed

+83
-10
lines changed

server/src/__tests__/analyzer.test.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -623,8 +623,29 @@ describe('findDeclarationsMatchingWord', () => {
623623
]
624624
`)
625625

626-
// FIXME: this should return the a reference to the function
627-
expect(findWordFromLine('GLOBAL_1', 1000)).toHaveLength(1)
626+
// Global variable defined inside a function
627+
expect(findWordFromLine('GLOBAL_1', 1000)).toMatchInlineSnapshot(`
628+
Array [
629+
Object {
630+
"containerName": "g",
631+
"kind": 13,
632+
"location": Object {
633+
"range": Object {
634+
"end": Object {
635+
"character": 23,
636+
"line": 13,
637+
},
638+
"start": Object {
639+
"character": 4,
640+
"line": 13,
641+
},
642+
},
643+
"uri": "file://${FIXTURE_FOLDER}scope.sh",
644+
},
645+
"name": "GLOBAL_1",
646+
},
647+
]
648+
`)
628649
})
629650
})
630651

server/src/util/declarations.ts

Lines changed: 60 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@ export type Declarations = { [word: string]: LSP.SymbolInformation[] }
1515

1616
/**
1717
* Returns declarations (functions or variables) from a given root node
18-
* that would be available after sourcing the file.
18+
* that would be available after sourcing the file. This currently does
19+
* not include global variables defined inside function as we do not do
20+
* any flow tracing.
1921
*
2022
* Will only return one declaration per symbol name – the latest definition.
2123
* This behavior is consistent with how Bash behaves, but differs between
@@ -64,6 +66,7 @@ export function getGlobalDeclarations({
6466

6567
/**
6668
* Returns all declarations (functions or variables) from a given tree.
69+
* This includes local variables.
6770
*/
6871
export function getAllDeclarationsInTree({
6972
tree,
@@ -91,12 +94,10 @@ export function getAllDeclarationsInTree({
9194

9295
/**
9396
* Returns declarations available for the given file and location.
94-
* Done by traversing the tree upwards (which is a simplification for
95-
* actual bash behaviour but deemed good enough, compared to the complexity of flow tracing).
96-
* Filters out duplicate definitions. Used when getting declarations for the current scope.
97+
* The heuristics used is a simplification compared to bash behaviour,
98+
* but deemed good enough, compared to the complexity of flow tracing.
9799
*
98-
* FIXME: unfortunately this doesn't capture all global variables defined inside functions.
99-
* Wondering if getGlobalDeclarations should return this or we should make that a custom feature of this function.
100+
* Used when getting declarations for the current scope.
100101
*/
101102
export function getLocalDeclarations({
102103
node,
@@ -107,8 +108,7 @@ export function getLocalDeclarations({
107108
}): Declarations {
108109
const declarations: Declarations = {}
109110

110-
// bottom up traversal of the tree to capture all local declarations
111-
111+
// Bottom up traversal to capture all local and scoped declarations
112112
const walk = (node: Parser.SyntaxNode | null) => {
113113
// NOTE: there is also node.walk
114114
if (node) {
@@ -145,6 +145,58 @@ export function getLocalDeclarations({
145145

146146
walk(node)
147147

148+
// Top down traversal to add missing global variables from within functions
149+
if (node) {
150+
const rootNode =
151+
node.type === 'program'
152+
? node
153+
: TreeSitterUtil.findParent(node, (p) => p.type === 'program')
154+
if (!rootNode) {
155+
throw new Error('did not find root node')
156+
}
157+
158+
Object.entries(
159+
getAllGlobalVariableDeclarations({
160+
rootNode,
161+
uri,
162+
}),
163+
).map(([name, symbols]) => {
164+
if (!declarations[name]) {
165+
declarations[name] = symbols
166+
}
167+
})
168+
}
169+
170+
return declarations
171+
}
172+
173+
function getAllGlobalVariableDeclarations({
174+
uri,
175+
rootNode,
176+
}: {
177+
uri: string
178+
rootNode: Parser.SyntaxNode
179+
}) {
180+
const declarations: Declarations = {}
181+
182+
TreeSitterUtil.forEach(rootNode, (node: Parser.SyntaxNode) => {
183+
if (
184+
node.type === 'variable_assignment' &&
185+
// exclude local variables
186+
node.parent?.type !== 'declaration_command'
187+
) {
188+
const symbol = nodeToSymbolInformation({ node, uri })
189+
if (symbol) {
190+
if (!declarations[symbol.name]) {
191+
declarations[symbol.name] = []
192+
}
193+
declarations[symbol.name].push(symbol)
194+
}
195+
}
196+
197+
return
198+
})
199+
148200
return declarations
149201
}
150202

0 commit comments

Comments
 (0)