From 2dfb9c86fbca93542598a9ef87d049b98ffe21d0 Mon Sep 17 00:00:00 2001 From: Curtis Myzie Date: Fri, 8 May 2026 11:09:59 -0400 Subject: [PATCH] Add ?. and ?[ optional-access operators What: - Add internal/optaccess: a token-level pre-parse rewrite that turns `obj?.field` into `__try_select__(obj, "field")` and `obj?[i]` into `__try_index__(obj, i)`. The walker matches balanced parens/brackets and treats earlier `?` tokens as continuation links so chained `a?.b?.c` rewrites cleanly. Strings, runes, and comments are not touched. - Wire optaccess into the Compile pipeline ahead of jsonlit and preprocessSource, so `?[1]` can carry the inner index expression through unmolested. - Register sentinel forms `__try_select__` / `__try_index__` in higher_order.go alongside the other special forms. Each form short-circuits on a nil receiver and treats missing-key / out-of-range as nil; wrong-kind type errors still surface. - Add language-level tests in optional_access_test.go (chained selects, fallback via ||, predicates, type-error propagation, string-literal preservation) and unit + fuzz tests in internal/optaccess. - Add fuzz seeds covering `?.` and `?[` to FuzzCompile / FuzzEval. - Update spec.md, llms.txt, and the higher-order-patterns and examples guides (plus their matching test) to use `find(...)?.x` instead of the unsafe `find(...).x`. Why: - `?.` plus operand-returning `||` plus `try` covers the same use cases as `??` without needing precedence-aware token rewriting. - The rewrite layer mirrors internal/jsonlit so the disambiguation logic stays simple and is reasoning about tokens, not strings. --- docs/guides/examples.md | 2 +- docs/guides/higher-order-patterns.md | 2 +- docs/reference/spec.md | 37 +++- docs_examples_test.go | 2 +- engine.go | 20 +- fuzz_test.go | 11 + higher_order.go | 169 +++++++++++++++ internal/optaccess/optaccess.go | 301 +++++++++++++++++++++++++++ internal/optaccess/optaccess_test.go | 154 ++++++++++++++ llms.txt | 23 +- optional_access_test.go | 167 +++++++++++++++ 11 files changed, 880 insertions(+), 8 deletions(-) create mode 100644 internal/optaccess/optaccess.go create mode 100644 internal/optaccess/optaccess_test.go create mode 100644 optional_access_test.go diff --git a/docs/guides/examples.md b/docs/guides/examples.md index a093c27..0fae98a 100644 --- a/docs/guides/examples.md +++ b/docs/guides/examples.md @@ -185,7 +185,7 @@ composite literal. "views": count(events, it.kind == "view"), "purchases": count(events, it.kind == "purchase"), "has_sale": any(events, it.kind == "purchase"), - "top_user": find(events, it.kind == "purchase").user, + "top_user": find(events, it.kind == "purchase")?.user, } ``` diff --git a/docs/guides/higher-order-patterns.md b/docs/guides/higher-order-patterns.md index 6f1d8e5..84daa4b 100644 --- a/docs/guides/higher-order-patterns.md +++ b/docs/guides/higher-order-patterns.md @@ -76,7 +76,7 @@ a list: "views": count(events, it.kind == "view"), "purchases": count(events, it.kind == "purchase"), "has_sale": any(events, it.kind == "purchase"), - "top_user": find(events, it.kind == "purchase").user, + "top_user": find(events, it.kind == "purchase")?.user, } ``` diff --git a/docs/reference/spec.md b/docs/reference/spec.md index 94ee6e5..3c993d4 100644 --- a/docs/reference/spec.md +++ b/docs/reference/spec.md @@ -394,7 +394,7 @@ expression itself surface unchanged. Combine with operand-returning `||` for the common case of presenting `nil` as a sentinel: ``` -try(find(events, it.kind == "purchase").user, "—") +try(find(events, it.kind == "purchase")?.user, "—") try(int(input), 0) > 0 try(user.nickname, nil) || "(none)" ``` @@ -408,6 +408,41 @@ internal token before parsing so the form can still be called as `map(xs, it * 2)`, and translates it back for error messages and method lookups. +## Optional access (`?.` and `?[`) + +`?.field` and `?[idx]` are pre-parse rewrites for "look this up, but +return `nil` if the receiver is missing or the lookup falls off the +end." They cover the common case where a JSON-shaped env may or may +not include a particular branch, without forcing the user to wrap +every access in `try(...)`. + +``` +user?.profile?.nickname || "(none)" +events?[0]?.user +config?.feature?.enabled +``` + +The semantics: + +- If the receiver is `nil`, the result is `nil` and the right-hand + side is not consulted. +- For `?.`, a missing struct field or absent map key resolves to + `nil`. +- For `?[`, a missing map key or an out-of-range slice/string index + resolves to `nil`. +- A wrong-kind error (selecting on a value that is not a struct or + map, indexing a slice with a non-integer, indexing into a map with + the wrong key type) still surfaces as `ErrEvaluate`. `?.` and `?[` + swallow "not there" errors, not "real bugs." + +`?.` and `?[` are pure source-level sugar. The rewrite happens +before the parser sees the source, so they behave like calls on +internal sentinel functions (`__try_select__` and `__try_index__`). +Users do not interact with those names directly, but they may +appear in error chains for diagnostic purposes. Strings, runes, and +comments are not rewritten — `?.` written inside `"..."` or a +comment is preserved verbatim. + ## Helpful errors expr annotates "not found" errors with a short hint drawn from the diff --git a/docs_examples_test.go b/docs_examples_test.go index 0ba63fc..88e1d32 100644 --- a/docs_examples_test.go +++ b/docs_examples_test.go @@ -143,7 +143,7 @@ func TestDocsExample4_EventSummary(t *testing.T) { "views": count(events, it.kind == "view"), "purchases": count(events, it.kind == "purchase"), "has_sale": any(events, it.kind == "purchase"), - "top_user": find(events, it.kind == "purchase").user, + "top_user": find(events, it.kind == "purchase")?.user, }` env := map[string]any{ "events": []any{ diff --git a/engine.go b/engine.go index 5284e82..52f196d 100644 --- a/engine.go +++ b/engine.go @@ -41,6 +41,7 @@ import ( "strings" "github.com/deepnoodle-ai/expr/internal/jsonlit" + "github.com/deepnoodle-ai/expr/internal/optaccess" ) // ErrCompile wraps parse failures so callers can match with errors.Is. @@ -151,7 +152,12 @@ func Compile(code string, opts ...Option) (*Program, error) { for _, opt := range opts { opt(cfg) } - parsed := preprocessSource(jsonlit.Rewrite(code)) + // 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(jsonlit.Rewrite(optaccess.Rewrite(code))) fset := token.NewFileSet() node, err := parser.ParseExprFrom(fset, "", parsed, 0) if err != nil { @@ -187,6 +193,18 @@ const mapFormName = "__expr_map__" // in error messages and method lookups. const ifFuncName = "__expr_if__" +// trySelectFormName and tryIndexFormName are the internal sentinel +// identifiers emitted by the optaccess pre-parse rewrite for the +// optional-access operators `?.` and `?[`. The evaluator dispatches +// them as special forms in higherOrderForms so they can short-circuit +// on a nil receiver and treat missing fields / out-of-range indices +// as nil rather than as errors. Users never type these names +// directly. +const ( + trySelectFormName = "__try_select__" + tryIndexFormName = "__try_index__" +) + // 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 diff --git a/fuzz_test.go b/fuzz_test.go index 3a27652..1aa6e46 100644 --- a/fuzz_test.go +++ b/fuzz_test.go @@ -68,6 +68,17 @@ var fuzzCorpus = []string{ "1 ^ 2", "1i", "", + // optional access (`?.` / `?[`) + "state.user?.nickname", + "state.user?.profile?.nickname", + "state.items?[0]", + "state.items?[99]", + "state.user?.nickname || \"(none)\"", + `"contains?.field"`, + "state /* ?.x */ .name", + "?.x", + "a?.", + "a?[", } // fuzzEnv is the environment FuzzEval runs every mutated expression diff --git a/higher_order.go b/higher_order.go index e5bc365..a46d754 100644 --- a/higher_order.go +++ b/higher_order.go @@ -54,6 +54,11 @@ func init() { "find": formFind, "count": formCount, "try": formTry, + // Sentinel forms emitted by the optaccess pre-parse rewrite. + // Users do not type these names; they appear only as the + // callee of synthesized CallExpr nodes. + trySelectFormName: formTrySelect, + tryIndexFormName: formTryIndex, } } @@ -308,6 +313,170 @@ func formCount(p *Program, ctx context.Context, n *ast.CallExpr, env any, depth return total, nil } +// formTrySelect implements the optaccess `?.` rewrite target, +// `__try_select__(receiver, "field")`. It evaluates the receiver; +// when the receiver is nil, returns nil. Otherwise it performs a +// field/key lookup via trySelectName, which returns nil for missing +// fields or keys without producing an error. Type errors (e.g., +// selecting on a non-struct, non-map value) propagate so real bugs +// still surface. +func formTrySelect(p *Program, ctx context.Context, n *ast.CallExpr, env any, depth int) (any, error) { + if len(n.Args) != 2 { + return nil, fmt.Errorf("%w: %s expects 2 arguments (receiver, field), got %d", + ErrEvaluate, trySelectFormName, len(n.Args)) + } + recv, err := p.eval(ctx, n.Args[0], env, depth) + if err != nil { + return nil, err + } + if recv == nil { + return nil, nil + } + name, err := p.eval(ctx, n.Args[1], env, depth) + if err != nil { + return nil, err + } + s, ok := name.(string) + if !ok { + return nil, fmt.Errorf("%w: %s expects a string field name, got %T", + ErrEvaluate, trySelectFormName, name) + } + return trySelectName(recv, s, p.fieldTags) +} + +// formTryIndex implements the optaccess `?[` rewrite target, +// `__try_index__(receiver, idx)`. Mirrors formTrySelect: nil +// receiver returns nil, missing keys and out-of-range indices return +// nil, and type errors (wrong index kind for the receiver) propagate. +func formTryIndex(p *Program, ctx context.Context, n *ast.CallExpr, env any, depth int) (any, error) { + if len(n.Args) != 2 { + return nil, fmt.Errorf("%w: %s expects 2 arguments (receiver, index), got %d", + ErrEvaluate, tryIndexFormName, len(n.Args)) + } + recv, err := p.eval(ctx, n.Args[0], env, depth) + if err != nil { + return nil, err + } + if recv == nil { + return nil, nil + } + idx, err := p.eval(ctx, n.Args[1], env, depth) + if err != nil { + return nil, err + } + return tryIndexValue(recv, idx) +} + +// trySelectName mirrors selectField but returns nil for missing +// fields or keys. Method values are not surfaced; selectField does +// not surface them either, so the two paths agree on what counts as +// a "field" lookup. +func trySelectName(recv any, name string, fieldTags *structTagConfig) (any, error) { + if recv == nil { + return nil, nil + } + if m, ok := recv.(map[string]any); ok { + v := m[name] + return v, nil + } + rv := reflect.ValueOf(recv) + if rv.Kind() == reflect.Pointer { + if rv.IsNil() { + return nil, nil + } + rv = rv.Elem() + } + switch rv.Kind() { + case reflect.Struct: + fv, ok, err := structFieldByName(rv, name, fieldTags) + if err != nil { + return nil, err + } + if !ok || !fv.IsValid() || !fv.CanInterface() { + return nil, nil + } + return fv.Interface(), nil + case reflect.Map: + if rv.Type().Key().Kind() != reflect.String { + return nil, fmt.Errorf("%w: cannot select %q on map with non-string keys", + ErrEvaluate, name) + } + mv := rv.MapIndex(mapStringKey(rv.Type().Key(), name)) + if !mv.IsValid() { + return nil, nil + } + return mv.Interface(), nil + } + return nil, fmt.Errorf("%w: cannot select %q on %T", ErrEvaluate, name, recv) +} + +// tryIndexValue mirrors indexValue but returns nil for missing +// keys and out-of-range slice/string indices. Wrong-kind errors +// (e.g., string index into a slice, non-integer slice index) still +// propagate. +func tryIndexValue(recv, idx any) (any, error) { + if recv == nil { + return nil, nil + } + if m, ok := recv.(map[string]any); ok { + key, ok := idx.(string) + if !ok { + return nil, fmt.Errorf("%w: map index must be string, got %T", + ErrEvaluate, idx) + } + v := m[key] + return v, nil + } + rv := reflect.ValueOf(recv) + switch rv.Kind() { + case reflect.Slice, reflect.Array: + i, err := toIndexInt(idx) + if err != nil { + return nil, err + } + if i < 0 || i >= int64(rv.Len()) { + return nil, nil + } + return rv.Index(int(i)).Interface(), nil + case reflect.String: + i, err := toIndexInt(idx) + if err != nil { + return nil, err + } + runes := []rune(rv.String()) + if i < 0 || i >= int64(len(runes)) { + return nil, nil + } + return string(runes[i]), nil + case reflect.Map: + keyType := rv.Type().Key() + if idx == nil { + return nil, fmt.Errorf("%w: cannot use nil as map key %v", ErrEvaluate, keyType) + } + kv := reflect.ValueOf(idx) + if !kv.Type().AssignableTo(keyType) { + if isNumericKind(keyType.Kind()) && isNumericKind(kv.Kind()) { + converted, err := safeNumericConvert(kv, keyType) + if err != nil { + return nil, fmt.Errorf("%w: map key conversion: %v", ErrEvaluate, err) + } + kv = converted + } else if kv.Type().ConvertibleTo(keyType) { + kv = kv.Convert(keyType) + } else { + return nil, fmt.Errorf("%w: cannot use %T as map key %v", + ErrEvaluate, idx, keyType) + } + } + mv := rv.MapIndex(kv) + if !mv.IsValid() { + return nil, nil + } + return mv.Interface(), nil + } + return nil, fmt.Errorf("%w: cannot index %T", ErrEvaluate, recv) +} + // formTry implements `try(value, default)`. It evaluates the first // argument; if evaluation returns an ErrEvaluate, it evaluates and // returns the second argument instead. The default expression is diff --git a/internal/optaccess/optaccess.go b/internal/optaccess/optaccess.go new file mode 100644 index 0000000..cb21d63 --- /dev/null +++ b/internal/optaccess/optaccess.go @@ -0,0 +1,301 @@ +// Package optaccess rewrites the optional-access operators `?.` and +// `?[` into calls on internal sentinel functions that the evaluator +// dispatches as special forms. +// +// obj?.field becomes __try_select__(obj, "field") +// obj?[i] becomes __try_index__(obj, i) +// +// The rewrite is token-based. Strings, runes, and comments are +// scanner tokens we never look inside, so a `?.` written inside a +// string literal or comment is preserved verbatim. The transform is +// also a no-op when src contains no `?` byte: the scanner pass is +// skipped entirely. +// +// The LHS of `?.` / `?[` is the primary expression that ends just +// before the `?`. The walker matches balanced parens and brackets so +// `f(a)?.b`, `a[0]?.b`, and `(a + b)?.c` all rewrite with the +// expected LHS. Chained optional access (`a?.b?.c`) is processed by +// repeated single-rewrite passes until the source no longer contains +// `?.` / `?[`, producing nested calls like +// `__try_select__(__try_select__(a, "b"), "c")`. +// +// optaccess does not validate. Anything it cannot classify +// unambiguously is left alone for the parser to reject. +package optaccess + +import ( + "go/scanner" + "go/token" + "strings" +) + +// Rewrite returns src with `?.field` and `?[idx]` rewritten to calls +// on the sentinel functions __try_select__ and __try_index__. +// +// When src contains no `?`, Rewrite returns src unchanged without +// invoking the scanner. Chained optional access is handled by +// iteratively rewriting the leftmost remaining `?.` or `?[` until +// the source contains none, so each rewrite sees the previous one +// already rewritten as its LHS. +func Rewrite(src string) string { + if !strings.Contains(src, "?") { + return src + } + for { + next, changed := rewriteOnce(src) + if !changed { + return src + } + src = next + } +} + +// rewriteOnce rewrites a single leftmost `?.` or `?[` occurrence in +// src and returns the new source. The boolean is false when no +// rewritable occurrence was found, in which case src is returned +// unchanged. +func rewriteOnce(src string) (string, bool) { + toks := scanTokens(src) + if len(toks) == 0 { + return src, false + } + for i := 0; i < len(toks); i++ { + if !isQuestion(src, toks[i]) { + continue + } + if i+1 >= len(toks) { + continue + } + next := toks[i+1] + switch next.kind { + case token.PERIOD: + if i+2 >= len(toks) || toks[i+2].kind != token.IDENT { + continue + } + lhsStart := lhsStartIdx(toks, src, i) + if lhsStart < 0 { + continue + } + field := toks[i+2].lit + if field == "" { + field = src[toks[i+2].pos:toks[i+2].end] + } + lhsBytes := src[toks[lhsStart].pos:toks[i].pos] + rep := "__try_select__(" + lhsBytes + ", " + quoteString(field) + ")" + return splice(src, toks[lhsStart].pos, toks[i+2].end, rep), true + case token.LBRACK: + closeIdx := matchBracketForward(toks, i+1) + if closeIdx < 0 { + continue + } + lhsStart := lhsStartIdx(toks, src, i) + if lhsStart < 0 { + continue + } + lhsBytes := src[toks[lhsStart].pos:toks[i].pos] + idxBytes := src[toks[i+1].end:toks[closeIdx].pos] + rep := "__try_index__(" + lhsBytes + ", " + idxBytes + ")" + return splice(src, toks[lhsStart].pos, toks[closeIdx].end, rep), true + } + } + return src, false +} + +// tokenInfo records one scanned token's byte range and kind. lit is +// retained for IDENT tokens so we can splice the field name into the +// rewrite without re-reading the source. +type tokenInfo struct { + pos int + end int + kind token.Token + lit string +} + +// scanTokens runs go/scanner over src and returns every significant +// token. Comments and auto-inserted semicolons are filtered so +// "previous token" tracking sees only meaningful tokens. +func scanTokens(src string) []tokenInfo { + fs := token.NewFileSet() + file := fs.AddFile("", fs.Base(), len(src)) + var s scanner.Scanner + s.Init(file, []byte(src), nil, scanner.ScanComments) + + var out []tokenInfo + for { + pos, t, lit := s.Scan() + if t == token.EOF { + break + } + if t == token.COMMENT || t == token.SEMICOLON { + continue + } + off := file.Offset(pos) + out = append(out, tokenInfo{ + pos: off, + end: off + tokLen(t, lit), + kind: t, + lit: lit, + }) + } + return out +} + +func tokLen(t token.Token, lit string) int { + if lit != "" { + return len(lit) + } + return len(t.String()) +} + +// isQuestion reports whether t is a single `?` byte. The scanner +// flags `?` as ILLEGAL but other bytes (e.g. `@`, `#`) end up there +// too, so the byte check is needed to avoid rewriting around them. +func isQuestion(src string, t tokenInfo) bool { + return t.kind == token.ILLEGAL && t.end-t.pos == 1 && src[t.pos] == '?' +} + +// matchBracketForward finds the index of the `]` that closes the `[` +// at lbrack. Returns -1 if no matching close exists. +func matchBracketForward(toks []tokenInfo, lbrack int) int { + depth := 1 + for j := lbrack + 1; j < len(toks); j++ { + switch toks[j].kind { + case token.LBRACK: + depth++ + case token.RBRACK: + depth-- + if depth == 0 { + return j + } + } + } + return -1 +} + +// lhsStartIdx returns the token index where the LHS of the `?` at +// qIdx starts. The walk extends backwards through identifiers, +// selector chains (`.IDENT`), balanced index brackets, balanced +// parens (call args or paren groups), and previous `?` tokens. +// +// Returns -1 if the LHS would extend past the start of the input +// without ever closing — that is, when the source is unbalanced or +// the `?` has nothing to the left of it. +func lhsStartIdx(toks []tokenInfo, src string, qIdx int) int { + if qIdx == 0 { + return -1 + } + i := qIdx - 1 + for i >= 0 { + t := toks[i] + switch t.kind { + case token.IDENT, token.INT, token.FLOAT, token.STRING, token.CHAR: + if i-1 >= 0 && toks[i-1].kind == token.PERIOD { + i -= 2 + continue + } + return i + case token.RBRACK: + j := matchBracketBack(toks, i) + if j < 0 { + return -1 + } + i = j - 1 + continue + case token.RPAREN: + j := matchParenBack(toks, i) + if j < 0 { + return -1 + } + prev := j - 1 + if prev >= 0 && extendsPrimary(toks[prev].kind) { + i = prev + continue + } + return j + case token.ILLEGAL: + if isQuestion(src, t) { + i-- + continue + } + return i + 1 + default: + return i + 1 + } + } + // Walking off the start of the stream means the LHS would + // extend past the beginning of the input — i.e., it never + // resolved to a complete primary. Decline the rewrite and let + // the parser produce its normal error on the `?` token. + return -1 +} + +// matchBracketBack returns the index of the `[` that opens the `]` +// at rbrack. Negative on imbalance. +func matchBracketBack(toks []tokenInfo, rbrack int) int { + depth := 1 + for j := rbrack - 1; j >= 0; j-- { + switch toks[j].kind { + case token.RBRACK: + depth++ + case token.LBRACK: + depth-- + if depth == 0 { + return j + } + } + } + return -1 +} + +// matchParenBack returns the index of the `(` that opens the `)` +// at rparen. Negative on imbalance. +func matchParenBack(toks []tokenInfo, rparen int) int { + depth := 1 + for j := rparen - 1; j >= 0; j-- { + switch toks[j].kind { + case token.RPAREN: + depth++ + case token.LPAREN: + depth-- + if depth == 0 { + return j + } + } + } + return -1 +} + +// extendsPrimary reports whether t can directly precede a `(` that +// is part of a call expression. A `(` after one of these tokens is +// always a call; after anything else it begins a paren group. +func extendsPrimary(t token.Token) bool { + switch t { + case token.IDENT, token.RBRACK, token.RPAREN: + return true + } + return false +} + +// quoteString returns a Go double-quoted string literal whose +// decoded value is s. Field names from the scanner are valid Go +// identifiers, so the body never contains characters that need +// escaping. The simple `"` wrapping is therefore correct without +// invoking strconv.Quote. +func quoteString(s string) string { + var b strings.Builder + b.Grow(len(s) + 2) + b.WriteByte('"') + b.WriteString(s) + b.WriteByte('"') + return b.String() +} + +// splice replaces src[pos:end] with str. +func splice(src string, pos, end int, str string) string { + var b strings.Builder + b.Grow(len(src) + len(str)) + b.WriteString(src[:pos]) + b.WriteString(str) + b.WriteString(src[end:]) + return b.String() +} diff --git a/internal/optaccess/optaccess_test.go b/internal/optaccess/optaccess_test.go new file mode 100644 index 0000000..50f6278 --- /dev/null +++ b/internal/optaccess/optaccess_test.go @@ -0,0 +1,154 @@ +package optaccess + +import ( + "strings" + "testing" +) + +func TestRewrite(t *testing.T) { + cases := []struct { + in string + want string + }{ + // no `?` at all: untouched + {`a + b`, `a + b`}, + {`f(x, y)`, `f(x, y)`}, + {`xs[0]`, `xs[0]`}, + + // simplest selector + {`obj?.field`, `__try_select__(obj, "field")`}, + // LHS is a selector chain + {`a.b.c?.d`, `__try_select__(a.b.c, "d")`}, + // LHS is an index + {`xs[0]?.name`, `__try_select__(xs[0], "name")`}, + // LHS is a paren group + {`(a + b)?.c`, `__try_select__((a + b), "c")`}, + // LHS is a call + {`find(xs, it.id == 1)?.name`, `__try_select__(find(xs, it.id == 1), "name")`}, + // chained `?.` + {`a?.b?.c`, `__try_select__(__try_select__(a, "b"), "c")`}, + // chained `?.` deep + {`a?.b?.c?.d`, `__try_select__(__try_select__(__try_select__(a, "b"), "c"), "d")`}, + + // optional index + {`xs?[0]`, `__try_index__(xs, 0)`}, + {`obj?["key"]`, `__try_index__(obj, "key")`}, + // optional index with expression + {`obj?[i + 1]`, `__try_index__(obj, i + 1)`}, + // chained `?.` then `?[` + {`a?.b?[0]`, `__try_index__(__try_select__(a, "b"), 0)`}, + // `?[` then `?.` + {`xs?[0]?.name`, `__try_select__(__try_index__(xs, 0), "name")`}, + + // composes with operator-returning || + {`user?.nickname || "(none)"`, `__try_select__(user, "nickname") || "(none)"`}, + + // inside higher-order predicate + {`map(users, it?.name)`, `map(users, __try_select__(it, "name"))`}, + + // `?` not followed by `.` or `[` is left alone + {`a ? b : c`, `a ? b : c`}, + // `?` at start of source: nothing to rewrite + {`?.x`, `?.x`}, + } + for _, tc := range cases { + t.Run(tc.in, func(t *testing.T) { + got := Rewrite(tc.in) + if got != tc.want { + t.Fatalf("Rewrite(%q)\n got %q\n want %q", tc.in, got, tc.want) + } + }) + } +} + +// `?.` and `?[` written inside a string literal must be preserved +// verbatim — string contents are scanner tokens we never look inside. +func TestRewrite_StringLiteralUntouched(t *testing.T) { + cases := []string{ + `"obj?.field"`, + `"a?[0]"`, + `'?'`, + "`obj?.field`", + `f("obj?.field")`, + `a + "?.b"`, + } + for _, src := range cases { + t.Run(src, func(t *testing.T) { + got := Rewrite(src) + if got != src { + t.Fatalf("Rewrite(%q) modified a string literal\n got %q", src, got) + } + }) + } +} + +// `?.` and `?[` written inside a comment must be preserved verbatim. +func TestRewrite_CommentUntouched(t *testing.T) { + src := `a /* obj?.field */ + b` + got := Rewrite(src) + if got != src { + t.Fatalf("Rewrite altered comment content\n got %q", got) + } + src2 := "a // obj?.field\n + b" + got2 := Rewrite(src2) + if got2 != src2 { + t.Fatalf("Rewrite altered line-comment content\n got %q", got2) + } +} + +// Real `?.` outside a string is still rewritten when the same source +// also contains `?.` inside a string. +func TestRewrite_MixedStringAndOperator(t *testing.T) { + src := `obj?.field + "obj?.field"` + want := `__try_select__(obj, "field") + "obj?.field"` + got := Rewrite(src) + if got != want { + t.Fatalf("got %q\nwant %q", got, want) + } +} + +// FuzzRewrite checks that the rewriter never panics and produces an +// output that no longer contains `?.` or `?[` outside strings, +// comments, or other unrewritable contexts. The latter half is +// approximate, so we only assert the no-panic guarantee. +func FuzzRewrite(f *testing.F) { + seeds := []string{ + ``, + `a`, + `a?.b`, + `a?[0]`, + `a?.b?.c`, + `(a + b)?.c`, + `xs?[0]?.name`, + `f("?.")`, + `a // ?.b`, + `a /* ?.b */`, + `?.x`, + `a ? b`, + `?[`, + `a?.`, + `a?[`, + } + for _, s := range seeds { + f.Add(s) + } + f.Fuzz(func(t *testing.T, s string) { + // Bound the input so a fuzzer-found 1 GiB string doesn't OOM. + if len(s) > 8192 { + return + } + // Reject strings the scanner can't process at all. + defer func() { + if r := recover(); r != nil { + t.Fatalf("panic on %q: %v", s, r) + } + }() + out := Rewrite(s) + // Output should be a string of bounded growth: each `?` byte + // expands to at most ~30 bytes of replacement. + if len(out) > 64*len(s)+128 { + t.Fatalf("Rewrite blew up size: in %d, out %d", len(s), len(out)) + } + _ = strings.Contains(out, "?") + }) +} diff --git a/llms.txt b/llms.txt index 2dd6f47..4207c94 100644 --- a/llms.txt +++ b/llms.txt @@ -142,9 +142,9 @@ The `default` expression is lazy: it runs only when the primary fails. `context.DeadlineExceeded`, or anything wrapping `ErrCompile`. ``` -try(int(s), 0) // parse with fallback -try(find(events, it.kind == "purchase").user, "—") // optional chain -try(user.nickname, nil) || "(none)" // present nil +try(int(s), 0) // parse with fallback +try(find(events, it.kind == "purchase")?.user, "—") // optional chain +try(user.nickname, nil) || "(none)" // present nil ``` Names can be shadowed by `WithFunctions` or an env entry of the same @@ -152,6 +152,23 @@ name. The literal token `map` is rewritten to an internal sentinel before parsing (Go treats `map` as a keyword), then translated back for error messages. +## Optional access (`?.` and `?[`) + +`obj?.field` and `obj?[idx]` are pre-parse rewrites that turn a +"missing-key" or "out-of-range" lookup into `nil` instead of an +error. nil receivers also resolve to nil. Wrong-kind type errors +(selecting on a non-struct, indexing with a non-integer, ...) still +surface, so real bugs are not hidden. + +``` +user?.profile?.nickname || "(none)" +events?[0]?.user +config?.feature?.enabled +``` + +The rewrite is token-level and skips strings, runes, and comments, +so `"obj?.field"` and `// obj?.field` survive untouched. + ## Custom functions ```go diff --git a/optional_access_test.go b/optional_access_test.go new file mode 100644 index 0000000..e030e01 --- /dev/null +++ b/optional_access_test.go @@ -0,0 +1,167 @@ +package expr + +import ( + "testing" + + "github.com/deepnoodle-ai/expr/internal/require" +) + +// --- ?. (optional selector) --- + +func TestOptionalSelect_PresentField(t *testing.T) { + env := map[string]any{ + "user": map[string]any{"nickname": "ada"}, + } + got, err := evalExpr(t.Context(), `user?.nickname`, env) + require.NoError(t, err) + require.Equal(t, "ada", got) +} + +func TestOptionalSelect_MissingKeyReturnsNil(t *testing.T) { + env := map[string]any{ + "user": map[string]any{}, + } + got, err := evalExpr(t.Context(), `user?.nickname`, env) + require.NoError(t, err) + require.Equal(t, nil, got) +} + +func TestOptionalSelect_NilReceiverReturnsNil(t *testing.T) { + env := map[string]any{"user": nil} + got, err := evalExpr(t.Context(), `user?.nickname`, env) + require.NoError(t, err) + require.Equal(t, nil, got) +} + +func TestOptionalSelect_NilPointerReturnsNil(t *testing.T) { + type profile struct{ Name string } + var p *profile + env := map[string]any{"profile": p} + got, err := evalExpr(t.Context(), `profile?.Name`, env) + require.NoError(t, err) + require.Equal(t, nil, got) +} + +func TestOptionalSelect_StructPresent(t *testing.T) { + type profile struct{ Name string } + env := map[string]any{"profile": &profile{Name: "Ada"}} + got, err := evalExpr(t.Context(), `profile?.Name`, env) + require.NoError(t, err) + require.Equal(t, "Ada", got) +} + +// Chained optional selects collapse to nil at the first missing +// link, so the caller does not have to guard each level. +func TestOptionalSelect_Chained(t *testing.T) { + env := map[string]any{ + "user": map[string]any{}, + } + got, err := evalExpr(t.Context(), `user?.profile?.nickname`, env) + require.NoError(t, err) + require.Equal(t, nil, got) +} + +// Composes with operand-returning || to provide a fallback for the +// missing case. +func TestOptionalSelect_WithFallback(t *testing.T) { + env := map[string]any{ + "user": map[string]any{}, + } + got, err := evalExpr(t.Context(), `user?.nickname || "(none)"`, env) + require.NoError(t, err) + require.Equal(t, "(none)", got) +} + +// `?.` only swallows the missing-key case. A real type error (e.g., +// trying to select on a value that isn't a struct or map) still +// surfaces so genuine bugs are not hidden. +func TestOptionalSelect_TypeErrorPropagates(t *testing.T) { + env := map[string]any{"x": 42} + _, err := evalExpr(t.Context(), `x?.field`, env) + require.ErrorIs(t, err, ErrEvaluate) +} + +// --- ?[ (optional index) --- + +func TestOptionalIndex_PresentSlice(t *testing.T) { + env := map[string]any{"xs": []any{int64(10), int64(20), int64(30)}} + got, err := evalExpr(t.Context(), `xs?[1]`, env) + require.NoError(t, err) + require.Equal(t, int64(20), got) +} + +func TestOptionalIndex_OutOfRangeReturnsNil(t *testing.T) { + env := map[string]any{"xs": []any{int64(10)}} + got, err := evalExpr(t.Context(), `xs?[5]`, env) + require.NoError(t, err) + require.Equal(t, nil, got) +} + +func TestOptionalIndex_NilReceiverReturnsNil(t *testing.T) { + env := map[string]any{"xs": nil} + got, err := evalExpr(t.Context(), `xs?[0]`, env) + require.NoError(t, err) + require.Equal(t, nil, got) +} + +func TestOptionalIndex_MissingMapKeyReturnsNil(t *testing.T) { + env := map[string]any{"obj": map[string]any{"a": 1}} + got, err := evalExpr(t.Context(), `obj?["missing"]`, env) + require.NoError(t, err) + require.Equal(t, nil, got) +} + +func TestOptionalIndex_TypeErrorPropagates(t *testing.T) { + env := map[string]any{"xs": []any{int64(1)}} + // Indexing a slice with a string is a wrong-kind error, not a + // "missing index". It must propagate. + _, err := evalExpr(t.Context(), `xs?["zero"]`, env) + require.ErrorIs(t, err, ErrEvaluate) +} + +// `?[` then `?.` and vice versa compose without surprises. +func TestOptionalIndex_ChainedWithSelect(t *testing.T) { + env := map[string]any{ + "users": []any{ + map[string]any{"name": "Ada"}, + map[string]any{}, + }, + } + cases := []struct { + expr string + want any + }{ + {`users?[0]?.name`, "Ada"}, + {`users?[1]?.name`, nil}, + {`users?[5]?.name`, nil}, + } + for _, tc := range cases { + t.Run(tc.expr, func(t *testing.T) { + got, err := evalExpr(t.Context(), tc.expr, env) + require.NoError(t, err) + require.Equal(t, tc.want, got) + }) + } +} + +// `?.` inside a higher-order predicate is the natural shape for +// "first user with a nickname" and similar shapes. +func TestOptionalSelect_InPredicate(t *testing.T) { + env := map[string]any{ + "users": []any{ + map[string]any{}, + map[string]any{"nickname": "Ada"}, + }, + } + got, err := evalExpr(t.Context(), `find(users, it?.nickname)`, env) + require.NoError(t, err) + require.Equal(t, map[string]any{"nickname": "Ada"}, got) +} + +// `?.` and `?[` written inside string literals must be preserved +// verbatim: the rewrite must skip string content. +func TestOptionalAccess_StringLiteralUntouched(t *testing.T) { + got, err := evalExpr(t.Context(), `"obj?.field"`, nil) + require.NoError(t, err) + require.Equal(t, "obj?.field", got) +}