From 176c597c8a26f7eedca5d8bb08c14a007e3ce898 Mon Sep 17 00:00:00 2001 From: Curtis Myzie Date: Fri, 8 May 2026 10:50:42 -0400 Subject: [PATCH] Add eager if(cond, t, f) builtin What: - Register `if(cond, t, f)` in Builtins(). Truthy condition picks t; falsey picks f. Both branches always evaluate. - Rewrite the Go `if` keyword token to a sentinel identifier in preprocessSource so the parser accepts the call. The rewrite uses the same scheme already in place for `map`; the rewrite list is factored out so future additions stay symmetric. - Translate the sentinel back via displayIdent in evalIdent, resolveCallable, and prewalk so user-visible env entries, registered shadows, error messages, and the prepared-func cache all key on "if". - Update templates.md and higher-order-patterns.md to use `if(cond, t, f)` instead of the brittle `cond && t || f` idiom. Update spec.md and llms.txt to document the new builtin. Why: - `if(cond, t, f)` reads cleanly even when both branches are non-nil values, where `cond && t || f` silently picks `f` whenever `t` happens to be falsey. - Consolidating the keyword-rewrite plumbing keeps the next addition (`?.` / `?[` in step 7) easy and consistent. --- boundaries1_test.go | 94 ++++++++++++++++++++++++++++ builtins.go | 17 +++++ docs/guides/higher-order-patterns.md | 9 +-- docs/guides/templates.md | 11 ++-- docs/reference/spec.md | 1 + engine.go | 94 +++++++++++++++++++++------- llms.txt | 1 + program.go | 32 ++++++---- 8 files changed, 217 insertions(+), 42 deletions(-) diff --git a/boundaries1_test.go b/boundaries1_test.go index 427b714..6be4ee1 100644 --- a/boundaries1_test.go +++ b/boundaries1_test.go @@ -714,6 +714,100 @@ func TestBuiltin_Bool(t *testing.T) { } } +func TestBuiltin_If(t *testing.T) { + opts := []Option{WithBuiltins()} + cases := []struct { + expr string + env map[string]any + want any + }{ + {`if(true, "yes", "no")`, nil, "yes"}, + {`if(false, "yes", "no")`, nil, "no"}, + {`if(1, "yes", "no")`, nil, "yes"}, + {`if(0, "yes", "no")`, nil, "no"}, + {`if("", "yes", "no")`, nil, "no"}, + {`if("x", "yes", "no")`, nil, "yes"}, + {`if(nil, "yes", "no")`, nil, "no"}, + {`if(score > 90, "A", "B")`, map[string]any{"score": 95}, "A"}, + {`if(score > 90, "A", "B")`, map[string]any{"score": 80}, "B"}, + } + for _, tc := range cases { + t.Run(tc.expr, func(t *testing.T) { + got, err := evalExpr(t.Context(), tc.expr, tc.env, opts...) + require.NoError(t, err) + require.Equal(t, tc.want, got) + }) + } +} + +func TestBuiltin_If_Arity(t *testing.T) { + opts := []Option{WithBuiltins()} + for _, expr := range []string{`if(true)`, `if(true, 1)`, `if(true, 1, 2, 3)`} { + _, err := evalExpr(t.Context(), expr, nil, opts...) + require.Error(t, err, expr) + require.ErrorIs(t, err, ErrEvaluate) + } +} + +// Without WithBuiltins, `if(...)` must report a clean "unknown +// function" error using the user-visible name, not the internal +// rewrite sentinel. +func TestBuiltin_If_NotRegistered(t *testing.T) { + _, err := evalExpr(t.Context(), `if(true, 1, 2)`, nil) + require.ErrorIs(t, err, ErrEvaluate) + require.Contains(t, err.Error(), `unknown function "if"`) +} + +// `if` is eager: the unselected branch still evaluates, so a +// runtime error there propagates. Users who need laziness reach for +// try, &&, or ||. +func TestBuiltin_If_EagerEvaluation(t *testing.T) { + opts := []Option{WithBuiltins()} + env := map[string]any{"xs": []any{1, 2, 3}} + _, err := evalExpr(t.Context(), `if(true, xs[0], xs[99])`, env, opts...) + require.Error(t, err) +} + +// A user-registered `if` must shadow the builtin, matching the +// shadow rules every other builtin obeys. +func TestBuiltin_If_UserShadows(t *testing.T) { + opts := []Option{ + WithBuiltins(), + WithFunctions(map[string]any{ + "if": Func(func(_ context.Context, _ []any) (any, error) { + return "shadow", nil + }), + }), + } + got, err := evalExpr(t.Context(), `if(true, 1, 2)`, nil, opts...) + require.NoError(t, err) + require.Equal(t, "shadow", got) +} + +// An env entry named `if` must shadow the builtin, matching the +// env-wins-over-funcs rule. +func TestBuiltin_If_EnvShadows(t *testing.T) { + opts := []Option{WithBuiltins()} + env := map[string]any{ + "if": Func(func(_ context.Context, _ []any) (any, error) { + return "envshadow", nil + }), + } + got, err := evalExpr(t.Context(), `if(true, 1, 2)`, env, opts...) + require.NoError(t, err) + require.Equal(t, "envshadow", got) +} + +// `if` must be usable inside higher-order predicates and templates; +// nothing about the keyword rewrite should break composition. +func TestBuiltin_If_InsidePredicate(t *testing.T) { + opts := []Option{WithBuiltins()} + env := map[string]any{"xs": []any{int64(1), int64(2), int64(3), int64(4)}} + got, err := evalExpr(t.Context(), `map(xs, if(it > 2, "big", "small"))`, env, opts...) + require.NoError(t, err) + require.Equal(t, []any{"small", "small", "big", "big"}, got) +} + func TestBuiltin_Contains_Nil(t *testing.T) { opts := []Option{WithBuiltins()} got, err := evalExpr(t.Context(), "contains(nil, 1)", nil, opts...) diff --git a/builtins.go b/builtins.go index 2ce8485..1aaf30c 100644 --- a/builtins.go +++ b/builtins.go @@ -24,6 +24,8 @@ import ( // as base-10 integers // float(v) numeric conversion to float64; strings parse strictly // bool(v) truthiness check (matches IsTruthy) +// if(cond, t, f) pick t when cond is truthy, else f; both branches +// are evaluated eagerly // contains(h, n) substring for strings, element membership for // slices/arrays (using loose numeric equality), or // key presence for string-keyed maps @@ -38,6 +40,7 @@ func Builtins() map[string]any { "int": Func(nativeInt), "float": Func(nativeFloat), "bool": Func(nativeBool), + "if": Func(nativeIf), "contains": Func(nativeContains), "has": Func(nativeHas), "keys": Func(nativeKeys), @@ -90,6 +93,20 @@ func nativeBool(_ context.Context, args []any) (any, error) { return IsTruthy(args[0]), nil } +// nativeIf is the eager three-argument selector. Both branches are +// evaluated before the call, so use try / && / || when laziness +// matters. The condition is interpreted via IsTruthy, so any value +// (number, string, slice, ...) can drive the choice. +func nativeIf(_ context.Context, args []any) (any, error) { + if err := checkArity("if", 3, len(args)); err != nil { + return nil, err + } + if IsTruthy(args[0]) { + return args[1], nil + } + return args[2], nil +} + func nativeContains(_ context.Context, args []any) (any, error) { if err := checkArity("contains", 2, len(args)); err != nil { return nil, err diff --git a/docs/guides/higher-order-patterns.md b/docs/guides/higher-order-patterns.md index 44d601f..6f1d8e5 100644 --- a/docs/guides/higher-order-patterns.md +++ b/docs/guides/higher-order-patterns.md @@ -1,10 +1,11 @@ # Higher-order patterns `map`, `filter`, `any`, `all`, `find`, `count` are the closest thing -expr has to control flow. There's no `if`, no `for`, no `let`. These -six forms — plus Go's short-circuit `&&` / `||` — are how you make -decisions and shape data. This guide walks the idioms that come up -most often and the ones you have to work around. +expr has to control flow over collections. There's no `for` and no +`let`. These six forms, plus the eager `if(cond, t, f)` builtin and +Go's short-circuit `&&` / `||`, are how you make decisions and shape +data. This guide walks the idioms that come up most often and the +ones you have to work around. A runnable companion lives in [`../../examples/higher_order_patterns/`](../../examples/higher_order_patterns/). diff --git a/docs/guides/templates.md b/docs/guides/templates.md index f79696b..38577b8 100644 --- a/docs/guides/templates.md +++ b/docs/guides/templates.md @@ -56,11 +56,12 @@ can't distinguish "value was nil" from "value was the empty string" in its output. If you need to, emit a sentinel in the expression: ``` -Nickname: ${user.nickname == nil && "(none)" || user.nickname} +Nickname: ${if(user.nickname == nil, "(none)", user.nickname)} ``` -(That's the standard expr idiom for a ternary: -`cond && ifTrue || ifFalse` — works when `ifTrue` is truthy.) +`if(cond, t, f)` is the canonical ternary in expr. Both branches +evaluate eagerly, so reach for `try(...)` or operand-returning +`||` when you need to dodge a runtime error in one branch. ## The list-stringification footgun @@ -167,8 +168,8 @@ is either "here's the full rendered string" or "here's an error," not - **Boolean flags:** `${admin && " (admin)"}` emits `" (admin)"` when truthy and `false` otherwise; `false` renders as `false` via `%v`, which is usually what you want for debugging but - wrong for user-facing text. Terminate boolean branches explicitly: - `${admin && " (admin)" || ""}`. + wrong for user-facing text. Use `if` to give the false branch an + explicit value: `${if(admin, " (admin)", "")}`. - **Counts with the right singular/plural:** register a `pluralize` helper, or compute the label in Go and pass it in. - **Currency:** format in Go. Templates are not the right place to diff --git a/docs/reference/spec.md b/docs/reference/spec.md index 7914e59..0527fad 100644 --- a/docs/reference/spec.md +++ b/docs/reference/spec.md @@ -322,6 +322,7 @@ The standard set is: | `int(v)` | `(any) -> int64, error` | Numeric values convert (float truncates toward zero). Strings are parsed strictly with `strconv.ParseInt` base-10 (trimmed whitespace, no `0x`, no trailing garbage). | | `float(v)` | `(any) -> float64, error` | Like `int`, but `strconv.ParseFloat` 64-bit. | | `bool(v)` | `(any) -> bool` | Same semantics as [truthiness](#truthiness). | +| `if(c,t,f)` | `(any, any, any) -> any` | Eager three-argument selector: returns `t` when `c` is truthy, else `f`. Both branches always evaluate; reach for `try`, `&&`, or `\|\|` when one branch must be skipped. | | `contains(h,n)` | `(any, any) -> bool, error` | Substring for string haystacks, element membership for slices/arrays (using [loose equality](#equality)), key presence for string-keyed maps. | | `has(m,k)` | `(any, string) -> bool, error` | True if map `m` has key `k`. Maps only. Nil → `false`. | | `keys(m)` | `(any) -> []any, error` | Sorted string keys. Other key types → error. | diff --git a/engine.go b/engine.go index b370e10..5284e82 100644 --- a/engine.go +++ b/engine.go @@ -180,25 +180,45 @@ func Compile(code string, opts ...Option) (*Program, error) { // identifier. const mapFormName = "__expr_map__" +// ifFuncName is the internal identifier that `if` is rewritten to +// before parsing. `if` is a Go statement keyword, so the expression +// parser refuses it as an operand; the rewrite makes the builtin +// callable as `if(cond, t, f)` while keeping the user-visible name +// in error messages and method lookups. +const ifFuncName = "__expr_if__" + +// keywordRewrites lists the Go keyword tokens that expr accepts as +// ordinary identifiers. Each entry maps the source spelling to its +// internal sentinel; preprocessSource walks tokens and substitutes +// occurrences in place. +var keywordRewrites = []struct { + tok token.Token + src string + internal string +}{ + {token.MAP, "map", mapFormName}, + {token.IF, "if", ifFuncName}, +} + // preprocessSource rewrites Go keyword tokens that expr wants to -// accept as ordinary identifiers. The only such token is `map`: Go's -// parser treats `map` as the start of a map-type literal everywhere -// it appears, so expr cannot accept `map(xs, pred)`, `obj.map(...)`, -// or any other construct that names `map` as an identifier. +// accept as ordinary identifiers (`map`, `if`). Go's parser treats +// these as the start of a map-type literal or an if-statement, so +// expr cannot accept `map(xs, pred)` or `if(cond, t, f)` as-is; +// preprocessSource substitutes a sentinel identifier so the parser +// sees a plain CallExpr, and the evaluator translates the sentinel +// back via displayIdent for lookups and error messages. // -// The rewrite replaces every `map` token with mapFormName *unless* -// the next token is `[`, which would indicate a Go map type literal -// like `map[string]int{}`. Composite literals are unsupported by -// expr anyway, so leaving that one case alone lets the parser emit -// its normal error for unsupported syntax. Selector, call, and -// method-call forms all carry the rewritten identifier through to -// the evaluator, which translates it back to "map" in lookups and -// error messages via mapFormDisplayName. +// The `map` rewrite skips occurrences immediately followed by `[`, so +// Go map type literals such as `map[string]int{}` still produce the +// usual "unsupported syntax" error at evaluation rather than being +// silently turned into a meaningless call. `if` has no analogous +// safe context inside expr expressions, so every `if` token is +// rewritten unconditionally. func preprocessSource(src string) string { - // Fast path: most expr expressions do not contain `map` at all, - // so the scanner pass is skipped. `strings.Contains` on a short - // expression is much cheaper than spinning up go/scanner. - if !strings.Contains(src, "map") { + // Fast path: skip the scanner unless the source mentions one of + // the rewritable keywords. Substring checks on short inputs are + // much cheaper than spinning up go/scanner. + if !containsAnyRewriteKeyword(src) { return src } fs := token.NewFileSet() @@ -223,18 +243,19 @@ func preprocessSource(src string) string { out.Grow(len(src) + 16) last := 0 for i := 0; i < len(toks); i++ { - if toks[i].tok != token.MAP { + rule, ok := matchRewrite(toks[i].tok) + if !ok { continue } // Leave `map[...]` alone so Go map type literals continue to // produce a normal "unsupported syntax" error at eval time. - if i+1 < len(toks) && toks[i+1].tok == token.LBRACK { + if rule.tok == token.MAP && i+1 < len(toks) && toks[i+1].tok == token.LBRACK { continue } off := file.Offset(toks[i].pos) out.WriteString(src[last:off]) - out.WriteString(mapFormName) - last = off + len("map") + out.WriteString(rule.internal) + last = off + len(rule.src) } if last == 0 { return src @@ -243,12 +264,41 @@ func preprocessSource(src string) string { return out.String() } +func containsAnyRewriteKeyword(src string) bool { + for _, r := range keywordRewrites { + if strings.Contains(src, r.src) { + return true + } + } + return false +} + +func matchRewrite(tok token.Token) (struct { + tok token.Token + src string + internal string +}, bool) { + for _, r := range keywordRewrites { + if r.tok == tok { + return r, true + } + } + return struct { + tok token.Token + src string + internal string +}{}, false +} + // displayIdent converts an internal rewritten identifier back to the // name the user originally typed, for use in error messages and -// field/method lookups. Currently this only matters for `map`. +// field/method lookups. func displayIdent(name string) string { - if name == mapFormName { + switch name { + case mapFormName: return "map" + case ifFuncName: + return "if" } return name } diff --git a/llms.txt b/llms.txt index 2646e53..e90fa17 100644 --- a/llms.txt +++ b/llms.txt @@ -100,6 +100,7 @@ registered yourself. This keeps the sandbox surface as narrow as you want. | `int(v)` | `(any) -> int64` | Truncates floats; strict base-10 parse for strings | | `float(v)` | `(any) -> float64` | `strconv.ParseFloat` 64-bit for strings | | `bool(v)` | `(any) -> bool` | Same rules as truthiness | +| `if(c, t, f)` | `(any, any, any) -> any` | Eager ternary; `t` and `f` both evaluate before the call | | `contains(h, n)` | `(any, any) -> bool` | Substring, element, or map-key presence | | `has(m, k)` | `(any, string) -> bool` | Key presence on a map (nil → false) | | `keys(m)` | `(any) -> []any` | Sorted string keys | diff --git a/program.go b/program.go index f474502..649d3ee 100644 --- a/program.go +++ b/program.go @@ -149,9 +149,12 @@ func (p *Program) prewalk(node ast.Expr) ast.Expr { // Only cache bare-identifier call targets that resolve to a // registered, prepared function AND are not higher-order // special forms. The form dispatcher needs the raw CallExpr. + // Keyword sentinels (`__expr_if__`, etc.) are translated back + // to their user-visible names so the prepared lookup keys on + // what the user actually wrote. if ident, ok := n.Fun.(*ast.Ident); ok { - name := ident.Name - if _, isForm := higherOrderForms[name]; !isForm && name != mapFormName { + if _, isForm := higherOrderForms[ident.Name]; !isForm { + name := displayIdent(ident.Name) if pf, ok := p.prepared[name]; ok && (pf.native != nil || pf.fv.IsValid()) { p.callCache[n] = pf } @@ -384,17 +387,20 @@ func evalIdent(n *ast.Ident, env any, funcs map[string]any, fieldTags *structTag case "nil": return nil, nil } - if v, ok, err := lookupEnv(env, n.Name, fieldTags); err != nil { + // Keyword sentinels (`__expr_if__`, ...) lookup under their + // user-visible name so env entries and registered functions + // match what the user actually wrote. + name := displayIdent(n.Name) + if v, ok, err := lookupEnv(env, name, fieldTags); err != nil { return nil, err } else if ok { return v, nil } - if fn, ok := funcs[n.Name]; ok { + if fn, ok := funcs[name]; ok { return fn, nil } - user := displayIdent(n.Name) return nil, fmt.Errorf("%w: undefined identifier %q%s", - ErrEvaluate, user, identHint(env, funcs, user, fieldTags)) + ErrEvaluate, name, identHint(env, funcs, name, fieldTags)) } // lookupEnv resolves a top-level identifier against env. env may be a @@ -1139,16 +1145,20 @@ func (p *Program) callValue(ctx context.Context, name string, fn any, argExprs [ func (p *Program) resolveCallable(ctx context.Context, fun ast.Expr, env any, depth int) (string, any, error) { switch f := fun.(type) { case *ast.Ident: - if v, ok, err := lookupEnv(env, f.Name, p.fieldTags); err != nil { + // Translate keyword sentinels (`__expr_if__`, ...) back to + // the user-visible name so env/funcs lookups and the error + // message both see what the user wrote. + name := displayIdent(f.Name) + if v, ok, err := lookupEnv(env, name, p.fieldTags); err != nil { return "", nil, err } else if ok { - return f.Name, v, nil + return name, v, nil } - if v, ok := p.funcs[f.Name]; ok { - return f.Name, v, nil + if v, ok := p.funcs[name]; ok { + return name, v, nil } return "", nil, fmt.Errorf("%w: unknown function %q%s", - ErrEvaluate, f.Name, identHint(env, p.funcs, f.Name, p.fieldTags)) + ErrEvaluate, name, identHint(env, p.funcs, name, p.fieldTags)) case *ast.SelectorExpr: recv, err := p.eval(ctx, f.X, env, depth) if err != nil {