From 56fbd1afa41784dbe06f4408ae4fc092aa64c49f Mon Sep 17 00:00:00 2001 From: Curtis Myzie Date: Wed, 28 Oct 2026 10:01:29 -0400 Subject: [PATCH 1/9] Add static type checking and keyword field names Program.Check(env) type-checks an expression against the types of its environment and returns its result type. It reports unknown names and fields, reads of maybe-nil values without ?. or try, wrong-kind operations, and comparisons that are never true, including against an enum. Template.Check checks every segment. WithFuncType types registered functions. Field names may now be Go keywords (event.type, x?.range), which JSON payloads use freely. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 1 + check.go | 599 ++++++++++++++++++++++++++++++++ check_test.go | 127 +++++++ docs/reference/spec.md | 34 ++ engine.go | 53 ++- internal/optaccess/optaccess.go | 21 +- kwfield_test.go | 50 +++ llms.txt | 20 ++ types.go | 232 +++++++++++++ 9 files changed, 1126 insertions(+), 11 deletions(-) create mode 100644 check.go create mode 100644 check_test.go create mode 100644 kwfield_test.go create mode 100644 types.go diff --git a/CLAUDE.md b/CLAUDE.md index 446ced8..1c104a1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -25,6 +25,7 @@ parameter interpolation. | `builtin_groups.go` | Opt-in `MathFuncs()` / `StringFuncs()` / `CollectionFuncs()` sets | | `higher_order.go` | `map`, `filter`, `any`, `all`, `find`, `count`, `try`, `if` forms | | `identifiers.go` | `Program.Identifiers()` — env-referenced name collection | +| `types.go` + `check.go` | `Type` and `Program.Check` / `Template.Check` — static type checking | | `template.go` | `${...}` interpolation via `NewTemplate` / `Render` | | `truthy.go` | `IsTruthy` rules used by `!`, `&&`, `\|\|`, `bool(v)` | | `suggest.go` | "Did you mean…" for unknown identifiers | diff --git a/check.go b/check.go new file mode 100644 index 0000000..22a7d02 --- /dev/null +++ b/check.go @@ -0,0 +1,599 @@ +package expr + +import ( + "errors" + "fmt" + "go/ast" + "go/token" + "sort" + "strings" +) + +// ErrType wraps every error [Program.Check] returns. +var ErrType = errors.New("expr: type error") + +// TypeError lists every problem Check found. +type TypeError struct { + Problems []string +} + +func (e *TypeError) Error() string { + return "expr: type error: " + strings.Join(e.Problems, "; ") +} + +func (e *TypeError) Unwrap() error { return ErrType } + +// FuncType computes a function's result type from its argument types, for +// [WithFuncType]. Return an error to report a misuse. +type FuncType func(args []*Type) (*Type, error) + +// CheckOption configures [Program.Check]. +type CheckOption func(*checkConfig) + +type checkConfig struct { + funcTypes map[string]FuncType +} + +// WithFuncType declares the result type of a function registered with +// [WithFunctions]. Without it, calls to registered functions produce Any. +func WithFuncType(name string, fn FuncType) CheckOption { + return func(c *checkConfig) { + if c.funcTypes == nil { + c.funcTypes = map[string]FuncType{} + } + c.funcTypes[name] = fn + } +} + +// Check type-checks the expression against the types of its environment +// and returns the type of its result. +// +// It reports every problem it finds, as a *[TypeError]: names that are not +// in env, fields a closed object does not have, operations on the wrong +// kind of value, comparisons with a value outside an enum, and selecting +// from a value that may be nil without `?.`. Anything typed [Any] is +// unchecked, so a partially typed environment checks what it can. +// +// Check is static. It does not evaluate anything and cannot see values, +// so a successful check does not guarantee Run succeeds (a list index can +// still be out of range), and env must describe what Run will be given. +func (p *Program) Check(env map[string]*Type, opts ...CheckOption) (*Type, error) { + c := &checker{p: p, env: env} + for _, o := range opts { + o(&c.cfg) + } + t := c.check(p.root, nil) + if len(c.problems) > 0 { + return t, &TypeError{Problems: c.problems} + } + return t, nil +} + +// Check type-checks every expression in the template. A template always +// renders a string. +func (t *Template) Check(env map[string]*Type, opts ...CheckOption) error { + var problems []string + for _, seg := range t.Segments() { + if seg.Program == nil { + continue + } + if _, err := seg.Program.Check(env, opts...); err != nil { + var te *TypeError + if errors.As(err, &te) { + problems = append(problems, te.Problems...) + } + } + } + if len(problems) > 0 { + return &TypeError{Problems: problems} + } + return nil +} + +type checker struct { + p *Program + env map[string]*Type + cfg checkConfig + problems []string + // lenient is positive inside try's first argument, where a nil + // receiver is caught at run time and so is not a problem. + lenient int +} + +// scope holds the names a higher-order form binds. +type scope struct { + names map[string]*Type + parent *scope +} + +func (s *scope) lookup(name string) (*Type, bool) { + for ; s != nil; s = s.parent { + if t, ok := s.names[name]; ok { + return t, true + } + } + return nil, false +} + +func (c *checker) report(node ast.Expr, format string, args ...any) { + msg := fmt.Sprintf(format, args...) + if src := exprDisplayString(node); src != "" { + msg = "`" + src + "`: " + msg + } + c.problems = append(c.problems, msg) +} + +// nilCheck reports a value that may be nil where nil is not allowed. +func (c *checker) nilCheck(node ast.Expr, t *Type) { + if t != nil && t.Optional && c.lenient == 0 { + c.report(node, "may be nil here; use ?. or try(...)") + } +} + +func (c *checker) check(node ast.Expr, sc *scope) *Type { + switch n := node.(type) { + case *ast.Ident: + return c.ident(n, sc) + case *ast.BasicLit: + return c.literal(n) + case *ast.ParenExpr: + return c.check(n.X, sc) + case *ast.UnaryExpr: + x := c.check(n.X, sc) + if n.Op == token.NOT { + return Bool() + } + c.nilCheck(n.X, x) + if !x.isAny() && x.Kind != KindNumber { + c.report(n, "%s is not a number", x) + } + return Number() + case *ast.BinaryExpr: + return c.binary(n, sc) + case *ast.SelectorExpr: + return c.field(n, n.X, c.check(n.X, sc), n.Sel.Name, false) + case *ast.IndexExpr: + return c.index(n, n.X, c.check(n.X, sc), n.Index, c.check(n.Index, sc), false) + case *ast.CallExpr: + return c.call(n, sc) + case *ast.CompositeLit: + return c.composite(n, sc) + } + return Any +} + +func (c *checker) ident(n *ast.Ident, sc *scope) *Type { + switch n.Name { + case "true", "false": + return &Type{Kind: KindBool, Enum: []any{n.Name == "true"}} + case "nil": + return Null() + } + if t, ok := sc.lookup(n.Name); ok { + return t + } + if t, ok := c.env[n.Name]; ok { + if t == nil { + return Any + } + return t + } + if _, ok := c.p.funcs[displayIdent(n.Name)]; ok { + return Any + } + names := make([]string, 0, len(c.env)) + for k := range c.env { + names = append(names, k) + } + c.report(n, "undefined name %q%s", displayIdent(n.Name), formatHint(n.Name, names)) + return Any +} + +func (c *checker) literal(n *ast.BasicLit) *Type { + v, ok := c.p.litCache[n] + if !ok { + var err error + if v, err = evalLiteral(n); err != nil { + return Any + } + } + switch v.(type) { + case string: + return &Type{Kind: KindString, Enum: []any{v}} + case int64, float64, int: + return &Type{Kind: KindNumber, Enum: []any{v}} + } + return Any +} + +// plain drops literal values from a type: `"a" + x` is a string, not "a". +func plain(t *Type) *Type { + if t == nil || len(t.Enum) == 0 { + return t + } + c := *t + c.Enum = nil + return &c +} + +func (c *checker) binary(n *ast.BinaryExpr, sc *scope) *Type { + l, r := c.check(n.X, sc), c.check(n.Y, sc) + switch n.Op { + case token.LAND, token.LOR: + return Bool() + case token.EQL, token.NEQ: + c.comparable(n, l, r) + return Bool() + case token.LSS, token.GTR, token.LEQ, token.GEQ: + c.nilCheck(n.X, l) + c.nilCheck(n.Y, r) + if !l.isAny() && !r.isAny() && (l.Kind != r.Kind || (l.Kind != KindNumber && l.Kind != KindString)) { + c.report(n, "cannot order %s and %s", l, r) + } + return Bool() + case token.ADD: + c.nilCheck(n.X, l) + c.nilCheck(n.Y, r) + switch { + case l.isAny() && r.isAny(): + return Any + case l.isAny(): + return plain(r.NonNil()) + case r.isAny(): + return plain(l.NonNil()) + case l.Kind == r.Kind && (l.Kind == KindNumber || l.Kind == KindString): + return plain(l.NonNil()) + } + c.report(n, "cannot add %s and %s", l, r) + return Any + default: // - * / % + c.nilCheck(n.X, l) + c.nilCheck(n.Y, r) + for _, t := range []*Type{l, r} { + if !t.isAny() && t.Kind != KindNumber { + c.report(n, "%s is not a number", t) + break + } + } + return Number() + } +} + +// comparable reports == and != comparisons that can never be true. +func (c *checker) comparable(n *ast.BinaryExpr, l, r *Type) { + if l.isAny() || r.isAny() || l.Kind == KindNull || r.Kind == KindNull { + return + } + if l.Kind != r.Kind { + c.report(n, "comparing %s with %s is never true", l.NonNil(), r.NonNil()) + return + } + for _, pair := range [][2]*Type{{l, r}, {r, l}} { + set, lit := pair[0], pair[1] + if len(set.Enum) > 1 && len(lit.Enum) == 1 && !set.hasEnumValue(lit.Enum[0]) { + c.report(n, "the value is one of %s, never %s", set.NonNil(), formatEnumValue(lit.Enum[0])) + return + } + } +} + +// field types recv.name. optional is set for `?.`. +func (c *checker) field(n, recvNode ast.Expr, recv *Type, name string, optional bool) *Type { + switch { + case recv.isAny(): + return Any + case recv.Kind == KindNull: + if !optional && c.lenient == 0 { + c.report(n, "the value is always nil") + } + return Null() + } + if !optional { + c.nilCheck(recvNode, recv) + } + if recv.Kind != KindObject { + c.report(n, "cannot read field %q of %s", name, recv) + return Any + } + f, ok := recv.Fields[name] + switch { + case ok: + case recv.Open: + f = recv.Elem + if f == nil { + f = Any + } + default: + names := make([]string, 0, len(recv.Fields)) + for k := range recv.Fields { + names = append(names, k) + } + sort.Strings(names) + c.report(n, "no field %q in %s%s", name, recv.NonNil(), formatHint(name, names)) + return Any + } + if optional && recv.Optional { + return f.OrNil() + } + return f +} + +func (c *checker) index(n, recvNode ast.Expr, recv *Type, idxNode ast.Expr, idx *Type, optional bool) *Type { + if recv.isAny() { + return Any + } + if recv.Kind == KindNull { + return Null() + } + if !optional { + c.nilCheck(recvNode, recv) + } + var out *Type + switch recv.Kind { + case KindList: + if !idx.isAny() && idx.Kind != KindNumber { + c.report(n, "list index must be a number, not %s", idx) + } + out = recv.Elem + case KindString: + out = String() + case KindObject: + if idx.Kind == KindString && len(idx.Enum) == 1 { + name, _ := idx.Enum[0].(string) + return c.field(n, recvNode, recv, name, optional) + } + out = Any + if recv.Open && recv.Elem != nil { + out = recv.Elem + } + default: + c.report(n, "cannot index %s", recv) + return Any + } + if out == nil { + out = Any + } + if optional { + return out.OrNil() + } + return out +} + +var iteratingForms = map[string]bool{ + mapFormName: true, "filter": true, "flatMap": true, "any": true, + "all": true, "find": true, "count": true, "sortBy": true, +} + +func (c *checker) call(n *ast.CallExpr, sc *scope) *Type { + ident, _ := n.Fun.(*ast.Ident) + if ident == nil { + c.check(n.Fun, sc) + for _, a := range n.Args { + c.check(a, sc) + } + return Any + } + name := ident.Name + switch { + case name == trySelectFormName && len(n.Args) == 2: + recv := c.check(n.Args[0], sc) + lit, _ := n.Args[1].(*ast.BasicLit) + if lit == nil { + return Any + } + field, ok := literalString(c.literal(lit)) + if !ok { + return Any + } + return c.field(n, n.Args[0], recv, field, true).OrNil() + case name == tryIndexFormName && len(n.Args) == 2: + recv := c.check(n.Args[0], sc) + return c.index(n, n.Args[0], recv, n.Args[1], c.check(n.Args[1], sc), true).OrNil() + case iteratingForms[name]: + return c.iterating(n, displayIdent(name), sc) + case name == "try" && (len(n.Args) == 1 || len(n.Args) == 2): + c.lenient++ + a := c.check(n.Args[0], sc) + c.lenient-- + if len(n.Args) == 1 { + return a.OrNil() + } + return Join(a, c.check(n.Args[1], sc)) + case name == ifFuncName && (len(n.Args) == 2 || len(n.Args) == 3): + c.check(n.Args[0], sc) + then := c.check(n.Args[1], sc) + if len(n.Args) == 2 { + return then.OrNil() + } + return Join(then, c.check(n.Args[2], sc)) + } + + args := make([]*Type, len(n.Args)) + for i, a := range n.Args { + args[i] = c.check(a, sc) + } + display := displayIdent(name) + fn := c.cfg.funcTypes[display] + if fn == nil { + if _, registered := c.p.funcs[display]; registered { + fn = builtinTypes[display] + } + } + if fn == nil { + if _, registered := c.p.funcs[display]; registered { + return Any + } + if _, inEnv := c.env[display]; inEnv { + return Any + } + if _, bound := sc.lookup(display); bound { + return Any + } + c.report(n, "undefined function %q", display) + return Any + } + t, err := fn(args) + if err != nil { + c.report(n, "%v", err) + return Any + } + return t +} + +func (c *checker) iterating(n *ast.CallExpr, form string, sc *scope) *Type { + coll, bind, body, err := splitFormArgs(form, n) + if err != nil { + c.report(n, "%s", strings.TrimPrefix(err.Error(), ErrEvaluate.Error()+": ")) + return Any + } + ct := c.check(coll, sc) + elem := Any + switch { + case ct.isAny(), ct.Kind == KindNull: + case ct.Kind == KindList: + elem = ct.Elem + if elem == nil { + elem = Any + } + default: + c.report(n, "%s expects a list, got %s", form, ct) + } + if bind == "" { + bind = "it" + } + inner := &scope{names: map[string]*Type{bind: elem, "index": Number()}, parent: sc} + bt := c.check(body, inner) + switch form { + case "map": + return ListOf(bt) + case "filter", "sortBy": + return ListOf(elem) + case "flatMap": + if bt.Kind == KindList { + return ListOf(bt.Elem) + } + return ListOf(Any) + case "any", "all": + return Bool() + case "find": + return elem.OrNil() + case "count": + return Number() + } + return Any +} + +func (c *checker) composite(n *ast.CompositeLit, sc *scope) *Type { + switch n.Type.(type) { + case *ast.ArrayType: + var elem *Type + for _, e := range n.Elts { + elem = Join(elem, c.check(e, sc)) + } + if elem == nil { + elem = Any + } + return ListOf(elem) + case *ast.MapType: + out := ObjectOf(map[string]*Type{}) + for _, e := range n.Elts { + kv, ok := e.(*ast.KeyValueExpr) + if !ok { + continue + } + kt := c.check(kv.Key, sc) + vt := c.check(kv.Value, sc) + if key, ok := literalString(kt); ok { + out.Fields[key] = vt + } else { + out.Open = true + } + } + return out + } + return Any +} + +func literalString(t *Type) (string, bool) { + if t.Kind != KindString || len(t.Enum) != 1 { + return "", false + } + s, ok := t.Enum[0].(string) + return s, ok +} + +// builtinTypes types the functions in Builtins, StringFuncs, MathFuncs and +// CollectionFuncs. +var builtinTypes = map[string]FuncType{ + "len": func(a []*Type) (*Type, error) { + if err := wantArgs("len", a, 1); err != nil { + return nil, err + } + if t := a[0]; !t.isAny() && t.Kind != KindNull && t.Kind != KindList && t.Kind != KindString && t.Kind != KindObject { + return nil, fmt.Errorf("len of %s", t) + } + return Number(), nil + }, + "string": returns(String()), "sprintf": returns(String()), "join": returns(String()), + "int": returns(Number()), "float": returns(Number()), "abs": returns(Number()), + "ceil": returns(Number()), "floor": returns(Number()), "round": returns(Number()), + "sum": returns(Number()), "min": returns(Number()), "max": returns(Number()), + "bool": returns(Bool()), "has": returns(Bool()), "contains": returns(Bool()), + "upper": stringFunc("upper", String()), "lower": stringFunc("lower", String()), + "trim": stringFunc("trim", String()), "replace": stringFunc("replace", String()), + "startsWith": stringFunc("startsWith", Bool()), "endsWith": stringFunc("endsWith", Bool()), + "split": stringFunc("split", ListOf(String())), + "keys": returns(ListOf(String())), + "first": elemFunc, "last": elemFunc, + "slice": sameFunc, "sort": sameFunc, "reverse": sameFunc, + "entries": func(a []*Type) (*Type, error) { + value := Any + if len(a) == 1 && a[0].Kind == KindObject { + var v *Type + for _, f := range a[0].Fields { + v = Join(v, f) + } + if a[0].Open { + v = Any + } + if v != nil { + value = v + } + } + return ListOf(ObjectOf(map[string]*Type{"key": String(), "value": value})), nil + }, +} + +func returns(t *Type) FuncType { + return func([]*Type) (*Type, error) { return t, nil } +} + +func stringFunc(name string, result *Type) FuncType { + return func(a []*Type) (*Type, error) { + if len(a) > 0 && !a[0].isAny() && a[0].Kind != KindString { + return nil, fmt.Errorf("%s expects a string, got %s", name, a[0]) + } + return result, nil + } +} + +func elemFunc(a []*Type) (*Type, error) { + if len(a) == 1 && a[0].Kind == KindList && a[0].Elem != nil { + return a[0].Elem.OrNil(), nil + } + return Any, nil +} + +func sameFunc(a []*Type) (*Type, error) { + if len(a) > 0 && (a[0].Kind == KindList || a[0].Kind == KindString) { + return plain(a[0].NonNil()), nil + } + return Any, nil +} + +func wantArgs(name string, a []*Type, n int) error { + if len(a) != n { + return fmt.Errorf("%s expects %d argument(s), got %d", name, n, len(a)) + } + return nil +} diff --git a/check_test.go b/check_test.go new file mode 100644 index 0000000..7eacc81 --- /dev/null +++ b/check_test.go @@ -0,0 +1,127 @@ +package expr + +import ( + "errors" + "strings" + "testing" + + "github.com/deepnoodle-ai/expr/internal/require" +) + +var checkEnv = map[string]*Type{ + "classify": ObjectOf(map[string]*Type{ + "kind": EnumOf("bug_fix", "feature", "other"), + "confidence": Number(), + }), + "findings": ObjectOf(map[string]*Type{ + "items": ListOf(ObjectOf(map[string]*Type{"title": String(), "severity": Number()})), + }), + "report": ObjectOf(map[string]*Type{"verdict": String(), "body": String()}).OrNil(), + "event": OpenObject(map[string]*Type{"pull_request": OpenObject(map[string]*Type{"number": Number()})}), + "inputs": ObjectOf(map[string]*Type{"note": String().OrNil(), "repo": String()}), + "blob": Any, +} + +func checkSrc(t *testing.T, src string) (*Type, error) { + t.Helper() + p, err := Compile(src, WithBuiltins(), WithFunctions(StringFuncs()), WithFunctions(CollectionFuncs())) + require.NoError(t, err, src) + return p.Check(checkEnv) +} + +func TestCheckAccepts(t *testing.T) { + for src, want := range map[string]string{ + `classify.kind == "bug_fix"`: "bool", + `classify.confidence > 0.5 && classify.kind != "other"`: "bool", + `findings.items`: "list<{severity: number, title: string}>", + `map(findings.items, it.title)`: "list", + `filter(findings.items, f, f.severity > 2)`: "list<{severity: number, title: string}>", + `len(findings.items) + 1`: "number", + `report?.verdict`: "string?", + `try(report.verdict, "none")`: "string", + `event.pull_request.number`: "number", + `event.anything.at.all`: "any", + `blob.x.y[3]`: "any", + `"#" + string(event.pull_request.number)`: "string", + `{"n": len(findings.items), "k": classify.kind}`: `{k: "bug_fix" | "feature" | "other", n: number}`, + `[classify.kind, "x"]`: `list<"bug_fix" | "feature" | "other" | "x">`, + `first(findings.items)?.title`: "string?", + `inputs.note == nil`: "bool", + `if(classify.confidence > 1, "high", "low")`: `"high" | "low"`, + `upper(inputs.repo)`: "string", + `findings.items | map(it.severity) | sum()`: "number", + `find(findings.items, it.severity > 3)`: "{severity: number, title: string}?", + } { + got, err := checkSrc(t, src) + require.NoError(t, err, src) + require.Equal(t, want, got.String(), src) + } +} + +func TestCheckRejects(t *testing.T) { + for src, want := range map[string]string{ + `classify.knd`: `no field "knd" in {confidence: number, kind: "bug_fix" | "feature" | "other"} (did you mean "kind"?)`, + `classify.kind == "bugfix"`: `never "bugfix"`, + `classify.kind == 3`: "comparing", + `report.verdict`: "`report`: may be nil here", + `inputs.note + "!"`: "may be nil", + `classif.kind`: `undefined name "classif" (did you mean "classify"?)`, + `findings.items.title`: `cannot read field "title" of list`, + `map(classify, it)`: "map expects a list", + `classify.confidence + "x"`: "cannot add number and", + `-inputs.repo`: "not a number", + `upper(classify.confidence)`: "upper expects a string", + `nosuch(1)`: `undefined function "nosuch"`, + `map(findings.items, it.titel)`: `no field "titel"`, + `findings.items["x"]`: "list index must be a number", + `classify?.missing`: `no field "missing"`, + } { + _, err := checkSrc(t, src) + require.Error(t, err, src) + require.True(t, errors.Is(err, ErrType), src) + require.Contains(t, err.Error(), want, src) + } +} + +func TestCheckReportsEveryProblem(t *testing.T) { + _, err := checkSrc(t, `classify.knd == "x" || report.body == "" || nope`) + var te *TypeError + require.True(t, errors.As(err, &te)) + require.Len(t, te.Problems, 3, strings.Join(te.Problems, "\n")) +} + +func TestCheckFuncTypes(t *testing.T) { + p, err := Compile(`concat(findings.items, findings.items)`, WithFunctions(map[string]any{ + "concat": func(a, b []any) []any { return append(a, b...) }, + })) + require.NoError(t, err) + got, err := p.Check(checkEnv) + require.NoError(t, err) + require.Equal(t, "any", got.String()) + + got, err = p.Check(checkEnv, WithFuncType("concat", func(args []*Type) (*Type, error) { + var elem *Type + for _, a := range args { + elem = Join(elem, a.Elem) + } + return ListOf(elem), nil + })) + require.NoError(t, err) + require.Equal(t, "list<{severity: number, title: string}>", got.String()) +} + +func TestCheckTemplate(t *testing.T) { + tmpl, err := NewTemplate(`PR #${event.pull_request.number}: ${classify.knd}`, WithBuiltins()) + require.NoError(t, err) + err = tmpl.Check(checkEnv) + require.Error(t, err) + require.Contains(t, err.Error(), `no field "knd"`) +} + +func TestJoin(t *testing.T) { + a := ObjectOf(map[string]*Type{"verdict": String(), "body": String()}) + b := ObjectOf(map[string]*Type{"verdict": EnumOf("APPROVE"), "extra": Number()}) + require.Equal(t, "{body: string?, extra: number?, verdict: string}", Join(a, b).String()) + require.Equal(t, "string?", Join(String(), Null()).String()) + require.Equal(t, "any", Join(String(), Number()).String()) +} diff --git a/docs/reference/spec.md b/docs/reference/spec.md index edae42e..77fadf9 100644 --- a/docs/reference/spec.md +++ b/docs/reference/spec.md @@ -297,6 +297,10 @@ rather than choosing one by declaration order. Selectors chain left-to-right: `a.b.c` is `(a.b).c`. Selector chains are limited by [evaluation depth](#limits-and-safety). +The field name may be a Go keyword: `event.type`, `item.range`, +`x.default`, `x?.type`. JSON payloads use these names freely. Keywords +stay keywords everywhere else. + ## Index expressions (`x[i]`) - `map[string]any`: `i` must be a string. Missing key → `ErrEvaluate`. @@ -718,6 +722,36 @@ covers the same use cases. When the LHS is a meaningful falsey value that should be kept (`0`, `""`, `[]`), use `try(x, default)` explicitly. +## Static type checking + +`Program.Check(env map[string]*Type, opts...)` checks an expression +against the types of its environment without evaluating it, and returns +the result type. `Template.Check` does the same for every `${...}`. + +A `Type` has a kind (any, nil, bool, number, string, list, object), an +element type for lists, fields for objects (closed unless `Open`), an +`Optional` flag (the value may be nil), and an optional `Enum`. Literals +type as single-value enums, so `{"verdict": "APPROVE"}` types as +`{verdict: "APPROVE"}`. + +Check reports, as a `*TypeError` wrapping `ErrType`, every: + +- name not in env, scope or the registered functions; +- field a closed object does not have (also through `?.`); +- field read, index or arithmetic on a value that may be nil, except + through `?.`, `?[` or inside the first argument of `try`; +- operation on the wrong kind (`len` of a number, adding a number and a + string, ordering a bool, iterating a non-list); +- `==`/`!=` that can never be true: different kinds, or a literal outside + the other side's enum (`kind == "bugfix"` when kind is + `"bug_fix" | "feature"`). + +`Any` disables checking for the value and everything derived from it, so a +partly typed env checks what it can. Builtins and the optional groups have +result types; other registered functions return `Any` unless declared with +`WithFuncType`. Check is static: a passing check does not prove Run +succeeds (a list index can still be out of range). + ## Helpful errors expr annotates "not found" errors with a short hint drawn from the diff --git a/engine.go b/engine.go index af82aa3..b939ce6 100644 --- a/engine.go +++ b/engine.go @@ -36,6 +36,7 @@ package expr import ( "errors" "fmt" + "go/ast" "go/parser" "go/scanner" "go/token" @@ -211,12 +212,13 @@ func compileWithConfig(code string, cfg *compileConfig) (*Program, error) { // rewrites bare composite literals; preprocessSource handles the // keyword rewrites (`map`) so the parser accepts them as // identifiers. - parsed := preprocessSource(jsonlit.Rewrite(optaccess.Rewrite(code))) + parsed := preprocessSource(rewriteKeywordFields(jsonlit.Rewrite(optaccess.Rewrite(code)))) fset := token.NewFileSet() node, err := parser.ParseExprFrom(fset, "", parsed, 0) if err != nil { return nil, fmt.Errorf("%w: %v", ErrCompile, err) } + restoreKeywordFields(node) // Desugar pipe expressions (`a | f(x)` → `f(a, x)`) before // validation so validate and the evaluator only ever see ordinary // call nodes. See pipe.go. @@ -344,6 +346,55 @@ func preprocessSource(src string) string { return out.String() } +// keywordFieldPrefix marks a field name that is a Go keyword. Go's parser +// rejects `x.type`, so rewriteKeywordFields renames the field before +// parsing and restoreKeywordFields puts the real name back on the +// SelectorExpr, which the evaluator then looks up as written. +const keywordFieldPrefix = "__expr_kw_" + +// rewriteKeywordFields renames every Go keyword that directly follows a +// `.` (`event.type`, `item.range`, `x.map`) so the expression parses. +func rewriteKeywordFields(src string) string { + if !strings.Contains(src, ".") { + return src + } + fs := token.NewFileSet() + file := fs.AddFile("expr", fs.Base(), len(src)) + var s scanner.Scanner + s.Init(file, []byte(src), nil, 0) + var out strings.Builder + last := 0 + prev := token.ILLEGAL + for { + pos, tok, lit := s.Scan() + if tok == token.EOF { + break + } + if prev == token.PERIOD && tok.IsKeyword() { + off := file.Offset(pos) + out.WriteString(src[last:off]) + out.WriteString(keywordFieldPrefix + lit) + last = off + len(lit) + } + prev = tok + } + if last == 0 { + return src + } + out.WriteString(src[last:]) + return out.String() +} + +// restoreKeywordFields undoes rewriteKeywordFields on the parsed tree. +func restoreKeywordFields(node ast.Node) { + ast.Inspect(node, func(n ast.Node) bool { + if sel, ok := n.(*ast.SelectorExpr); ok { + sel.Sel.Name = strings.TrimPrefix(sel.Sel.Name, keywordFieldPrefix) + } + return true + }) +} + func containsAnyRewriteKeyword(src string) bool { for _, r := range keywordRewrites { if strings.Contains(src, r.src) { diff --git a/internal/optaccess/optaccess.go b/internal/optaccess/optaccess.go index 7912cbb..e711059 100644 --- a/internal/optaccess/optaccess.go +++ b/internal/optaccess/optaccess.go @@ -157,17 +157,12 @@ func isQuestion(src string, t tokenInfo) bool { } // isFieldToken reports whether t can be the field name after `?.`. -// Plain identifiers are the normal case; `map` and `if` are accepted -// too because expr lets those keywords act as ordinary identifiers -// (the engine rewrites `.map` / `.if` selectors on the non-optional -// path), and the field name is spliced into a string literal here, so -// no keyword rewrite is needed downstream. +// Plain identifiers are the normal case; any Go keyword is accepted too, +// because expr allows keywords as field names (`event?.type`). The field +// name is spliced into a string literal here, so no keyword rewrite is +// needed downstream. func isFieldToken(t token.Token) bool { - switch t { - case token.IDENT, token.MAP, token.IF: - return true - } - return false + return t == token.IDENT || t.IsKeyword() } // matchBracketForward finds the index of the `]` that closes the `[` @@ -203,6 +198,12 @@ func lhsStartIdx(toks []tokenInfo, src string, qIdx int) int { i := qIdx - 1 for i >= 0 { t := toks[i] + // A keyword used as a field name (`a.type?.b`) walks like an + // identifier in a selector chain. + if t.kind.IsKeyword() && i-1 >= 0 && toks[i-1].kind == token.PERIOD { + i -= 2 + continue + } switch t.kind { case token.IDENT, token.INT, token.FLOAT, token.STRING, token.CHAR, token.MAP, token.IF: diff --git a/kwfield_test.go b/kwfield_test.go new file mode 100644 index 0000000..526f9f3 --- /dev/null +++ b/kwfield_test.go @@ -0,0 +1,50 @@ +package expr + +import ( + "context" + "testing" + + "github.com/deepnoodle-ai/expr/internal/require" +) + +func TestKeywordFieldNames(t *testing.T) { + env := map[string]any{ + "event": map[string]any{"type": "push", "range": []any{1, 2}, "map": "m", "if": true, + "sender": map[string]any{"type": "Bot"}}, + } + for src, want := range map[string]any{ + `event.type`: "push", + `event.sender.type == "Bot"`: true, + `len(event.range) == 2`: true, + `event.map`: "m", + `event.if`: true, + `event?.type`: "push", + `map(event.range, it * 2)[1] == 4`: true, + `"a.type"`: "a.type", + `{"type": event.type}.type`: "push", + } { + p, err := Compile(src, WithBuiltins()) + require.NoError(t, err, src) + got, err := p.Run(context.Background(), env) + require.NoError(t, err, src) + require.Equal(t, want, got, src) + } + // Keywords are still keywords where they are not field names. + _, err := Compile(`type`) + require.Error(t, err) +} + +func TestKeywordFieldNamesOptional(t *testing.T) { + env := map[string]any{"a": map[string]any{"type": map[string]any{"b": 1}}, "n": nil} + for src, want := range map[string]any{ + `a?.type?.b == 1`: true, + `a.type?.b == 1`: true, + `n?.type`: nil, + } { + p, err := Compile(src, WithBuiltins()) + require.NoError(t, err, src) + got, err := p.Run(context.Background(), env) + require.NoError(t, err, src) + require.Equal(t, want, got, src) + } +} diff --git a/llms.txt b/llms.txt index 70fdbda..a4d23d7 100644 --- a/llms.txt +++ b/llms.txt @@ -452,6 +452,26 @@ if errors.Is(err, expr.ErrCompile) { /* ... */ } if errors.Is(err, expr.ErrEvaluate) { /* ... */ } ``` +## Static type checking + +```go +env := map[string]*expr.Type{ + "classify": expr.ObjectOf(map[string]*expr.Type{"kind": expr.EnumOf("bug_fix", "feature")}), + "report": expr.ObjectOf(map[string]*expr.Type{"verdict": expr.String()}).OrNil(), + "event": expr.Any, +} +t, err := p.Check(env, expr.WithFuncType("concat", concatType)) +// errors.Is(err, expr.ErrType); err.(*expr.TypeError).Problems lists every problem +``` + +Types: `expr.Any`, `Null()`, `Bool()`, `Number()`, `String()`, `EnumOf(...)`, +`ListOf(t)`, `ObjectOf(fields)` (closed), `OpenObject(fields)`, `t.OrNil()`, +`t.NonNil()`, `Join(a, b)`. Check catches unknown names and fields, +reads of maybe-nil values without `?.`/`try`, wrong-kind operations, and +comparisons that are never true (including against an enum). + +Field names may be Go keywords: `event.type`, `x?.range`. + ## Cancellation `Program.Run(ctx, env)` checks `ctx.Err()` before dispatching each AST diff --git a/types.go b/types.go new file mode 100644 index 0000000..a8112cf --- /dev/null +++ b/types.go @@ -0,0 +1,232 @@ +package expr + +import ( + "fmt" + "sort" + "strconv" + "strings" +) + +// Kind is the shape of a Type. +type Kind int + +const ( + // KindAny is a value the checker knows nothing about. Every operation + // on it is allowed and produces Any. + KindAny Kind = iota + KindNull + KindBool + KindNumber + KindString + KindList + KindObject +) + +// Type describes the values an expression may produce, for static checking +// with [Program.Check]. Build types with the constructors ([String], +// [ListOf], [ObjectOf], ...) or as literals. +// +// Types are structural and deliberately small: they catch misspelled +// fields, operations on the wrong kind of value, comparisons that can +// never be true, and reads of values that may be nil. +type Type struct { + Kind Kind + // Elem is the element type of a list. For an open object it is the + // type of fields not listed in Fields (nil means Any). + Elem *Type + // Fields are an object's known fields. + Fields map[string]*Type + // Open allows fields beyond Fields. Selecting an unlisted field of a + // closed object is an error. + Open bool + // Optional means the value may be nil. + Optional bool + // Enum, if set, lists every value the type can hold. + Enum []any +} + +// Any is the type of values the checker knows nothing about. +var Any = &Type{Kind: KindAny} + +func Null() *Type { return &Type{Kind: KindNull} } +func Bool() *Type { return &Type{Kind: KindBool} } +func Number() *Type { return &Type{Kind: KindNumber} } +func String() *Type { return &Type{Kind: KindString} } + +// ListOf is a list whose elements have type elem. +func ListOf(elem *Type) *Type { return &Type{Kind: KindList, Elem: elem} } + +// ObjectOf is a closed object with exactly the given fields. +func ObjectOf(fields map[string]*Type) *Type { return &Type{Kind: KindObject, Fields: fields} } + +// OpenObject is an object that has the given fields and possibly others. +func OpenObject(fields map[string]*Type) *Type { + return &Type{Kind: KindObject, Fields: fields, Open: true} +} + +// EnumOf is a string type limited to the given values. +func EnumOf(values ...string) *Type { + t := String() + for _, v := range values { + t.Enum = append(t.Enum, v) + } + return t +} + +// OrNil returns a copy of t that may also be nil. +func (t *Type) OrNil() *Type { + if t == nil || t.Kind == KindAny || t.Kind == KindNull { + return t + } + c := *t + c.Optional = true + return &c +} + +// NonNil returns a copy of t that is never nil. +func (t *Type) NonNil() *Type { + if t == nil || !t.Optional { + return t + } + c := *t + c.Optional = false + return &c +} + +func (t *Type) isAny() bool { return t == nil || t.Kind == KindAny } + +// String renders the type, for example `{kind: "bug_fix" | "feature", items: list}?`. +func (t *Type) String() string { + if t == nil { + return "any" + } + var s string + switch t.Kind { + case KindAny: + return "any" + case KindNull: + return "nil" + case KindBool: + s = "bool" + case KindNumber: + s = "number" + case KindString: + s = "string" + case KindList: + s = "list<" + t.Elem.String() + ">" + case KindObject: + names := make([]string, 0, len(t.Fields)) + for k := range t.Fields { + names = append(names, k) + } + sort.Strings(names) + parts := make([]string, 0, len(names)+1) + for _, k := range names { + parts = append(parts, k+": "+t.Fields[k].String()) + } + if t.Open { + parts = append(parts, "...") + } + s = "{" + strings.Join(parts, ", ") + "}" + } + if len(t.Enum) > 0 { + vals := make([]string, len(t.Enum)) + for i, v := range t.Enum { + vals[i] = formatEnumValue(v) + } + s = strings.Join(vals, " | ") + if t.Optional && len(t.Enum) > 1 { + s = "(" + s + ")" + } + } + if t.Optional { + s += "?" + } + return s +} + +func formatEnumValue(v any) string { + if s, ok := v.(string); ok { + return strconv.Quote(s) + } + return fmt.Sprint(v) +} + +func (t *Type) hasEnumValue(v any) bool { + for _, e := range t.Enum { + if enumEqual(e, v) { + return true + } + } + return false +} + +func enumEqual(a, b any) bool { + if af, ok := enumFloat(a); ok { + bf, ok := enumFloat(b) + return ok && af == bf + } + return a == b +} + +func enumFloat(v any) (float64, bool) { + switch n := v.(type) { + case int: + return float64(n), true + case int64: + return float64(n), true + case float64: + return n, true + } + return 0, false +} + +// Join is the type of a value that has either type a or type b. +func Join(a, b *Type) *Type { + switch { + case a == nil: + return b + case b == nil: + return a + case a.isAny() || b.isAny(): + return Any + case a.Kind == KindNull: + return b.OrNil() + case b.Kind == KindNull: + return a.OrNil() + case a.Kind != b.Kind: + return Any + } + out := &Type{Kind: a.Kind, Optional: a.Optional || b.Optional} + switch a.Kind { + case KindList: + out.Elem = Join(a.Elem, b.Elem) + case KindObject: + out.Open = a.Open || b.Open + out.Fields = map[string]*Type{} + for k, at := range a.Fields { + if bt, ok := b.Fields[k]; ok { + out.Fields[k] = Join(at, bt) + } else { + out.Fields[k] = at.OrNil() + } + } + for k, bt := range b.Fields { + if _, ok := a.Fields[k]; !ok { + out.Fields[k] = bt.OrNil() + } + } + if a.Open && b.Open { + out.Elem = Join(a.Elem, b.Elem) + } + } + if len(a.Enum) > 0 && len(b.Enum) > 0 { + out.Enum = append([]any(nil), a.Enum...) + for _, v := range b.Enum { + if !out.hasEnumValue(v) { + out.Enum = append(out.Enum, v) + } + } + } + return out +} From 7be56ac600ee38b8bbadd8cbf3f325bbbb635da7 Mon Sep 17 00:00:00 2001 From: Curtis Myzie Date: Wed, 28 Oct 2026 10:22:18 -0400 Subject: [PATCH 2/9] Check: match runtime semantics found in review - && and || type as the join of their operands, as Run returns operands. - Nil checks narrow (x != nil && x.a, x == nil || x.a, if(x != nil, x.a, b)). - Builtins that fail on nil (upper, round, split, ...) report nil arguments. - try needs 2 arguments and if needs 3, as Run requires. - == and != on lists and objects are reported. - Unlisted fields of an open object are maybe-absent: read them with ?. - Join treats a missing element type as Any. Co-Authored-By: Claude Opus 5.5 --- check.go | 134 +++++++++++++++++++++++++++++++++++++---- check_test.go | 50 +++++++++++++++ docs/reference/spec.md | 7 +++ types.go | 18 +++++- 4 files changed, 194 insertions(+), 15 deletions(-) diff --git a/check.go b/check.go index 22a7d02..4a7fbc6 100644 --- a/check.go +++ b/check.go @@ -98,6 +98,71 @@ type checker struct { // lenient is positive inside try's first argument, where a nil // receiver is caught at run time and so is not a problem. lenient int + // narrowed counts, by source text, expressions known not to be nil + // here: the right side of `x != nil && ...`, the then-branch of + // `if(x != nil, ...)`. + narrowed map[string]int +} + +// narrowings returns the expressions that cond being truthy (or falsy) +// proves are not nil. +func narrowings(cond ast.Expr, truthy bool) []string { + switch n := cond.(type) { + case *ast.ParenExpr: + return narrowings(n.X, truthy) + case *ast.UnaryExpr: + if n.Op == token.NOT { + return narrowings(n.X, !truthy) + } + case *ast.BinaryExpr: + switch { + case n.Op == token.LAND && truthy, n.Op == token.LOR && !truthy: + return append(narrowings(n.X, truthy), narrowings(n.Y, truthy)...) + case n.Op == token.NEQ && truthy, n.Op == token.EQL && !truthy: + if isNilIdent(n.Y) { + return []string{exprDisplayString(n.X)} + } + if isNilIdent(n.X) { + return []string{exprDisplayString(n.Y)} + } + } + case *ast.Ident, *ast.SelectorExpr, *ast.IndexExpr, *ast.CallExpr: + if truthy { + return []string{exprDisplayString(cond)} + } + } + return nil +} + +func isNilIdent(e ast.Expr) bool { + id, ok := e.(*ast.Ident) + return ok && id.Name == "nil" +} + +// narrow checks node with the given expressions known not to be nil. +func (c *checker) narrow(names []string, node ast.Expr, sc *scope) *Type { + if c.narrowed == nil { + c.narrowed = map[string]int{} + } + for _, n := range names { + c.narrowed[n]++ + } + t := c.check(node, sc) + for _, n := range names { + c.narrowed[n]-- + } + return t +} + +// applyNarrowing drops Optional from a value narrowed by its context. +func (c *checker) applyNarrowing(node ast.Expr, t *Type) *Type { + if len(c.narrowed) == 0 || t == nil || !t.Optional { + return t + } + if c.narrowed[exprDisplayString(node)] > 0 { + return t.NonNil() + } + return t } // scope holds the names a higher-order form binds. @@ -131,6 +196,14 @@ func (c *checker) nilCheck(node ast.Expr, t *Type) { } func (c *checker) check(node ast.Expr, sc *scope) *Type { + switch node.(type) { + case *ast.Ident, *ast.SelectorExpr, *ast.IndexExpr, *ast.CallExpr: + return c.applyNarrowing(node, c.checkNode(node, sc)) + } + return c.checkNode(node, sc) +} + +func (c *checker) checkNode(node ast.Expr, sc *scope) *Type { switch n := node.(type) { case *ast.Ident: return c.ident(n, sc) @@ -217,10 +290,20 @@ func plain(t *Type) *Type { } func (c *checker) binary(n *ast.BinaryExpr, sc *scope) *Type { + switch n.Op { + case token.LAND: + // `a && b` is a when a is falsy, else b. + l := c.check(n.X, sc) + r := c.narrow(narrowings(n.X, true), n.Y, sc) + return Join(l, r) + case token.LOR: + // `a || b` is a when a is truthy (so never nil), else b. + l := c.check(n.X, sc) + r := c.narrow(narrowings(n.X, false), n.Y, sc) + return Join(l.NonNil(), r) + } l, r := c.check(n.X, sc), c.check(n.Y, sc) switch n.Op { - case token.LAND, token.LOR: - return Bool() case token.EQL, token.NEQ: c.comparable(n, l, r) return Bool() @@ -264,6 +347,10 @@ func (c *checker) comparable(n *ast.BinaryExpr, l, r *Type) { if l.isAny() || r.isAny() || l.Kind == KindNull || r.Kind == KindNull { return } + if l.Kind == KindList || l.Kind == KindObject || r.Kind == KindList || r.Kind == KindObject { + c.report(n, "cannot compare %s and %s with %s", l.NonNil(), r.NonNil(), n.Op) + return + } if l.Kind != r.Kind { c.report(n, "comparing %s with %s is never true", l.NonNil(), r.NonNil()) return @@ -299,10 +386,18 @@ func (c *checker) field(n, recvNode ast.Expr, recv *Type, name string, optional switch { case ok: case recv.Open: + // An unlisted field may be absent, and selecting a missing key + // fails at run time; only ?. reads it safely. f = recv.Elem if f == nil { f = Any } + if f.Kind != KindAny { + if !optional && c.lenient == 0 { + c.report(n, "field %q may be absent; use ?.", name) + } + f = f.OrNil() + } default: names := make([]string, 0, len(recv.Fields)) for k := range recv.Fields { @@ -391,21 +486,23 @@ func (c *checker) call(n *ast.CallExpr, sc *scope) *Type { return c.index(n, n.Args[0], recv, n.Args[1], c.check(n.Args[1], sc), true).OrNil() case iteratingForms[name]: return c.iterating(n, displayIdent(name), sc) - case name == "try" && (len(n.Args) == 1 || len(n.Args) == 2): + case name == "try": + if len(n.Args) != 2 { + c.report(n, "try expects 2 arguments (value, default), got %d", len(n.Args)) + return Any + } c.lenient++ a := c.check(n.Args[0], sc) c.lenient-- - if len(n.Args) == 1 { - return a.OrNil() - } return Join(a, c.check(n.Args[1], sc)) - case name == ifFuncName && (len(n.Args) == 2 || len(n.Args) == 3): - c.check(n.Args[0], sc) - then := c.check(n.Args[1], sc) - if len(n.Args) == 2 { - return then.OrNil() + case name == ifFuncName: + if len(n.Args) != 3 { + c.report(n, "if expects 3 arguments (cond, then, else), got %d", len(n.Args)) + return Any } - return Join(then, c.check(n.Args[2], sc)) + c.check(n.Args[0], sc) + then := c.narrow(narrowings(n.Args[0], true), n.Args[1], sc) + return Join(then, c.narrow(narrowings(n.Args[0], false), n.Args[2], sc)) } args := make([]*Type, len(n.Args)) @@ -432,6 +529,11 @@ func (c *checker) call(n *ast.CallExpr, sc *scope) *Type { c.report(n, "undefined function %q", display) return Any } + if nilIntolerant[display] { + for i, a := range args { + c.nilCheck(n.Args[i], a) + } + } t, err := fn(args) if err != nil { c.report(n, "%v", err) @@ -440,6 +542,14 @@ func (c *checker) call(n *ast.CallExpr, sc *scope) *Type { return t } +// nilIntolerant are the builtins that fail on a nil argument. The rest +// (len, string, int, first, keys, sum, ...) treat nil as empty. +var nilIntolerant = map[string]bool{ + "abs": true, "ceil": true, "floor": true, "round": true, "min": true, "max": true, + "upper": true, "lower": true, "trim": true, "replace": true, "split": true, + "startsWith": true, "endsWith": true, "slice": true, +} + func (c *checker) iterating(n *ast.CallExpr, form string, sc *scope) *Type { coll, bind, body, err := splitFormArgs(form, n) if err != nil { diff --git a/check_test.go b/check_test.go index 7eacc81..375997c 100644 --- a/check_test.go +++ b/check_test.go @@ -125,3 +125,53 @@ func TestJoin(t *testing.T) { require.Equal(t, "string?", Join(String(), Null()).String()) require.Equal(t, "any", Join(String(), Number()).String()) } + +// Regressions from review: each was accepted by Check and failed in Run, or +// the reverse. +func TestCheckMatchesRuntime(t *testing.T) { + env := map[string]*Type{ + "a": ObjectOf(map[string]*Type{ + "n": Number().OrNil(), + "name": String().OrNil(), + "items": ListOf(String()).OrNil(), + "flag": Bool(), + "t": ListOf(String()), + }), + "scores": &Type{Kind: KindObject, Open: true, Elem: Number()}, + "loose": OpenObject(nil), + } + check := func(src string) (*Type, error) { + p, err := Compile(src, WithBuiltins(), WithFunctions(StringFuncs()), WithFunctions(MathFuncs())) + require.NoError(t, err, src) + return p.Check(env) + } + for src, want := range map[string]string{ + `a.flag && a.flag`: "bool", + `a.items && len(a.items) > 0`: "any", // a list or a bool: not a boolean + `a.name || "anon"`: "string", + `a.n != nil && a.n > 3`: "bool", + `a.n == nil || a.n > 3`: "bool", + `if(a.name != nil, upper(a.name), "")`: "string", + `scores?.alice`: "number?", + `loose.anything`: "any", + `len(a.items)`: "number", + `string(a.n)`: "string", + } { + got, err := check(src) + require.NoError(t, err, src) + require.Equal(t, want, got.String(), src) + } + for src, want := range map[string]string{ + `upper(a.name)`: "may be nil", + `round(a.n)`: "may be nil", + `try(a.n)`: "try expects 2 arguments", + `if(a.flag, 1)`: "if expects 3 arguments", + `a.t == a.t`: "cannot compare", + `scores.alice + 1`: `field "alice" may be absent`, + `a.n > 3 && a.n != nil`: "may be nil", + } { + _, err := check(src) + require.Error(t, err, src) + require.Contains(t, err.Error(), want, src) + } +} diff --git a/docs/reference/spec.md b/docs/reference/spec.md index 77fadf9..d8c4522 100644 --- a/docs/reference/spec.md +++ b/docs/reference/spec.md @@ -746,6 +746,13 @@ Check reports, as a `*TypeError` wrapping `ErrType`, every: the other side's enum (`kind == "bugfix"` when kind is `"bug_fix" | "feature"`). +`&&` and `||` return one of their operands (see Logical), so they type +as the join of their operand types: `bool` only when both sides are. A +nil check narrows: in `x != nil && x.a > 3`, `x == nil || x.a > 3` and +`if(x != nil, x.a, 0)`, `x` is known not to be nil where it is read. A field +an open object does not list may be absent, so it is read with `?.`. +`==` and `!=` on lists or objects are reported, since Run rejects them. + `Any` disables checking for the value and everything derived from it, so a partly typed env checks what it can. Builtins and the optional groups have result types; other registered functions return `Any` unless declared with diff --git a/types.go b/types.go index a8112cf..628a6bd 100644 --- a/types.go +++ b/types.go @@ -181,6 +181,13 @@ func enumFloat(v any) (float64, bool) { return 0, false } +func orAny(t *Type) *Type { + if t == nil { + return Any + } + return t +} + // Join is the type of a value that has either type a or type b. func Join(a, b *Type) *Type { switch { @@ -200,7 +207,7 @@ func Join(a, b *Type) *Type { out := &Type{Kind: a.Kind, Optional: a.Optional || b.Optional} switch a.Kind { case KindList: - out.Elem = Join(a.Elem, b.Elem) + out.Elem = Join(orAny(a.Elem), orAny(b.Elem)) case KindObject: out.Open = a.Open || b.Open out.Fields = map[string]*Type{} @@ -216,8 +223,13 @@ func Join(a, b *Type) *Type { out.Fields[k] = bt.OrNil() } } - if a.Open && b.Open { - out.Elem = Join(a.Elem, b.Elem) + switch { + case a.Open && b.Open: + out.Elem = Join(orAny(a.Elem), orAny(b.Elem)) + case a.Open: + out.Elem = a.Elem + case b.Open: + out.Elem = b.Elem } } if len(a.Enum) > 0 && len(b.Enum) > 0 { From 7d6c5fd36c54d66a2a8a53da2ea3f14bb86586b3 Mon Sep 17 00:00:00 2001 From: Curtis Myzie Date: Wed, 28 Oct 2026 10:25:03 -0400 Subject: [PATCH 3/9] Check: unions for joins of different kinds Join of different known kinds now keeps the variants instead of collapsing to Any, so (s || 0) + 1 and a.items && len(a.items) > 0 used as a boolean are reported rather than unchecked. Co-Authored-By: Claude Opus 5.5 --- check.go | 26 +++++++++++++++++ check_test.go | 28 ++++++++++++++++-- docs/reference/spec.md | 4 ++- types.go | 64 ++++++++++++++++++++++++++++++++++++++---- 4 files changed, 114 insertions(+), 8 deletions(-) diff --git a/check.go b/check.go index 4a7fbc6..275ec66 100644 --- a/check.go +++ b/check.go @@ -188,6 +188,15 @@ func (c *checker) report(node ast.Expr, format string, args ...any) { c.problems = append(c.problems, msg) } +// oneKind reports a union where an operation needs a single kind. +func (c *checker) oneKind(node ast.Expr, t *Type) bool { + if t.IsUnion() { + c.report(node, "may be %s; the operation needs one kind", t) + return false + } + return true +} + // nilCheck reports a value that may be nil where nil is not allowed. func (c *checker) nilCheck(node ast.Expr, t *Type) { if t != nil && t.Optional && c.lenient == 0 { @@ -217,6 +226,7 @@ func (c *checker) checkNode(node ast.Expr, sc *scope) *Type { return Bool() } c.nilCheck(n.X, x) + c.oneKind(n.X, x) if !x.isAny() && x.Kind != KindNumber { c.report(n, "%s is not a number", x) } @@ -310,6 +320,9 @@ func (c *checker) binary(n *ast.BinaryExpr, sc *scope) *Type { case token.LSS, token.GTR, token.LEQ, token.GEQ: c.nilCheck(n.X, l) c.nilCheck(n.Y, r) + if !c.oneKind(n.X, l) || !c.oneKind(n.Y, r) { + return Bool() + } if !l.isAny() && !r.isAny() && (l.Kind != r.Kind || (l.Kind != KindNumber && l.Kind != KindString)) { c.report(n, "cannot order %s and %s", l, r) } @@ -317,6 +330,9 @@ func (c *checker) binary(n *ast.BinaryExpr, sc *scope) *Type { case token.ADD: c.nilCheck(n.X, l) c.nilCheck(n.Y, r) + if !c.oneKind(n.X, l) || !c.oneKind(n.Y, r) { + return Any + } switch { case l.isAny() && r.isAny(): return Any @@ -332,6 +348,9 @@ func (c *checker) binary(n *ast.BinaryExpr, sc *scope) *Type { default: // - * / % c.nilCheck(n.X, l) c.nilCheck(n.Y, r) + if !c.oneKind(n.X, l) || !c.oneKind(n.Y, r) { + return Number() + } for _, t := range []*Type{l, r} { if !t.isAny() && t.Kind != KindNumber { c.report(n, "%s is not a number", t) @@ -367,6 +386,9 @@ func (c *checker) comparable(n *ast.BinaryExpr, l, r *Type) { // field types recv.name. optional is set for `?.`. func (c *checker) field(n, recvNode ast.Expr, recv *Type, name string, optional bool) *Type { switch { + case recv.IsUnion(): + c.oneKind(recvNode, recv) + return Any case recv.isAny(): return Any case recv.Kind == KindNull: @@ -414,6 +436,10 @@ func (c *checker) field(n, recvNode ast.Expr, recv *Type, name string, optional } func (c *checker) index(n, recvNode ast.Expr, recv *Type, idxNode ast.Expr, idx *Type, optional bool) *Type { + if recv.IsUnion() { + c.oneKind(recvNode, recv) + return Any + } if recv.isAny() { return Any } diff --git a/check_test.go b/check_test.go index 375997c..76a975f 100644 --- a/check_test.go +++ b/check_test.go @@ -123,7 +123,7 @@ func TestJoin(t *testing.T) { b := ObjectOf(map[string]*Type{"verdict": EnumOf("APPROVE"), "extra": Number()}) require.Equal(t, "{body: string?, extra: number?, verdict: string}", Join(a, b).String()) require.Equal(t, "string?", Join(String(), Null()).String()) - require.Equal(t, "any", Join(String(), Number()).String()) + require.Equal(t, "number | string", Join(String(), Number()).String()) } // Regressions from review: each was accepted by Check and failed in Run, or @@ -147,7 +147,7 @@ func TestCheckMatchesRuntime(t *testing.T) { } for src, want := range map[string]string{ `a.flag && a.flag`: "bool", - `a.items && len(a.items) > 0`: "any", // a list or a bool: not a boolean + `a.items && len(a.items) > 0`: "(bool | list)?", `a.name || "anon"`: "string", `a.n != nil && a.n > 3`: "bool", `a.n == nil || a.n > 3`: "bool", @@ -175,3 +175,27 @@ func TestCheckMatchesRuntime(t *testing.T) { require.Contains(t, err.Error(), want, src) } } + +func TestUnions(t *testing.T) { + env := map[string]*Type{"s": String().OrNil(), "n": Number(), "l": ListOf(String()).OrNil()} + check := func(src string) (*Type, error) { + p, err := Compile(src, WithBuiltins()) + require.NoError(t, err, src) + return p.Check(env) + } + got, err := check(`s || 0`) + require.NoError(t, err) + require.Equal(t, "0 | string", got.String()) + require.True(t, got.IsUnion()) + + got, err = check(`l && len(l) > 0`) + require.NoError(t, err) + require.Equal(t, "(bool | list)?", got.String()) + + for _, src := range []string{`(s || 0) + 1`, `(s || 0) > 1`, `(s || 0).x`} { + _, err := check(src) + require.Error(t, err, src) + require.Contains(t, err.Error(), "the operation needs one kind", src) + } + require.Equal(t, "string", Join(EnumOf("a"), String()).String()) +} diff --git a/docs/reference/spec.md b/docs/reference/spec.md index d8c4522..750116f 100644 --- a/docs/reference/spec.md +++ b/docs/reference/spec.md @@ -747,7 +747,9 @@ Check reports, as a `*TypeError` wrapping `ErrType`, every: `"bug_fix" | "feature"`). `&&` and `||` return one of their operands (see Logical), so they type -as the join of their operand types: `bool` only when both sides are. A +as the join of their operand types: `bool` only when both sides are. Joining +types of different kinds gives a union (`string | 0` for `s || 0`), which +arithmetic, ordering, field reads and indexing reject: they need one kind. A nil check narrows: in `x != nil && x.a > 3`, `x == nil || x.a > 3` and `if(x != nil, x.a, 0)`, `x` is known not to be nil where it is read. A field an open object does not list may be absent, so it is read with `?.`. diff --git a/types.go b/types.go index 628a6bd..3796f71 100644 --- a/types.go +++ b/types.go @@ -43,6 +43,10 @@ type Type struct { Optional bool // Enum, if set, lists every value the type can hold. Enum []any + // Variants, on a KindAny type, are the known types of different kinds + // the value may have, as Join produces for `x || 0` when x is a + // string. Operations that need one kind report a union. + Variants []*Type } // Any is the type of values the checker knows nothing about. @@ -75,7 +79,7 @@ func EnumOf(values ...string) *Type { // OrNil returns a copy of t that may also be nil. func (t *Type) OrNil() *Type { - if t == nil || t.Kind == KindAny || t.Kind == KindNull { + if t == nil || (t.Kind == KindAny && len(t.Variants) == 0) || t.Kind == KindNull { return t } c := *t @@ -95,6 +99,9 @@ func (t *Type) NonNil() *Type { func (t *Type) isAny() bool { return t == nil || t.Kind == KindAny } +// IsUnion reports whether t is a union of known types of different kinds. +func (t *Type) IsUnion() bool { return t != nil && t.Kind == KindAny && len(t.Variants) > 0 } + // String renders the type, for example `{kind: "bug_fix" | "feature", items: list}?`. func (t *Type) String() string { if t == nil { @@ -103,7 +110,18 @@ func (t *Type) String() string { var s string switch t.Kind { case KindAny: - return "any" + if len(t.Variants) == 0 { + return "any" + } + parts := make([]string, len(t.Variants)) + for i, v := range t.Variants { + parts[i] = v.String() + } + s = strings.Join(parts, " | ") + if t.Optional { + return "(" + s + ")?" + } + return s case KindNull: return "nil" case KindBool: @@ -181,6 +199,42 @@ func enumFloat(v any) (float64, bool) { return 0, false } +// union joins types of different kinds, merging those of the same kind. +func union(a, b *Type) *Type { + var parts []*Type + optional := false + for _, t := range []*Type{a, b} { + optional = optional || t.Optional + if t.IsUnion() { + parts = append(parts, t.Variants...) + } else { + parts = append(parts, t.NonNil()) + } + } + var out []*Type + for _, p := range parts { + merged := false + for i, o := range out { + if o.Kind == p.Kind { + out[i] = Join(o, p) + merged = true + break + } + } + if !merged { + out = append(out, p.NonNil()) + } + } + sort.SliceStable(out, func(i, j int) bool { return out[i].Kind < out[j].Kind }) + if len(out) == 1 { + if optional { + return out[0].OrNil() + } + return out[0] + } + return &Type{Kind: KindAny, Variants: out, Optional: optional} +} + func orAny(t *Type) *Type { if t == nil { return Any @@ -195,14 +249,14 @@ func Join(a, b *Type) *Type { return b case b == nil: return a - case a.isAny() || b.isAny(): + case (a.isAny() && !a.IsUnion()) || (b.isAny() && !b.IsUnion()): return Any case a.Kind == KindNull: return b.OrNil() case b.Kind == KindNull: return a.OrNil() - case a.Kind != b.Kind: - return Any + case a.IsUnion() || b.IsUnion() || a.Kind != b.Kind: + return union(a, b) } out := &Type{Kind: a.Kind, Optional: a.Optional || b.Optional} switch a.Kind { From 2fb599e399ea6739d59d1090478623b0caeeff10 Mon Sep 17 00:00:00 2001 From: Curtis Myzie Date: Wed, 28 Oct 2026 10:32:14 -0400 Subject: [PATCH 4/9] Check: fixes from review of #29 - S1: a nil field type is Any, like a nil env entry; no panics. - S2: a form's binding hides nil checks on the name it shadows. - S3: builtin result types apply only when the registered function is the builtin, not a host function that replaced it. - S4: indexing a value that is always nil is reported. - S5: comparisons against single-value enums are checked. - N1: only keyword field names are restored by the keyword rewrite. Co-Authored-By: Claude Opus 5.5 --- check.go | 55 +++++++++++++++++++++++++++++++++++++++++++-------- check_test.go | 45 +++++++++++++++++++++++++++++++++++++++++ engine.go | 5 ++++- 3 files changed, 96 insertions(+), 9 deletions(-) diff --git a/check.go b/check.go index 275ec66..98c2481 100644 --- a/check.go +++ b/check.go @@ -5,6 +5,7 @@ import ( "fmt" "go/ast" "go/token" + "reflect" "sort" "strings" ) @@ -376,7 +377,7 @@ func (c *checker) comparable(n *ast.BinaryExpr, l, r *Type) { } for _, pair := range [][2]*Type{{l, r}, {r, l}} { set, lit := pair[0], pair[1] - if len(set.Enum) > 1 && len(lit.Enum) == 1 && !set.hasEnumValue(lit.Enum[0]) { + if len(set.Enum) > 0 && len(lit.Enum) == 1 && !set.hasEnumValue(lit.Enum[0]) { c.report(n, "the value is one of %s, never %s", set.NonNil(), formatEnumValue(lit.Enum[0])) return } @@ -407,6 +408,7 @@ func (c *checker) field(n, recvNode ast.Expr, recv *Type, name string, optional f, ok := recv.Fields[name] switch { case ok: + f = orAny(f) // a nil field type is unknown, like a nil env entry case recv.Open: // An unlisted field may be absent, and selecting a missing key // fails at run time; only ?. reads it safely. @@ -444,6 +446,9 @@ func (c *checker) index(n, recvNode ast.Expr, recv *Type, idxNode ast.Expr, idx return Any } if recv.Kind == KindNull { + if !optional && c.lenient == 0 { + c.report(n, "the value is always nil") + } return Null() } if !optional { @@ -471,9 +476,7 @@ func (c *checker) index(n, recvNode ast.Expr, recv *Type, idxNode ast.Expr, idx c.report(n, "cannot index %s", recv) return Any } - if out == nil { - out = Any - } + out = orAny(out) if optional { return out.OrNil() } @@ -538,7 +541,7 @@ func (c *checker) call(n *ast.CallExpr, sc *scope) *Type { display := displayIdent(name) fn := c.cfg.funcTypes[display] if fn == nil { - if _, registered := c.p.funcs[display]; registered { + if registered, ok := c.p.funcs[display]; ok && isBuiltin(display, registered) { fn = builtinTypes[display] } } @@ -555,7 +558,7 @@ func (c *checker) call(n *ast.CallExpr, sc *scope) *Type { c.report(n, "undefined function %q", display) return Any } - if nilIntolerant[display] { + if nilIntolerant[display] && fn != nil && c.cfg.funcTypes[display] == nil { for i, a := range args { c.nilCheck(n.Args[i], a) } @@ -568,6 +571,28 @@ func (c *checker) call(n *ast.CallExpr, sc *scope) *Type { return t } +// builtinFuncs maps each builtin name to its function's code pointer, so +// Check uses a builtin's type only when the registered function is the +// builtin, not a host function that replaced it. +var builtinFuncs = func() map[string]uintptr { + out := map[string]uintptr{} + for _, group := range []map[string]any{Builtins(), StringFuncs(), MathFuncs(), CollectionFuncs()} { + for name, fn := range group { + out[name] = reflect.ValueOf(fn).Pointer() + } + } + return out +}() + +func isBuiltin(name string, fn any) bool { + ptr, ok := builtinFuncs[name] + if !ok { + return false + } + rv := reflect.ValueOf(fn) + return rv.Kind() == reflect.Func && rv.Pointer() == ptr +} + // nilIntolerant are the builtins that fail on a nil argument. The rest // (len, string, int, first, keys, sum, ...) treat nil as empty. var nilIntolerant = map[string]bool{ @@ -598,7 +623,21 @@ func (c *checker) iterating(n *ast.CallExpr, form string, sc *scope) *Type { bind = "it" } inner := &scope{names: map[string]*Type{bind: elem, "index": Number()}, parent: sc} + // The binding shadows any outer name, so nil checks on that name do + // not reach into the body. + hidden := map[string]int{} + for k, v := range c.narrowed { + for _, name := range []string{bind, "index"} { + if k == name || strings.HasPrefix(k, name+".") || strings.HasPrefix(k, name+"?") || strings.HasPrefix(k, name+"[") { + hidden[k] = v + delete(c.narrowed, k) + } + } + } bt := c.check(body, inner) + for k, v := range hidden { + c.narrowed[k] = v + } switch form { case "map": return ListOf(bt) @@ -714,14 +753,14 @@ func stringFunc(name string, result *Type) FuncType { } func elemFunc(a []*Type) (*Type, error) { - if len(a) == 1 && a[0].Kind == KindList && a[0].Elem != nil { + if len(a) == 1 && a[0] != nil && a[0].Kind == KindList && a[0].Elem != nil { return a[0].Elem.OrNil(), nil } return Any, nil } func sameFunc(a []*Type) (*Type, error) { - if len(a) > 0 && (a[0].Kind == KindList || a[0].Kind == KindString) { + if len(a) > 0 && a[0] != nil && (a[0].Kind == KindList || a[0].Kind == KindString) { return plain(a[0].NonNil()), nil } return Any, nil diff --git a/check_test.go b/check_test.go index 76a975f..192c6e5 100644 --- a/check_test.go +++ b/check_test.go @@ -1,6 +1,7 @@ package expr import ( + "context" "errors" "strings" "testing" @@ -199,3 +200,47 @@ func TestUnions(t *testing.T) { } require.Equal(t, "string", Join(EnumOf("a"), String()).String()) } + +// Regressions from review of #29. +func TestCheckReviewFixes(t *testing.T) { + env := map[string]*Type{ + "obj": ObjectOf(map[string]*Type{"items": nil, "k": nil}), + "x": ObjectOf(map[string]*Type{"a": Number()}).OrNil(), + "xs": ListOf(ObjectOf(map[string]*Type{"a": Number()}).OrNil()), + "e": EnumOf("a"), + "z": Null(), + } + check := func(src string, opts ...Option) error { + p, err := Compile(src, append([]Option{WithBuiltins(), WithFunctions(CollectionFuncs())}, opts...)...) + require.NoError(t, err, src) + _, err = p.Check(env) + return err + } + // S1: nil field types are unknown, never a panic. + for _, src := range []string{`first(obj.items)`, `reverse(obj.items)`, `obj[obj.k]`, `obj.items.deep`} { + require.NoError(t, check(src), src) + } + // S2: a binding that shadows a narrowed name is not narrowed. + err := check(`x != nil && any(xs, x, x.a > 1)`) + require.Error(t, err) + require.Contains(t, err.Error(), "may be nil") + require.NoError(t, check(`x != nil && any(xs, y, y != nil && y.a > 1 && x.a > 0)`)) + // S3: a replaced builtin does not keep the builtin's type. + require.NoError(t, check(`len(5)`, WithFunctions(map[string]any{"len": func(n float64) float64 { return n }}))) + require.Error(t, check(`len(5)`)) + // S4: indexing a value that is always nil. + require.Error(t, check(`z[0]`)) + // S5: single-value enums are compared too. + err = check(`e == "b"`) + require.Error(t, err) + require.Contains(t, err.Error(), `never "b"`) + require.NoError(t, check(`e == "a"`)) +} + +func TestKeywordPrefixOnlyRestoresKeywords(t *testing.T) { + p, err := Compile(`x.__expr_kw_foo`) + require.NoError(t, err) + v, err := p.Run(context.Background(), map[string]any{"x": map[string]any{"__expr_kw_foo": 1}}) + require.NoError(t, err) + require.EqualValues(t, 1, v) +} diff --git a/engine.go b/engine.go index b939ce6..a66675d 100644 --- a/engine.go +++ b/engine.go @@ -389,7 +389,10 @@ func rewriteKeywordFields(src string) string { func restoreKeywordFields(node ast.Node) { ast.Inspect(node, func(n ast.Node) bool { if sel, ok := n.(*ast.SelectorExpr); ok { - sel.Sel.Name = strings.TrimPrefix(sel.Sel.Name, keywordFieldPrefix) + // Restore only names the rewrite made: the rest must be a keyword. + if name, cut := strings.CutPrefix(sel.Sel.Name, keywordFieldPrefix); cut && token.Lookup(name).IsKeyword() { + sel.Sel.Name = name + } } return true }) From ae64ca38fa1adc1698a2b7ad10848a6fe88b3324 Mon Sep 17 00:00:00 2001 From: Curtis Myzie Date: Wed, 28 Oct 2026 10:32:28 -0400 Subject: [PATCH 5/9] Fix test build in keyword prefix test Co-Authored-By: Claude Opus 5.5 --- check_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/check_test.go b/check_test.go index 192c6e5..1d73fe7 100644 --- a/check_test.go +++ b/check_test.go @@ -3,6 +3,7 @@ package expr import ( "context" "errors" + "fmt" "strings" "testing" @@ -242,5 +243,5 @@ func TestKeywordPrefixOnlyRestoresKeywords(t *testing.T) { require.NoError(t, err) v, err := p.Run(context.Background(), map[string]any{"x": map[string]any{"__expr_kw_foo": 1}}) require.NoError(t, err) - require.EqualValues(t, 1, v) + require.Equal(t, "1", fmt.Sprint(v)) } From cbb2e259e5d4a532cde782578075eae3efad9911 Mon Sep 17 00:00:00 2001 From: Curtis Myzie Date: Wed, 28 Oct 2026 12:42:17 -0400 Subject: [PATCH 6/9] Check: fixes from second review of #29 Keyword fields: - B1: rename keyword field names before optaccess and jsonlit run, so event.range[0] and a.type()?.b compile. Check now agrees with Run on: - B2: unions. isAny no longer covers them, so builtins, iteration, indexing and == check each variant. - B3: index types. Nil and union indexes, string receivers, and object keys that may be absent (use ?[). - B4: object literal keys that are not strings or may be nil. - B5: literal comparisons. == and != are no longer constant-folded, so "a" == 1 reaches Check. - B6, B7: name resolution in Run's order (env, then registered functions, then special forms), using the user-visible name. - M1: builtin calls. A signature table checks arity, argument kinds, nil and list element kinds; it replaces builtinTypes and nilIntolerant. - M2: nil field types join as Any; entries keeps an open object's Elem. - M3: a binding hides every narrowing that mentions it, not only prefix matches. - try: a missing key read inside try is not nil. Also: hints for undefined functions and scope names, and the spec now says a nil *Type means Any. Co-Authored-By: Claude Opus 5.5 --- check.go | 523 +++++++++++++++++++++++--------- check_test.go | 166 +++++++++- docs/reference/spec.md | 22 +- engine.go | 25 +- internal/optaccess/optaccess.go | 6 - kwfield_test.go | 11 +- llms.txt | 5 +- program.go | 6 +- types.go | 15 +- 9 files changed, 602 insertions(+), 177 deletions(-) diff --git a/check.go b/check.go index 98c2481..d836aed 100644 --- a/check.go +++ b/check.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "go/ast" + "go/scanner" "go/token" "reflect" "sort" @@ -227,7 +228,9 @@ func (c *checker) checkNode(node ast.Expr, sc *scope) *Type { return Bool() } c.nilCheck(n.X, x) - c.oneKind(n.X, x) + if !c.oneKind(n.X, x) { + return Number() + } if !x.isAny() && x.Kind != KindNumber { c.report(n, "%s is not a number", x) } @@ -253,23 +256,26 @@ func (c *checker) ident(n *ast.Ident, sc *scope) *Type { case "nil": return Null() } - if t, ok := sc.lookup(n.Name); ok { + name := displayIdent(n.Name) + if t, ok := sc.lookup(name); ok { return t } - if t, ok := c.env[n.Name]; ok { - if t == nil { - return Any - } - return t + if t, ok := c.env[name]; ok { + return orAny(t) } - if _, ok := c.p.funcs[displayIdent(n.Name)]; ok { + if _, ok := c.p.funcs[name]; ok { return Any } names := make([]string, 0, len(c.env)) for k := range c.env { names = append(names, k) } - c.report(n, "undefined name %q%s", displayIdent(n.Name), formatHint(n.Name, names)) + for s := sc; s != nil; s = s.parent { + for k := range s.names { + names = append(names, k) + } + } + c.report(n, "undefined name %q%s", name, formatHint(name, names)) return Any } @@ -362,19 +368,32 @@ func (c *checker) binary(n *ast.BinaryExpr, sc *scope) *Type { } } -// comparable reports == and != comparisons that can never be true. +// comparable reports == and != comparisons that fail at run time (a list +// or object compared with anything but nil) or can never be true. func (c *checker) comparable(n *ast.BinaryExpr, l, r *Type) { if l.isAny() || r.isAny() || l.Kind == KindNull || r.Kind == KindNull { return } - if l.Kind == KindList || l.Kind == KindObject || r.Kind == KindList || r.Kind == KindObject { - c.report(n, "cannot compare %s and %s with %s", l.NonNil(), r.NonNil(), n.Op) - return + lk, rk := variants(l), variants(r) + for _, t := range append(lk, rk...) { + if t.Kind == KindList || t.Kind == KindObject { + c.report(n, "cannot compare %s and %s with %s", l.NonNil(), r.NonNil(), n.Op) + return + } } - if l.Kind != r.Kind { + shared := false + for _, a := range lk { + for _, b := range rk { + shared = shared || a.Kind == b.Kind + } + } + if !shared { c.report(n, "comparing %s with %s is never true", l.NonNil(), r.NonNil()) return } + if l.IsUnion() || r.IsUnion() { + return + } for _, pair := range [][2]*Type{{l, r}, {r, l}} { set, lit := pair[0], pair[1] if len(set.Enum) > 0 && len(lit.Enum) == 1 && !set.hasEnumValue(lit.Enum[0]) { @@ -384,6 +403,14 @@ func (c *checker) comparable(n *ast.BinaryExpr, l, r *Type) { } } +// variants returns the types a union may have, or t alone. +func variants(t *Type) []*Type { + if t.IsUnion() { + return t.Variants + } + return []*Type{t} +} + // field types recv.name. optional is set for `?.`. func (c *checker) field(n, recvNode ast.Expr, recv *Type, name string, optional bool) *Type { switch { @@ -411,16 +438,14 @@ func (c *checker) field(n, recvNode ast.Expr, recv *Type, name string, optional f = orAny(f) // a nil field type is unknown, like a nil env entry case recv.Open: // An unlisted field may be absent, and selecting a missing key - // fails at run time; only ?. reads it safely. - f = recv.Elem - if f == nil { - f = Any - } - if f.Kind != KindAny { - if !optional && c.lenient == 0 { - c.report(n, "field %q may be absent; use ?.", name) - } + // fails at run time; only ?. reads it safely, as nil. + f = orAny(recv.Elem) + switch { + case f.isAny(): + case optional: f = f.OrNil() + case c.lenient == 0: + c.report(n, "field %q may be absent; use ?.", name) } default: names := make([]string, 0, len(recv.Fields)) @@ -454,23 +479,59 @@ func (c *checker) index(n, recvNode ast.Expr, recv *Type, idxNode ast.Expr, idx if !optional { c.nilCheck(recvNode, recv) } + // A nil index fails even after ?[. + if idx.Kind == KindNull { + if c.lenient == 0 { + c.report(idxNode, "the index is always nil") + } + return Any + } + c.nilCheck(idxNode, idx) + if !c.oneKind(idxNode, idx) { + return Any + } var out *Type switch recv.Kind { - case KindList: + case KindList, KindString: + what := "list" + out = recv.Elem + if recv.Kind == KindString { + what, out = "string", String() + } if !idx.isAny() && idx.Kind != KindNumber { - c.report(n, "list index must be a number, not %s", idx) + c.report(n, "%s index must be a number, not %s", what, describe(idx)) } - out = recv.Elem - case KindString: - out = String() case KindObject: - if idx.Kind == KindString && len(idx.Enum) == 1 { - name, _ := idx.Enum[0].(string) - return c.field(n, recvNode, recv, name, optional) + if !idx.isAny() && idx.Kind != KindString { + c.report(n, "object key must be a string, not %s", describe(idx)) + return Any + } + if len(idx.Enum) > 0 { + // Each key the index may be. A nil receiver is reported above. + r := recv + if !optional { + r = recv.NonNil() + } + var t *Type + for _, v := range idx.Enum { + name, _ := v.(string) + t = Join(t, c.field(n, recvNode, r, name, optional)) + } + return t } - out = Any - if recv.Open && recv.Elem != nil { - out = recv.Elem + if idx.isAny() || (recv.Open && recv.Elem == nil) { + return Any + } + // Any string: the key may be absent, which fails unless read + // with ?[, as nil. + if !optional && c.lenient == 0 { + c.report(n, "the key may be absent; use ?[") + } + for _, f := range recv.Fields { + out = Join(out, orAny(f)) + } + if recv.Open { + out = Join(out, recv.Elem) } default: c.report(n, "cannot index %s", recv) @@ -492,9 +553,7 @@ func (c *checker) call(n *ast.CallExpr, sc *scope) *Type { ident, _ := n.Fun.(*ast.Ident) if ident == nil { c.check(n.Fun, sc) - for _, a := range n.Args { - c.check(a, sc) - } + c.args(n, sc) return Any } name := ident.Name @@ -513,8 +572,34 @@ func (c *checker) call(n *ast.CallExpr, sc *scope) *Type { case name == tryIndexFormName && len(n.Args) == 2: recv := c.check(n.Args[0], sc) return c.index(n, n.Args[0], recv, n.Args[1], c.check(n.Args[1], sc), true).OrNil() + } + + // Resolve the name as Run does: the environment, then registered + // functions, then the special forms. + display := displayIdent(name) + _, bound := sc.lookup(display) + if _, inEnv := c.env[display]; bound || inEnv { + c.args(n, sc) + return Any + } + if registered, ok := c.p.funcs[display]; ok { + args := c.args(n, sc) + if fn := c.cfg.funcTypes[display]; fn != nil { + t, err := fn(args) + if err != nil { + c.report(n, "%v", err) + return Any + } + return t + } + if sig, ok := builtinSigs[display]; ok && isBuiltin(display, registered) { + return c.builtin(n, display, sig, args) + } + return Any + } + switch { case iteratingForms[name]: - return c.iterating(n, displayIdent(name), sc) + return c.iterating(n, display, sc) case name == "try": if len(n.Args) != 2 { c.report(n, "try expects 2 arguments (value, default), got %d", len(n.Args)) @@ -533,42 +618,26 @@ func (c *checker) call(n *ast.CallExpr, sc *scope) *Type { then := c.narrow(narrowings(n.Args[0], true), n.Args[1], sc) return Join(then, c.narrow(narrowings(n.Args[0], false), n.Args[2], sc)) } + c.args(n, sc) + names := make([]string, 0, len(c.p.funcs)+len(specialForms)) + for k := range c.p.funcs { + names = append(names, k) + } + names = append(names, specialForms...) + c.report(n, "undefined function %q%s", display, formatHint(display, names)) + return Any +} + +// specialForms are the user-visible names of the forms call handles +// itself. +var specialForms = []string{"map", "filter", "flatMap", "any", "all", "find", "count", "sortBy", "try", "if"} +func (c *checker) args(n *ast.CallExpr, sc *scope) []*Type { args := make([]*Type, len(n.Args)) for i, a := range n.Args { args[i] = c.check(a, sc) } - display := displayIdent(name) - fn := c.cfg.funcTypes[display] - if fn == nil { - if registered, ok := c.p.funcs[display]; ok && isBuiltin(display, registered) { - fn = builtinTypes[display] - } - } - if fn == nil { - if _, registered := c.p.funcs[display]; registered { - return Any - } - if _, inEnv := c.env[display]; inEnv { - return Any - } - if _, bound := sc.lookup(display); bound { - return Any - } - c.report(n, "undefined function %q", display) - return Any - } - if nilIntolerant[display] && fn != nil && c.cfg.funcTypes[display] == nil { - for i, a := range args { - c.nilCheck(n.Args[i], a) - } - } - t, err := fn(args) - if err != nil { - c.report(n, "%v", err) - return Any - } - return t + return args } // builtinFuncs maps each builtin name to its function's code pointer, so @@ -593,14 +662,6 @@ func isBuiltin(name string, fn any) bool { return rv.Kind() == reflect.Func && rv.Pointer() == ptr } -// nilIntolerant are the builtins that fail on a nil argument. The rest -// (len, string, int, first, keys, sum, ...) treat nil as empty. -var nilIntolerant = map[string]bool{ - "abs": true, "ceil": true, "floor": true, "round": true, "min": true, "max": true, - "upper": true, "lower": true, "trim": true, "replace": true, "split": true, - "startsWith": true, "endsWith": true, "slice": true, -} - func (c *checker) iterating(n *ast.CallExpr, form string, sc *scope) *Type { coll, bind, body, err := splitFormArgs(form, n) if err != nil { @@ -627,14 +688,20 @@ func (c *checker) iterating(n *ast.CallExpr, form string, sc *scope) *Type { // not reach into the body. hidden := map[string]int{} for k, v := range c.narrowed { - for _, name := range []string{bind, "index"} { - if k == name || strings.HasPrefix(k, name+".") || strings.HasPrefix(k, name+"?") || strings.HasPrefix(k, name+"[") { - hidden[k] = v - delete(c.narrowed, k) - } + if mentions(k, bind) || mentions(k, "index") { + hidden[k] = v + delete(c.narrowed, k) } } bt := c.check(body, inner) + if form == "sortBy" { + // Keys must all be numbers or all strings, and not nil. + if bt.IsUnion() { + c.report(body, "sortBy: keys may be %s; they must all be one kind", bt) + } else { + c.argument(body, "sortBy", 2, param{kinds: numberOrString}, bt) + } + } for k, v := range hidden { c.narrowed[k] = v } @@ -658,6 +725,26 @@ func (c *checker) iterating(n *ast.CallExpr, form string, sc *scope) *Type { return Any } +// mentions reports whether the source text src refers to the name as an +// identifier, not as a field (`x.name`) or inside a string. +func mentions(src, name string) bool { + fs := token.NewFileSet() + file := fs.AddFile("", fs.Base(), len(src)) + var s scanner.Scanner + s.Init(file, []byte(src), nil, 0) + prev := token.ILLEGAL + for { + _, tok, lit := s.Scan() + if tok == token.EOF { + return false + } + if tok == token.IDENT && lit == name && prev != token.PERIOD { + return true + } + prev = tok + } +} + func (c *checker) composite(n *ast.CompositeLit, sc *scope) *Type { switch n.Type.(type) { case *ast.ArrayType: @@ -680,8 +767,17 @@ func (c *checker) composite(n *ast.CompositeLit, sc *scope) *Type { vt := c.check(kv.Value, sc) if key, ok := literalString(kt); ok { out.Fields[key] = vt - } else { - out.Open = true + continue + } + out.Open = true + switch { + case kt.Kind == KindNull: + c.report(kv.Key, "an object key is always nil") + case !c.oneKind(kv.Key, kt): + case !kt.isAny() && kt.Kind != KindString: + c.report(kv.Key, "an object key must be a string, not %s", describe(kt)) + default: + c.nilCheck(kv.Key, kt) } } return out @@ -697,78 +793,225 @@ func literalString(t *Type) (string, bool) { return s, ok } -// builtinTypes types the functions in Builtins, StringFuncs, MathFuncs and +// kinds is a set of kinds, for the arguments a builtin accepts. The zero +// set accepts any kind. +type kinds uint8 + +func kindSet(ks ...Kind) kinds { + var s kinds + for _, k := range ks { + s |= 1 << k + } + return s +} + +func (s kinds) has(k Kind) bool { return s == 0 || s&(1< 0 && !a[0].isAny() && a[0].Kind != KindString { - return nil, fmt.Errorf("%s expects a string, got %s", name, a[0]) +func entriesType(a []*Type) *Type { + value := Any + if len(a) == 1 && a[0].Kind == KindObject && !(a[0].Open && a[0].Elem == nil) { + var v *Type + for _, f := range a[0].Fields { + v = Join(v, orAny(f)) } - return result, nil + if a[0].Open { + v = Join(v, a[0].Elem) + } + value = orAny(v) } + return ListOf(ObjectOf(map[string]*Type{"key": String(), "value": value})) } -func elemFunc(a []*Type) (*Type, error) { - if len(a) == 1 && a[0] != nil && a[0].Kind == KindList && a[0].Elem != nil { - return a[0].Elem.OrNil(), nil +func elemOrNil(a []*Type) *Type { + if len(a) == 1 && a[0].Kind == KindList { + return orAny(a[0].Elem).OrNil() } - return Any, nil + return Any } -func sameFunc(a []*Type) (*Type, error) { - if len(a) > 0 && a[0] != nil && (a[0].Kind == KindList || a[0].Kind == KindString) { - return plain(a[0].NonNil()), nil +// listType is the result of sort and reverse: a new list, empty for nil. +func listType(a []*Type) *Type { + if len(a) == 1 && a[0].Kind == KindList { + return ListOf(orAny(a[0].Elem)) } - return Any, nil + return ListOf(Any) } -func wantArgs(name string, a []*Type, n int) error { - if len(a) != n { - return fmt.Errorf("%s expects %d argument(s), got %d", name, n, len(a)) +// sliceType is a list for a list or nil, and a string for a string. +func sliceType(a []*Type) *Type { + if len(a) == 0 || a[0].isAny() { + return Any } - return nil + if a[0].Kind == KindString { + if a[0].Optional { + return Join(String(), ListOf(Any)) + } + return String() + } + return listType(a[:1]) } diff --git a/check_test.go b/check_test.go index 1d73fe7..ff1403b 100644 --- a/check_test.go +++ b/check_test.go @@ -3,7 +3,6 @@ package expr import ( "context" "errors" - "fmt" "strings" "testing" @@ -72,7 +71,7 @@ func TestCheckRejects(t *testing.T) { `map(classify, it)`: "map expects a list", `classify.confidence + "x"`: "cannot add number and", `-inputs.repo`: "not a number", - `upper(classify.confidence)`: "upper expects a string", + `upper(classify.confidence)`: "upper: argument 1 must be string, not number", `nosuch(1)`: `undefined function "nosuch"`, `map(findings.items, it.titel)`: `no field "titel"`, `findings.items["x"]`: "list index must be a number", @@ -243,5 +242,166 @@ func TestKeywordPrefixOnlyRestoresKeywords(t *testing.T) { require.NoError(t, err) v, err := p.Run(context.Background(), map[string]any{"x": map[string]any{"__expr_kw_foo": 1}}) require.NoError(t, err) - require.Equal(t, "1", fmt.Sprint(v)) + require.Equal(t, 1, v) +} + +// Regressions from the second review of #29. Each rejected expression +// also fails at run time with these values, and each accepted one runs. +func TestCheckMatchesRun(t *testing.T) { + item := ObjectOf(map[string]*Type{"a": Number()}) + env := map[string]*Type{ + "s": String().OrNil(), + "w": String().OrNil(), + "xs": ListOf(Number()), + "ys": ListOf(Number()).OrNil(), + "a": ObjectOf(map[string]*Type{"t": ListOf(String()), "n": Number().OrNil()}), + "obj": ObjectOf(map[string]*Type{"k": String()}), + "scores": &Type{Kind: KindObject, Open: true, Elem: Number()}, + "k": String(), + "x": ListOf(item), + "xss": ListOf(ListOf(item)), + "map": Number(), + } + values := map[string]any{ + "s": nil, "w": "x", "xs": []any{1, 2}, "ys": nil, + "a": map[string]any{"t": []any{"q"}, "n": nil}, + "obj": map[string]any{"k": "v"}, "scores": map[string]any{"alice": 1}, "k": "bob", + "x": []any{map[string]any{"a": 1}}, "xss": []any{[]any{}}, "map": 3, + } + compile := func(src string) *Program { + p, err := Compile(src, WithBuiltins(), WithFunctions(StringFuncs()), WithFunctions(MathFuncs()), + WithFunctions(CollectionFuncs()), WithFunctions(map[string]any{"count": func(xs []any) int { return len(xs) }})) + require.NoError(t, err, src) + return p + } + for src, want := range map[string]string{ + // B1 is in kwfield_test.go. B2: a union is not Any. + `upper(s || 0)`: "may be number, and must be string", + `len(s || 0)`: "may be number", + `sum(ys || "none")`: "may be string, and must be a list", + `xs[w || 0]`: "the operation needs one kind", + `(s || 0) == [1, 2]`: "cannot compare", + `(s || 0) > (w || "")`: "the operation needs one kind", + `sort(w || [1])`: "may be string, and must be a list", + `slice(xs, w || 0, 1)`: "may be string, and must be number", + `-(w || 0)`: "the operation needs one kind", + `{(s || 0): 1}`: "the operation needs one kind", + `has(obj, s || 0)`: "may be number, and must be string", + `join(xs, w || 0)`: "must have string elements", + `contains("abc", s || 0)`: "needs a string", + `a.t[a.n]`: "`a.n`: may be nil", + `a.t?[a.n]`: "`a.n`: may be nil", + `"hello"["x"]`: "string index must be a number", + `obj[0]`: "object key must be a string", + `scores[k]`: "the key may be absent; use ?[", + `obj[k]`: "the key may be absent; use ?[", + `{1: "a"}`: "an object key must be a string", + `{s: "a"}`: "`s`: may be nil", + `upper()`: "upper expects 1 argument(s), got 0", + `split("a", 1)`: "split: argument 2 must be string, not number", + `replace("a", "b")`: "replace expects 3 argument(s)", + `replace("a", 1, "c")`: "replace: argument 2 must be string", + `join([1, 2], ",")`: "join: argument 1 must have string elements, not number", + `join(["a"], nil)`: "join: argument 2 is always nil", + `sprintf(1)`: "sprintf: argument 1 must be string", + `sprintf()`: "sprintf expects at least 1 argument(s)", + `round("x")`: "round: argument 1 must be number, not string", + `abs(true)`: "abs: argument 1 must be number, not bool", + `min("a", "b")`: "min: argument 1 must be number", + `min()`: "min expects at least 1 argument(s)", + `sum("x")`: "sum: argument 1 must be a list", + `sum(["a"])`: "sum: argument 1 must have number elements", + `sort("hello")`: "sort: argument 1 must be a list, not string", + `reverse("hello")`: "reverse: argument 1 must be a list", + `sort([s, "a"])`: "elements that may be nil", + `sortBy(xs, s)`: "`s`: may be nil", + `sortBy(xs, it > 1)`: "sortBy: argument 2 must be number or string, not bool", + `first(1)`: "first: argument 1 must be a list", + `last("s")`: "last: argument 1 must be a list", + `keys([1])`: "keys: argument 1 must be an object", + `entries(1)`: "entries: argument 1 must be an object", + `has([1], "a")`: "has: argument 1 must be an object", + `has(obj, nil)`: "has: argument 2 is always nil", + `slice(xs, 0)`: "slice expects 3 argument(s)", + `slice(xs, nil, 1)`: "slice: argument 2 is always nil", + `int([1])`: "int: argument 1 must be number or string", + `contains("abc", 1)`: "contains: looking in string needs a string", + `contains(obj, 1)`: "contains: looking in an object needs a string", + `first(x) != nil && any(xss, x, first(x).a > 1)`: "`first(x)`: may be nil", + } { + p := compile(src) + _, err := p.Check(env) + require.Error(t, err, src) + require.Contains(t, err.Error(), want, src) + _, err = p.Run(context.Background(), values) + require.Error(t, err, "Run should fail too: "+src) + } + + // Never true, though they do not fail. B5: literals are not folded + // away before Check. + for src, want := range map[string]string{ + `(s || 0) == true`: "is never true", + `"a" == 1`: "is never true", + `"a" == "b"`: `never "b"`, + `1 != "1"`: "is never true", + } { + _, err := compile(src).Check(env) + require.Error(t, err, src) + require.Contains(t, err.Error(), want, src) + } + + for src, want := range map[string]string{ + `count(xs)`: "any", // B6: a host function shadows a form + `map + 1`: "number", // B7: env names that are keywords + `scores?[k]`: "number?", + `obj["k"]`: "string", + `entries(scores)[0].value`: "number", + `slice(s, 0, 1)`: "string | list", + `slice(ys, 0, 1)`: "list", + `sort(xs)`: "list", + `sum(ys)`: "number", + `keys(obj)`: "list", + `contains(xs, "a")`: "bool", + `len(scores) + len(s)`: "number", + `sprintf("%d-%v", 1, nil)`: "string", + `min(1)`: "number", + `(s || 0) == 0`: "bool", + `try(scores[k], 0)`: "number", + `{(w || "k"): 1}`: "{...}", + `string(s) + upper(w || "")`: "string", + } { + p := compile(src) + got, err := p.Check(env) + require.NoError(t, err, src) + require.Equal(t, want, got.String(), src) + _, err = p.Run(context.Background(), values) + require.NoError(t, err, src) + } + + // M2: a nil field type is Any when joined. + require.Equal(t, "{k: any}", Join(ObjectOf(map[string]*Type{"k": nil}), ObjectOf(map[string]*Type{"k": String()})).String()) + require.Equal(t, "{k: any}", Join(ObjectOf(map[string]*Type{"k": nil}), ObjectOf(map[string]*Type{})).String()) +} + +func TestCheckHints(t *testing.T) { + for src, want := range map[string]string{ + `any([1], item, iten > 1)`: `undefined name "iten" (did you mean "item"?)`, + `uper("a")`: `undefined function "uper" (did you mean "upper"?)`, + `fiter([1], it > 1)`: `undefined function "fiter" (did you mean "filter"?)`, + } { + p, err := Compile(src, WithBuiltins()) + require.NoError(t, err, src) + _, err = p.Check(nil) + require.Error(t, err, src) + require.Contains(t, err.Error(), want, src) + } +} + +func TestCheckIndexReportsOnce(t *testing.T) { + _, err := checkSrc(t, `report["verdict"]`) + var te *TypeError + require.True(t, errors.As(err, &te)) + require.Len(t, te.Problems, 1, strings.Join(te.Problems, "\n")) + _, err = checkSrc(t, `try(findings.items[nil], 0)`) + require.NoError(t, err) } diff --git a/docs/reference/spec.md b/docs/reference/spec.md index 750116f..6dbc885 100644 --- a/docs/reference/spec.md +++ b/docs/reference/spec.md @@ -740,8 +740,17 @@ Check reports, as a `*TypeError` wrapping `ErrType`, every: - field a closed object does not have (also through `?.`); - field read, index or arithmetic on a value that may be nil, except through `?.`, `?[` or inside the first argument of `try`; -- operation on the wrong kind (`len` of a number, adding a number and a - string, ordering a bool, iterating a non-list); +- operation on the wrong kind (adding a number and a string, ordering a + bool, iterating a non-list, indexing a list with a string or an object + with a number); +- builtin call with the wrong number of arguments, or an argument of a + kind the builtin rejects at run time (`len(5)`, `split(s, 1)`, + `join([1, 2], ",")`, `sort("abc")`), or nil where it does not accept + nil (`upper(nil)`, `has(m, nil)`); +- object key that may be absent: an unlisted field of an open object + with a typed `Elem`, or an index by a string that is not a literal + (`scores[name]`), unless read with `?.`/`?[` or inside `try`; +- object literal key that is not a string or may be nil; - `==`/`!=` that can never be true: different kinds, or a literal outside the other side's enum (`kind == "bugfix"` when kind is `"bug_fix" | "feature"`). @@ -756,9 +765,12 @@ an open object does not list may be absent, so it is read with `?.`. `==` and `!=` on lists or objects are reported, since Run rejects them. `Any` disables checking for the value and everything derived from it, so a -partly typed env checks what it can. Builtins and the optional groups have -result types; other registered functions return `Any` unless declared with -`WithFuncType`. Check is static: a passing check does not prove Run +partly typed env checks what it can. A nil `*Type`, in env or in `Fields`, +means `Any`. Calls resolve as Run resolves them: +a name in env, then a registered function, then a special form, so a host +function named `count` replaces the form. Builtins and the optional groups +have signatures; other registered functions return `Any` unless declared +with `WithFuncType`. Check is static: a passing check does not prove Run succeeds (a list index can still be out of range). ## Helpful errors diff --git a/engine.go b/engine.go index a66675d..2686d3a 100644 --- a/engine.go +++ b/engine.go @@ -207,12 +207,13 @@ func compileWithConfig(code string, cfg *compileConfig) (*Program, error) { if len(cfg.errs) > 0 { return nil, fmt.Errorf("%w: %w", ErrCompile, errors.Join(cfg.errs...)) } - // Pipeline order: optaccess turns `?.`/`?[` into sentinel calls - // while the source still uses raw operator syntax; jsonlit then - // rewrites bare composite literals; preprocessSource handles the - // keyword rewrites (`map`) so the parser accepts them as - // identifiers. - parsed := preprocessSource(rewriteKeywordFields(jsonlit.Rewrite(optaccess.Rewrite(code)))) + // Pipeline order: rewriteKeywordFields renames keyword field names + // (`x.type`) first, so the later rewrites see an identifier there; + // optaccess turns `?.`/`?[` into sentinel calls while the source + // still uses raw operator syntax; jsonlit then rewrites bare + // composite literals; preprocessSource handles the keyword rewrites + // (`map`) so the parser accepts them as identifiers. + parsed := preprocessSource(jsonlit.Rewrite(optaccess.Rewrite(rewriteKeywordFields(code)))) fset := token.NewFileSet() node, err := parser.ParseExprFrom(fset, "", parsed, 0) if err != nil { @@ -353,7 +354,8 @@ func preprocessSource(src string) string { const keywordFieldPrefix = "__expr_kw_" // rewriteKeywordFields renames every Go keyword that directly follows a -// `.` (`event.type`, `item.range`, `x.map`) so the expression parses. +// `.` (`event.type`, `item.range`, `x.map`) so the expression parses. It +// leaves `?.type` alone: optaccess puts that name in a string literal. func rewriteKeywordFields(src string) string { if !strings.Contains(src, ".") { return src @@ -364,19 +366,22 @@ func rewriteKeywordFields(src string) string { s.Init(file, []byte(src), nil, 0) var out strings.Builder last := 0 - prev := token.ILLEGAL + // period is set when the previous token is a `.` that is not part + // of `?.`. + period, question := false, false for { pos, tok, lit := s.Scan() if tok == token.EOF { break } - if prev == token.PERIOD && tok.IsKeyword() { + if period && tok.IsKeyword() { off := file.Offset(pos) out.WriteString(src[last:off]) out.WriteString(keywordFieldPrefix + lit) last = off + len(lit) } - prev = tok + period = tok == token.PERIOD && !question + question = tok == token.ILLEGAL && lit == "?" } if last == 0 { return src diff --git a/internal/optaccess/optaccess.go b/internal/optaccess/optaccess.go index e711059..c95bc7d 100644 --- a/internal/optaccess/optaccess.go +++ b/internal/optaccess/optaccess.go @@ -198,12 +198,6 @@ func lhsStartIdx(toks []tokenInfo, src string, qIdx int) int { i := qIdx - 1 for i >= 0 { t := toks[i] - // A keyword used as a field name (`a.type?.b`) walks like an - // identifier in a selector chain. - if t.kind.IsKeyword() && i-1 >= 0 && toks[i-1].kind == token.PERIOD { - i -= 2 - continue - } switch t.kind { case token.IDENT, token.INT, token.FLOAT, token.STRING, token.CHAR, token.MAP, token.IF: diff --git a/kwfield_test.go b/kwfield_test.go index 526f9f3..1dde96a 100644 --- a/kwfield_test.go +++ b/kwfield_test.go @@ -22,6 +22,10 @@ func TestKeywordFieldNames(t *testing.T) { `map(event.range, it * 2)[1] == 4`: true, `"a.type"`: "a.type", `{"type": event.type}.type`: "push", + `event.range[1]`: 2, + `event.type[0]`: "p", + `event.sender.type[0] == "B"`: true, + `event?.range[0]`: 1, } { p, err := Compile(src, WithBuiltins()) require.NoError(t, err, src) @@ -37,9 +41,10 @@ func TestKeywordFieldNames(t *testing.T) { func TestKeywordFieldNamesOptional(t *testing.T) { env := map[string]any{"a": map[string]any{"type": map[string]any{"b": 1}}, "n": nil} for src, want := range map[string]any{ - `a?.type?.b == 1`: true, - `a.type?.b == 1`: true, - `n?.type`: nil, + `a?.type?.b == 1`: true, + `a.type?.b == 1`: true, + `n?.type`: nil, + `a.type?.b == 1 && a.type.b == 1`: true, } { p, err := Compile(src, WithBuiltins()) require.NoError(t, err, src) diff --git a/llms.txt b/llms.txt index a4d23d7..921ef1d 100644 --- a/llms.txt +++ b/llms.txt @@ -467,8 +467,9 @@ t, err := p.Check(env, expr.WithFuncType("concat", concatType)) Types: `expr.Any`, `Null()`, `Bool()`, `Number()`, `String()`, `EnumOf(...)`, `ListOf(t)`, `ObjectOf(fields)` (closed), `OpenObject(fields)`, `t.OrNil()`, `t.NonNil()`, `Join(a, b)`. Check catches unknown names and fields, -reads of maybe-nil values without `?.`/`try`, wrong-kind operations, and -comparisons that are never true (including against an enum). +reads of maybe-nil values without `?.`/`try`, wrong-kind operations, +builtin calls with the wrong arguments, and comparisons that are never +true (including against an enum). A nil `*Type` means `Any`. Field names may be Go keywords: `event.type`, `x?.range`. diff --git a/program.go b/program.go index 18ddb85..815ab05 100644 --- a/program.go +++ b/program.go @@ -154,8 +154,10 @@ func (p *Program) prewalk(node ast.Expr) ast.Expr { // Skip logical ops: their short-circuit semantics are visible // to side-effects, and the LAND/LOR path in evalBinary is // fast enough that folding a pure-literal `true && false` - // isn't worth a second code path. - if n.Op != token.LAND && n.Op != token.LOR { + // isn't worth a second code path. Skip == and != too: they + // never fail, so folding `"a" == 1` to false would hide a + // comparison that is never true from Check. + if n.Op != token.LAND && n.Op != token.LOR && n.Op != token.EQL && n.Op != token.NEQ { if xv, ok := p.constValue(n.X); ok { if yv, ok2 := p.constValue(n.Y); ok2 { if v, err := applyBinary(n.Op, xv, yv); err == nil { diff --git a/types.go b/types.go index 3796f71..ddfbbb1 100644 --- a/types.go +++ b/types.go @@ -79,7 +79,7 @@ func EnumOf(values ...string) *Type { // OrNil returns a copy of t that may also be nil. func (t *Type) OrNil() *Type { - if t == nil || (t.Kind == KindAny && len(t.Variants) == 0) || t.Kind == KindNull { + if t.isAny() || t.Kind == KindNull { return t } c := *t @@ -97,7 +97,9 @@ func (t *Type) NonNil() *Type { return &c } -func (t *Type) isAny() bool { return t == nil || t.Kind == KindAny } +// isAny reports whether t is unknown. A union is not: its variants are +// known, and each must allow an operation. +func (t *Type) isAny() bool { return t == nil || (t.Kind == KindAny && len(t.Variants) == 0) } // IsUnion reports whether t is a union of known types of different kinds. func (t *Type) IsUnion() bool { return t != nil && t.Kind == KindAny && len(t.Variants) > 0 } @@ -249,7 +251,7 @@ func Join(a, b *Type) *Type { return b case b == nil: return a - case (a.isAny() && !a.IsUnion()) || (b.isAny() && !b.IsUnion()): + case a.isAny() || b.isAny(): return Any case a.Kind == KindNull: return b.OrNil() @@ -266,15 +268,16 @@ func Join(a, b *Type) *Type { out.Open = a.Open || b.Open out.Fields = map[string]*Type{} for k, at := range a.Fields { + // A nil field type is Any, not "no type yet". if bt, ok := b.Fields[k]; ok { - out.Fields[k] = Join(at, bt) + out.Fields[k] = Join(orAny(at), orAny(bt)) } else { - out.Fields[k] = at.OrNil() + out.Fields[k] = orAny(at).OrNil() } } for k, bt := range b.Fields { if _, ok := a.Fields[k]; !ok { - out.Fields[k] = bt.OrNil() + out.Fields[k] = orAny(bt).OrNil() } } switch { From e636a20133a4f991a4bf59bf6b5106756d54d7d4 Mon Sep 17 00:00:00 2001 From: Curtis Myzie Date: Wed, 28 Oct 2026 12:46:29 -0400 Subject: [PATCH 7/9] Check: an open object with an Any Elem is untyped Co-Authored-By: Claude Opus 5.5 --- check.go | 4 ++-- check_test.go | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/check.go b/check.go index d836aed..df2396b 100644 --- a/check.go +++ b/check.go @@ -519,7 +519,7 @@ func (c *checker) index(n, recvNode ast.Expr, recv *Type, idxNode ast.Expr, idx } return t } - if idx.isAny() || (recv.Open && recv.Elem == nil) { + if idx.isAny() || (recv.Open && recv.Elem.isAny()) { return Any } // Any string: the key may be absent, which fails unless read @@ -974,7 +974,7 @@ func containsNeedle(a []*Type) error { func entriesType(a []*Type) *Type { value := Any - if len(a) == 1 && a[0].Kind == KindObject && !(a[0].Open && a[0].Elem == nil) { + if len(a) == 1 && a[0].Kind == KindObject && !(a[0].Open && a[0].Elem.isAny()) { var v *Type for _, f := range a[0].Fields { v = Join(v, orAny(f)) diff --git a/check_test.go b/check_test.go index ff1403b..e18a7bd 100644 --- a/check_test.go +++ b/check_test.go @@ -378,6 +378,14 @@ func TestCheckMatchesRun(t *testing.T) { require.NoError(t, err, src) } + // An open object whose Elem is Any is untyped, like one with no Elem. + anyElem := map[string]*Type{"m": &Type{Kind: KindObject, Open: true, Elem: Any}, "k": String()} + for src, want := range map[string]string{`m[k]`: "any", `entries(m)[0].value`: "any"} { + got, err := compile(src).Check(anyElem) + require.NoError(t, err, src) + require.Equal(t, want, got.String(), src) + } + // M2: a nil field type is Any when joined. require.Equal(t, "{k: any}", Join(ObjectOf(map[string]*Type{"k": nil}), ObjectOf(map[string]*Type{"k": String()})).String()) require.Equal(t, "{k: any}", Join(ObjectOf(map[string]*Type{"k": nil}), ObjectOf(map[string]*Type{})).String()) From 9aae17d8c9d197678df6c7a092ccfa9e7d35b5a8 Mon Sep 17 00:00:00 2001 From: Curtis Myzie Date: Wed, 28 Oct 2026 12:52:55 -0400 Subject: [PATCH 8/9] Docs: type checking guide, example and README section Adds docs/guides/type-checking.md with a runnable example and tests for every claim in it and in the README section. Points the templates, env and function guides at it, documents WithFuncType, adds Check to the package doc and llms.txt, and tells contributors that Check must agree with Run. Corrects the README's size comparison, which was stale. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 7 +- README.md | 50 ++++-- docs/guides/designing-an-env.md | 9 ++ docs/guides/registering-functions.md | 21 +++ docs/guides/templates.md | 4 + docs/guides/type-checking.md | 223 +++++++++++++++++++++++++++ docs/reference/spec.md | 4 +- docs_guides_test.go | 113 ++++++++++++++ engine.go | 10 ++ examples/README.md | 17 +- examples/type_checking/main.go | 84 ++++++++++ llms.txt | 8 + 12 files changed, 531 insertions(+), 19 deletions(-) create mode 100644 docs/guides/type-checking.md create mode 100644 examples/type_checking/main.go diff --git a/CLAUDE.md b/CLAUDE.md index 1c104a1..45d6556 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -18,7 +18,7 @@ parameter interpolation. | File | Purpose | | ------------------- | ------------------------------------------------------------------ | -| `engine.go` | `Compile`, `Option`, `map`/`if` keyword preprocessing | +| `engine.go` | `Compile`, `Option`, `map`/`if` and keyword field preprocessing | | `program.go` | AST walker — the evaluator | | `reflect.go` + `prepared.go` | Env lookup, function dispatch, cached signatures | | `builtins.go` | Default function set exposed by `Builtins()` / `WithBuiltins()` | @@ -41,6 +41,11 @@ parameter interpolation. - The `map` identifier is rewritten to an internal sentinel (`mapFormName`) before parsing because Go treats `map` as a keyword. - Errors wrap `ErrCompile` or `ErrEvaluate` — preserve that so callers can `errors.Is`. + `Check` errors wrap `ErrType`. +- `Check` must agree with `Run`. When you change what an operator, builtin or + form accepts at run time, change `check.go` to match (`builtinSigs` for + builtins), and add the case to `TestCheckMatchesRun`, which runs every + rejected expression to confirm `Run` fails too. - When changing language behavior, update `docs/reference/spec.md` in the same change; if the change affects how expressions *look*, update `docs/guides/examples.md` and `llms.txt` too. - Guide pages under `docs/guides/` have runnable companions under `examples/` and diff --git a/README.md b/README.md index 868c1f1..291b703 100644 --- a/README.md +++ b/README.md @@ -133,6 +133,39 @@ p, err := expr.Compile( ) ``` +## Type checking + +Expressions are often written ahead of time, in a config file, a rule, or +a workflow definition. `Program.Check` finds their mistakes when they are +saved instead of when some input finally hits the broken branch. Describe +the env's types once: + +```go +env := map[string]*expr.Type{ + "issue": expr.ObjectOf(map[string]*expr.Type{ + "kind": expr.EnumOf("bug", "feature", "question"), + "title": expr.String(), + }), + "review": expr.ObjectOf(map[string]*expr.Type{"body": expr.String()}).OrNil(), + "event": expr.Any, // unchecked +} + +p, _ := expr.Compile(`issue.kind == "bugfix" || len(review.body) > 0`, expr.WithBuiltins()) +t, err := p.Check(env) +// expr: type error: +// `issue.kind == "bugfix"`: the value is one of "bug" | "feature" | "question", never "bugfix"; +// `review`: may be nil here; use ?. or try(...) +``` + +`Check` reports every problem at once: unknown names and fields (with +"did you mean"), operations on the wrong kind of value, builtin calls +with the wrong arguments, reads of values that may be nil, and +comparisons that can never be true. It returns the result type too, so a +host can insist that a condition is a `bool`. `Any` turns checking off +for a value, so you can start loosely and add types where mistakes hurt. +[The type checking guide](docs/guides/type-checking.md) covers the +details. + ## JSON-style literals Object and array literals work the way you'd hope, without the Go ceremony: @@ -219,14 +252,13 @@ define new functions from inside an expression. [`cel-go`](https://github.com/google/cel-go) are the obvious neighbors. Both are excellent, and both are much larger. `expr` is deliberately smaller: -| Library | Non-test Go LOC | Parser | Notes | -| ----------------- | --------------- | --------------- | -------------------------------------------- | -| `deepnoodle/expr` | ~4k | `go/parser` | Go subset, JSON literals, `${...}` templates | -| `expr-lang/expr` | ~30k | Custom | Own grammar, optimizer, type checker | -| `cel-go` | ~70.0k | Custom (protos) | CEL spec, protobuf-native, type system | +| Library | Non-test Go LOC | Parser | Notes | +| ----------------- | --------------- | --------------- | ---------------------------------------------------------- | +| `deepnoodle/expr` | ~7k | `go/parser` | Go subset, JSON literals, `${...}` templates, type checker | +| `expr-lang/expr` | ~30k | Custom | Own grammar, optimizer, type checker | +| `cel-go` | ~70.0k | Custom (protos) | CEL spec, protobuf-native, type system | -That's an order of magnitude smaller than `expr-lang`, nearly two than -`cel-go`. You give up a pile of features you may never need. In exchange: no +That's about a quarter the size of `expr-lang` and a tenth of `cel-go`. You give up a pile of features you may never need. In exchange: no dependencies, low complexity, and syntax anyone who writes Go already knows. ## Performance @@ -271,8 +303,8 @@ go test -run=^$ -bench=. -benchtime=300ms ./... | go run ./chart ../../docs/asse - [`docs/reference/spec.md`](docs/reference/spec.md) is the authoritative language reference. - [`docs/guides/`](docs/guides/) has deeper guides on registering Go - functions, designing an env, sandboxing untrusted expressions, using - templates in anger, and higher-order patterns. Start with + functions, designing an env, type checking, sandboxing untrusted + expressions, using templates in anger, and higher-order patterns. Start with [`examples.md`](docs/guides/examples.md) for worked multi-line expressions. - [`examples/`](examples/) has runnable versions of everything above. - [`llms.txt`](llms.txt) is a condensed reference sized for LLM context diff --git a/docs/guides/designing-an-env.md b/docs/guides/designing-an-env.md index 579d0bf..d027cbf 100644 --- a/docs/guides/designing-an-env.md +++ b/docs/guides/designing-an-env.md @@ -175,6 +175,15 @@ Each top-level key is an identifier, and expressions select into whichever shape fits. This composes well because you can add a new root without touching existing expressions. +## Describing the env's types + +If expressions are written ahead of time, describe the env's shape as a +`map[string]*expr.Type` and run [`Program.Check`](type-checking.md) when +an expression is saved. The same choices carry over: a map whose keys +you control is a closed `ObjectOf`; a JSON payload you only partly know +is an `OpenObject`; a key you normalize to an explicit `nil` is a type +with `.OrNil()`, which the checker makes every read handle. + ## When to preprocess in Go vs in the expression If the same derivation shows up in every expression — `totalCents / 100`, diff --git a/docs/guides/registering-functions.md b/docs/guides/registering-functions.md index fd593c0..09bc404 100644 --- a/docs/guides/registering-functions.md +++ b/docs/guides/registering-functions.md @@ -186,6 +186,27 @@ feature is gone. The second argument is an *evaluated value*, not an AST re-evaluated per element. If you want per-element evaluation, don't shadow the form — register a different name. +## Types for `Check` + +[`Program.Check`](type-checking.md) knows the signatures of the builtins +and the opt-in groups. A function you register returns `Any` to the +checker, which turns checking off for everything derived from the call. +Declare its type with `WithFuncType`, passed to `Check`: + +```go +t, err := p.Check(env, expr.WithFuncType("concat", func(args []*expr.Type) (*expr.Type, error) { + if len(args) != 2 { + return nil, fmt.Errorf("concat expects 2 arguments, got %d", len(args)) + } + return expr.Join(args[0], args[1]), nil +})) +``` + +The function gets the argument types and returns the result type, or an +error that `Check` reports at the call. Replacing a builtin with your own +function of the same name drops the builtin's signature, since `Check` +can no longer know what the call does. + ## Errors from registered functions A `(T, error)` return becomes an `ErrEvaluate`-wrapped error out of diff --git a/docs/guides/templates.md b/docs/guides/templates.md index 81fd452..0f09447 100644 --- a/docs/guides/templates.md +++ b/docs/guides/templates.md @@ -174,6 +174,10 @@ template: invalid expression `${x +}` at 2:3 (offset 11): ... template: unclosed `${` at 3:1 (offset 18, missing `}`) ``` +To catch mistakes in the expressions before anything renders, +`Template.Check(env)` type-checks every `${...}` against the env's types +(see [Type checking](type-checking.md)). + ## Inspecting segments: `Template.Segments()` `Segments()` returns the parsed segments in source order as diff --git a/docs/guides/type-checking.md b/docs/guides/type-checking.md new file mode 100644 index 0000000..51720f7 --- /dev/null +++ b/docs/guides/type-checking.md @@ -0,0 +1,223 @@ +# Type checking + +`Run` finds an expression's mistakes when it runs. If expressions are +written ahead of time, in a config file, a rule, or a workflow +definition, that may be days after someone saved them, against data +that happened to hit the broken branch. `Program.Check` finds the same +mistakes when the expression is saved, with nothing to run against. + +You describe the types of the env once. `Check` walks the same parsed +expression that `Run` evaluates and reports every problem it finds. A +runnable companion lives in +[`../../examples/type_checking/`](../../examples/type_checking/). + +## Describing the env + +The env for `Check` is a `map[string]*expr.Type` with the same names as +the env you will pass to `Run`: + +```go +env := map[string]*expr.Type{ + "issue": expr.ObjectOf(map[string]*expr.Type{ + "kind": expr.EnumOf("bug", "feature", "question"), + "title": expr.String(), + "labels": expr.ListOf(expr.String()), + "score": expr.Number(), + }), + "review": expr.ObjectOf(map[string]*expr.Type{ + "verdict": expr.EnumOf("approve", "reject"), + "body": expr.String(), + }).OrNil(), + "event": expr.OpenObject(map[string]*expr.Type{ + "type": expr.String(), + "sender": expr.OpenObject(map[string]*expr.Type{"login": expr.String()}), + }), +} +``` + +The types: + +| Type | Values | +| ------------------------ | ---------------------------------------------------------- | +| `expr.Any` | anything; unchecked | +| `Null()` | only nil | +| `Bool()`, `Number()`, `String()` | a bool, a number (int or float), a string | +| `EnumOf("a", "b")` | one of those strings | +| `ListOf(t)` | a list whose elements are `t` | +| `ObjectOf(fields)` | an object with exactly these fields (closed) | +| `OpenObject(fields)` | an object with these fields and possibly others | +| `t.OrNil()` | `t`, or nil | + +Use `ObjectOf` when you know every field, such as data your own code +builds. Reading a field it does not list is a mistake. Use `OpenObject` +for payloads you only partly know, such as webhooks: unlisted fields are +`Any`. Set `Elem` on an open object (`&expr.Type{Kind: expr.KindObject, +Open: true, Elem: expr.Number()}`) for a map whose keys you don't know +but whose values you do. + +`Any`, and a nil `*Type` anywhere in env or `Fields`, turns checking off +for that value and everything read from it. A partly typed env checks +what it can, so you can start with `Any` and add types where mistakes +hurt. + +If your data is described by JSON Schema, convert the schema to a +`Type`: `object` with `properties` becomes `ObjectOf` (or `OpenObject` +when `additionalProperties` is not false), `enum` becomes `EnumOf`, and +a property not in `required` becomes `.OrNil()`. + +## Running it + +```go +p, err := expr.Compile(src, expr.WithBuiltins(), expr.WithFunctions(expr.StringFuncs())) +if err != nil { + return err // a syntax error +} +t, err := p.Check(env) +var te *expr.TypeError +if errors.As(err, &te) { + for _, problem := range te.Problems { + fmt.Println(problem) + } +} +``` + +`Check` returns the result type and, if anything is wrong, a +`*TypeError` that lists every problem. It wraps `expr.ErrType`, so +`errors.Is(err, expr.ErrType)` works. Compile with the same options you +run with: `Check` uses the registered functions to type calls. + +The result type is useful too. A host that needs a boolean condition +can require `t.Kind == expr.KindBool`, and a host that stores the result +can check it against what it expects to store. + +```text +issue.kind == "bug" && issue.score > 0.5 + ok: bool +map(filter(issue.labels, startsWith(it, "area/")), upper(it)) + ok: list +{"title": issue.title, "kind": issue.kind} + ok: {kind: "bug" | "feature" | "question", title: string} +``` + +## What it reports + +Every problem is one that `Run` would hit with some input: + +```text +issue.titel + `issue.titel`: no field "titel" in {kind: "bug" | "feature" | "question", labels: list, score: number, title: string} (did you mean "title"?) +issue.kind == "bugfix" + `issue.kind == "bugfix"`: the value is one of "bug" | "feature" | "question", never "bugfix" +review.verdict + `review`: may be nil here; use ?. or try(...) +issue.score + issue.title + `issue.score + issue.title`: cannot add number and string +split(issue.title, 1) + `1`: split: argument 2 must be string, not number +issue.labels[issue.title] + `issue.labels[issue.title]`: list index must be a number, not string +``` + +In full, it reports: + +- names that are not in env, a binding (`it`, `index`, a named element) + or the registered functions; +- fields a closed object does not have; +- operations on the wrong kind of value: arithmetic on strings, + ordering bools, iterating something that is not a list, indexing a + list with a string; +- builtin calls with the wrong number of arguments or an argument the + builtin rejects; +- reads of values that may be nil, and keys that may be absent; +- `==` and `!=` that can never be true, including a string outside an + enum, and `==` on lists or objects, which `Run` rejects. + +The enum check is the one that pays for itself fastest: when the values +of `kind` change, every expression still comparing against the old +value fails its check. + +## Nil and absent values + +A type marked `.OrNil()` may be nil, and reading through nil fails at +run time. `Check` accepts the three ways to handle it: + +```go +review?.verdict // ("approve" | "reject")? +try(review.body, "no review") // string +review != nil && review.verdict == "approve" // bool +``` + +A nil check narrows the value where the check guarantees it: on the +right of `x != nil && ...` and `x == nil || ...`, and in the then +branch of `if(x != nil, ..., ...)`. Narrowing is by the text of the +expression, so `review.verdict` is narrowed by `review != nil` but not +by `r != nil` where `r` happens to hold the same value. + +A key may be absent from an object: an unlisted field of an open object +whose `Elem` is typed, or `scores[name]` where `name` is any string. +Reading a missing key fails, so `Check` asks for `?.` or `?[`, which +give nil instead. + +## `&&`, `||` and unions + +`&&` and `||` return one of their operands, not a boolean (see the +spec's Logical section), and `Check` types them that way: +`issue.title || "untitled"` is a string, and `issue.title || 0` is +`0 | string`, a union. A union is fine to store or compare with `==`, +but an operation that needs one kind reports it: + +```text +len(issue.title || 0) + `issue.title || 0`: len: argument 1 may be number, and must be string or a list or an object +``` + +## Functions + +The builtins and the optional groups (`MathFuncs`, `StringFuncs`, +`CollectionFuncs`) have signatures, so `Check` knows their arguments and +result types. A function you register returns `Any` unless you declare +its type with `WithFuncType`: + +```go +p, _ := expr.Compile(`concat(a, b)`, expr.WithFunctions(map[string]any{"concat": concat})) +t, err := p.Check(env, expr.WithFuncType("concat", func(args []*expr.Type) (*expr.Type, error) { + if len(args) != 2 { + return nil, fmt.Errorf("concat expects 2 arguments, got %d", len(args)) + } + return expr.Join(args[0], args[1]), nil +})) +``` + +The function receives the argument types and returns the result type, +or an error to report a misuse. `Join(a, b)` is the type of a value that +is either `a` or `b`. + +Names resolve the way `Run` resolves them: env first, then registered +functions, then the higher-order forms. Registering `count` replaces the +form for `Check` too, and a builtin you replace with your own function +loses the builtin's signature. + +## Templates + +`Template.Check` checks every `${...}` in a template and returns one +`*TypeError` for all of them: + +```go +tmpl, _ := expr.NewTemplate("${issue.title} (${issue.knd})", expr.WithBuiltins()) +err := tmpl.Check(env) +// `issue.knd`: no field "knd" in {kind: ..., title: string} (did you mean "kind"?) +``` + +## What it does not promise + +`Check` is static. It does not see values, so: + +- a list index can still be out of range at run time; +- an `Any` value is unchecked, and so is everything read from it; +- a registered function without `WithFuncType` returns `Any`; +- the env passed to `Run` must match the types passed to `Check`. If it + doesn't, `Check` was checking a different program. + +Within those limits, `Check` agrees with `Run`: an expression that +passes should not fail with any of the problems listed above. One that +does is a bug in `Check`. diff --git a/docs/reference/spec.md b/docs/reference/spec.md index 6dbc885..d3ff104 100644 --- a/docs/reference/spec.md +++ b/docs/reference/spec.md @@ -757,8 +757,10 @@ Check reports, as a `*TypeError` wrapping `ErrType`, every: `&&` and `||` return one of their operands (see Logical), so they type as the join of their operand types: `bool` only when both sides are. Joining -types of different kinds gives a union (`string | 0` for `s || 0`), which +types of different kinds gives a union (`0 | string` for `s || 0`), which arithmetic, ordering, field reads and indexing reject: they need one kind. A +builtin accepts a union only when it accepts every member, and `==` reports +one that shares no kind with the other side. A nil check narrows: in `x != nil && x.a > 3`, `x == nil || x.a > 3` and `if(x != nil, x.a, 0)`, `x` is known not to be nil where it is read. A field an open object does not list may be absent, so it is read with `?.`. diff --git a/docs_guides_test.go b/docs_guides_test.go index c4963b1..0fa0911 100644 --- a/docs_guides_test.go +++ b/docs_guides_test.go @@ -731,3 +731,116 @@ func TestGuide_HigherOrder_Pipelines(t *testing.T) { t.Fatal("expected non-call pipe right side to fail compilation") } } + +// --- type-checking.md --------------------------------------------------------- + +func TestGuide_TypeChecking(t *testing.T) { + env := map[string]*Type{ + "issue": ObjectOf(map[string]*Type{ + "kind": EnumOf("bug", "feature", "question"), + "title": String(), + "labels": ListOf(String()), + "score": Number(), + }), + "review": ObjectOf(map[string]*Type{ + "verdict": EnumOf("approve", "reject"), + "body": String(), + }).OrNil(), + "event": OpenObject(map[string]*Type{ + "type": String(), + "sender": OpenObject(map[string]*Type{"login": String()}), + }), + } + check := func(src string, opts ...CheckOption) (*Type, error) { + p, err := Compile(src, WithBuiltins(), WithFunctions(StringFuncs())) + if err != nil { + t.Fatalf("compile %s: %v", src, err) + } + return p.Check(env, opts...) + } + for src, want := range map[string]string{ + `issue.kind == "bug" && issue.score > 0.5`: "bool", + `map(filter(issue.labels, startsWith(it, "area/")), upper(it))`: "list", + `{"title": issue.title, "kind": issue.kind}`: `{kind: "bug" | "feature" | "question", title: string}`, + `review?.verdict`: `("approve" | "reject")?`, + `try(review.body, "no review")`: "string", + `review != nil && review.verdict == "approve"`: "bool", + `event.action`: "any", + `issue.title || "untitled"`: "string", + `issue.title || 0`: "0 | string", + } { + got, err := check(src) + if err != nil { + t.Fatalf("%s: %v", src, err) + } + if got.String() != want { + t.Fatalf("%s: got %s, want %s", src, got, want) + } + } + for src, want := range map[string]string{ + `issue.titel`: `no field "titel" in {kind: "bug" | "feature" | "question", labels: list, score: number, title: string} (did you mean "title"?)`, + `issue.kind == "bugfix"`: `the value is one of "bug" | "feature" | "question", never "bugfix"`, + `review.verdict`: "`review`: may be nil here; use ?. or try(...)", + `issue.score + issue.title`: "cannot add number and string", + `split(issue.title, 1)`: "split: argument 2 must be string, not number", + `issue.labels[issue.title]`: "list index must be a number, not string", + `len(issue.title || 0)`: "len: argument 1 may be number, and must be string or a list or an object", + } { + _, err := check(src) + if !errors.Is(err, ErrType) || !strings.Contains(err.Error(), want) { + t.Fatalf("%s: got %v, want %q", src, err, want) + } + } + + // WithFuncType. + p, err := Compile(`concat(issue.labels, issue.labels)`, WithFunctions(map[string]any{ + "concat": func(a, b []any) []any { return append(a, b...) }, + })) + if err != nil { + t.Fatal(err) + } + got, err := p.Check(env, WithFuncType("concat", func(args []*Type) (*Type, error) { + if len(args) != 2 { + return nil, fmt.Errorf("concat expects 2 arguments, got %d", len(args)) + } + return Join(args[0], args[1]), nil + })) + if err != nil || got.String() != "list" { + t.Fatalf("concat: %v, %v", got, err) + } + + // Template.Check. + tmpl, err := NewTemplate("${issue.title} (${issue.knd})", WithBuiltins()) + if err != nil { + t.Fatal(err) + } + if err := tmpl.Check(env); err == nil || !strings.Contains(err.Error(), `(did you mean "kind"?)`) { + t.Fatalf("template: %v", err) + } +} + +// The README's type checking example. +func TestReadme_TypeChecking(t *testing.T) { + env := map[string]*Type{ + "issue": ObjectOf(map[string]*Type{ + "kind": EnumOf("bug", "feature", "question"), + "title": String(), + }), + "review": ObjectOf(map[string]*Type{"body": String()}).OrNil(), + "event": Any, + } + p, err := Compile(`issue.kind == "bugfix" || len(review.body) > 0`, WithBuiltins()) + if err != nil { + t.Fatal(err) + } + _, err = p.Check(env) + var te *TypeError + if !errors.As(err, &te) { + t.Fatalf("got %v", err) + } + want := []string{ + "`issue.kind == \"bugfix\"`: the value is one of \"bug\" | \"feature\" | \"question\", never \"bugfix\"", + "`review`: may be nil here; use ?. or try(...)", + } + assertDeepEqual(t, te.Problems, want) +} diff --git a/engine.go b/engine.go index 2686d3a..8989696 100644 --- a/engine.go +++ b/engine.go @@ -31,6 +31,16 @@ // // t, err := expr.NewTemplate("Hello ${user.name}!", expr.WithBuiltins()) // out, err := t.Render(ctx, env) +// +// # Type checking +// +// [Program.Check] checks an expression against the [Type] of each env +// name without running it, and reports every problem as a [*TypeError]: +// unknown names and fields, wrong-kind operations and builtin arguments, +// reads of values that may be nil, and comparisons that are never true. +// +// env := map[string]*expr.Type{"user": expr.ObjectOf(map[string]*expr.Type{"name": expr.String()})} +// t, err := p.Check(env) package expr import ( diff --git a/examples/README.md b/examples/README.md index e964163..5d51ec5 100644 --- a/examples/README.md +++ b/examples/README.md @@ -4,14 +4,15 @@ Small, runnable programs showing how the `expr` expression language looks and works. Each subdirectory is a standalone `main.go`; run any of them with `go run ./examples/`. -| Directory | What it shows | -| --------------- | ---------------------------------------------------------- | -| `basic/` | default built-ins over a `map[string]any` environment | -| `structs/` | struct envs with fields and bound methods | -| `funcs/` | registering custom Go functions as callable identifiers | -| `compile_once/` | compile once, evaluate many, the hot-path pattern | -| `template/` | `${...}` string interpolation via `NewTemplate` / `Render` | -| `higher_order/` | `map` / `filter` / `any` / `all` / `find` / `count` | +| Directory | What it shows | +| ---------------- | ---------------------------------------------------------- | +| `basic/` | default built-ins over a `map[string]any` environment | +| `structs/` | struct envs with fields and bound methods | +| `funcs/` | registering custom Go functions as callable identifiers | +| `compile_once/` | compile once, evaluate many, the hot-path pattern | +| `template/` | `${...}` string interpolation via `NewTemplate` / `Render` | +| `higher_order/` | `map` / `filter` / `any` / `all` / `find` / `count` | +| `type_checking/` | `Program.Check` against typed env, accepted and rejected | Each of the first five directories corresponds to one of the code blocks in the top-level [README](../README.md); you can copy the diff --git a/examples/type_checking/main.go b/examples/type_checking/main.go new file mode 100644 index 0000000..ecfeec4 --- /dev/null +++ b/examples/type_checking/main.go @@ -0,0 +1,84 @@ +// Type checking: describe the env's types once, then check each +// expression when it is saved instead of finding its mistakes when it +// runs. +package main + +import ( + "errors" + "fmt" + "log" + + "github.com/deepnoodle-ai/expr" +) + +// env describes what Run will be given: a classified issue, an optional +// review, and a webhook payload whose fields are not all known. +var env = map[string]*expr.Type{ + "issue": expr.ObjectOf(map[string]*expr.Type{ + "kind": expr.EnumOf("bug", "feature", "question"), + "title": expr.String(), + "labels": expr.ListOf(expr.String()), + "score": expr.Number(), + }), + "review": expr.ObjectOf(map[string]*expr.Type{ + "verdict": expr.EnumOf("approve", "reject"), + "body": expr.String(), + }).OrNil(), + "event": expr.OpenObject(map[string]*expr.Type{ + "type": expr.String(), + "sender": expr.OpenObject(map[string]*expr.Type{"login": expr.String()}), + }), +} + +func check(src string) { + p, err := expr.Compile(src, expr.WithBuiltins(), expr.WithFunctions(expr.StringFuncs())) + if err != nil { + log.Fatal(err) + } + t, err := p.Check(env) + var te *expr.TypeError + if errors.As(err, &te) { + fmt.Printf("%s\n", src) + for _, problem := range te.Problems { + fmt.Printf(" %s\n", problem) + } + return + } + fmt.Printf("%s\n ok: %s\n", src, t) +} + +func main() { + fmt.Println("== Accepted, with the result type") + for _, src := range []string{ + `issue.kind == "bug" && issue.score > 0.5`, + `map(filter(issue.labels, startsWith(it, "area/")), upper(it))`, + `review?.verdict`, + `review != nil && review.verdict == "approve"`, + `try(review.body, "no review")`, + `event.type == "issues" && event.sender.login != ""`, + `event.action`, // an open object's unlisted fields are Any + `{"title": issue.title, "kind": issue.kind}`, + } { + check(src) + } + + fmt.Println("\n== Rejected") + for _, src := range []string{ + `issue.titel`, + `issue.kind == "bugfix"`, + `review.verdict`, + `issue.score + issue.title`, + `split(issue.title, 1)`, + `issue.labels[issue.title]`, + `len(issue.title || 0)`, + } { + check(src) + } + + fmt.Println("\n== A template checks every ${...}") + tmpl, err := expr.NewTemplate("${issue.title} (${issue.knd})", expr.WithBuiltins()) + if err != nil { + log.Fatal(err) + } + fmt.Println(tmpl.Check(env)) +} diff --git a/llms.txt b/llms.txt index 921ef1d..a1c0d65 100644 --- a/llms.txt +++ b/llms.txt @@ -471,6 +471,14 @@ reads of maybe-nil values without `?.`/`try`, wrong-kind operations, builtin calls with the wrong arguments, and comparisons that are never true (including against an enum). A nil `*Type` means `Any`. +`Template.Check(env)` checks every `${...}`. `WithFuncType(name, fn)` types +a registered function: `fn(args []*expr.Type) (*expr.Type, error)` returns +the result type or an error to report. Without it, registered functions +return `Any`. Names resolve as in Run: env, then registered functions, then +the forms. An open object's unlisted fields are `Any` unless `Elem` is set; +reading a key that may be absent needs `?.`/`?[`. Guide: +`docs/guides/type-checking.md`. + Field names may be Go keywords: `event.type`, `x?.range`. ## Cancellation From ee08a3b3cb5cb729b5dface55a9023ed647b0404 Mon Sep 17 00:00:00 2001 From: Curtis Myzie Date: Wed, 28 Oct 2026 12:57:30 -0400 Subject: [PATCH 9/9] Spec: identifiers beginning with __expr_ or __try_ are reserved Co-Authored-By: Claude Opus 5.5 --- docs/reference/spec.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/reference/spec.md b/docs/reference/spec.md index d3ff104..6a71058 100644 --- a/docs/reference/spec.md +++ b/docs/reference/spec.md @@ -301,6 +301,9 @@ The field name may be a Go keyword: `event.type`, `item.range`, `x.default`, `x?.type`. JSON payloads use these names freely. Keywords stay keywords everywhere else. +Identifiers beginning with `__expr_` or `__try_` are reserved for expr's +internal use. This applies to field names as well. + ## Index expressions (`x[i]`) - `map[string]any`: `i` must be a string. Missing key → `ErrEvaluate`.