From 0f3d4dd4927388bd6ea7364276cd9d28345cdc12 Mon Sep 17 00:00:00 2001
From: Arpit Jain <arpitjain099@gmail.com>
Date: Thu, 20 Aug 2026 00:16:35 -0400
Subject: [PATCH] Report a reformatted failpoint header instead of panicking

ToComments detects a generated header by looking for ", __fpErr := __fp_" on
an "if" line, then reads the type out of the same line with

  strings.Split(strings.Split(l, ".("), 1)[0]

The generator writes that header as one line, but gofmt splits it, which
leaves the type assertion on a later line. The header match still fires on
the "if" line, so the split returns one element and indexing it panics.

Reproduced with three ordinary commands and no hand-edited file:

  gofail enable .
  gofmt -w svc.go        # or any editor format-on-save
  gofail disable .
  panic: runtime error: index out of range [1] with length 1
    go.etcd.io/gofail/code.ToComments code/rewrite.go:115
    main.xfrmFile gofail.go:62
    main.main gofail.go:177

The crash also leaves a zero-byte svc.go.tmp behind, and every later enable
or disable on that tree then fails with "open ...svc.go.tmp: file exists"
until it is deleted by hand.

Return an error naming the failpoint instead. gofail.go already reports the
error and stops, so the source file is left intact and no temp file is
orphaned. Teaching ToComments to read the reformatted shape would be a
larger change; this only stops the panic.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
---
 code/rewrite.go      | 10 +++++++++-
 code/rewrite_test.go | 31 +++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/code/rewrite.go b/code/rewrite.go
index fcf53cf..e5d2e66 100644
--- a/code/rewrite.go
+++ b/code/rewrite.go
@@ -112,7 +112,15 @@ func ToComments(wdst io.Writer, rsrc io.Reader) ([]*Failpoint, error) {
 
 			ws = strings.Split(l, "i")[0]
 			n := strings.Split(strings.Split(l, "__fp_")[1], ".")[0]
-			t := strings.Split(strings.Split(l, ".(")[1], ")")[0]
+			// The generated header is written as a single line, but gofmt splits
+			// it across several, which leaves the type assertion on a later line
+			// than the one matched above.
+			typeParts := strings.SplitN(l, ".(", 2)
+			if len(typeParts) < 2 {
+				return fps, fmt.Errorf("gofail: failpoint %q header is missing its type assertion, "+
+					"the generated code may have been reformatted: %q", n, strings.TrimRight(l, "\n"))
+			}
+			t := strings.Split(typeParts[1], ")")[0]
 			dst.WriteString(ws + pfx + " var " + n + " " + t + "\n")
 			if !strings.Contains(l, "; goto __nomock") {
 				// not single liner
diff --git a/code/rewrite_test.go b/code/rewrite_test.go
index 07dc685..37eefc5 100644
--- a/code/rewrite_test.go
+++ b/code/rewrite_test.go
@@ -124,3 +124,34 @@ func TestToComment(t *testing.T) {
 		require.Equalf(t, len(fps), ex.wfps, "%d: got %d failpoints but expected %d", i, len(fps), ex.wfps)
 	}
 }
+
+// gofmt splits the single-line header the generator writes across several
+// lines, which leaves the type assertion on a later line than the "if". The
+// header match still fires on the "if" line, so the type lookup has to cope
+// with not finding one.
+func TestToCommentReformattedHeader(t *testing.T) {
+	reformatted := `package mypkg
+
+import "fmt"
+
+func Serve() {
+	if vSlowDown, __fpErr := __fp_SlowDown.Acquire(); __fpErr == nil {
+		_, __fpTypeOK := vSlowDown.(struct{})
+		if !__fpTypeOK {
+			goto __badTypeSlowDown
+		}
+		goto __nomockSlowDown
+	__badTypeSlowDown:
+		__fp_SlowDown.BadType(vSlowDown, "struct{}")
+	__nomockSlowDown:
+	}
+	fmt.Println("serving")
+}
+`
+
+	dst := bytes.NewBuffer(make([]byte, 0, 1024))
+	_, err := ToComments(dst, strings.NewReader(reformatted))
+
+	require.Error(t, err, "expected an error rather than a panic")
+	require.Contains(t, err.Error(), "missing its type assertion")
+}