diff --git a/docs/reference/spec.md b/docs/reference/spec.md index c16c1d8..7914e59 100644 --- a/docs/reference/spec.md +++ b/docs/reference/spec.md @@ -361,6 +361,22 @@ The `list` argument must be a slice or array (or `nil`, which is treated as empty). Maps are not iterated by these forms; use `keys(m)` (from `WithBuiltins`) to drive a map iteration manually. +When a predicate raises an `ErrEvaluate`, the iterating forms wrap +the error with the form name, the predicate's source text, and the +failing element's index. A typo inside `map(users, it.Nmae)` therefore +reads: + +``` +map predicate `it.Nmae` failed on element 0: ... field "Nmae" not found ... +``` + +Nested forms each contribute their own layer, so the wrapping reads +top-down through the iteration tree. The internal `map` rewrite is +reversed in the printed predicate, so users see the source as they +typed it. The wrapping preserves the underlying error chain +(`errors.Is(err, ErrEvaluate)` still matches); context cancellation +passes through unchanged. + `try(value, default)` is the odd one out: it does not iterate a list and binds no implicit `it`/`index`. Both arguments are arbitrary expressions. The `default` is **only** evaluated when `value` failed, diff --git a/higher_order.go b/higher_order.go index 9576e7a..e5bc365 100644 --- a/higher_order.go +++ b/higher_order.go @@ -1,11 +1,15 @@ package expr import ( + "bytes" "context" "errors" "fmt" "go/ast" + "go/printer" + "go/token" "reflect" + "strings" ) // itEnv is a scope chain used by the higher-order special forms. It @@ -99,8 +103,16 @@ func checkFormArity(name string, got int) error { // forEach is the shared loop used by every higher-order form. The // body closure observes `ctx.Err()` through `p.eval` automatically, // so a cancelled context aborts the iteration at the next element. +// +// Predicate errors are wrapped with the form name, the failing +// element's index, and the predicate's source text so users can +// locate the failure inside nested expressions. Cancellation errors +// and anything else that is not an ErrEvaluate pass through unchanged +// so callers can still match on context.Canceled / +// context.DeadlineExceeded. func (p *Program) forEach( ctx context.Context, + name string, items []any, predicate ast.Expr, env any, @@ -113,7 +125,7 @@ func (p *Program) forEach( scope.index = int64(i) v, err := p.eval(ctx, predicate, scope, depth) if err != nil { - return err + return wrapPredicateErr(name, predicate, i, err) } stop, err := body(item, v) if err != nil { @@ -126,6 +138,44 @@ func (p *Program) forEach( return nil } +// wrapPredicateErr decorates a predicate-evaluation error with the +// form name, the predicate's source text, and the failing element's +// index. Errors that do not wrap ErrEvaluate (notably +// context.Canceled and context.DeadlineExceeded, which are returned +// raw by the evaluator) pass through unchanged so callers can still +// match on them with errors.Is. +func wrapPredicateErr(name string, predicate ast.Expr, index int, err error) error { + if err == nil { + return nil + } + if !errors.Is(err, ErrEvaluate) { + return err + } + src := formatPredicate(predicate) + if src == "" { + return fmt.Errorf("%s predicate failed on element %d: %w", name, index, err) + } + return fmt.Errorf("%s predicate `%s` failed on element %d: %w", name, src, index, err) +} + +// formatPredicate prints a predicate AST back to source text for +// inclusion in error messages. The internal map sentinel is +// translated back to `map` so nested forms read the way users wrote +// them. Returns "" if printing fails or if the result contains a +// backtick (which would clash with the surrounding error format), +// letting wrapPredicateErr fall back to a position-only message. +func formatPredicate(node ast.Expr) string { + var buf bytes.Buffer + if err := printer.Fprint(&buf, token.NewFileSet(), node); err != nil { + return "" + } + out := strings.ReplaceAll(buf.String(), mapFormName, "map") + if strings.ContainsRune(out, '`') { + return "" + } + return out +} + func formMap(p *Program, ctx context.Context, n *ast.CallExpr, env any, depth int) (any, error) { if err := checkFormArity("map", len(n.Args)); err != nil { return nil, err @@ -135,7 +185,7 @@ func formMap(p *Program, ctx context.Context, n *ast.CallExpr, env any, depth in return nil, err } out := make([]any, 0, len(items)) - err = p.forEach(ctx, items, n.Args[1], env, depth, func(_ any, v any) (bool, error) { + err = p.forEach(ctx, "map", items, n.Args[1], env, depth, func(_ any, v any) (bool, error) { out = append(out, v) return false, nil }) @@ -154,7 +204,7 @@ func formFilter(p *Program, ctx context.Context, n *ast.CallExpr, env any, depth return nil, err } out := make([]any, 0, len(items)) - err = p.forEach(ctx, items, n.Args[1], env, depth, func(item any, v any) (bool, error) { + err = p.forEach(ctx, "filter", items, n.Args[1], env, depth, func(item any, v any) (bool, error) { if isTruthy(v) { out = append(out, item) } @@ -175,7 +225,7 @@ func formAny(p *Program, ctx context.Context, n *ast.CallExpr, env any, depth in return nil, err } found := false - err = p.forEach(ctx, items, n.Args[1], env, depth, func(_ any, v any) (bool, error) { + err = p.forEach(ctx, "any", items, n.Args[1], env, depth, func(_ any, v any) (bool, error) { if isTruthy(v) { found = true return true, nil @@ -197,7 +247,7 @@ func formAll(p *Program, ctx context.Context, n *ast.CallExpr, env any, depth in return nil, err } ok := true - err = p.forEach(ctx, items, n.Args[1], env, depth, func(_ any, v any) (bool, error) { + err = p.forEach(ctx, "all", items, n.Args[1], env, depth, func(_ any, v any) (bool, error) { if !isTruthy(v) { ok = false return true, nil @@ -220,7 +270,7 @@ func formFind(p *Program, ctx context.Context, n *ast.CallExpr, env any, depth i } var match any matched := false - err = p.forEach(ctx, items, n.Args[1], env, depth, func(item any, v any) (bool, error) { + err = p.forEach(ctx, "find", items, n.Args[1], env, depth, func(item any, v any) (bool, error) { if isTruthy(v) { match = item matched = true @@ -246,7 +296,7 @@ func formCount(p *Program, ctx context.Context, n *ast.CallExpr, env any, depth return nil, err } var total int64 - err = p.forEach(ctx, items, n.Args[1], env, depth, func(_ any, v any) (bool, error) { + err = p.forEach(ctx, "count", items, n.Args[1], env, depth, func(_ any, v any) (bool, error) { if isTruthy(v) { total++ } diff --git a/higher_order_test.go b/higher_order_test.go index 2c608ed..a2566c5 100644 --- a/higher_order_test.go +++ b/higher_order_test.go @@ -3,6 +3,7 @@ package expr import ( "context" "errors" + "strings" "testing" "github.com/deepnoodle-ai/expr/internal/require" @@ -166,10 +167,126 @@ func TestHigherOrder_PredicateError(t *testing.T) { env := map[string]any{"users": sampleUsers()} _, err := evalExpr(t.Context(), "map(users, it.Nmae)", env) require.ErrorIs(t, err, ErrEvaluate) + require.Contains(t, err.Error(), "map predicate `it.Nmae` failed on element 0:") require.Contains(t, err.Error(), `field "Nmae" not found`) require.Contains(t, err.Error(), `did you mean "Name"?`) } +// Predicate errors include the index of the failing element so users +// can locate the failure inside long collections. The first two +// elements pass through cleanly; the third has no Name field, which is +// where the wrapping should report. +func TestHigherOrder_PredicateErrorReportsIndex(t *testing.T) { + env := map[string]any{ + "items": []any{ + map[string]any{"Name": "a"}, + map[string]any{"Name": "b"}, + map[string]any{"Other": "c"}, + }, + } + _, err := evalExpr(t.Context(), "map(items, it.Name)", env) + require.ErrorIs(t, err, ErrEvaluate) + require.Contains(t, err.Error(), "map predicate `it.Name` failed on element 2:") +} + +// Nested forms each contribute their own wrapping layer. The inner +// map prints as `map(it, it.bad)` and shows the inner failing element; +// the outer map prints the full inner expression and reports the +// outer index. Internal sentinel rewriting is reversed so the printed +// source reads as the user originally typed it. +func TestHigherOrder_PredicateErrorNestedForms(t *testing.T) { + env := map[string]any{ + "matrix": []any{ + []any{int64(1)}, + []any{int64(2)}, + }, + } + _, err := evalExpr(t.Context(), "map(matrix, map(it, it.bad))", env) + require.ErrorIs(t, err, ErrEvaluate) + // The inner map prints as `map(it, it.bad)` and reports element 0 + // of the inner list. + require.Contains(t, err.Error(), "map predicate `it.bad` failed on element 0:") + // The outer map wraps with the full inner predicate text. + require.Contains(t, err.Error(), "map predicate `map(it, it.bad)` failed on element 0:") + // The internal `__expr_map__` sentinel must not leak into messages. + require.False(t, + strings.Contains(err.Error(), mapFormName), + "map sentinel leaked into error: %s", err.Error()) +} + +// Each form name appears in its own wrapping. Spot-check the names +// rather than enumerating: filter, find, count, any, all all flow +// through the same forEach so a single fixture per name suffices. +func TestHigherOrder_PredicateErrorFormNames(t *testing.T) { + cases := []struct { + expr string + want string + }{ + {`filter(items, it.bad)`, "filter predicate `it.bad` failed on element 0"}, + {`find(items, it.bad)`, "find predicate `it.bad` failed on element 0"}, + {`count(items, it.bad)`, "count predicate `it.bad` failed on element 0"}, + {`any(items, it.bad)`, "any predicate `it.bad` failed on element 0"}, + {`all(items, it.bad)`, "all predicate `it.bad` failed on element 0"}, + } + env := map[string]any{"items": []any{map[string]any{"good": 1}}} + for _, tc := range cases { + t.Run(tc.expr, func(t *testing.T) { + _, err := evalExpr(t.Context(), tc.expr, env) + require.ErrorIs(t, err, ErrEvaluate) + require.Contains(t, err.Error(), tc.want) + }) + } +} + +// The filter(xs, p)[N] fast path in program.go has its own iteration +// loop. Predicate failures there should match the slow path's wrapping +// so the fast path is not a debuggability regression. The first item +// matches and the second is missing the field, so reaching index 1 of +// the filtered result forces the predicate to evaluate on the second +// element and fail there. +func TestHigherOrder_PredicateErrorFilterIndexFastPath(t *testing.T) { + env := map[string]any{ + "items": []any{ + map[string]any{"Name": "a"}, + map[string]any{"Other": "b"}, + }, + } + _, err := evalExpr(t.Context(), "filter(items, it.Name)[1]", env) + require.ErrorIs(t, err, ErrEvaluate) + require.Contains(t, err.Error(), "filter predicate `it.Name` failed on element 1:") +} + +// A predicate that contains a backtick character would clash with the +// backtick delimiters used in the error message, so the wrapping +// falls back to a position-only form. Reaches the formatPredicate +// fallback path defensively. +func TestHigherOrder_PredicateErrorBacktickFallback(t *testing.T) { + env := map[string]any{"items": []any{map[string]any{"good": 1}}} + _, err := evalExpr(t.Context(), "map(items, it.bad == `oops`)", env) + require.ErrorIs(t, err, ErrEvaluate) + require.Contains(t, err.Error(), "map predicate failed on element 0:") + require.False(t, + strings.Contains(err.Error(), "predicate `"), + "backtick-bearing predicate should not be embedded, got %s", err.Error()) +} + +// Cancellation must not be wrapped as a predicate error so callers can +// still match it with errors.Is(err, context.Canceled). +func TestHigherOrder_PredicateErrorIgnoresCancellation(t *testing.T) { + ctxC, cancel := context.WithCancel(context.Background()) + defer cancel() + opts := []Option{WithFunctions(map[string]any{ + "stop": func() bool { cancel(); return true }, + })} + env := map[string]any{"xs": []any{int64(1), int64(2), int64(3)}} + _, err := evalExpr(ctxC, "map(xs, stop() && it)", env, opts...) + require.True(t, errors.Is(err, context.Canceled), + "expected context.Canceled, got %v", err) + require.False(t, + strings.Contains(err.Error(), "failed on element"), + "cancellation should not be wrapped, got %v", err) +} + func TestHigherOrder_UserFuncOverride(t *testing.T) { // User-registered `map` wins over the special form, matching the // env→funcs identifier-resolution order used everywhere else. diff --git a/llms.txt b/llms.txt index 9782eed..2646e53 100644 --- a/llms.txt +++ b/llms.txt @@ -125,6 +125,14 @@ you need to). | `count(list, pred)` | `int64` | Number of matches | | `try(value, default)` | value or default | `value` if it evaluates cleanly, else `default` | +When a predicate inside an iterating form errors, the form wraps +the error with its name, the predicate's source text, and the failing +element's index, e.g. +``map predicate `it.Nmae` failed on element 3: ...``. Nested forms +add their own layer. The wrapping preserves the underlying chain so +`errors.Is(err, ErrEvaluate)` still matches; cancellation passes +through unchanged. + `try` is the odd one out: it does not iterate a list, binds no `it` or `index`, and traps anything wrapping `ErrEvaluate` (missing keys, nil selectors, out-of-range indices, `int`/`float` parse failures). diff --git a/program.go b/program.go index dc33e73..f474502 100644 --- a/program.go +++ b/program.go @@ -962,7 +962,7 @@ func (p *Program) tryFilterIndex(ctx context.Context, n *ast.IndexExpr, env any, scope.index = int64(i) v, err := p.eval(ctx, call.Args[1], scope, depth) if err != nil { - return nil, true, err + return nil, true, wrapPredicateErr("filter", call.Args[1], i, err) } if !isTruthy(v) { continue