Skip to content

Commit

Permalink
Add positional information to semantic errors (#5121)
Browse files Browse the repository at this point in the history
This commit changes errors produced from semantic analysis so they
include a reference to the AST node which produced the error. The allows
for highlighting the specific portion of the source that caused a
semantic error.

This commit also changes how semantic errors are collected in that the
semantic analyzer no longer exits as soon as the first error is found
and instead goes on the analyze the rest of the AST, reporting all
semantic errors in the AST.
mattnibs authored May 31, 2024
1 parent 9db943d commit 68f35c9
Showing 44 changed files with 740 additions and 808 deletions.
16 changes: 8 additions & 8 deletions cli/queryflags/flags.go
Original file line number Diff line number Diff line change
@@ -32,7 +32,7 @@ func (f *Flags) SetFlags(fs *flag.FlagSet) {
fs.Var(&f.Includes, "I", "source file containing Zed query text (may be used multiple times)")
}

func (f *Flags) ParseSourcesAndInputs(paths []string) ([]string, ast.Seq, bool, error) {
func (f *Flags) ParseSourcesAndInputs(paths []string) ([]string, ast.Seq, *parser.SourceSet, bool, error) {
var src string
if len(paths) != 0 && !cli.FileExists(paths[0]) && !isURLWithKnownScheme(paths[0], "http", "https", "s3") {
src = paths[0]
@@ -41,25 +41,25 @@ func (f *Flags) ParseSourcesAndInputs(paths []string) ([]string, ast.Seq, bool,
// Consider a lone argument to be a query if it compiles
// and appears to start with a from or yield operator.
// Otherwise, consider it a file.
query, err := compiler.Parse(src, f.Includes...)
query, sset, err := compiler.Parse(src, f.Includes...)
if err == nil {
if s, err := semantic.Analyze(context.Background(), query, data.NewSource(storage.NewLocalEngine(), nil), nil); err == nil {
if semantic.HasSource(s) {
return nil, query, false, nil
return nil, query, sset, false, nil
}
if semantic.StartsWithYield(s) {
return nil, query, true, nil
return nil, query, sset, true, nil
}
}
}
return nil, nil, false, singleArgError(src, err)
return nil, nil, nil, false, singleArgError(src, err)
}
}
query, err := compiler.Parse(src, f.Includes...)
query, sset, err := parser.ParseZed(f.Includes, src)
if err != nil {
return nil, nil, false, err
return nil, nil, nil, false, err
}
return paths, query, false, nil
return paths, query, sset, false, nil
}

func isURLWithKnownScheme(path string, schemes ...string) bool {
4 changes: 2 additions & 2 deletions cli/zq/command.go
Original file line number Diff line number Diff line change
@@ -129,7 +129,7 @@ func (c *Command) Run(args []string) error {
// Prevent ParseSourcesAndInputs from treating args[0] as a path.
args = append(args, "-")
}
paths, flowgraph, null, err := c.queryFlags.ParseSourcesAndInputs(args)
paths, flowgraph, sset, null, err := c.queryFlags.ParseSourcesAndInputs(args)
if err != nil {
return fmt.Errorf("zq: %w", err)
}
@@ -154,7 +154,7 @@ func (c *Command) Run(args []string) error {
return err
}
comp := compiler.NewFileSystemCompiler(local)
query, err := runtime.CompileQuery(ctx, zctx, comp, flowgraph, readers)
query, err := runtime.CompileQuery(ctx, zctx, comp, flowgraph, sset, readers)
if err != nil {
return err
}
7 changes: 6 additions & 1 deletion cmd/zed/dev/compile/command.go
Original file line number Diff line number Diff line change
@@ -12,6 +12,7 @@ import (
"github.com/brimdata/zed/compiler/ast"
"github.com/brimdata/zed/compiler/ast/dag"
"github.com/brimdata/zed/compiler/data"
"github.com/brimdata/zed/compiler/parser"
"github.com/brimdata/zed/lake"
"github.com/brimdata/zed/pkg/charm"
"github.com/brimdata/zed/runtime"
@@ -130,7 +131,7 @@ func (c *Command) header(msg string) {
}

func (c *Command) parse(z string, lk *lake.Root) error {
seq, err := compiler.Parse(z, c.includes...)
seq, sset, err := compiler.Parse(z, c.includes...)
if err != nil {
return err
}
@@ -143,6 +144,7 @@ func (c *Command) parse(z string, lk *lake.Root) error {
fmt.Println(normalize(b))
}
if c.proc {
c.header("proc")
c.writeAST(seq)
}

@@ -151,6 +153,9 @@ func (c *Command) parse(z string, lk *lake.Root) error {
}
runtime, err := compiler.NewJob(runtime.DefaultContext(), seq, data.NewSource(nil, lk), nil)
if err != nil {
if list, ok := err.(parser.ErrorList); ok {
list.SetSourceSet(sset)
}
return err
}
if c.semantic {
4 changes: 4 additions & 0 deletions cmd/zed/log/command.go
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@ import (
"github.com/brimdata/zed/cli/outputflags"
"github.com/brimdata/zed/cli/poolflags"
"github.com/brimdata/zed/cmd/zed/root"
"github.com/brimdata/zed/compiler/parser"
"github.com/brimdata/zed/pkg/charm"
"github.com/brimdata/zed/pkg/storage"
"github.com/brimdata/zed/zio"
@@ -70,6 +71,9 @@ func (c *Command) Run(args []string) error {
defer w.Close()
q, err := lake.Query(ctx, nil, query)
if err != nil {
if list := (parser.ErrorList)(nil); errors.As(err, &list) && len(list) == 1 {
return errors.New(list[0].Msg)
}
return err
}
defer q.Close()
55 changes: 30 additions & 25 deletions compiler/ast/ast.go
Original file line number Diff line number Diff line change
@@ -110,7 +110,7 @@ func (c *Call) End() int {
if c.Where != nil {
return c.Where.End()
}
return c.Rparen
return c.Rparen + 1
}

type Cast struct {
@@ -180,7 +180,7 @@ type Regexp struct {
}

func (r *Regexp) Pos() int { return r.PatternPos }
func (r *Regexp) End() int { return r.PatternPos + len(r.Pattern) }
func (r *Regexp) End() int { return r.PatternPos + len(r.Pattern) + 2 }

type String struct {
Kind string `json:"kind" unpack:""`
@@ -432,9 +432,9 @@ type (
NullsFirst bool `json:"nullsfirst"`
}
Cut struct {
Kind string `json:"kind" unpack:""`
KeywordPos int `json:"keyword_pos"`
Args []Assignment `json:"args"`
Kind string `json:"kind" unpack:""`
KeywordPos int `json:"keyword_pos"`
Args Assignments `json:"args"`
}
Drop struct {
Kind string `json:"kind" unpack:""`
@@ -471,10 +471,10 @@ type (
Kind string `json:"kind" unpack:""`
// StartPos is not called KeywordPos for Summarize since the "summarize"
// keyword is optional.
StartPos int `json:"start_pos"`
Limit int `json:"limit"`
Keys []Assignment `json:"keys"`
Aggs []Assignment `json:"aggs"`
StartPos int `json:"start_pos"`
Limit int `json:"limit"`
Keys Assignments `json:"keys"`
Aggs Assignments `json:"aggs"`
}
Top struct {
Kind string `json:"kind" unpack:""`
@@ -484,9 +484,9 @@ type (
Flush bool `json:"flush"`
}
Put struct {
Kind string `json:"kind" unpack:""`
KeywordPos int `json:"keyword_pos"`
Args []Assignment `json:"args"`
Kind string `json:"kind" unpack:""`
KeywordPos int `json:"keyword_pos"`
Args Assignments `json:"args"`
}
Merge struct {
Kind string `json:"kind" unpack:""`
@@ -520,8 +520,8 @@ type (
// is unknown: It could be a Summarize or Put operator. This will be
// determined in the semantic phase.
OpAssignment struct {
Kind string `json:"kind" unpack:""`
Assignments []Assignment `json:"assignments"`
Kind string `json:"kind" unpack:""`
Assignments Assignments `json:"assignments"`
}
// An OpExpr operator is an expression that appears as an operator
// and requires semantic analysis to determine if it is a filter, a yield,
@@ -531,22 +531,22 @@ type (
Expr Expr `json:"expr"`
}
Rename struct {
Kind string `json:"kind" unpack:""`
KeywordPos int `json:"keyword_pos"`
Args []Assignment `json:"args"`
Kind string `json:"kind" unpack:""`
KeywordPos int `json:"keyword_pos"`
Args Assignments `json:"args"`
}
Fuse struct {
Kind string `json:"kind" unpack:""`
KeywordPos int `json:"keyword_pos"`
}
Join struct {
Kind string `json:"kind" unpack:""`
KeywordPos int `json:"keyword_pos"`
Style string `json:"style"`
RightInput Seq `json:"right_input"`
LeftKey Expr `json:"left_key"`
RightKey Expr `json:"right_key"`
Args []Assignment `json:"args"`
Kind string `json:"kind" unpack:""`
KeywordPos int `json:"keyword_pos"`
Style string `json:"style"`
RightInput Seq `json:"right_input"`
LeftKey Expr `json:"left_key"`
RightKey Expr `json:"right_key"`
Args Assignments `json:"args"`
}
Sample struct {
Kind string `json:"kind" unpack:""`
@@ -678,7 +678,12 @@ func (a *Assignment) Pos() int {
return a.RHS.Pos()
}

func (a *Assignment) End() int { return a.RHS.Pos() }
func (a *Assignment) End() int { return a.RHS.End() }

type Assignments []Assignment

func (a Assignments) Pos() int { return a[0].Pos() }
func (a Assignments) End() int { return a[len(a)-1].End() }

// Def is like Assignment but the LHS is an identifier that may be later
// referenced. This is used for const blocks in Sequential and var blocks
6 changes: 6 additions & 0 deletions compiler/ast/dag/expr.go
Original file line number Diff line number Diff line change
@@ -30,6 +30,11 @@ type (
LHS Expr `json:"lhs"`
RHS Expr `json:"rhs"`
}
// A BadExpr node is a placeholder for an expression containing semantic
// errors.
BadExpr struct {
Kind string `json:"kind" unpack:""`
}
BinaryExpr struct {
Kind string `json:"kind" unpack:""`
Op string `json:"op"`
@@ -131,6 +136,7 @@ type (
func (*Agg) ExprDAG() {}
func (*ArrayExpr) ExprDAG() {}
func (*Assignment) ExprDAG() {}
func (*BadExpr) ExprDAG() {}
func (*BinaryExpr) ExprDAG() {}
func (*Call) ExprDAG() {}
func (*Conditional) ExprDAG() {}
6 changes: 6 additions & 0 deletions compiler/ast/dag/op.go
Original file line number Diff line number Diff line change
@@ -26,6 +26,11 @@ type Seq []Op
// Ops

type (
// A BadOp node is a placeholder for an expression containing semantic
// errors.
BadOp struct {
Kind string `json:"kind" unpack:""`
}
Combine struct {
Kind string `json:"kind" unpack:""`
}
@@ -281,6 +286,7 @@ type (
}
)

func (*BadOp) OpNode() {}
func (*Fork) OpNode() {}
func (*Scatter) OpNode() {}
func (*Switch) OpNode() {}
2 changes: 2 additions & 0 deletions compiler/ast/dag/unpack.go
Original file line number Diff line number Diff line change
@@ -10,6 +10,8 @@ var unpacker = unpack.New(
Agg{},
ArrayExpr{},
Assignment{},
BadOp{},
BadExpr{},
BinaryExpr{},
Call{},
Combine{},
19 changes: 12 additions & 7 deletions compiler/job.go
Original file line number Diff line number Diff line change
@@ -88,14 +88,13 @@ func (j *Job) Parallelize(n int) error {
return err
}

func Parse(src string, filenames ...string) (ast.Seq, error) {
seq, _, err := parser.ParseZed(filenames, src)
return seq, err
func Parse(src string, filenames ...string) (ast.Seq, *parser.SourceSet, error) {
return parser.ParseZed(filenames, src)
}

// MustParse is like Parse but panics if an error is encountered.
func MustParse(query string) ast.Seq {
seq, err := (*anyCompiler)(nil).Parse(query)
seq, _, err := (*anyCompiler)(nil).Parse(query)
if err != nil {
panic(err)
}
@@ -136,7 +135,7 @@ type anyCompiler struct{}

// Parse concatenates the source files in filenames followed by src and parses
// the resulting program.
func (*anyCompiler) Parse(src string, filenames ...string) (ast.Seq, error) {
func (*anyCompiler) Parse(src string, filenames ...string) (ast.Seq, *parser.SourceSet, error) {
return Parse(src, filenames...)
}

@@ -145,13 +144,16 @@ func (*anyCompiler) Parse(src string, filenames ...string) (ast.Seq, error) {
// nor does it compute the demand of the query to prune the projection
// from the vcache.
func VectorCompile(rctx *runtime.Context, query string, object *vcache.Object) (zbuf.Puller, error) {
seq, err := Parse(query)
seq, sset, err := Parse(query)
if err != nil {
return nil, err
}
src := &data.Source{}
entry, err := semantic.Analyze(rctx.Context, seq, src, nil)
if err != nil {
if list := (parser.ErrorList)(nil); errors.As(err, &list) {
list.SetSourceSet(sset)
}
return nil, err
}
puller := vam.NewVectorProjection(rctx.Zctx, object, nil) //XXX project all
@@ -177,12 +179,15 @@ func VectorFilterCompile(rctx *runtime.Context, query string, src *data.Source,
if err != nil {
return nil, err
}
seq, err := Parse(query)
seq, sset, err := Parse(query)
if err != nil {
return nil, err
}
entry, err := semantic.Analyze(rctx.Context, seq, src, nil)
if err != nil {
if list := (parser.ErrorList)(nil); errors.As(err, &list) {
list.SetSourceSet(sset)
}
return nil, err
}
if len(entry) != 1 {
2 changes: 1 addition & 1 deletion compiler/parser/parser_test.go
Original file line number Diff line number Diff line change
@@ -40,7 +40,7 @@ func searchForZed() ([]string, error) {
}

func parseOp(z string) ([]byte, error) {
o, err := compiler.Parse(z)
o, _, err := compiler.Parse(z)
if err != nil {
return nil, err
}
Loading

0 comments on commit 68f35c9

Please sign in to comment.