Skip to content

Commit 61621d2

Browse files
skovhusskovhus
committed
Add additional context in diagnostics
1 parent 784424f commit 61621d2

File tree

7 files changed

+144
-60
lines changed

7 files changed

+144
-60
lines changed

‎server/src/__tests__/__snapshots__/analyzer.test.ts.snap

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,5 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

3-
exports[`analyze returns a list of errors for a file with a missing node 1`] = `
4-
Array [
5-
Object {
6-
"message": "Syntax error: expected \\"fi\\" somewhere in the file",
7-
"range": Object {
8-
"end": Object {
9-
"character": 0,
10-
"line": 12,
11-
},
12-
"start": Object {
13-
"character": 0,
14-
"line": 12,
15-
},
16-
},
17-
"severity": 2,
18-
},
19-
]
20-
`;
21-
22-
exports[`analyze returns a list of errors for a file with parsing errors 1`] = `
23-
Array [
24-
Object {
25-
"message": "Failed to parse",
26-
"range": Object {
27-
"end": Object {
28-
"character": 1,
29-
"line": 9,
30-
},
31-
"start": Object {
32-
"character": 0,
33-
"line": 2,
34-
},
35-
},
36-
"severity": 1,
37-
},
38-
]
39-
`;
40-
413
exports[`findReferences returns a list of locations if parameter is found 1`] = `
424
Array [
435
Object {

‎server/src/__tests__/analyzer.test.ts

Lines changed: 91 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -36,30 +36,110 @@ beforeAll(async () => {
3636
})
3737

3838
describe('analyze', () => {
39-
it('returns an empty list of errors for a file with no parsing errors', () => {
40-
const result = analyzer.analyze({
39+
it('returns an empty list of diagnostics for a file with no parsing errors', () => {
40+
const diagnostics = analyzer.analyze({
4141
uri: CURRENT_URI,
4242
document: FIXTURE_DOCUMENT.INSTALL,
4343
})
44-
expect(result).toEqual([])
44+
expect(diagnostics).toEqual([])
4545
})
4646

47-
it('returns a list of errors for a file with a missing node', () => {
48-
const result = analyzer.analyze({
47+
it('returns a list of diagnostics for a file with a missing node', () => {
48+
const diagnostics = analyzer.analyze({
4949
uri: CURRENT_URI,
5050
document: FIXTURE_DOCUMENT.MISSING_NODE,
5151
})
52-
expect(result).not.toEqual([])
53-
expect(result).toMatchSnapshot()
52+
expect(diagnostics).not.toEqual([])
53+
expect(diagnostics).toMatchInlineSnapshot(`
54+
Array [
55+
Object {
56+
"message": "Syntax error: expected \\"fi\\" somewhere in the file",
57+
"range": Object {
58+
"end": Object {
59+
"character": 0,
60+
"line": 12,
61+
},
62+
"start": Object {
63+
"character": 0,
64+
"line": 12,
65+
},
66+
},
67+
"severity": 2,
68+
"source": "bash-language-server",
69+
},
70+
]
71+
`)
5472
})
5573

56-
it('returns a list of errors for a file with parsing errors', () => {
57-
const result = analyzer.analyze({
74+
it('returns a list of diagnostics for a file with parsing errors', () => {
75+
const diagnostics = analyzer.analyze({
5876
uri: CURRENT_URI,
5977
document: FIXTURE_DOCUMENT.PARSE_PROBLEMS,
6078
})
61-
expect(result).not.toEqual([])
62-
expect(result).toMatchSnapshot()
79+
expect(diagnostics).not.toEqual([])
80+
expect(diagnostics).toMatchInlineSnapshot(`
81+
Array [
82+
Object {
83+
"message": "Failed to parse",
84+
"range": Object {
85+
"end": Object {
86+
"character": 1,
87+
"line": 9,
88+
},
89+
"start": Object {
90+
"character": 0,
91+
"line": 2,
92+
},
93+
},
94+
"severity": 1,
95+
"source": "bash-language-server",
96+
},
97+
]
98+
`)
99+
})
100+
101+
it('returns a list of diagnostics for a file with sourcing issues', async () => {
102+
const parser = await initializeParser()
103+
const newAnalyzer = new Analyzer({
104+
parser,
105+
workspaceFolder: FIXTURE_FOLDER,
106+
})
107+
const diagnostics = newAnalyzer.analyze({
108+
uri: CURRENT_URI,
109+
document: FIXTURE_DOCUMENT.SOURCING,
110+
})
111+
expect(diagnostics).not.toEqual([])
112+
expect(diagnostics).toMatchInlineSnapshot(`
113+
Array [
114+
Object {
115+
"message": "Source command could not be analyzed: expansion not supported.
116+
117+
Note that enabling the configuration flag \\"includeAllWorkspaceSymbols\\"
118+
would include all symbols in the workspace regardless of source commands.
119+
But be aware that this will lead to false positive suggestions.",
120+
"range": Object {
121+
"end": Object {
122+
"character": 16,
123+
"line": 21,
124+
},
125+
"start": Object {
126+
"character": 2,
127+
"line": 21,
128+
},
129+
},
130+
"severity": 3,
131+
"source": "bash-language-server",
132+
},
133+
]
134+
`)
135+
136+
// unless setIncludeAllWorkspaceSymbols set
137+
newAnalyzer.setIncludeAllWorkspaceSymbols(true)
138+
const diagnostics2 = newAnalyzer.analyze({
139+
uri: CURRENT_URI,
140+
document: FIXTURE_DOCUMENT.SOURCING,
141+
})
142+
expect(diagnostics2).toEqual([])
63143
})
64144
})
65145

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,23 @@ describe('server', () => {
208208
},
209209
"name": "BOLD",
210210
},
211+
Object {
212+
"kind": 12,
213+
"location": Object {
214+
"range": Object {
215+
"end": Object {
216+
"character": 1,
217+
"line": 22,
218+
},
219+
"start": Object {
220+
"character": 0,
221+
"line": 20,
222+
},
223+
},
224+
"uri": "file://${FIXTURE_FOLDER}sourcing.sh",
225+
},
226+
"name": "loadlib",
227+
},
211228
]
212229
`)
213230
})

