From b7a4eba2ef4d661e5fbc67b43d75e18150cf16a1 Mon Sep 17 00:00:00 2001 From: Robert Kolmos Date: Wed, 22 May 2024 13:39:58 -0700 Subject: [PATCH] fix: map go formatting errors to their locations in templ files (#737) Co-authored-by: Adrian Hesketh --- cmd/templ/generatecmd/eventhandler.go | 28 ++++- .../test-eventhandler/eventhandler_test.go | 100 ++++++++++++++++++ .../multiple_errors.templ.error | 10 ++ .../single_error.templ.error | 5 + 4 files changed, 142 insertions(+), 1 deletion(-) create mode 100644 cmd/templ/generatecmd/test-eventhandler/eventhandler_test.go create mode 100644 cmd/templ/generatecmd/test-eventhandler/multiple_errors.templ.error create mode 100644 cmd/templ/generatecmd/test-eventhandler/single_error.templ.error diff --git a/cmd/templ/generatecmd/eventhandler.go b/cmd/templ/generatecmd/eventhandler.go index ed8bce18a..e3d2da2d4 100644 --- a/cmd/templ/generatecmd/eventhandler.go +++ b/cmd/templ/generatecmd/eventhandler.go @@ -7,6 +7,8 @@ import ( "crypto/sha256" "fmt" "go/format" + "go/scanner" + "go/token" "log/slog" "os" "path" @@ -221,7 +223,8 @@ func (h *FSEventHandler) generate(ctx context.Context, fileName string) (goUpdat formattedGoCode, err := format.Source(b.Bytes()) if err != nil { - return false, false, nil, fmt.Errorf("%s source formatting error: %w", fileName, err) + err = remapErrorList(err, sourceMap, fileName, targetFileName) + return false, false, nil, fmt.Errorf("%s source formatting error %w", fileName, err) } // Hash output, and write out the file if the goCodeHash has changed. @@ -257,6 +260,29 @@ func (h *FSEventHandler) generate(ctx context.Context, fileName string) (goUpdat return goUpdated, textUpdated, parsedDiagnostics, err } +// Takes an error from the formatter and attempts to convert the positions reported in the target file to their positions +// in the source file. +func remapErrorList(err error, sourceMap *parser.SourceMap, fileName string, targetFileName string) error { + list, ok := err.(scanner.ErrorList) + if !ok || len(list) == 0 { + return err + } + for i, e := range list { + // The positions in the source map are off by one line because of the package definition. + srcPos, ok := sourceMap.SourcePositionFromTarget(uint32(e.Pos.Line-1), uint32(e.Pos.Column)) + if !ok { + continue + } + list[i].Pos = token.Position{ + Filename: fileName, + Offset: int(srcPos.Index), + Line: int(srcPos.Line) + 1, + Column: int(srcPos.Col), + } + } + return list +} + func generateSourceMapVisualisation(ctx context.Context, templFileName, goFileName string, sourceMap *parser.SourceMap) error { if err := ctx.Err(); err != nil { return err diff --git a/cmd/templ/generatecmd/test-eventhandler/eventhandler_test.go b/cmd/templ/generatecmd/test-eventhandler/eventhandler_test.go new file mode 100644 index 000000000..c41bd8b73 --- /dev/null +++ b/cmd/templ/generatecmd/test-eventhandler/eventhandler_test.go @@ -0,0 +1,100 @@ +package testeventhandler + +import ( + "context" + "errors" + "fmt" + "go/scanner" + "go/token" + "io" + "log/slog" + "os" + "testing" + + "github.com/a-h/templ/cmd/templ/generatecmd" + "github.com/a-h/templ/generator" + "github.com/fsnotify/fsnotify" + "github.com/google/go-cmp/cmp" +) + +func TestErrorLocationMapping(t *testing.T) { + tests := []struct { + name string + rawFileName string + errorPositions []token.Position + }{ + { + name: "single error outputs location in srcFile", + rawFileName: "single_error.templ.error", + errorPositions: []token.Position{ + {Offset: 46, Line: 3, Column: 20}, + }, + }, + { + name: "multiple errors all output locations in srcFile", + rawFileName: "multiple_errors.templ.error", + errorPositions: []token.Position{ + {Offset: 41, Line: 3, Column: 15}, + {Offset: 101, Line: 7, Column: 22}, + {Offset: 126, Line: 10, Column: 1}, + }, + }, + } + + slog := slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{})) + fseh := generatecmd.NewFSEventHandler(slog, ".", false, []generator.GenerateOpt{}, false, false, true) + for _, test := range tests { + // The raw files cannot end in .templ because they will cause the generator to fail. Instead, + // we create a tmp file that ends in .templ only for the duration of the test. + rawFile, err := os.Open(test.rawFileName) + if err != nil { + t.Errorf("%s: Failed to open file %s: %v", test.name, test.rawFileName, err) + break + } + file, err := os.CreateTemp("", fmt.Sprintf("*%s.templ", test.rawFileName)) + if err != nil { + t.Errorf("%s: Failed to create a tmp file at %s: %v", test.name, file.Name(), err) + break + } + defer os.Remove(file.Name()) + if _, err = io.Copy(file, rawFile); err != nil { + t.Errorf("%s: Failed to copy contents from raw file %s to tmp %s: %v", test.name, test.rawFileName, file.Name(), err) + } + + event := fsnotify.Event{Name: file.Name(), Op: fsnotify.Write} + _, _, err = fseh.HandleEvent(context.Background(), event) + if err == nil { + t.Errorf("%s: no error was thrown", test.name) + break + } + list, ok := err.(scanner.ErrorList) + for !ok { + err = errors.Unwrap(err) + if err == nil { + t.Errorf("%s: reached end of error wrapping before finding an ErrorList", test.name) + break + } else { + list, ok = err.(scanner.ErrorList) + } + } + if !ok { + break + } + + if len(list) != len(test.errorPositions) { + t.Errorf("%s: expected %d errors but got %d", test.name, len(test.errorPositions), len(list)) + break + } + for i, err := range list { + test.errorPositions[i].Filename = file.Name() + diff := cmp.Diff(test.errorPositions[i], err.Pos) + if diff != "" { + t.Error(diff) + t.Error("expected:") + t.Error(test.errorPositions[i]) + t.Error("actual:") + t.Error(err.Pos) + } + } + } +} diff --git a/cmd/templ/generatecmd/test-eventhandler/multiple_errors.templ.error b/cmd/templ/generatecmd/test-eventhandler/multiple_errors.templ.error new file mode 100644 index 000000000..14f0c4182 --- /dev/null +++ b/cmd/templ/generatecmd/test-eventhandler/multiple_errors.templ.error @@ -0,0 +1,10 @@ +package testeventhandler + +func invalid(a: string) string { + return "foo" +} + +templ multipleError(a: string) { +
+} +l diff --git a/cmd/templ/generatecmd/test-eventhandler/single_error.templ.error b/cmd/templ/generatecmd/test-eventhandler/single_error.templ.error new file mode 100644 index 000000000..05c9e5234 --- /dev/null +++ b/cmd/templ/generatecmd/test-eventhandler/single_error.templ.error @@ -0,0 +1,5 @@ +package testeventhandler + +templ singleError(a: string) { +
+}