Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: build errors indicate if they are compiler errors #3197

Merged
merged 4 commits into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
627 changes: 347 additions & 280 deletions backend/protos/xyz/block/ftl/v1/language/language.pb.go

Large diffs are not rendered by default.

11 changes: 10 additions & 1 deletion backend/protos/xyz/block/ftl/v1/language/language.proto
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,18 @@ message Error {
ERROR = 2;
}

enum ErrorType {
FTL = 0;
// Compiler errors are errors that are from the compiler. This is useful to avoid duplicate errors
// being shown to the user when combining errors from multiple sources (eg: an IDE showing compiler
// errors and FTL errors via LSP).
COMPILER = 1;
}

string msg = 1;
ErrorLevel level = 4;
Position pos = 5;
optional Position pos = 5;
ErrorType type = 6;
}

message Position {
Expand Down
27 changes: 16 additions & 11 deletions backend/protos/xyz/block/ftl/v1/language/mixins.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package languagepb
import (
"fmt"

"github.com/alecthomas/types/optional"
structpb "google.golang.org/protobuf/types/known/structpb"

"github.com/TBD54566975/ftl/internal/builderrors"
Expand Down Expand Up @@ -57,28 +58,32 @@ func errorFromProto(e *Error) builderrors.Error {
}

func errorToProto(e builderrors.Error) *Error {
var pos *Position
if bpos, ok := e.Pos.Get(); ok {
pos = &Position{
Filename: bpos.Filename,
StartColumn: int64(bpos.StartColumn),
EndColumn: int64(bpos.EndColumn),
Line: int64(bpos.Line),
}
}
return &Error{
Msg: e.Msg,
Pos: &Position{
Filename: e.Pos.Filename,
StartColumn: int64(e.Pos.StartColumn),
EndColumn: int64(e.Pos.EndColumn),
Line: int64(e.Pos.Line),
},
Msg: e.Msg,
Pos: pos,
Level: levelToProto(e.Level),
}
}

func PosFromProto(pos *Position) builderrors.Position {
func PosFromProto(pos *Position) optional.Option[builderrors.Position] {
if pos == nil {
return builderrors.Position{}
return optional.None[builderrors.Position]()
}
return builderrors.Position{
return optional.Some(builderrors.Position{
Line: int(pos.Line),
StartColumn: int(pos.StartColumn),
EndColumn: int(pos.EndColumn),
Filename: pos.Filename,
}
})
}

func LogLevelFromProto(level LogMessage_LogLevel) log.Level {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions go-runtime/compile/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,21 +336,21 @@ func Build(ctx context.Context, projectRootDir, stubsRoot string, config modulec

ftlTypesFilename := "types.ftl.go"
wg.Go(func() error {
if err := exec.Command(wgctx, log.Debug, config.Dir, "go", "mod", "tidy").RunBuffered(wgctx); err != nil {
if err := exec.Command(wgctx, log.Debug, config.Dir, "go", "mod", "tidy").RunStderrError(wgctx); err != nil {
return fmt.Errorf("%s: failed to tidy go.mod: %w", config.Dir, err)
}

if err := exec.Command(wgctx, log.Debug, config.Dir, "go", "fmt", ftlTypesFilename).RunBuffered(wgctx); err != nil {
if err := exec.Command(wgctx, log.Debug, config.Dir, "go", "fmt", ftlTypesFilename).RunStderrError(wgctx); err != nil {
return fmt.Errorf("%s: failed to format module dir: %w", config.Dir, err)
}
return filesTransaction.ModifiedFiles(filepath.Join(config.Dir, "go.mod"), filepath.Join(config.Dir, "go.sum"), filepath.Join(config.Dir, ftlTypesFilename))
})
mainDir := filepath.Join(buildDir, "go", "main")
wg.Go(func() error {
if err := exec.Command(wgctx, log.Debug, mainDir, "go", "mod", "tidy").RunBuffered(wgctx); err != nil {
if err := exec.Command(wgctx, log.Debug, mainDir, "go", "mod", "tidy").RunStderrError(wgctx); err != nil {
return fmt.Errorf("%s: failed to tidy go.mod: %w", mainDir, err)
}
if err := exec.Command(wgctx, log.Debug, mainDir, "go", "fmt", "./...").RunBuffered(wgctx); err != nil {
if err := exec.Command(wgctx, log.Debug, mainDir, "go", "fmt", "./...").RunStderrError(wgctx); err != nil {
return fmt.Errorf("%s: failed to format main dir: %w", mainDir, err)
}
return filesTransaction.ModifiedFiles(filepath.Join(mainDir, "go.mod"), filepath.Join(config.Dir, "go.sum"))
Expand All @@ -367,7 +367,7 @@ func Build(ctx context.Context, projectRootDir, stubsRoot string, config modulec
// We have seen lots of upstream HTTP/2 failures that make CI unstable.
// Disable HTTP/2 for now during the build. This can probably be removed later
buildEnv = append(buildEnv, "GODEBUG=http2client=0")
err = exec.CommandWithEnv(ctx, log.Debug, mainDir, buildEnv, "go", args...).RunBuffered(ctx)
err = exec.CommandWithEnv(ctx, log.Debug, mainDir, buildEnv, "go", args...).RunStderrError(ctx)
if err != nil {
return moduleSch, nil, fmt.Errorf("failed to compile: %w", err)
}
Expand Down
4 changes: 2 additions & 2 deletions go-runtime/schema/extract.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ func combineAllPackageResults(results map[*analysis.Analyzer][]any, diagnostics
fqName, err := goQualifiedNameForWidenedType(obj, d.Metadata)
if err != nil {
cd.error(builderrors.Error{
Pos: d.Position().ToErrorPos(),
Pos: optional.Some(d.Position().ToErrorPos()),
Msg: err.Error(),
Level: builderrors.ERROR})
}
Expand Down Expand Up @@ -395,7 +395,7 @@ func diagnosticsToSchemaErrors(diagnostics []analysis.SimpleDiagnostic) []builde
errors := make([]builderrors.Error, 0, len(diagnostics))
for _, d := range diagnostics {
errors = append(errors, builderrors.Error{
Pos: simplePosToErrorPos(d.Pos, d.End.Column),
Pos: optional.Some(simplePosToErrorPos(d.Pos, d.End.Column)),
Msg: d.Message,
Level: common.DiagnosticCategory(d.Category).ToErrorLevel(),
})
Expand Down
2 changes: 1 addition & 1 deletion go-runtime/schema/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ func TestErrorReporting(t *testing.T) {
for _, e := range r.Errors {
str := strings.ReplaceAll(e.Error(), subFilename+":", "")
str = strings.ReplaceAll(str, filename+":", "")
if e.Pos.Filename == filename {
if pos, ok := e.Pos.Get(); ok && pos.Filename == filename {
actualParent = append(actualParent, str)
} else {
actualChild = append(actualChild, str)
Expand Down
7 changes: 6 additions & 1 deletion internal/buildengine/languageplugin/go_plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
goruntime "github.com/TBD54566975/ftl/go-runtime"
"github.com/TBD54566975/ftl/go-runtime/compile"
"github.com/TBD54566975/ftl/internal"
"github.com/TBD54566975/ftl/internal/builderrors"
"github.com/TBD54566975/ftl/internal/exec"
"github.com/TBD54566975/ftl/internal/log"
"github.com/TBD54566975/ftl/internal/moduleconfig"
Expand Down Expand Up @@ -140,7 +141,11 @@ func buildGo(ctx context.Context, projectRoot, stubsRoot string, bctx BuildConte
config := bctx.Config.Abs()
moduleSch, buildErrs, err := compile.Build(ctx, projectRoot, stubsRoot, config, bctx.Schema, transaction, buildEnv, devMode)
if err != nil {
return BuildResult{}, CompilerBuildError{err: fmt.Errorf("failed to build module %q: %w", config.Module, err)}
return BuildResult{}, builderrors.Error{
Msg: "compile: " + err.Error(),
Level: builderrors.ERROR,
Type: builderrors.COMPILER,
}
}
return BuildResult{
Errors: buildErrs,
Expand Down
12 changes: 0 additions & 12 deletions internal/buildengine/languageplugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,18 +173,6 @@ func (getDependenciesCommand) pluginCmd() {}

type buildFunc = func(ctx context.Context, projectRoot, stubsRoot string, bctx BuildContext, buildEnv []string, rebuildAutomatically bool, transaction watch.ModifyFilesTransaction) (BuildResult, error)

type CompilerBuildError struct {
err error
}

func (e CompilerBuildError) Error() string {
return e.err.Error()
}

func (e CompilerBuildError) Unwrap() error {
return e.err
}

// internalPlugin is used by languages that have not been split off into their own external plugins yet.
// It has standard behaviours around building and watching files.
type internalPlugin struct {
Expand Down
36 changes: 28 additions & 8 deletions internal/builderrors/builderrors.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"errors"
"fmt"
"sort"

"github.com/alecthomas/types/optional"
)

type Position struct {
Expand Down Expand Up @@ -33,16 +35,32 @@ const (
ERROR
)

type ErrorType int

const (
// FTL errors are errors that are generated by the FTL runtime or the language plugin.
FTL ErrorType = iota
// COMPILER errors are errors that are generated by the compiler itself.
// LSP integration will not show these errors as they will be generated by the compiler.
COMPILER
)

type Error struct {
Type ErrorType
Msg string
Pos Position
Pos optional.Option[Position]
Level ErrorLevel
}

func (e Error) Error() string { return fmt.Sprintf("%s: %s", e.Pos, e.Msg) }
func (e Error) Error() string {
if pos, ok := e.Pos.Get(); ok {
return fmt.Sprintf("%s: %s", pos, e.Msg)
}
return e.Msg
}

func makeError(level ErrorLevel, pos Position, format string, args ...any) Error {
return Error{Msg: fmt.Sprintf(format, args...), Pos: pos, Level: level}
return Error{Type: FTL, Msg: fmt.Sprintf(format, args...), Pos: optional.Some(pos), Level: level}
}

func Infof(pos Position, format string, args ...any) Error {
Expand All @@ -66,7 +84,9 @@ func Wrapf(pos Position, endColumn int, err error, format string, args ...any) E
// Propagate existing error position if available
var newPos Position
if perr := (Error{}); errors.As(err, &perr) {
newPos = perr.Pos
if existingPos, ok := perr.Pos.Get(); ok {
newPos = existingPos
}
args = append(args, perr.Msg)
} else {
newPos = pos
Expand All @@ -80,11 +100,11 @@ func SortErrorsByPosition(merr []Error) {
return
}
sort.Slice(merr, func(i, j int) bool {
ipp := merr[i].Pos
jpp := merr[j].Pos
ipp := merr[i].Pos.Default(Position{})
jpp := merr[j].Pos.Default(Position{})
return ipp.Line < jpp.Line || (ipp.Line == jpp.Line && ipp.StartColumn < jpp.StartColumn) ||
(ipp.Line == jpp.Line && ipp.StartColumn == jpp.StartColumn && merr[i].Pos.EndColumn < merr[j].Pos.EndColumn) ||
(ipp.Line == jpp.Line && ipp.StartColumn == jpp.StartColumn && merr[i].Pos.EndColumn == merr[j].Pos.EndColumn && merr[i].Msg < merr[j].Msg)
(ipp.Line == jpp.Line && ipp.StartColumn == jpp.StartColumn && ipp.EndColumn < jpp.EndColumn) ||
(ipp.Line == jpp.Line && ipp.StartColumn == jpp.StartColumn && ipp == jpp && merr[i].Msg < merr[j].Msg)
})
}

Expand Down
32 changes: 20 additions & 12 deletions internal/lsp/lsp.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"github.com/tliron/kutil/version"

"github.com/TBD54566975/ftl/internal/buildengine"
"github.com/TBD54566975/ftl/internal/buildengine/languageplugin"
"github.com/TBD54566975/ftl/internal/builderrors"
ftlErrors "github.com/TBD54566975/ftl/internal/errors"
"github.com/TBD54566975/ftl/internal/log"
Expand Down Expand Up @@ -130,20 +129,25 @@ func (s *Server) post(err error) {
if !ftlErrors.Innermost(e) {
continue
}

var ce builderrors.Error
var cbe languageplugin.CompilerBuildError
if errors.As(e, &ce) {
filename := ce.Pos.Filename
if _, exists := errByFilename[filename]; !exists {
errByFilename[filename] = errSet{}
}
errByFilename[filename] = append(errByFilename[filename], ce)
} else if errors.As(e, &cbe) {
if !errors.As(e, &ce) {
errUnspecified = append(errUnspecified, err)
continue
}
if ce.Type == builderrors.COMPILER {
// ignore compiler errors
continue
} else {
}
pos, ok := ce.Pos.Get()
if !ok {
errUnspecified = append(errUnspecified, err)
continue
}
filename := pos.Filename
if _, exists := errByFilename[filename]; !exists {
errByFilename[filename] = errSet{}
}
errByFilename[filename] = append(errByFilename[filename], ce)
}

go publishPositionalErrors(errByFilename, s)
Expand All @@ -154,7 +158,11 @@ func publishPositionalErrors(errByFilename map[string]errSet, s *Server) {
for filename, errs := range errByFilename {
var diagnostics []protocol.Diagnostic
for _, e := range errs {
pp := e.Pos
pp, ok := e.Pos.Get()
if !ok {
// Errors without positions are not expected in this function.
continue
}
sourceName := "ftl"
var severity protocol.DiagnosticSeverity

Expand Down
6 changes: 4 additions & 2 deletions python-runtime/python-plugin/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ func (s *Service) Build(ctx context.Context, req *connect.Request[langpb.BuildRe
return err
}

//TODO: Use real build errors
_, _, err = compile.Build(ctx, req.Msg.ProjectRoot, req.Msg.StubsRoot, buildCtx.Config, nil, nil, false)
logger.Errorf(err, "build failed")

Expand All @@ -123,7 +122,10 @@ func (s *Service) Build(ctx context.Context, req *connect.Request[langpb.BuildRe
ContextId: req.Msg.BuildContext.Id,
IsAutomaticRebuild: false,
Errors: langpb.ErrorsToProto([]builderrors.Error{
builderrors.Errorf(builderrors.Position{}, "not implemented"),
{
Level: builderrors.ERROR,
Msg: "not implemented",
},
}),
InvalidateDependencies: false,
},
Expand Down
Loading