Skip to content

Commit 409a498

Browse files
skovhusskovhus
committed
Improve sourcing diagnostics
1 parent 65fb035 commit 409a498

File tree

5 files changed

+281
-114
lines changed

5 files changed

+281
-114
lines changed

‎server/src/analyser.ts

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
} from './util/declarations'
1818
import { getFilePaths } from './util/fs'
1919
import { logger } from './util/logger'
20+
import { isPositionIncludedInRange } from './util/lsp'
2021
import { analyzeShebang } from './util/shebang'
2122
import * as sourcing from './util/sourcing'
2223
import * as TreeSitterUtil from './util/tree-sitter'
@@ -27,6 +28,7 @@ type AnalyzedDocument = {
2728
document: TextDocument
2829
globalDeclarations: GlobalDeclarations
2930
sourcedUris: Set<string>
31+
sourceCommands: sourcing.SourceCommand[]
3032
tree: Parser.Tree
3133
}
3234

@@ -74,17 +76,38 @@ export default class Analyzer {
7476

7577
const { diagnostics, globalDeclarations } = getGlobalDeclarations({ tree, uri })
7678

79+
const sourceCommands = sourcing.getSourceCommands({
80+
fileUri: uri,
81+
rootPath: this.workspaceFolder,
82+
tree,
83+
})
84+
85+
const sourcedUris = new Set(
86+
sourceCommands
87+
.map((sourceCommand) => sourceCommand.uri)
88+
.filter((uri): uri is string => uri !== null),
89+
)
90+
7791
this.uriToAnalyzedDocument[uri] = {
7892
document,
7993
globalDeclarations,
80-
sourcedUris: sourcing.getSourcedUris({
81-
fileUri: uri,
82-
rootPath: this.workspaceFolder,
83-
tree,
84-
}),
94+
sourcedUris,
95+
sourceCommands: sourceCommands.filter((sourceCommand) => !sourceCommand.error),
8596
tree,
8697
}
8798

99+
sourceCommands
100+
.filter((sourceCommand) => sourceCommand.error)
101+
.forEach((sourceCommand) => {
102+
diagnostics.push(
103+
LSP.Diagnostic.create(
104+
sourceCommand.range,
105+
`Source command could not be analyzed: ${sourceCommand.error}.`,
106+
LSP.DiagnosticSeverity.Information,
107+
),
108+
)
109+
})
110+
88111
function findMissingNodes(node: Parser.SyntaxNode) {
89112
if (node.isMissing()) {
90113
diagnostics.push(
@@ -197,20 +220,13 @@ export default class Analyzer {
197220
uri: string
198221
word: string
199222
}): LSP.Location[] {
200-
const tree = this.uriToAnalyzedDocument[uri]?.tree
223+
// If the word is sourced, return the location of the source file
224+
const sourcedUri = this.uriToAnalyzedDocument[uri]?.sourceCommands
225+
.filter((sourceCommand) => isPositionIncludedInRange(position, sourceCommand.range))
226+
.map((sourceCommand) => sourceCommand.uri)[0]
201227

202-
if (position && tree) {
203-
// NOTE: when a word is a file path to a sourced file, we return a location to it.
204-
const sourcedLocation = sourcing.getSourcedLocation({
205-
position,
206-
rootPath: this.workspaceFolder,
207-
tree,
208-
uri,
209-
word,
210-
})
211-
if (sourcedLocation) {
212-
return [sourcedLocation]
213-
}
228+
if (sourcedUri) {
229+
return [LSP.Location.create(sourcedUri, LSP.Range.create(0, 0, 0, 0))]
214230
}
215231

216232
return this.findDeclarationsMatchingWord({

‎server/src/util/__tests__/__snapshots__/sourcing.test.ts.snap

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`getSourcedUris returns a set of sourced files 2`] = `
4+
Array [
5+
Object {
6+
"error": null,
7+
"range": Object {
8+
"end": Object {
9+
"character": 28,
10+
"line": 1,
11+
},
12+
"start": Object {
13+
"character": 6,
14+
"line": 1,
15+
},
16+
},
17+
"uri": "file:///Users/bash/file-in-path.sh",
18+
},
19+
Object {
20+
"error": null,
21+
"range": Object {
22+
"end": Object {
23+
"character": 31,
24+
"line": 3,
25+
},
26+
"start": Object {
27+
"character": 6,
28+
"line": 3,
29+
},
30+
},
31+
"uri": "file:///bin/extension.inc",
32+
},
33+
Object {
34+
"error": null,
35+
"range": Object {
36+
"end": Object {
37+
"character": 22,
38+
"line": 5,
39+
},
40+
"start": Object {
41+
"character": 6,
42+
"line": 5,
43+
},
44+
},
45+
"uri": "file:///Users/bash/x",
46+
},
47+
Object {
48+
"error": null,
49+
"range": Object {
50+
"end": Object {
51+
"character": 36,
52+
"line": 7,
53+
},
54+
"start": Object {
55+
"character": 6,
56+
"line": 7,
57+
},
58+
},
59+
"uri": "file:///Users/scripts/release-client.sh",
60+
},
61+
Object {
62+
"error": null,
63+
"range": Object {
64+
"end": Object {
65+
"character": 23,
66+
"line": 9,
67+
},
68+
"start": Object {
69+
"character": 6,
70+
"line": 9,
71+
},
72+
},
73+
"uri": "file:///Users/bash-user/myscript",
74+
},
75+
Object {
76+
"error": "expansion not supported",
77+
"range": Object {
78+
"end": Object {
79+
"character": 23,
80+
"line": 13,
81+
},
82+
"start": Object {
83+
"character": 6,
84+
"line": 13,
85+
},
86+
},
87+
"uri": null,
88+
},
89+
Object {
90+
"error": "expansion not supported",
91+
"range": Object {
92+
"end": Object {
93+
"character": 66,
94+
"line": 16,
95+
},
96+
"start": Object {
97+
"character": 49,
98+
"line": 16,
99+
},
100+
},
101+
"uri": null,
102+
},
103+
Object {
104+
"error": "expansion not supported",
105+
"range": Object {
106+
"end": Object {
107+
"character": 66,
108+
"line": 18,
109+
},
110+
"start": Object {
111+
"character": 6,
112+
"line": 18,
113+
},
114+
},
115+
"uri": null,
116+
},
117+
Object {
118+
"error": null,
119+
"range": Object {
120+
"end": Object {
121+
"character": 30,
122+
"line": 25,
123+
},
124+
"start": Object {
125+
"character": 8,
126+
"line": 25,
127+
},
128+
},
129+
"uri": "file:///Users/bash/issue206.sh",
130+
},
131+
Object {
132+
"error": "expansion not supported",
133+
"range": Object {
134+
"end": Object {
135+
"character": 22,
136+
"line": 43,
137+
},
138+
"start": Object {
139+
"character": 8,
140+
"line": 43,
141+
},
142+
},
143+
"uri": null,
144+
},
145+
]
146+
`;

‎server/src/util/__tests__/sourcing.test.ts

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import * as os from 'os'
33
import * as Parser from 'web-tree-sitter'
44

55
import { initializeParser } from '../../parser'
6-
import { getSourcedUris } from '../sourcing'
6+
import { getSourceCommands } from '../sourcing'
77

88
const fileDirectory = '/Users/bash'
99
const fileUri = `${fileDirectory}/file.sh`
@@ -21,12 +21,12 @@ jest.spyOn(os, 'homedir').mockImplementation(() => '/Users/bash-user')
2121
describe('getSourcedUris', () => {
2222
it('returns an empty set if no files were sourced', () => {
2323
const fileContent = ''
24-
const result = getSourcedUris({
24+
const sourceCommands = getSourceCommands({
2525
fileUri,
2626
rootPath: null,
2727
tree: parser.parse(fileContent),
2828
})
29-
expect(result).toEqual(new Set([]))
29+
expect(sourceCommands).toEqual([])
3030
})
3131

3232
it('returns a set of sourced files', () => {
@@ -72,15 +72,26 @@ describe('getSourcedUris', () => {
7272
fi
7373
done
7474
75+
loadlib () {
76+
source "$1.sh"
77+
}
78+
79+
loadlib "issue101"
7580
`
7681

77-
const result = getSourcedUris({
82+
const sourceCommands = getSourceCommands({
7883
fileUri,
7984
rootPath: null,
8085
tree: parser.parse(fileContent),
8186
})
8287

83-
expect(result).toMatchInlineSnapshot(`
88+
const sourcedUris = new Set(
89+
sourceCommands
90+
.map((sourceCommand) => sourceCommand.uri)
91+
.filter((uri) => uri !== null),
92+
)
93+
94+
expect(sourcedUris).toMatchInlineSnapshot(`
8495
Set {
8596
"file:///Users/bash/file-in-path.sh",
8697
"file:///bin/extension.inc",
@@ -90,5 +101,7 @@ describe('getSourcedUris', () => {
90101
"file:///Users/bash/issue206.sh",
91102
}
92103
`)
104+
105+
expect(sourceCommands).toMatchSnapshot()
93106
})
94107
})

‎server/src/util/lsp.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import * as LSP from 'vscode-languageserver/node'
2+
3+
/**
4+
* Determine if a position is included in a range.
5+
*/
6+
export function isPositionIncludedInRange(position: LSP.Position, range: LSP.Range) {
7+
return (
8+
range.start.line <= position.line &&
9+
range.end.line >= position.line &&
10+
range.start.character <= position.character &&
11+
range.end.character >= position.character
12+
)
13+
}

0 commit comments

Comments
 (0)