From 57d685a1e5c28fb6eff40584d8dd68990eb38a00 Mon Sep 17 00:00:00 2001 From: David Kang Date: Mon, 21 Oct 2024 22:32:45 -0700 Subject: [PATCH] Update sync positional errors to be position range (#476) * Update sync positional errors to be position range * Add handling Go parser errors and fix TS error regex --- pkg/sync/error.go | 24 ++++++++++++++---------- pkg/sync/golang.go | 21 ++++++++++++++++----- pkg/sync/ts.go | 16 ++++++++++++---- proto/lekko/bff/v1beta1/bff.proto | 8 ++++++++ 4 files changed, 50 insertions(+), 19 deletions(-) diff --git a/pkg/sync/error.go b/pkg/sync/error.go index 803697a8..e64eed71 100644 --- a/pkg/sync/error.go +++ b/pkg/sync/error.go @@ -36,23 +36,27 @@ func (e *SyncError) Unwrap() error { // Error that can be directly attributed to incorrect/unsupported code type SyncPosError struct { - Inner error - Filename string - Line int - Col int + Inner error + Filename string + StartLine int + StartCol int + EndLine int + EndCol int } -func NewSyncPosError(inner error, filename string, line, col int) *SyncPosError { +func NewSyncPosError(inner error, filename string, startLine, startCol, endLine, endCol int) *SyncPosError { return &SyncPosError{ - Inner: inner, - Filename: filename, - Line: line, - Col: col, + Inner: inner, + Filename: filename, + StartLine: startLine, + StartCol: startCol, + EndLine: endLine, + EndCol: endCol, } } func (e *SyncPosError) Error() string { - return fmt.Sprintf("sync %s:%d:%d: %v", e.Filename, e.Line, e.Col, e.Inner) + return fmt.Sprintf("sync %s:%d:%d:%d:%d: %v", e.Filename, e.StartLine, e.StartCol, e.EndLine, e.EndCol, e.Inner) } func (e *SyncPosError) Unwrap() error { diff --git a/pkg/sync/golang.go b/pkg/sync/golang.go index 4f00e0b2..0f240384 100644 --- a/pkg/sync/golang.go +++ b/pkg/sync/golang.go @@ -19,6 +19,7 @@ import ( "fmt" "go/ast" "go/parser" + "go/scanner" "go/token" "io/fs" "os" @@ -390,7 +391,7 @@ func (g *goSyncer) Sync(filePaths ...string) (*featurev1beta1.RepositoryContents for _, filePath := range filePaths { astf, err := parser.ParseFile(g.fset, filePath, nil, parser.ParseComments|parser.AllErrors|parser.SkipObjectResolution) if err != nil { - return nil, errors.Wrapf(err, "parse %s", filePath) + return nil, errors.Wrapf(g.parseParseError(err), "parse %s", filePath) } ns, err := g.AstToNamespace(astf, ret.FileDescriptorSet) if err != nil { @@ -412,7 +413,7 @@ func (g *goSyncer) SyncContents(contentMap map[string]string) (*featurev1beta1.R for namespace, contents := range contentMap { astf, err := parser.ParseFile(g.fset, namespace, contents, parser.ParseComments|parser.AllErrors|parser.SkipObjectResolution) if err != nil { - return nil, errors.Wrapf(err, "parse %s", namespace) + return nil, errors.Wrapf(g.parseParseError(err), "parse %s", namespace) } ns, err := g.AstToNamespace(astf, ret.FileDescriptorSet) if err != nil { @@ -425,6 +426,15 @@ func (g *goSyncer) SyncContents(contentMap map[string]string) (*featurev1beta1.R return ret, nil } +func (g *goSyncer) parseParseError(err error) error { + if perr, ok := err.(scanner.ErrorList); ok && len(perr) >= 1 { + return NewSyncPosError(errors.New(perr[0].Msg), perr[0].Pos.Filename, perr[0].Pos.Line, perr[0].Pos.Column, perr[0].Pos.Line, perr[0].Pos.Column+1) + } else if err == nil { + return nil + } + return NewSyncError(err) +} + // TODO - is this only used for context keys, or other things? func (g *goSyncer) exprToValue(expr ast.Expr) (string, error) { switch v := expr.(type) { @@ -433,7 +443,7 @@ func (g *goSyncer) exprToValue(expr ast.Expr) (string, error) { case *ast.SelectorExpr: return strcase.ToSnake(v.Sel.Name), nil default: - return "", g.posErr(expr, "unsupported syntax") + return "", g.posErr(expr, "unsupported context key syntax") } } @@ -1195,8 +1205,9 @@ func (g *goSyncer) posErr(node ast.Node, err any) error { // This means there's an internal bug panic("invalid inner error type") } - p := g.fset.Position(node.Pos()) - return NewSyncPosError(inner, p.Filename, p.Line, p.Column) + startPos := g.fset.Position(node.Pos()) + endPos := g.fset.Position(node.End()) + return NewSyncPosError(inner, startPos.Filename, startPos.Line, startPos.Column, endPos.Line, endPos.Column) } func (g *goSyncer) structToMap(structType *ast.StructType) (map[string]string, error) { diff --git a/pkg/sync/ts.go b/pkg/sync/ts.go index b4b6b3f4..9fbd94ab 100644 --- a/pkg/sync/ts.go +++ b/pkg/sync/ts.go @@ -87,18 +87,26 @@ func checkSyncTSError(output []byte) error { if strings.Contains(o, "404") && strings.Contains(o, syncTSExec) { return NoSyncTSError } - r := regexp.MustCompile("LekkoParseError: (.*):(\\d+):(\\d+) - (.*)") + r := regexp.MustCompile("LekkoParseError: (.*):(\\d+):(\\d+):(\\d+):(\\d+) - (.*)") matches := r.FindStringSubmatch(o) if matches == nil { return NewSyncError(errors.New(o)) } - line, err := strconv.Atoi(matches[2]) + startLine, err := strconv.Atoi(matches[2]) if err != nil { return NewSyncError(errors.New(o)) } - col, err := strconv.Atoi(matches[3]) + startCol, err := strconv.Atoi(matches[3]) if err != nil { return NewSyncError(errors.New(o)) } - return NewSyncPosError(errors.New(matches[4]), matches[1], line, col) + endLine, err := strconv.Atoi(matches[4]) + if err != nil { + return NewSyncError(errors.New(o)) + } + endCol, err := strconv.Atoi(matches[5]) + if err != nil { + return NewSyncError(errors.New(o)) + } + return NewSyncPosError(errors.New(matches[6]), matches[1], startLine, startCol, endLine, endCol) } diff --git a/proto/lekko/bff/v1beta1/bff.proto b/proto/lekko/bff/v1beta1/bff.proto index a1ec0098..9a02e02d 100644 --- a/proto/lekko/bff/v1beta1/bff.proto +++ b/proto/lekko/bff/v1beta1/bff.proto @@ -831,6 +831,14 @@ message SaveNativeLangResponse { string formatted = 2; } +message SaveNativeLangError { + string message = 1; + uint32 start_line = 2; + uint32 start_col = 3; + uint32 end_line = 4; + uint32 end_col = 5; +} + message ConvertRuleToStringRequest { lekko.rules.v1beta3.Rule rule = 1; }