Skip to content

Commit 62a26f7

Browse files
committed
Resolve sourced paths
1 parent 0bb7cbb commit 62a26f7

File tree

5 files changed

+120
-58
lines changed

5 files changed

+120
-58
lines changed

Diff for: server/src/__tests__/analyzer.test.ts

+30-22
Original file line numberDiff line numberDiff line change
@@ -22,34 +22,39 @@ beforeAll(async () => {
2222

2323
describe('analyze', () => {
2424
it('returns an empty list of errors for a file with no parsing errors', () => {
25-
const result = analyzer.analyze(CURRENT_URI, FIXTURES.INSTALL)
25+
const result = analyzer.analyze({ uri: CURRENT_URI, document: FIXTURES.INSTALL })
2626
expect(result).toEqual([])
2727
})
2828

2929
it('returns a list of errors for a file with a missing node', () => {
30-
const result = analyzer.analyze(CURRENT_URI, FIXTURES.MISSING_NODE)
30+
const result = analyzer.analyze({ uri: CURRENT_URI, document: FIXTURES.MISSING_NODE })
3131
expect(result).not.toEqual([])
3232
expect(result).toMatchSnapshot()
3333
})
3434

3535
it('returns a list of errors for a file with parsing errors', () => {
36-
const result = analyzer.analyze(CURRENT_URI, FIXTURES.PARSE_PROBLEMS)
36+
const result = analyzer.analyze({
37+
uri: CURRENT_URI,
38+
document: FIXTURES.PARSE_PROBLEMS,
39+
})
3740
expect(result).not.toEqual([])
3841
expect(result).toMatchSnapshot()
3942
})
4043
})
4144

4245
describe('findDefinition', () => {
4346
it('returns an empty list if word is not found', () => {
44-
analyzer.analyze(CURRENT_URI, FIXTURES.INSTALL)
47+
analyzer.analyze({ uri: CURRENT_URI, document: FIXTURES.INSTALL })
4548
const result = analyzer.findDefinition({ uri: CURRENT_URI, word: 'foobar' })
4649
expect(result).toEqual([])
4750
})
4851

4952
it('returns a location to a file if word is the path in a sourcing statement', () => {
50-
analyzer.analyze(CURRENT_URI, FIXTURES.SOURCING)
53+
const document = FIXTURES.SOURCING
54+
const { uri } = document
55+
analyzer.analyze({ uri, document })
5156
const result = analyzer.findDefinition({
52-
uri: CURRENT_URI,
57+
uri,
5358
word: './extension.inc',
5459
position: { character: 10, line: 2 },
5560
})
@@ -66,14 +71,14 @@ describe('findDefinition', () => {
6671
"line": 0,
6772
},
6873
},
69-
"uri": "extension.inc",
74+
"uri": "file://${FIXTURE_FOLDER}extension.inc",
7075
},
7176
]
7277
`)
7378
})
7479

7580
it('returns a list of locations if parameter is found', () => {
76-
analyzer.analyze(CURRENT_URI, FIXTURES.INSTALL)
81+
analyzer.analyze({ uri: CURRENT_URI, document: FIXTURES.INSTALL })
7782
const result = analyzer.findDefinition({
7883
uri: CURRENT_URI,
7984
word: 'node_version',
@@ -101,13 +106,13 @@ describe('findDefinition', () => {
101106

102107
describe('findReferences', () => {
103108
it('returns empty list if parameter is not found', () => {
104-
analyzer.analyze(CURRENT_URI, FIXTURES.INSTALL)
109+
analyzer.analyze({ uri: CURRENT_URI, document: FIXTURES.INSTALL })
105110
const result = analyzer.findReferences('foobar')
106111
expect(result).toEqual([])
107112
})
108113

109114
it('returns a list of locations if parameter is found', () => {
110-
analyzer.analyze(CURRENT_URI, FIXTURES.INSTALL)
115+
analyzer.analyze({ uri: CURRENT_URI, document: FIXTURES.INSTALL })
111116
const result = analyzer.findReferences('node_version')
112117
expect(result).not.toEqual([])
113118
expect(result).toMatchSnapshot()
@@ -116,20 +121,20 @@ describe('findReferences', () => {
116121

117122
describe('findSymbolsForFile', () => {
118123
it('returns empty list if uri is not found', () => {
119-
analyzer.analyze(CURRENT_URI, FIXTURES.INSTALL)
124+
analyzer.analyze({ uri: CURRENT_URI, document: FIXTURES.INSTALL })
120125
const result = analyzer.findSymbolsForFile({ uri: 'foobar.sh' })
121126
expect(result).toEqual([])
122127
})
123128

124129
it('returns a list of SymbolInformation if uri is found', () => {
125-
analyzer.analyze(CURRENT_URI, FIXTURES.INSTALL)
130+
analyzer.analyze({ uri: CURRENT_URI, document: FIXTURES.INSTALL })
126131
const result = analyzer.findSymbolsForFile({ uri: CURRENT_URI })
127132
expect(result).not.toEqual([])
128133
expect(result).toMatchSnapshot()
129134
})
130135

131136
it('issue 101', () => {
132-
analyzer.analyze(CURRENT_URI, FIXTURES.ISSUE101)
137+
analyzer.analyze({ uri: CURRENT_URI, document: FIXTURES.ISSUE101 })
133138
const result = analyzer.findSymbolsForFile({ uri: CURRENT_URI })
134139
expect(result).not.toEqual([])
135140
expect(result).toMatchSnapshot()
@@ -169,7 +174,10 @@ describe('findAllSourcedUris', () => {
169174
})
170175

171176
// Parse the file without extension
172-
newAnalyzer.analyze(FIXTURE_URI.MISSING_EXTENSION, FIXTURES.MISSING_EXTENSION)
177+
newAnalyzer.analyze({
178+
uri: FIXTURE_URI.MISSING_EXTENSION,
179+
document: FIXTURES.MISSING_EXTENSION,
180+
})
173181

174182
const result = newAnalyzer.findAllSourcedUris({ uri: FIXTURE_URI.MISSING_EXTENSION })
175183
expect(result).toEqual(
@@ -184,7 +192,7 @@ describe('findAllSourcedUris', () => {
184192

185193
describe('wordAtPoint', () => {
186194
it('returns current word at a given point', () => {
187-
analyzer.analyze(CURRENT_URI, FIXTURES.INSTALL)
195+
analyzer.analyze({ uri: CURRENT_URI, document: FIXTURES.INSTALL })
188196
expect(analyzer.wordAtPoint(CURRENT_URI, 25, 0)).toEqual(null)
189197
expect(analyzer.wordAtPoint(CURRENT_URI, 25, 1)).toEqual(null)
190198
expect(analyzer.wordAtPoint(CURRENT_URI, 25, 2)).toEqual(null)
@@ -212,7 +220,7 @@ describe('wordAtPoint', () => {
212220

213221
describe('commandNameAtPoint', () => {
214222
it('returns current command name at a given point', () => {
215-
analyzer.analyze(CURRENT_URI, FIXTURES.INSTALL)
223+
analyzer.analyze({ uri: CURRENT_URI, document: FIXTURES.INSTALL })
216224
expect(analyzer.commandNameAtPoint(CURRENT_URI, 15, 0)).toEqual(null)
217225

218226
expect(analyzer.commandNameAtPoint(CURRENT_URI, 20, 2)).toEqual('curl')
@@ -409,40 +417,40 @@ Array [
409417

410418
describe('commentsAbove', () => {
411419
it('returns a string of a comment block above a line', () => {
412-
analyzer.analyze(CURRENT_URI, FIXTURES.COMMENT_DOC)
420+
analyzer.analyze({ uri: CURRENT_URI, document: FIXTURES.COMMENT_DOC })
413421
expect(analyzer.commentsAbove(CURRENT_URI, 22)).toEqual(
414422
'```txt\ndoc for func_one\n```',
415423
)
416424
})
417425

418426
it('handles line breaks in comments', () => {
419-
analyzer.analyze(CURRENT_URI, FIXTURES.COMMENT_DOC)
427+
analyzer.analyze({ uri: CURRENT_URI, document: FIXTURES.COMMENT_DOC })
420428
expect(analyzer.commentsAbove(CURRENT_URI, 28)).toEqual(
421429
'```txt\ndoc for func_two\nhas two lines\n```',
422430
)
423431
})
424432

425433
it('only returns connected comments', () => {
426-
analyzer.analyze(CURRENT_URI, FIXTURES.COMMENT_DOC)
434+
analyzer.analyze({ uri: CURRENT_URI, document: FIXTURES.COMMENT_DOC })
427435
expect(analyzer.commentsAbove(CURRENT_URI, 36)).toEqual(
428436
'```txt\ndoc for func_three\n```',
429437
)
430438
})
431439

432440
it('returns null if no comment found', () => {
433-
analyzer.analyze(CURRENT_URI, FIXTURES.COMMENT_DOC)
441+
analyzer.analyze({ uri: CURRENT_URI, document: FIXTURES.COMMENT_DOC })
434442
expect(analyzer.commentsAbove(CURRENT_URI, 45)).toEqual(null)
435443
})
436444

437445
it('works for variables', () => {
438-
analyzer.analyze(CURRENT_URI, FIXTURES.COMMENT_DOC)
446+
analyzer.analyze({ uri: CURRENT_URI, document: FIXTURES.COMMENT_DOC })
439447
expect(analyzer.commentsAbove(CURRENT_URI, 42)).toEqual(
440448
'```txt\nworks for variables\n```',
441449
)
442450
})
443451

444452
it('returns connected comments with empty comment line', () => {
445-
analyzer.analyze(CURRENT_URI, FIXTURES.COMMENT_DOC)
453+
analyzer.analyze({ uri: CURRENT_URI, document: FIXTURES.COMMENT_DOC })
446454
expect(analyzer.commentsAbove(CURRENT_URI, 51)).toEqual(
447455
'```txt\nthis is also included\n\ndoc for func_four\n```',
448456
)

Diff for: server/src/analyser.ts

+15-4
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,7 @@ export default class Analyzer {
3737
// We need this to find the word at a given point etc.
3838
private uriToFileContent: Texts = {}
3939
private uriToDeclarations: FileDeclarations = {}
40-
4140
private uriToSourcedUris: { [uri: string]: Set<string> } = {}
42-
4341
private treeSitterTypeToLSPKind: Kinds = {
4442
// These keys are using underscores as that's the naming convention in tree-sitter.
4543
environment_variable_assignment: LSP.SymbolKind.Variable,
@@ -124,7 +122,11 @@ export default class Analyzer {
124122
continue
125123
}
126124

127-
this.analyze(uri, TextDocument.create(uri, 'shell', 1, fileContent))
125+
this.analyze({
126+
document: TextDocument.create(uri, 'shell', 1, fileContent),
127+
rootPath,
128+
uri,
129+
})
128130
} catch (error) {
129131
const errorMessage = error instanceof Error ? error.message : error
130132
this.console.warn(
@@ -325,7 +327,15 @@ export default class Analyzer {
325327
* Returns all, if any, syntax errors that occurred while parsing the file.
326328
*
327329
*/
328-
public analyze(uri: string, document: TextDocument): LSP.Diagnostic[] {
330+
public analyze({
331+
document,
332+
rootPath,
333+
uri, // NOTE: we don't use document.uri to make testing easier
334+
}: {
335+
document: TextDocument
336+
rootPath?: string
337+
uri: string
338+
}): LSP.Diagnostic[] {
329339
const contents = document.getText()
330340

331341
const tree = this.parser.parse(contents)
@@ -339,6 +349,7 @@ export default class Analyzer {
339349
this.uriToSourcedUris[uri] = sourcing.getSourcedUris({
340350
fileContent: contents,
341351
fileUri: uri,
352+
rootPath,
342353
})
343354

344355
const problems: LSP.Diagnostic[] = []

Diff for: server/src/server.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ export default class BashServer {
245245
let diagnostics: LSP.Diagnostic[] = []
246246

247247
// Load the tree for the modified contents into the analyzer:
248-
const analyzeDiagnostics = this.analyzer.analyze(uri, document)
248+
const analyzeDiagnostics = this.analyzer.analyze({ uri, document })
249249
// Treesitter's diagnostics can be a bit inaccurate, so we only merge the
250250
// analyzer's diagnostics if the setting is enabled:
251251
if (this.config.highlightParsingErrors) {

Diff for: server/src/util/__tests__/sourcing.test.ts

+23-14
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,18 @@
1-
import { homedir } from 'os'
1+
import * as os from 'os'
22

33
import { getSourcedUris } from '../sourcing'
44

55
const fileDirectory = '/Users/bash'
66
const fileUri = `${fileDirectory}/file.sh`
77

8+
// mock fs.existsSync to always return true
9+
jest.mock('fs', () => ({
10+
existsSync: () => true,
11+
}))
12+
13+
// mock os.homedir() to return a fixed path
14+
jest.spyOn(os, 'homedir').mockImplementation(() => '/Users/bash-user')
15+
816
describe('getSourcedUris', () => {
917
it('returns an empty set if no files were sourced', () => {
1018
const result = getSourcedUris({ fileContent: '', fileUri })
@@ -17,17 +25,17 @@ describe('getSourcedUris', () => {
1725
1826
source file-in-path.sh # does not contain a slash (i.e. is maybe somewhere on the path)
1927
20-
source /bin/f.inc
28+
source /bin/extension.inc # absolute path
2129
2230
source ./x a b c # some arguments
2331
24-
. ./relative/to-this.sh
32+
. ../scripts/release-client.sh
2533
2634
source ~/myscript
2735
2836
# source ...
2937
30-
source "./my_quoted_file.sh"
38+
source "./issue206.sh" # quoted file in fixtures folder
3139
3240
source "$LIBPATH" # dynamic imports not supported
3341
@@ -36,15 +44,16 @@ describe('getSourcedUris', () => {
3644
`,
3745
fileUri,
3846
})
39-
expect(result).toEqual(
40-
new Set([
41-
`${fileDirectory}/file-in-path.sh`, // as we don't resolve it, we hope it is here
42-
`${fileDirectory}/bin/f.inc`,
43-
`${fileDirectory}/x`,
44-
`${fileDirectory}/relative/to-this.sh`,
45-
`${homedir()}/myscript`,
46-
`${fileDirectory}/my_quoted_file.sh`,
47-
]),
48-
)
47+
48+
expect(result).toMatchInlineSnapshot(`
49+
Set {
50+
"file:///Users/bash/file-in-path.sh",
51+
"file:///bin/extension.inc",
52+
"file:///Users/bash/x",
53+
"file:///Users/scripts/release-client.sh",
54+
"file:///Users/bash-user/myscript",
55+
"file:///Users/bash/issue206.sh",
56+
}
57+
`)
4958
})
5059
})

0 commit comments

Comments
 (0)