Skip to content

Commit fe63f3d

Browse files
julienduchesnejulienduchesne
authored
Fix diagnostics line parsing (#50)
Fixes #49 The parsing was all wrong in some cases and was quite unsafe. This should no longer panic and it fixes a few issues with parsing All tested now, too
1 parent 4a6d376 commit fe63f3d

File tree

2 files changed

+168
-51
lines changed

2 files changed

+168
-51
lines changed

‎pkg/server/diagnostics.go‎

Lines changed: 58 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,62 @@ import (
1717

1818
var (
1919
// errRegexp matches the various Jsonnet location formats in errors.
20-
// file:line msg
21-
// file:line:col-endCol msg
22-
// file:(line:endLine)-(col:endCol) msg
23-
// Has 10 matching groups.
24-
errRegexp = regexp.MustCompile(`/.*:(?:(\d+)|(?:(\d+):(\d+)-(\d+))|(?:\((\d+):(\d+)\)-\((\d+):(\d+))\))\s(.*)`)
20+
// 1. file:line msg
21+
// 2. file:line:col msg
22+
// 3. file:line:col-endCol msg
23+
// 4. file:(line:col)-(endLine:endCol) msg
24+
// https://regex101.com/r/tL5VWi/2
25+
errRegexp = regexp.MustCompile(`/[^:]*:` +
26+
`(?:(?P<startLine1>\d+)` +
27+
`|(?P<startLine2>\d+):(?P<startCol2>\d+)` +
28+
`|(?:(?P<startLine3>\d+):(?P<startCol3>\d+)-(?P<endCol3>\d+))` +
29+
`|(?:\((?P<startLine4>\d+):(?P<startCol4>\d+)\)-\((?P<endLine4>\d+):(?P<endCol4>\d+))\))` +
30+
`\s(?P<message>.*)`)
2531
)
2632

33+
func parseErrRegexpMatch(match []string) (string, protocol.Range) {
34+
get := func(name string) string {
35+
idx := errRegexp.SubexpIndex(name)
36+
if len(match) <= idx {
37+
return ""
38+
}
39+
return match[idx]
40+
}
41+
42+
message, line, col, endLine, endCol := "", 1, 1, 1, 1
43+
if len(match) > 1 {
44+
if lineStr := get("startLine1"); lineStr != "" {
45+
line, _ = strconv.Atoi(lineStr)
46+
endLine = line
47+
}
48+
49+
if lineStr := get("startLine2"); lineStr != "" {
50+
line, _ = strconv.Atoi(lineStr)
51+
endLine = line
52+
col, _ = strconv.Atoi(get("startCol2"))
53+
endCol = col
54+
}
55+
56+
if lineStr := get("startLine3"); lineStr != "" {
57+
line, _ = strconv.Atoi(lineStr)
58+
endLine = line
59+
col, _ = strconv.Atoi(get("startCol3"))
60+
endCol, _ = strconv.Atoi(get("endCol3"))
61+
}
62+
63+
if lineStr := get("startLine4"); lineStr != "" {
64+
line, _ = strconv.Atoi(lineStr)
65+
endLine, _ = strconv.Atoi(get("endLine4"))
66+
col, _ = strconv.Atoi(get("startCol4"))
67+
endCol, _ = strconv.Atoi(get("endCol4"))
68+
}
69+
70+
message = get("message")
71+
}
72+
73+
return message, position.NewProtocolRange(line-1, col-1, endLine-1, endCol-1)
74+
}
75+
2776
func (s *server) queueDiagnostics(uri protocol.DocumentURI) {
2877
s.cache.diagMutex.Lock()
2978
defer s.cache.diagMutex.Unlock()
@@ -109,9 +158,7 @@ func (s *server) getEvalDiags(doc *document) (diags []protocol.Diagnostic) {
109158
doc.val, doc.err = vm.EvaluateAnonymousSnippet(doc.item.URI.SpanURI().Filename(), doc.item.Text)
110159
}
111160

112-
// Initialize with 1 because we indiscriminately subtract one to map error ranges to LSP ranges.
113161
if doc.err != nil {
114-
line, col, endLine, endCol := 1, 1, 1, 1
115162
diag := protocol.Diagnostic{Source: "jsonnet evaluation"}
116163
lines := strings.Split(doc.err.Error(), "\n")
117164
if len(lines) == 0 {
@@ -127,34 +174,17 @@ func (s *server) getEvalDiags(doc *document) (diags []protocol.Diagnostic) {
127174
} else {
128175
match = errRegexp.FindStringSubmatch(lines[0])
129176
}
130-
if len(match) == 10 {
131-
if match[1] != "" {
132-
line, _ = strconv.Atoi(match[1])
133-
endLine = line + 1
134-
}
135-
if match[2] != "" {
136-
line, _ = strconv.Atoi(match[2])
137-
col, _ = strconv.Atoi(match[3])
138-
endLine = line
139-
endCol, _ = strconv.Atoi(match[4])
140-
}
141-
if match[5] != "" {
142-
line, _ = strconv.Atoi(match[5])
143-
col, _ = strconv.Atoi(match[6])
144-
endLine, _ = strconv.Atoi(match[7])
145-
endCol, _ = strconv.Atoi(match[8])
146-
}
147-
}
148177

178+
message, rang := parseErrRegexpMatch(match)
149179
if runtimeErr {
150180
diag.Message = doc.err.Error()
151181
diag.Severity = protocol.SeverityWarning
152182
} else {
153-
diag.Message = match[9]
183+
diag.Message = message
154184
diag.Severity = protocol.SeverityError
155185
}
156186

157-
diag.Range = position.NewProtocolRange(line-1, col-1, endLine-1, endCol-1)
187+
diag.Range = rang
158188
diags = append(diags, diag)
159189
}
160190

@@ -167,31 +197,8 @@ func (s *server) getLintDiags(doc *document) (diags []protocol.Diagnostic) {
167197
log.Errorf("getLintDiags: %s: %v\n", errorRetrievingDocument, err)
168198
} else {
169199
for _, match := range errRegexp.FindAllStringSubmatch(result, -1) {
170-
line, col, endLine, endCol := 1, 1, 1, 1
171200
diag := protocol.Diagnostic{Source: "lint", Severity: protocol.SeverityWarning}
172-
173-
if len(match) == 10 {
174-
if match[1] != "" {
175-
line, _ = strconv.Atoi(match[1])
176-
endLine = line + 1
177-
}
178-
if match[2] != "" {
179-
line, _ = strconv.Atoi(match[2])
180-
col, _ = strconv.Atoi(match[3])
181-
endLine = line
182-
endCol, _ = strconv.Atoi(match[4])
183-
}
184-
if match[5] != "" {
185-
line, _ = strconv.Atoi(match[5])
186-
col, _ = strconv.Atoi(match[6])
187-
endLine, _ = strconv.Atoi(match[7])
188-
endCol, _ = strconv.Atoi(match[8])
189-
}
190-
}
191-
192-
diag.Message = match[9]
193-
194-
diag.Range = position.NewProtocolRange(line-1, col-1, endLine-1, endCol-1)
201+
diag.Message, diag.Range = parseErrRegexpMatch(match)
195202
diags = append(diags, diag)
196203
}
197204
}

‎pkg/server/diagnostics_test.go‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,28 @@ func TestGetLintDiags(t *testing.T) {
1313
fileContent string
1414
expected []protocol.Diagnostic
1515
}{
16+
{
17+
name: "no error",
18+
fileContent: `{}`,
19+
},
20+
{
21+
name: "invalid function call",
22+
fileContent: `function(notPassed) {
23+
this: 'is wrong',
24+
}()
25+
`,
26+
expected: []protocol.Diagnostic{
27+
{
28+
Range: protocol.Range{
29+
Start: protocol.Position{Line: 0, Character: 20},
30+
End: protocol.Position{Line: 2, Character: 3},
31+
},
32+
Severity: protocol.SeverityWarning,
33+
Source: "lint",
34+
Message: "Called value must be a function, but it is assumed to be an object",
35+
},
36+
},
37+
},
1638
{
1739
name: "unused variable",
1840
fileContent: `
@@ -45,3 +67,91 @@ local unused = 'test';
4567
})
4668
}
4769
}
70+
71+
func TestGetEvalDiags(t *testing.T) {
72+
testCases := []struct {
73+
name string
74+
fileContent string
75+
expected []protocol.Diagnostic
76+
}{
77+
{
78+
name: "no error",
79+
fileContent: `{}`,
80+
},
81+
{
82+
name: "syntax error 1",
83+
fileContent: `{ s }`,
84+
expected: []protocol.Diagnostic{
85+
{
86+
Range: protocol.Range{
87+
Start: protocol.Position{Line: 0, Character: 4},
88+
End: protocol.Position{Line: 0, Character: 5},
89+
},
90+
Severity: protocol.SeverityError,
91+
Source: "jsonnet evaluation",
92+
Message: `Expected token OPERATOR but got "}"`,
93+
},
94+
},
95+
},
96+
{
97+
name: "syntax error 2",
98+
fileContent: `{ s: }`,
99+
expected: []protocol.Diagnostic{
100+
{
101+
Range: protocol.Range{
102+
Start: protocol.Position{Line: 0, Character: 5},
103+
End: protocol.Position{Line: 0, Character: 6},
104+
},
105+
Severity: protocol.SeverityError,
106+
Source: "jsonnet evaluation",
107+
Message: `Unexpected: "}" while parsing terminal`,
108+
},
109+
},
110+
},
111+
{
112+
name: "syntax error 3",
113+
fileContent: `{`,
114+
expected: []protocol.Diagnostic{
115+
{
116+
Range: protocol.Range{
117+
Start: protocol.Position{Line: 0, Character: 1},
118+
End: protocol.Position{Line: 0, Character: 1},
119+
},
120+
Severity: protocol.SeverityError,
121+
Source: "jsonnet evaluation",
122+
Message: `Unexpected: end of file while parsing field definition`,
123+
},
124+
},
125+
},
126+
{
127+
name: "syntax error 4",
128+
fileContent: `{
129+
s: |||
130+
|||
131+
}`,
132+
expected: []protocol.Diagnostic{
133+
{
134+
Range: protocol.Range{
135+
Start: protocol.Position{Line: 1, Character: 7},
136+
End: protocol.Position{Line: 1, Character: 7},
137+
},
138+
Severity: protocol.SeverityError,
139+
Source: "jsonnet evaluation",
140+
Message: `Text block's first line must start with whitespace`,
141+
},
142+
},
143+
},
144+
}
145+
for _, tc := range testCases {
146+
t.Run(tc.name, func(t *testing.T) {
147+
s, fileURI := testServerWithFile(t, nil, tc.fileContent)
148+
doc, err := s.cache.get(fileURI)
149+
if err != nil {
150+
t.Fatalf("%s: %v", errorRetrievingDocument, err)
151+
}
152+
153+
diags := s.getEvalDiags(doc)
154+
assert.Equal(t, tc.expected, diags)
155+
})
156+
}
157+
}

0 commit comments

Comments
 (0)