Skip to content

Commit b0a4a00

Browse files
authored
fix: parse go compile errors better (#4867)
closes #4369 - All paths prefixed with `../../../` are cut out (previously only the first path in each error was) - Better whitespace handling - Added unit test
1 parent 81e3c51 commit b0a4a00

File tree

2 files changed

+89
-9
lines changed

2 files changed

+89
-9
lines changed

go-runtime/compile/build.go

+29-9
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"path"
88
"path/filepath"
99
stdreflect "reflect"
10+
"regexp"
1011
"runtime"
1112
"slices"
1213
"strconv"
@@ -39,8 +40,14 @@ import (
3940
"github.com/block/ftl/internal/watch"
4041
)
4142

42-
var ftlTypesFilename = "types.ftl.go"
43-
var ftlQueriesFilename = "queries.ftl.go"
43+
var (
44+
ftlTypesFilename = "types.ftl.go"
45+
ftlQueriesFilename = "queries.ftl.go"
46+
// Searches for paths beginning with ../../../
47+
// Matches the previous character as well to avoid matching the middle deeper backtrack paths, and because
48+
// regex package does not support negative lookbehind assertions
49+
pathFromMainToModuleRegex = regexp.MustCompile(`(^|[^/])\.\./\.\./\.\./`)
50+
)
4451

4552
type MainWorkContext struct {
4653
GoVersion string
@@ -634,16 +641,27 @@ func compile(ctx context.Context, mainDir string, buildEnv []string, devMode boo
634641
buildEnv = slices.Clone(buildEnv)
635642
buildEnv = append(buildEnv, "GODEBUG=http2client=0")
636643
err := exec.CommandWithEnv(ctx, log.Debug, mainDir, buildEnv, "go", args...).RunStderrError(ctx)
637-
if err == nil {
638-
return nil
644+
if err != nil {
645+
return buildErrsFromCompilerErr(err.Error())
639646
}
647+
return nil
648+
}
649+
650+
// buildErrsFromCompilerErr converts a compiler error to a list of builderrors by trying to parse
651+
// the error text and extract multiple errors with file, line and column information
652+
func buildErrsFromCompilerErr(input string) []builderrors.Error {
640653
errs := []builderrors.Error{}
641-
lines := strings.Split(err.Error(), "\n")
642-
for i := 1; i < len(lines); {
643-
if strings.HasPrefix(lines[i], "\t") {
654+
lines := strings.Split(input, "\n")
655+
for i := 0; i < len(lines); {
656+
if i > 0 && strings.HasPrefix(lines[i], "\t") {
644657
lines[i-1] = lines[i-1] + ": " + strings.TrimSpace(lines[i])
645658
lines = append(lines[:i], lines[i+1:]...)
646659
} else {
660+
lines[i] = strings.TrimSpace(lines[i])
661+
if len(lines[i]) == 0 {
662+
lines = append(lines[:i], lines[i+1:]...)
663+
continue
664+
}
647665
i++
648666
}
649667
}
@@ -652,7 +670,8 @@ func compile(ctx context.Context, mainDir string, buildEnv []string, devMode boo
652670
continue
653671
}
654672
// ../../../example.go:11:59: undefined: lorem ipsum
655-
components := strings.SplitN(line, ":", 4)
673+
line = pathFromMainToModuleRegex.ReplaceAllString(line, "$1")
674+
components := islices.Map(strings.SplitN(line, ":", 4), strings.TrimSpace)
656675
if len(components) != 4 {
657676
errs = append(errs, builderrors.Error{
658677
Type: builderrors.COMPILER,
@@ -661,7 +680,8 @@ func compile(ctx context.Context, mainDir string, buildEnv []string, devMode boo
661680
})
662681
continue
663682
}
664-
path, _ := strings.CutPrefix(components[0], "../../../")
683+
684+
path := components[0]
665685
hasPos := true
666686
line, err := strconv.Atoi(components[1])
667687
if err != nil {

go-runtime/compile/build_test.go

+60
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,13 @@ import (
44
"os"
55
"path/filepath"
66
stdslices "slices"
7+
"strconv"
78
"testing"
89

910
"github.com/alecthomas/assert/v2"
1011
"github.com/alecthomas/types/optional"
1112

13+
"github.com/block/ftl/common/builderrors"
1214
"github.com/block/ftl/common/schema"
1315
"github.com/block/ftl/common/slices"
1416
"github.com/block/ftl/common/strcase"
@@ -143,3 +145,61 @@ module cyclic {
143145

144146
assert.EqualError(t, err, "cyclic references are not allowed: cyclic.a refers to cyclic.b refers to cyclic.c refers to cyclic.a")
145147
}
148+
149+
func TestParseCompilerErrs(t *testing.T) {
150+
t.Parallel()
151+
152+
for i, tt := range []struct {
153+
input string
154+
expected []builderrors.Error
155+
}{
156+
{
157+
input: "random text which is just an error",
158+
expected: []builderrors.Error{
159+
{
160+
Msg: "random text which is just an error",
161+
},
162+
},
163+
},
164+
{
165+
input: `
166+
# ftl/time
167+
../../../time.go:30:6: Internal redeclared in this block
168+
../../../time.go:25:6: other declaration of Internal
169+
../../../../../../example.go:1:2: An error I made up which refers to ../../../../../../another.go and another/../../../path.go
170+
`,
171+
expected: []builderrors.Error{
172+
{
173+
Msg: "Internal redeclared in this block: time.go:25:6: other declaration of Internal",
174+
Pos: optional.Some(builderrors.Position{
175+
Filename: "time.go",
176+
Line: 30,
177+
StartColumn: 6,
178+
EndColumn: 6,
179+
}),
180+
},
181+
{
182+
Msg: "An error I made up which refers to ../../../another.go and another/../../../path.go",
183+
Pos: optional.Some(builderrors.Position{
184+
Filename: "../../../example.go",
185+
Line: 1,
186+
StartColumn: 2,
187+
EndColumn: 2,
188+
}),
189+
},
190+
},
191+
},
192+
} {
193+
t.Run(strconv.Itoa(i), func(t *testing.T) {
194+
t.Parallel()
195+
// Add standard params to expected errs for convenience
196+
expected := slices.Map(tt.expected, func(e builderrors.Error) builderrors.Error {
197+
e.Type = builderrors.COMPILER
198+
e.Level = builderrors.ERROR
199+
return e
200+
})
201+
actual := buildErrsFromCompilerErr(tt.input)
202+
assert.Equal(t, expected, actual)
203+
})
204+
}
205+
}

0 commit comments

Comments
 (0)