‎server/src/analyser.ts

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -96,17 +96,26 @@ export default class Analyzer {
9696
tree,
9797
}
9898

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-
})
99+
if (!this.includeAllWorkspaceSymbols) {
100+
sourceCommands
101+
.filter((sourceCommand) => sourceCommand.error)
102+
.forEach((sourceCommand) => {
103+
diagnostics.push(
104+
LSP.Diagnostic.create(
105+
sourceCommand.range,
106+
[
107+
`Source command could not be analyzed: ${sourceCommand.error}.\n`,
108+
'Note that enabling the configuration flag "includeAllWorkspaceSymbols"',
109+
'would include all symbols in the workspace regardless of source commands.',
110+
'But be aware that this will lead to false positive suggestions.',
111+
].join('\n'),
112+
LSP.DiagnosticSeverity.Information,
113+
undefined,
114+
'bash-language-server',
115+
),
116+
)
117+
})
118+
}
110119

111120
function findMissingNodes(node: Parser.SyntaxNode) {
112121
if (node.isMissing()) {
@@ -115,6 +124,8 @@ export default class Analyzer {
115124
TreeSitterUtil.range(node),
116125
`Syntax error: expected "${node.type}" somewhere in the file`,
117126
LSP.DiagnosticSeverity.Warning,
127+
undefined,
128+
'bash-language-server',
118129
),
119130
)
120131
} else if (node.hasError()) {

‎server/src/util/declarations.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ export function getGlobalDeclarations({
4646
TreeSitterUtil.range(node),
4747
'Failed to parse',
4848
LSP.DiagnosticSeverity.Error,
49+
undefined,
50+
'bash-language-server',
4951
),
5052
)
5153
return

‎server/src/util/sourcing.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,12 @@ function getSourcedPathInfoFromNode({
6868
}
6969
}
7070

71+
if (argumentNode.type === 'simple_expansion') {
72+
return {
73+
parseError: 'expansion not supported',
74+
}
75+
}
76+
7177
if (argumentNode.type === 'string') {
7278
if (argumentNode.namedChildren.length === 0) {
7379
return {

‎testing/fixtures/sourcing.sh

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,3 +17,9 @@ echo "$"
1717
source ./scripts/tag-release.inc
1818

1919
tagRelease '1.0.0'
20+
21+
loadlib () {
22+
source "$1.sh"
23+
}
24+
25+
loadlib "issue206"

0 commit comments

Comments
 (0)