From 6d7b88a8d62d3a8b6d974cb80ce22b1edba08301 Mon Sep 17 00:00:00 2001 From: Spencer Brower <6729162+sbrow@users.noreply.github.com> Date: Tue, 28 Jul 2026 15:41:44 -0400 Subject: [PATCH] feat: Warns the user if they exceed 16 context frames. --- TODOS.md | 18 +- mustache/mustache.odin | 319 ++++++++++++++++++++---------------- mustache/mustache_test.odin | 38 +++++ mustache/suggest.odin | 5 +- mustache/suggest_test.odin | 12 +- 5 files changed, 240 insertions(+), 152 deletions(-) diff --git a/TODOS.md b/TODOS.md index 0950d08..1b098af 100644 --- a/TODOS.md +++ b/TODOS.md @@ -2,18 +2,25 @@ - Polish existing features before moving on to new ones. - [ ] Improve diagnostics + - [ ] show "stack traces" in template error diagnostics + - [ ] better diagnostics for syntax errors in treesitter. + - [ ] Ensure diagnostics for MAX_CONTEXT_DEPTH are good. + - [ ] show a proper diagnostic for timezones + - currently "unable to load timezone 'America/New_Yorkskie'" + - want rust style diagnostic and better message, maybe "unknown timezone 'America/New_Yorkskie'" - [x] Simplify / unify template context stack. Come up with a name for it. - [x] `render_template` should accept `Template_Context`, not `any` - [ ] Load grammars dynamically -- [ ] consider adding a limit to the context stack in mustache. -- [ ] better diagnostics for syntax errors in treesitter. +- [x] consider adding a limit to the context stack in mustache. - [x] Add heading ids as a default on extension. -- [ ] show "stack traces" in template error diagnostics - [ ] starred must be a param. -- [ ] Add a `#config(MAX_CONTEXT_DEPTH, 16?)` to `mustache`. +- [x] Add a `#config(MAX_CONTEXT_DEPTH, 16?)` to `mustache`. +- [ ] Documentation + - [ ] talk about the context stack (and its limit). - [ ] menu system - [ ] like Hugo's, but warn(/fail?) if menus are defined in the config *and* pages. - i.e. force the user to choose one or the other. +- [ ] Don't show annoying log output in tests. ## Performance @@ -69,9 +76,6 @@ - [ ] display an error when no part of the date appears in the output. - [ ] Handle 0 and whitespace padding i.e. "_2" -> " 2" - [ ] Do we *need* mustache.Date_Components, or can we use core:time/datetime.DateTime? -- [ ] show a proper diagnostic for timezones - - currently "unable to load timezone 'America/New_Yorkskie'" - - want rust style diagnostic and better message, maybe "unknown timezone 'America/New_Yorkskie'" ## General - [ ] get rid of the global variables in the `treesitter` package. diff --git a/mustache/mustache.odin b/mustache/mustache.odin index 93930e2..1435594 100644 --- a/mustache/mustache.odin +++ b/mustache/mustache.odin @@ -3,16 +3,26 @@ package mustache import "base:runtime" import "core:fmt" import "core:log" -import "core:reflect" import "core:strings" +// A hypothetical maximum context depth. Trying to pass more than this many items +// to render(tpl, data), or nesting templates further than this depth would be +// an error. +// May be enforced in a later version (for performance) +MAX_CONTEXT_DEPTH :: #config(MAX_CONTEXT_DEPTH, 16) + +// Context_Stack is the growable stack of data frames walked top-to-bottom by +// resolve_name. Frames are pushed on section descent and popped on exit; the +// root data (or each element of a root []any) forms the base frames. +Context_Stack :: [dynamic]any + // --------------------------------------------------------------------------- // Error types // --------------------------------------------------------------------------- Error_Kind :: enum { Syntax, // parse-time: malformed template - Data, // render-time: template fine, data wrong (e.g. filter misuse) + Data, // render-time: template fine, data wrong (e.g. filter misuse) } Error_Body :: struct { @@ -22,14 +32,18 @@ Error_Body :: struct { } // Error is nil when no error occurred. -Error :: union { Error_Body } +Error :: union { + Error_Body, +} // body unwraps the Error_Body from a non-nil Error. // Precondition: err != nil. body :: proc(err: Error) -> Error_Body { switch e in err { - case Error_Body: return e - case: return {} + case Error_Body: + return e + case: + return {} } } @@ -143,11 +157,9 @@ render :: proc( err: Error, ) { builder: strings.Builder - strings.builder_init(&builder, allocator) - defer strings.builder_destroy(&builder) + strings.builder_init(&builder, context.temp_allocator) - ctx := make([dynamic]any, 0, 4, allocator) - defer delete(ctx) + ctx := make(Context_Stack, 0, 4, context.temp_allocator) // If data is a []any, expand into individual context frames. // Otherwise, push as a single frame. @@ -287,14 +299,14 @@ parse_section :: proc( case .Section_Close: if strings.contains(tok.value, "|") { - return Error_Body { - msg = fmt.tprintf( - "pipe expression not allowed in close tag '{{{{/%s}}}}' — use the bare key", - tok.value, - ), - pos = tok.pos, - kind = .Syntax, - } + return Error_Body { + msg = fmt.tprintf( + "pipe expression not allowed in close tag '{{{{/%s}}}}' — use the bare key", + tok.value, + ), + pos = tok.pos, + kind = .Syntax, + } } if end_tag != "" && tok.value == end_tag { pos^ += 1 @@ -331,12 +343,7 @@ parse_section :: proc( idx := len(nodes) append( nodes, - Node { - kind = .Parent, - key = tok.value, - indent = tok.indent, - pos = tok.pos, - }, + Node{kind = .Parent, key = tok.value, indent = tok.indent, pos = tok.pos}, ) parse_section(tokens, pos, nodes, tok.value, source, allocator, tok.pos) or_return nodes[idx].children = nodes[idx + 1:len(nodes)] @@ -344,15 +351,7 @@ parse_section :: proc( case .Block_Open: pos^ += 1 idx := len(nodes) - append( - nodes, - Node { - kind = .Block, - key = tok.value, - indent = tok.indent, - pos = tok.pos, - }, - ) + append(nodes, Node{kind = .Block, key = tok.value, indent = tok.indent, pos = tok.pos}) parse_section(tokens, pos, nodes, tok.value, source, allocator, tok.pos) or_return nodes[idx].children = nodes[idx + 1:len(nodes)] } @@ -499,21 +498,29 @@ remove_line_indent :: proc(s: string, indent: string, allocator := context.alloc render_template :: proc( pt: Template, - ctx: ^[dynamic]any, + ctx: ^Context_Stack, partials: map[string]Template, b: ^strings.Builder, blocks: map[string]Block_Override, indent: string, ) -> Error { if len(indent) > 0 { - state := Indent_State{indent = indent, at_line_start = false} + state := Indent_State { + indent = indent, + at_line_start = false, + } strings.write_string(b, indent) // first line always gets indent return render_nodes(pt, pt.nodes[:], ctx, partials, b, blocks, &state) } return render_nodes(pt, pt.nodes[:], ctx, partials, b, blocks, nil) } -write_indented :: proc(b: ^strings.Builder, indent: string, content: string, at_line_start: ^bool) { +write_indented :: proc( + b: ^strings.Builder, + indent: string, + content: string, + at_line_start: ^bool, +) { if len(indent) == 0 || len(content) == 0 { strings.write_string(b, content) return @@ -544,7 +551,7 @@ write_indented :: proc(b: ^strings.Builder, indent: string, content: string, at_ render_nodes :: proc( current: Template, nodes: []Node, - ctx: ^[dynamic]any, + ctx: ^Context_Stack, partials: map[string]Template, b: ^strings.Builder, blocks: map[string]Block_Override = nil, @@ -588,16 +595,16 @@ render_nodes :: proc( if perr == nil { temp: strings.Builder strings.builder_init(&temp, context.temp_allocator) - render_nodes( - sub_tpl, - sub_tpl.nodes[:], - ctx, - partials, - &temp, - blocks, - nil, - ) or_return - write_value(b, strings.to_string(temp), escape = true) + render_nodes( + sub_tpl, + sub_tpl.nodes[:], + ctx, + partials, + &temp, + blocks, + nil, + ) or_return + write_value(b, strings.to_string(temp), escape = true) } } else { write_value(b, val, escape = true) @@ -630,16 +637,16 @@ render_nodes :: proc( if perr == nil { temp: strings.Builder strings.builder_init(&temp, context.temp_allocator) - render_nodes( - sub_tpl, - sub_tpl.nodes[:], - ctx, - partials, - &temp, - blocks, - nil, - ) or_return - write_value(b, strings.to_string(temp), escape = false) + render_nodes( + sub_tpl, + sub_tpl.nodes[:], + ctx, + partials, + &temp, + blocks, + nil, + ) or_return + write_value(b, strings.to_string(temp), escape = false) } } else { write_value(b, val, escape = false) @@ -666,23 +673,36 @@ render_nodes :: proc( context.temp_allocator, ) if perr == nil { - render_nodes( - sub_tpl, - sub_tpl.nodes[:], - ctx, - partials, - b, - blocks, - nil, - ) or_return + render_nodes( + sub_tpl, + sub_tpl.nodes[:], + ctx, + partials, + b, + blocks, + nil, + ) or_return } - } else if is_truthy(val) { - children := node.children - elem_info, count, data := list_info(val) - if elem_info != nil { - for j in 0 ..< count { - elem := extract_list_element(elem_info, data, j) - append(ctx, elem) + } else if is_truthy(val) { + children := node.children + elem_info, count, data := list_info(val) + if elem_info != nil { + for j in 0 ..< count { + elem := extract_list_element(elem_info, data, j) + context_push(ctx, elem, current, node) + defer pop(ctx) + render_nodes( + current, + children, + ctx, + partials, + b, + blocks, + indent_state, + ) or_return + } + } else { + context_push(ctx, val, current, node) defer pop(ctx) render_nodes( current, @@ -694,13 +714,8 @@ render_nodes :: proc( indent_state, ) or_return } - } else { - append(ctx, val) - defer pop(ctx) - render_nodes(current, children, ctx, partials, b, blocks, indent_state) or_return } - } - i += 1 + len(node.children) + i += 1 + len(node.children) case .Inverted: val := resolve_name(node.key, ctx[:]) @@ -714,10 +729,18 @@ render_nodes :: proc( } val = transformed } - if !is_truthy(val) { - render_nodes(current, node.children, ctx, partials, b, blocks, indent_state) or_return - } - i += 1 + len(node.children) + if !is_truthy(val) { + render_nodes( + current, + node.children, + ctx, + partials, + b, + blocks, + indent_state, + ) or_return + } + i += 1 + len(node.children) case .Partial: name := node.key @@ -736,64 +759,64 @@ render_nodes :: proc( } i += 1 - case .Block: - content_nodes: []Node - content_blocks := blocks - render_current := current + case .Block: + content_nodes: []Node + content_blocks := blocks + render_current := current - found_override := false - if blocks != nil { - if o, ok := blocks[node.key]; ok { - content_nodes = o.nodes - found_override = true - render_current = o.source + found_override := false + if blocks != nil { + if o, ok := blocks[node.key]; ok { + content_nodes = o.nodes + found_override = true + render_current = o.source + } } - } - if !found_override { - content_nodes = node.children - } - - if len(node.indent) > 0 { - temp: strings.Builder - strings.builder_init(&temp, context.temp_allocator) - render_nodes( - render_current, - content_nodes, - ctx, - partials, - &temp, - content_blocks, - nil, - ) or_return - at_ls := true - write_indented(b, node.indent, strings.to_string(temp), &at_ls) - } else { - render_nodes( - render_current, - content_nodes, - ctx, - partials, - b, - content_blocks, - indent_state, - ) or_return - } - i += 1 + len(node.children) - - case .Parent: - parent_children := node.children - merged := merge_block_overrides(parent_children, blocks, current) - pt, found := partials[node.key] - if !found { - warn_missing_partial(current, partials, node, node.key) - } else { - warn_unmatched_block_overrides(current, pt, parent_children) - render_template(pt, ctx, partials, b, merged, node.indent) or_return - if indent_state != nil { - indent_state.at_line_start = false + if !found_override { + content_nodes = node.children } - } - i += 1 + len(node.children) + + if len(node.indent) > 0 { + temp: strings.Builder + strings.builder_init(&temp, context.temp_allocator) + render_nodes( + render_current, + content_nodes, + ctx, + partials, + &temp, + content_blocks, + nil, + ) or_return + at_ls := true + write_indented(b, node.indent, strings.to_string(temp), &at_ls) + } else { + render_nodes( + render_current, + content_nodes, + ctx, + partials, + b, + content_blocks, + indent_state, + ) or_return + } + i += 1 + len(node.children) + + case .Parent: + parent_children := node.children + merged := merge_block_overrides(parent_children, blocks, current) + pt, found := partials[node.key] + if !found { + warn_missing_partial(current, partials, node, node.key) + } else { + warn_unmatched_block_overrides(current, pt, parent_children) + render_template(pt, ctx, partials, b, merged, node.indent) or_return + if indent_state != nil { + indent_state.at_line_start = false + } + } + i += 1 + len(node.children) } } return nil @@ -937,3 +960,25 @@ warn_unmatched_block_overrides :: proc( } } +context_push :: proc(ctx: ^Context_Stack, val: any, current: Template, node: Node) { + append(ctx, val) + if len(ctx^) == MAX_CONTEXT_DEPTH + 1 { + warn_context_depth(current, node) + } +} + +// warn_context_depth emits a diagnostic warning pointing at the section tag +// whose push carried the context stack past MAX_CONTEXT_DEPTH. +warn_context_depth :: proc(current: Template, node: Node) { + msg := fmt.tprintf( + "context stack depth exceeded %d (possible recursive section/partial)", + MAX_CONTEXT_DEPTH, + ) + path := current.path + if path == "" { + path = "" + } + diag := format_error(path, current.source, node.pos, msg, "", colorize = should_colorize()) + log.warnf("%s", diag) +} + diff --git a/mustache/mustache_test.odin b/mustache/mustache_test.odin index 1e2a15e..267abdd 100644 --- a/mustache/mustache_test.odin +++ b/mustache/mustache_test.odin @@ -1,7 +1,9 @@ #+test package mustache +import "core:log" import "core:mem" +import "core:strings" import "core:testing" @(test) @@ -124,3 +126,39 @@ leak_repeated_render :: proc(t: ^testing.T) { } } +// A deeply nested map pushes the context stack past MAX_CONTEXT_DEPTH (16). +// The depth warning must be non-fatal: rendering still succeeds. +@(test) +test_context_depth_warns :: proc(t: ^testing.T) { + context.logger = log.nil_logger() + + AMT :: MAX_CONTEXT_DEPTH + 2 + + // Build AMT nested {x: {...}} levels; the innermost holds `leaf`. + data := make(map[string]any, context.temp_allocator) + data["leaf"] = "found" + for _ in 0 ..< AMT { + outer := make(map[string]any, context.temp_allocator) + outer["x"] = data + data = outer + } + + // Template: AMT nested {{#x}} sections around {{leaf}}. + src: strings.Builder + strings.builder_init(&src, context.temp_allocator) + for _ in 0 ..< AMT do strings.write_string(&src, "{{#x}}") + strings.write_string(&src, "{{leaf}}") + for _ in 0 ..< AMT do strings.write_string(&src, "{{/x}}") + template := strings.to_string(src) + + tmpl, perr := parse(template, "", context.temp_allocator) + testing.expect(t, perr == nil, "should parse") + if perr != nil { + return + } + + result, rerr := render(tmpl, data, allocator = context.temp_allocator) + testing.expect(t, rerr == nil, "depth warning must be non-fatal") + testing.expect_value(t, result, "found") +} + diff --git a/mustache/suggest.odin b/mustache/suggest.odin index 257b112..ec4fe16 100644 --- a/mustache/suggest.odin +++ b/mustache/suggest.odin @@ -153,10 +153,7 @@ suggest_correction :: proc(available: []string, missing: string) -> string { if len(available) == 0 || len(missing) == 0 { return "" } - threshold := 2 - if len(missing) > 8 { - threshold = len(missing) / 4 - } + threshold := max(2, len(missing) / 3) best: string best_dist := threshold + 1 diff --git a/mustache/suggest_test.odin b/mustache/suggest_test.odin index 635ff3a..9f90f27 100644 --- a/mustache/suggest_test.odin +++ b/mustache/suggest_test.odin @@ -10,11 +10,15 @@ Inner :: struct { bar: int, } +Page :: struct { + title: string, +} + Outer :: struct { - title: string, - page.title: string, - inner: Inner, - numbers: [3]int, + title: string, + page: Page, + inner: Inner, + numbers: [3]int, } @(test)