Skip to content

Commit

Permalink
Update sync positional errors to be position range (#476)
Browse files Browse the repository at this point in the history
* Update sync positional errors to be position range

* Add handling Go parser errors and fix TS error regex
  • Loading branch information
DavidSGK authored Oct 22, 2024
1 parent 15487ef commit 57d685a
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 19 deletions.
24 changes: 14 additions & 10 deletions pkg/sync/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
21 changes: 16 additions & 5 deletions pkg/sync/golang.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"fmt"
"go/ast"
"go/parser"
"go/scanner"
"go/token"
"io/fs"
"os"
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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) {
Expand All @@ -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")
}
}

Expand Down Expand Up @@ -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) {
Expand Down
16 changes: 12 additions & 4 deletions pkg/sync/ts.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
8 changes: 8 additions & 0 deletions proto/lekko/bff/v1beta1/bff.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down

0 comments on commit 57d685a

Please sign in to comment.