command/views/json: Never generate invalid diagnostic snippet offsets
Because our snippet generator is trying to select whole lines to include in the snippet, it has some edge cases for odd situations where the relevant source range starts or ends directly at a newline, which were previously causing this logic to return out-of-bounds offsets into the code snippet string. Although arguably it'd be better for the original diagnostics to report more reasonable source ranges, it's better for us to report a slightly-inaccurate snippet than to crash altogether, and so we'll extend our existing range checks to check both bounds of the string and thus avoid downstreams having to deal with out-of-bounds indices. For completeness here I also added some similar logic to the human-oriented diagnostic formatter, which consumes the result of the JSON diagnostic builder. That's not really needed with the additional checks in the JSON diagnostic builder, but it's nice to reinforce that this code can't panic (in this way, at least) even if its input isn't valid.
This commit is contained in:
parent
c687ebeaf1
commit
70bc432f85
|
@ -250,6 +250,21 @@ func appendSourceSnippets(buf *bytes.Buffer, diag *viewsjson.Diagnostic, color *
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// If either start or end is out of range for the code buffer then
|
||||||
|
// we'll cap them at the bounds just to avoid a panic, although
|
||||||
|
// this would happen only if there's a bug in the code generating
|
||||||
|
// the snippet objects.
|
||||||
|
if start < 0 {
|
||||||
|
start = 0
|
||||||
|
} else if start > len(code) {
|
||||||
|
start = len(code)
|
||||||
|
}
|
||||||
|
if end < 0 {
|
||||||
|
end = 0
|
||||||
|
} else if end > len(code) {
|
||||||
|
end = len(code)
|
||||||
|
}
|
||||||
|
|
||||||
before, highlight, after := code[0:start], code[start:end], code[end:]
|
before, highlight, after := code[0:start], code[start:end], code[end:]
|
||||||
code = fmt.Sprintf(color.Color("%s[underline]%s[reset]%s"), before, highlight, after)
|
code = fmt.Sprintf(color.Color("%s[underline]%s[reset]%s"), before, highlight, after)
|
||||||
|
|
||||||
|
|
|
@ -221,12 +221,23 @@ func NewDiagnostic(diag tfdiags.Diagnostic, sources map[string][]byte) *Diagnost
|
||||||
// to the code snippet string.
|
// to the code snippet string.
|
||||||
start := highlightRange.Start.Byte - codeStartByte
|
start := highlightRange.Start.Byte - codeStartByte
|
||||||
end := start + (highlightRange.End.Byte - highlightRange.Start.Byte)
|
end := start + (highlightRange.End.Byte - highlightRange.Start.Byte)
|
||||||
if start > len(codeStr) {
|
|
||||||
|
// We can end up with some quirky results here in edge cases like
|
||||||
|
// when a source range starts or ends at a newline character,
|
||||||
|
// so we'll cap the results at the bounds of the highlight range
|
||||||
|
// so that consumers of this data don't need to contend with
|
||||||
|
// out-of-bounds errors themselves.
|
||||||
|
if start < 0 {
|
||||||
|
start = 0
|
||||||
|
} else if start > len(codeStr) {
|
||||||
start = len(codeStr)
|
start = len(codeStr)
|
||||||
}
|
}
|
||||||
if end > len(codeStr) {
|
if end < 0 {
|
||||||
|
end = 0
|
||||||
|
} else if end > len(codeStr) {
|
||||||
end = len(codeStr)
|
end = len(codeStr)
|
||||||
}
|
}
|
||||||
|
|
||||||
diagnostic.Snippet.HighlightStartOffset = start
|
diagnostic.Snippet.HighlightStartOffset = start
|
||||||
diagnostic.Snippet.HighlightEndOffset = end
|
diagnostic.Snippet.HighlightEndOffset = end
|
||||||
|
|
||||||
|
|
|
@ -30,6 +30,7 @@ func TestNewDiagnostic(t *testing.T) {
|
||||||
}
|
}
|
||||||
`),
|
`),
|
||||||
"short.tf": []byte("bad source code"),
|
"short.tf": []byte("bad source code"),
|
||||||
|
"odd-comment.tf": []byte("foo\n\n#\n"),
|
||||||
"values.tf": []byte(`[
|
"values.tf": []byte(`[
|
||||||
var.a,
|
var.a,
|
||||||
var.b,
|
var.b,
|
||||||
|
@ -285,6 +286,51 @@ func TestNewDiagnostic(t *testing.T) {
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
|
"error whose range starts at a newline": {
|
||||||
|
&hcl.Diagnostic{
|
||||||
|
Severity: hcl.DiagError,
|
||||||
|
Summary: "Invalid newline",
|
||||||
|
Detail: "How awkward!",
|
||||||
|
Subject: &hcl.Range{
|
||||||
|
Filename: "odd-comment.tf",
|
||||||
|
Start: hcl.Pos{Line: 2, Column: 5, Byte: 4},
|
||||||
|
End: hcl.Pos{Line: 3, Column: 1, Byte: 6},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
&Diagnostic{
|
||||||
|
Severity: "error",
|
||||||
|
Summary: "Invalid newline",
|
||||||
|
Detail: "How awkward!",
|
||||||
|
Range: &DiagnosticRange{
|
||||||
|
Filename: "odd-comment.tf",
|
||||||
|
Start: Pos{
|
||||||
|
Line: 2,
|
||||||
|
Column: 5,
|
||||||
|
Byte: 4,
|
||||||
|
},
|
||||||
|
End: Pos{
|
||||||
|
Line: 3,
|
||||||
|
Column: 1,
|
||||||
|
Byte: 6,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
Snippet: &DiagnosticSnippet{
|
||||||
|
Code: `#`,
|
||||||
|
StartLine: 2,
|
||||||
|
Values: []DiagnosticExpressionValue{},
|
||||||
|
|
||||||
|
// Due to the range starting at a newline on a blank
|
||||||
|
// line, we end up stripping off the initial newline
|
||||||
|
// to produce only a one-line snippet. That would
|
||||||
|
// therefore cause the start offset to naturally be
|
||||||
|
// -1, just before the Code we returned, but then we
|
||||||
|
// force it to zero so that the result will still be
|
||||||
|
// in range for a byte-oriented slice of Code.
|
||||||
|
HighlightStartOffset: 0,
|
||||||
|
HighlightEndOffset: 1,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
"error with source code subject and known expression": {
|
"error with source code subject and known expression": {
|
||||||
&hcl.Diagnostic{
|
&hcl.Diagnostic{
|
||||||
Severity: hcl.DiagError,
|
Severity: hcl.DiagError,
|
||||||
|
|
26
internal/command/views/json/testdata/diagnostic/error-whose-range-starts-at-a-newline.json
vendored
Normal file
26
internal/command/views/json/testdata/diagnostic/error-whose-range-starts-at-a-newline.json
vendored
Normal file
|
@ -0,0 +1,26 @@
|
||||||
|
{
|
||||||
|
"severity": "error",
|
||||||
|
"summary": "Invalid newline",
|
||||||
|
"detail": "How awkward!",
|
||||||
|
"range": {
|
||||||
|
"filename": "odd-comment.tf",
|
||||||
|
"start": {
|
||||||
|
"line": 2,
|
||||||
|
"column": 5,
|
||||||
|
"byte": 4
|
||||||
|
},
|
||||||
|
"end": {
|
||||||
|
"line": 3,
|
||||||
|
"column": 1,
|
||||||
|
"byte": 6
|
||||||
|
}
|
||||||
|
},
|
||||||
|
"snippet": {
|
||||||
|
"context": null,
|
||||||
|
"code": "#",
|
||||||
|
"start_line": 2,
|
||||||
|
"highlight_start_offset": 0,
|
||||||
|
"highlight_end_offset": 1,
|
||||||
|
"values": []
|
||||||
|
}
|
||||||
|
}
|
Loading…
Reference in New Issue