Merge pull request #28598 from hashicorp/alisdair/fix-diagnostic-snippet-crash

cli: Fix diagnostic snippet crash

command/format/diagnostic.go

@@ -240,6 +240,16 @@ func appendSourceSnippets(buf *bytes.Buffer, diag *viewsjson.Diagnostic, color *
 		// Split the snippet and render the highlighted section with underlines
 		start := snippet.HighlightStartOffset
 		end := snippet.HighlightEndOffset
+
+		// Only buggy diagnostics can have an end range before the start, but
+		// we need to ensure we don't crash here if that happens.
+		if end < start {
+			end = start + 1
+			if end > len(code) {
+				end = len(code)
+			}
+		}
+
 		before, highlight, after := code[0:start], code[start:end], code[end:]
 		code = fmt.Sprintf(color.Color("%s[underline]%s[reset]%s"), before, highlight, after)
 

command/format/diagnostic_test.go

@@ -10,6 +10,8 @@ import (
 	"github.com/mitchellh/colorstring"
 	"github.com/zclconf/go-cty/cty"
 
+	viewsjson "github.com/hashicorp/terraform/command/views/json"
+
 	"github.com/hashicorp/terraform/tfdiags"
 )
 
@@ -699,3 +701,56 @@ eventually make it onto multiple lines. THE END
 		t.Fatalf("unexpected output: got:\n%s\nwant\n%s\n", output, expected)
 	}
 }
+
+// Test cases covering invalid JSON diagnostics which should still render
+// correctly. These JSON diagnostic values cannot be generated from the
+// json.NewDiagnostic code path, but we may read and display JSON diagnostics
+// in future from other sources.
+func TestDiagnosticFromJSON_invalid(t *testing.T) {
+	tests := map[string]struct {
+		Diag *viewsjson.Diagnostic
+		Want string
+	}{
+		"zero-value end range and highlight end byte": {
+			&viewsjson.Diagnostic{
+				Severity: viewsjson.DiagnosticSeverityError,
+				Summary:  "Bad end",
+				Detail:   "It all went wrong.",
+				Range: &viewsjson.DiagnosticRange{
+					Filename: "ohno.tf",
+					Start:    viewsjson.Pos{Line: 1, Column: 23, Byte: 22},
+					End:      viewsjson.Pos{Line: 0, Column: 0, Byte: 0},
+				},
+				Snippet: &viewsjson.DiagnosticSnippet{
+					Code:                 `resource "foo_bar "baz" {`,
+					StartLine:            1,
+					HighlightStartOffset: 22,
+					HighlightEndOffset:   0,
+				},
+			},
+			`[red]╷[reset]
+[red]│[reset] [bold][red]Error: [reset][bold]Bad end[reset]
+[red]│[reset]
+[red]│[reset]   on ohno.tf line 1:
+[red]│[reset]    1: resource "foo_bar "baz[underline]"[reset] {
+[red]│[reset]
+[red]│[reset] It all went wrong.
+[red]╵[reset]
+`,
+		},
+	}
+
+	// This empty Colorize just passes through all of the formatting codes
+	// untouched, because it doesn't define any formatting keywords.
+	colorize := &colorstring.Colorize{}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			got := strings.TrimSpace(DiagnosticFromJSON(test.Diag, colorize, 40))
+			want := strings.TrimSpace(test.Want)
+			if got != want {
+				t.Errorf("wrong result\ngot:\n%s\n\nwant:\n%s\n\n", got, want)
+			}
+		})
+	}
+}

command/views/json/diagnostic.go

@@ -133,6 +133,12 @@ func NewDiagnostic(diag tfdiags.Diagnostic, sources map[string][]byte) *Diagnost
 		// We'll borrow HCL's range implementation here, because it has some
 		// handy features to help us produce a nice source code snippet.
 		highlightRange := sourceRefs.Subject.ToHCL()
+
+		// Some diagnostic sources fail to set the end of the subject range.
+		if highlightRange.End == (hcl.Pos{}) {
+			highlightRange.End = highlightRange.Start
+		}
+
 		snippetRange := highlightRange
 		if sourceRefs.Context != nil {
 			snippetRange = sourceRefs.Context.ToHCL()

command/views/json/diagnostic_test.go

@@ -246,6 +246,44 @@ func TestNewDiagnostic(t *testing.T) {
 				},
 			},
 		},
+		"error with unset highlight end position": {
+			&hcl.Diagnostic{
+				Severity: hcl.DiagError,
+				Summary:  "There is no end",
+				Detail:   "But there is a beginning",
+				Subject: &hcl.Range{
+					Filename: "test.tf",
+					Start:    hcl.Pos{Line: 1, Column: 16, Byte: 15},
+					End:      hcl.Pos{Line: 0, Column: 0, Byte: 0},
+				},
+			},
+			&Diagnostic{
+				Severity: "error",
+				Summary:  "There is no end",
+				Detail:   "But there is a beginning",
+				Range: &DiagnosticRange{
+					Filename: "test.tf",
+					Start: Pos{
+						Line:   1,
+						Column: 16,
+						Byte:   15,
+					},
+					End: Pos{
+						Line:   1,
+						Column: 17,
+						Byte:   16,
+					},
+				},
+				Snippet: &DiagnosticSnippet{
+					Context:              strPtr(`resource "test_resource" "test"`),
+					Code:                 `resource "test_resource" "test" {`,
+					StartLine:            1,
+					HighlightStartOffset: 15,
+					HighlightEndOffset:   16,
+					Values:               []DiagnosticExpressionValue{},
+				},
+			},
+		},
 		"error with source code subject and known expression": {
 			&hcl.Diagnostic{
 				Severity: hcl.DiagError,
@@ -698,6 +736,11 @@ func TestNewDiagnostic(t *testing.T) {
 				"diagnostic",
 				fmt.Sprintf("%s.json", strings.ReplaceAll(name, " ", "-")),
 			)
+
+			// Generate golden reference by uncommenting the next two lines:
+			// gotBytes = append(gotBytes, '\n')
+			// os.WriteFile(filename, gotBytes, 0644)
+
 			wantFile, err := os.Open(filename)
 			if err != nil {
 				t.Fatalf("failed to open golden file: %s", err)

command/views/json/testdata/diagnostic/error-with-unset-highlight-end-position.json

@@ -0,0 +1,26 @@
+{
+  "severity": "error",
+  "summary": "There is no end",
+  "detail": "But there is a beginning",
+  "range": {
+    "filename": "test.tf",
+    "start": {
+      "line": 1,
+      "column": 16,
+      "byte": 15
+    },
+    "end": {
+      "line": 1,
+      "column": 17,
+      "byte": 16
+    }
+  },
+  "snippet": {
+    "context": "resource \"test_resource\" \"test\"",
+    "code": "resource \"test_resource\" \"test\" {",
+    "start_line": 1,
+    "highlight_start_offset": 15,
+    "highlight_end_offset": 16,
+    "values": []
+  }
+}