Skip to content

Commit

Permalink
feat: pass kotlin build errors to the language server
Browse files Browse the repository at this point in the history
fixes #1123
fixes #1159
  • Loading branch information
worstell committed Apr 5, 2024
1 parent 3c29e86 commit 241e039
Show file tree
Hide file tree
Showing 9 changed files with 298 additions and 156 deletions.
28 changes: 27 additions & 1 deletion backend/schema/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package schema
import (
"errors"
"fmt"

schemapb "github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1/schema"
)

type Error struct {
Expand All @@ -12,8 +14,32 @@ type Error struct {
Err error `protobuf:"-"` // Wrapped error, if any
}

func errorFromProto(e *schemapb.Error) *Error {
return &Error{
Pos: posFromProto(e.Pos),
Msg: e.Msg,
EndColumn: int(e.EndColumn),
}
}

func errorsFromProto(errs []*schemapb.Error) []*Error {
var out []*Error
for _, pb := range errs {
s := errorFromProto(pb)
out = append(out, s)
}
return out
}

type ErrorList struct {
Errors []Error `json:"errors" protobuf:"1"`
Errors []*Error `json:"errors" protobuf:"1"`
}

// ErrorListFromProto converts a protobuf ErrorList to an ErrorList.
func ErrorListFromProto(e *schemapb.ErrorList) *ErrorList {
return &ErrorList{
Errors: errorsFromProto(e.Errors),
}
}

func (e Error) Error() string { return fmt.Sprintf("%s-%d: %s", e.Pos, e.EndColumn, e.Msg) }
Expand Down
4 changes: 2 additions & 2 deletions buildengine/build_go_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func Nothing(context.Context) error {
buildDir: "_ftl",
sch: sch,
}
testBuild(t, bctx, []assertion{
testBuild(t, bctx, false, []assertion{
assertGeneratedModule("go/modules/other/external_module.go", expected),
})
}
Expand Down Expand Up @@ -174,7 +174,7 @@ func Call(context.Context, Req) (Resp, error) {
buildDir: "_ftl",
sch: sch,
}
testBuild(t, bctx, []assertion{
testBuild(t, bctx, false, []assertion{
assertGeneratedModule("go/modules/test/external_module.go", expected),
})
}
Expand Down
32 changes: 31 additions & 1 deletion buildengine/build_kotlin.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package buildengine

import (
"context"
"errors"
"fmt"
"os"
"path/filepath"
Expand All @@ -13,8 +14,10 @@ import (
"github.com/beevik/etree"
sets "github.com/deckarep/golang-set/v2"
"golang.org/x/exp/maps"
"google.golang.org/protobuf/proto"

"github.com/TBD54566975/ftl"
schemapb "github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1/schema"
"github.com/TBD54566975/ftl/backend/schema"
"github.com/TBD54566975/ftl/internal"
"github.com/TBD54566975/ftl/internal/exec"
Expand Down Expand Up @@ -59,7 +62,16 @@ func buildKotlinModule(ctx context.Context, sch *schema.Schema, module Module) e
logger.Debugf("Using build command '%s'", module.Build)
err := exec.Command(ctx, log.Debug, module.Dir, "bash", "-c", module.Build).RunBuffered(ctx)
if err != nil {
return fmt.Errorf("failed to build module %q: %w", module.Module, err)
// read runtime-specific build errors from the build directory
errorList, err := loadProtoErrors(module.AbsDeployDir())
if err != nil {
return fmt.Errorf("failed to read build errors for module: %w", err)
}
errs := make([]error, 0, len(errorList.Errors))
for _, e := range errorList.Errors {
errs = append(errs, *e)
}
return errors.Join(errs...)
}

return nil
Expand Down Expand Up @@ -249,3 +261,21 @@ func genType(module *schema.Module, t schema.Type) string {
}
panic(fmt.Sprintf("unsupported type %T", t))
}

func loadProtoErrors(buildDir string) (*schema.ErrorList, error) {
f := filepath.Join(buildDir, "errors.pb")
if _, err := os.Stat(f); errors.Is(err, os.ErrNotExist) {
return &schema.ErrorList{Errors: make([]*schema.Error, 0)}, nil
}

content, err := os.ReadFile(f)
if err != nil {
return nil, err
}
errorspb := &schemapb.ErrorList{}
err = proto.Unmarshal(content, errorspb)
if err != nil {
return nil, err
}
return schema.ErrorListFromProto(errorspb), nil
}
44 changes: 14 additions & 30 deletions buildengine/build_kotlin_test.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,9 @@
package buildengine

import (
"bytes"
"context"
"os"
"testing"

"github.com/alecthomas/assert/v2"

"github.com/TBD54566975/ftl/backend/schema"
"github.com/TBD54566975/ftl/internal/log"
)

func TestGenerateBasicModule(t *testing.T) {
Expand All @@ -31,7 +25,7 @@ package ftl.test
buildDir: "target",
sch: sch,
}
testBuild(t, bctx, []assertion{
testBuild(t, bctx, false, []assertion{
assertGeneratedModule("generated-sources/ftl/test/Test.kt", expected),
})
}
Expand Down Expand Up @@ -158,7 +152,7 @@ data class TestResponse(
buildDir: "target",
sch: sch,
}
testBuild(t, bctx, []assertion{
testBuild(t, bctx, false, []assertion{
assertGeneratedModule("generated-sources/ftl/test/Test.kt", expected),
})
}
Expand Down Expand Up @@ -219,7 +213,7 @@ fun testVerb(context: Context, req: Request): ftl.builtin.Empty = throw
buildDir: "target",
sch: sch,
}
testBuild(t, bctx, []assertion{
testBuild(t, bctx, false, []assertion{
assertGeneratedModule("generated-sources/ftl/test/Test.kt", expected),
})
}
Expand Down Expand Up @@ -273,7 +267,7 @@ class Empty
buildDir: "target",
sch: sch,
}
testBuild(t, bctx, []assertion{
testBuild(t, bctx, false, []assertion{
assertGeneratedModule("generated-sources/ftl/builtin/Builtin.kt", expected),
})
}
Expand Down Expand Up @@ -317,7 +311,7 @@ fun emptyVerb(context: Context, req: ftl.builtin.Empty): ftl.builtin.Empty = thr
buildDir: "target",
sch: sch,
}
testBuild(t, bctx, []assertion{
testBuild(t, bctx, false, []assertion{
assertGeneratedModule("generated-sources/ftl/test/Test.kt", expected),
})
}
Expand Down Expand Up @@ -397,7 +391,7 @@ fun nothing(context: Context): Unit = throw
buildDir: "target",
sch: sch,
}
testBuild(t, bctx, []assertion{
testBuild(t, bctx, false, []assertion{
assertGeneratedModule("generated-sources/ftl/test/Test.kt", expected),
})
}
Expand All @@ -406,22 +400,12 @@ func TestKotlinExternalType(t *testing.T) {
if testing.Short() {
t.SkipNow()
}
moduleDir := "testdata/projects/externalkotlin"
buildDir := "_ftl"

ctx := log.ContextWithLogger(context.Background(), log.Configure(os.Stderr, log.Config{}))
module, err := LoadModule(moduleDir)
assert.NoError(t, err)

logBuffer := bytes.Buffer{}
logger := log.Configure(&logBuffer, log.Config{})
ctx = log.ContextWithLogger(ctx, logger)

sch := &schema.Schema{}
err = Build(ctx, sch, module)
assert.Error(t, err)
assert.Contains(t, logBuffer.String(), "Expected module name to be in the form ftl.<module>, but was com.google.type.DayOfWeek")

err = os.RemoveAll(buildDir)
assert.NoError(t, err, "Error removing build directory")
bctx := buildContext{
moduleDir: "testdata/projects/externalkotlin",
buildDir: "target",
sch: &schema.Schema{},
}
testBuild(t, bctx, true, []assertion{
assertBuildProtoErrors("expected module name to be in the form ftl.<module>, but was com.google.type.DayOfWeek"),
})
}
35 changes: 31 additions & 4 deletions buildengine/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,28 @@ type assertion func(t testing.TB, bctx buildContext) error
func testBuild(
t *testing.T,
bctx buildContext,
expectFail bool,
assertions []assertion,
) {
t.Helper()
ctx := log.ContextWithLogger(context.Background(), log.Configure(os.Stderr, log.Config{}))
module, err := LoadModule(bctx.moduleDir)
abs, err := filepath.Abs(bctx.moduleDir)
assert.NoError(t, err, "Error getting absolute path for module directory")
module, err := LoadModule(abs)
assert.NoError(t, err)

err = Build(ctx, bctx.sch, module)
assert.NoError(t, err)
if expectFail {
assert.Error(t, err)
} else {
assert.NoError(t, err)
}

for _, a := range assertions {
err = a(t, bctx)
assert.NoError(t, err)
}

err = os.RemoveAll(bctx.buildDir)
err = os.RemoveAll(filepath.Join(bctx.moduleDir, bctx.buildDir))
assert.NoError(t, err, "Error removing build directory")
}

Expand All @@ -54,3 +60,24 @@ func assertGeneratedModule(generatedModulePath string, expectedContent string) a
return nil
}
}

func assertBuildProtoErrors(msgs ...string) assertion {
return func(t testing.TB, bctx buildContext) error {
t.Helper()
errorList, err := loadProtoErrors(filepath.Join(bctx.moduleDir, bctx.buildDir))
assert.NoError(t, err, "Error loading proto errors")

expected := make([]*schema.Error, 0, len(msgs))
for _, msg := range msgs {
expected = append(expected, &schema.Error{Msg: msg})
}

// normalize results
for _, e := range errorList.Errors {
e.EndColumn = 0
}

assert.Equal(t, errorList.Errors, expected, assert.Exclude[schema.Position]())
return nil
}
}
3 changes: 1 addition & 2 deletions internal/exec/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package exec

import (
"context"
"fmt"
"os"
"os/exec" //nolint:depguard
"syscall"
Expand Down Expand Up @@ -59,7 +58,7 @@ func (c *Cmd) RunBuffered(ctx context.Context) error {

err := c.Run()
if err != nil {
fmt.Printf("%s", outputBuffer.Bytes())
log.FromContext(ctx).Infof("%s", outputBuffer.Bytes())
return err
}

Expand Down
Loading

0 comments on commit 241e039

Please sign in to comment.