feat: Added diagnostics to first and last.

This commit is contained in:
Spencer Brower
2026-08-07 11:33:23 -04:00
parent a7d471fa11
commit f0800b8e13
5 changed files with 120 additions and 20 deletions
+36
View File
@@ -229,3 +229,39 @@ expected: |
Show where the error was triggered. Should also show the format that will be used Show where the error was triggered. Should also show the format that will be used
actual: | actual: |
[ERROR] --- [333:apply_format()] format pipe used but no date format configured (set date.format in thor.json) Default will be used [ERROR] --- [333:apply_format()] format pipe used but no date format configured (set date.format in thor.json) Default will be used
---
name: first/last with zero argument
input: |
<ul>{{#items | first 0}}<li>{{.}}</li>{{/items}}</ul>
expected: Error with source location — "argument must not be zero"
actual: |
[ERROR] --- [render_template()] argument must not be zero
--> <file>:<line>:<col>
|
N | <ul>{{#items | first 0}}<li>{{.}}</li>{{/items}}</ul>
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
---
name: first/last with negative argument
input: |
<ul>{{#items | first -3}}<li>{{.}}</li>{{/items}}</ul>
expected: Warning with source location — "first -3 is equivalent to last 3 — consider using the clearer form"
actual: |
[WARN ] --- [take()] first -3 is equivalent to last 3
--> <file>:<line>:<col>
|
N | <ul>{{#items | first -3}}<li>{{.}}</li>{{/items}}</ul>
| ^^^^^^^^^^^^^^^^^^^^^^^^^ consider using the clearer form
|
---
name: first/last on string with no argument
input: |
{{word | first}}
expected: Warning with source location — "defaulting to n=1 for string; consider specifying n explicitly"
actual: |
[WARN ] --- [take()] defaulting to n=1 for string
--> <file>:<line>:<col>
|
N | {{word | first}}
| ^^^^^^^^^^^^^^^ consider specifying n explicitly
|
+12 -5
View File
@@ -17,12 +17,19 @@ Remember to polish existing features before moving on to new ones.
- current: `layouts/partials` - current: `layouts/partials`
- alt1: `layouts`, `partials` - alt1: `layouts`, `partials`
- alt2: `templates/layouts`, `templates/partials` - alt2: `templates/layouts`, `templates/partials`
- [ ] add `sort_by` pipe
- [ ] pipes - [ ] pipes
- [ ] first/last - [x] first/last
- [ ] warn when using negative numbers - [ ] write test for error when n == 0
- [ ] warn when using default arg on string - [x] warn when using negative numbers
- [ ] add the above warnings to DIAGNOSTIC_TODOS.md - [x] warn when using default arg on string
- [ ] sort_by - [x] add the above warnings to DIAGNOSTIC_TODOS.md
- [ ] Instead of "clearer form", be exact: "last 5"
- [ ] write tests for `first 0` & `last 0`
- [ ] update tests to assert diagnostics when appropriate
- do we want to do this or just use DIAGNOSTIC_TODOS.md as the tests?
- [ ] also update to use file logger so they don't clog up the output
- [ ] narrow highlight in diagnostic to relevent part of line.
## Documentation ## Documentation
+4 -4
View File
@@ -621,7 +621,7 @@ render_nodes :: proc(
warn_unknown_key(current, ctx[:], node) warn_unknown_key(current, ctx[:], node)
} }
if len(node.filters) > 0 { if len(node.filters) > 0 {
transformed, perr := apply_pipeline(val, node.filters[:], node.pos, ctx[:]) transformed, perr := apply_pipeline(val, node.filters[:], node.pos, ctx[:], current)
if perr != nil { if perr != nil {
return tag_error(perr, current) return tag_error(perr, current)
} }
@@ -640,7 +640,7 @@ render_nodes :: proc(
warn_unknown_key(current, ctx[:], node) warn_unknown_key(current, ctx[:], node)
} }
if len(node.filters) > 0 { if len(node.filters) > 0 {
transformed, perr := apply_pipeline(val, node.filters[:], node.pos, ctx[:]) transformed, perr := apply_pipeline(val, node.filters[:], node.pos, ctx[:], current)
if perr != nil { if perr != nil {
return tag_error(perr, current) return tag_error(perr, current)
} }
@@ -655,7 +655,7 @@ render_nodes :: proc(
warn_unknown_key(current, ctx[:], node) warn_unknown_key(current, ctx[:], node)
} }
if len(node.filters) > 0 { if len(node.filters) > 0 {
transformed, perr := apply_pipeline(val, node.filters[:], node.pos, ctx[:]) transformed, perr := apply_pipeline(val, node.filters[:], node.pos, ctx[:], current)
if perr != nil { if perr != nil {
return tag_error(perr, current) return tag_error(perr, current)
} }
@@ -701,7 +701,7 @@ render_nodes :: proc(
warn_unknown_key(current, ctx[:], node) warn_unknown_key(current, ctx[:], node)
} }
if len(node.filters) > 0 { if len(node.filters) > 0 {
transformed, perr := apply_pipeline(val, node.filters[:], node.pos, ctx[:]) transformed, perr := apply_pipeline(val, node.filters[:], node.pos, ctx[:], current)
if perr != nil { if perr != nil {
return tag_error(perr, current) return tag_error(perr, current)
} }
+66 -9
View File
@@ -220,13 +220,14 @@ apply_pipeline :: proc(
filters: []Pipe_Filter, filters: []Pipe_Filter,
pos: int, pos: int,
ctx: []any, ctx: []any,
tmpl: Template,
) -> ( ) -> (
current: any, current: any,
err: Error, err: Error,
) { ) {
current = value current = value
for &filter in filters { for &filter in filters {
current = apply_filter(current, &filter, pos, ctx) or_return current = apply_filter(current, &filter, pos, ctx, tmpl) or_return
log.debugf("applied: filter=%v before=%s after=%s pos=%d", filter, value, current, pos) log.debugf("applied: filter=%v before=%s after=%s pos=%d", filter, value, current, pos)
} }
return return
@@ -256,7 +257,16 @@ resolve_format_string :: proc(name: string, ctx: []any, pos: int) -> (string, Er
return str, nil return str, nil
} }
apply_filter :: proc(value: any, filter: ^Pipe_Filter, pos: int, ctx: []any) -> (any, Error) { apply_filter :: proc(
value: any,
filter: ^Pipe_Filter,
pos: int,
ctx: []any,
tmpl: Template,
) -> (
any,
Error,
) {
op, ok := pipe_op_from_string(filter.op) op, ok := pipe_op_from_string(filter.op)
if !ok { if !ok {
hint := "" hint := ""
@@ -315,9 +325,9 @@ apply_filter :: proc(value: any, filter: ^Pipe_Filter, pos: int, ctx: []any) ->
return any{new_clone(str2, context.temp_allocator), typeid_of(string)}, nil return any{new_clone(str2, context.temp_allocator), typeid_of(string)}, nil
} }
case .First: case .First:
return take(value, filter.args[:], pos, from_start = true) return take(value, filter.args[:], true, tmpl, pos, filter.op_pos)
case .Last: case .Last:
return take(value, filter.args[:], pos, from_start = false) return take(value, filter.args[:], false, tmpl, pos, filter.op_pos)
} }
return {}, nil return {}, nil
} }
@@ -414,7 +424,17 @@ apply_group_by :: proc(value: any, args: []string, pos: int) -> (result: any, er
return groups, nil return groups, nil
} }
take :: proc(value: any, args: []string, pos: int, from_start: bool) -> (any, Error) { take :: proc(
value: any,
args: []string,
from_start: bool,
tmpl: Template,
tag_pos: int,
op_pos: int,
) -> (
any,
Error,
) {
elem_info, count, data := list_info(value) elem_info, count, data := list_info(value)
is_list := elem_info != nil is_list := elem_info != nil
from_start := from_start from_start := from_start
@@ -423,7 +443,13 @@ take :: proc(value: any, args: []string, pos: int, from_start: bool) -> (any, Er
if !is_list { if !is_list {
s, is_str := reflect.as_string(value) s, is_str := reflect.as_string(value)
if !is_str { if !is_str {
return nil, Error_Body{msg = "requires a list or string", pos = pos, kind = .Data} return nil, Error_Body {
msg = "requires a list or string",
pos = tag_pos,
kind = .Data,
source = tmpl.source,
path = tmpl.path,
}
} }
str = s str = s
count = len(str) count = len(str)
@@ -431,23 +457,54 @@ take :: proc(value: any, args: []string, pos: int, from_start: bool) -> (any, Er
n := 1 n := 1
if !is_list && len(args) == 0 { if !is_list && len(args) == 0 {
return nil, Error_Body{msg = "requires an argument for strings", pos = pos, kind = .Data} path := tmpl.path != "" ? tmpl.path : "<input>"
diag := format_tag_error(
path,
tmpl.source,
op_pos,
"defaulting to n=1 for string",
"consider specifying n explicitly",
)
log.warnf("%s", diag)
} }
if len(args) > 0 { if len(args) > 0 {
parsed, ok := strconv.parse_int(args[0]) parsed, ok := strconv.parse_int(args[0])
if !ok { if !ok {
return nil, Error_Body { return nil, Error_Body {
msg = fmt.tprintf("invalid argument '%s'", args[0]), msg = fmt.tprintf("invalid argument '%s'", args[0]),
pos = pos, pos = tag_pos,
kind = .Data, kind = .Data,
source = tmpl.source,
path = tmpl.path,
} }
} }
n = parsed n = parsed
} }
if n == 0 {
return nil, Error_Body {
msg = "argument must not be zero",
pos = tag_pos,
kind = .Data,
source = tmpl.source,
path = tmpl.path,
}
}
if n < 0 { if n < 0 {
op_name := from_start ? "first" : "last"
alt_name := from_start ? "last" : "first"
path := tmpl.path != "" ? tmpl.path : "<input>"
diag := format_tag_error(
path,
tmpl.source,
op_pos,
fmt.tprintf("%s %d is equivalent to %s %d", op_name, n, alt_name, 0 - n),
"consider using the clearer form",
)
log.warnf("%s", diag)
from_start = !from_start from_start = !from_start
n = 0 - n // abs(n) n = 0 - n
} }
start: int start: int
+2 -2
View File
@@ -572,7 +572,7 @@ test_unknown_pipe_op_suggestion :: proc(t: ^testing.T) {
op = "formats", op = "formats",
op_pos = 0, op_pos = 0,
} }
_, err := apply_filter("2026-01-15", &filter, 0, nil) _, err := apply_filter("2026-01-15", &filter, 0, nil, {})
testing.expect(t, err != nil, "should error on unknown op") testing.expect(t, err != nil, "should error on unknown op")
b := body(err) b := body(err)
testing.expect(t, strings.contains(b.hint, "format"), "hint should suggest 'format'") testing.expect(t, strings.contains(b.hint, "format"), "hint should suggest 'format'")
@@ -585,7 +585,7 @@ test_unknown_pipe_op_no_suggestion :: proc(t: ^testing.T) {
op = "xyz", op = "xyz",
op_pos = 0, op_pos = 0,
} }
_, err := apply_filter("2026-01-15", &filter, 0, nil) _, err := apply_filter("2026-01-15", &filter, 0, nil, {})
testing.expect(t, err != nil, "should error on unknown op") testing.expect(t, err != nil, "should error on unknown op")
b := body(err) b := body(err)
testing.expect(t, b.hint == "", "no suggestion expected for 'xyz'") testing.expect(t, b.hint == "", "no suggestion expected for 'xyz'")