fix: don't ignore malformed rec: values
generateRecurringTask threw away the Atoi error, so rec:abc parsed as 0 and completing it spawned a "next" task with the same due date. now it validates and bails before writing anything. added the missing parser, smart date and recurrence tests while i was in there. closes 55effa9
diff --git a/actions.go b/actions.go
@@ -162,24 +162,32 @@ func CompleteTasks(cfg *Config, ids []int) ([]Task, error) {
}
func generateRecurringTask(t Task, now time.Time) (string, error) {
- recStr := strings.Trim(t.Recurrence, "[]+")
+ recStr := strings.Trim(t.Recurrence, "[]")
if len(recStr) == 0 {
- return "", fmt.Errorf("invalid recurrence format")
+ return "", fmt.Errorf("invalid recurrence format: %q", t.Recurrence)
}
- var amount int
- var unit string
- lastChar := string(recStr[len(recStr)-1])
- if strings.ContainsAny(lastChar, "dwmy") {
+ // a leading + means advance from the due date instead of from today
+ fromDue := strings.HasPrefix(recStr, "+")
+ recStr = strings.TrimPrefix(recStr, "+")
+ if len(recStr) == 0 {
+ return "", fmt.Errorf("invalid recurrence format: %q", t.Recurrence)
+ }
+
+ unit := "d"
+ amountStr := recStr
+ if lastChar := string(recStr[len(recStr)-1]); strings.ContainsAny(lastChar, "dwmy") {
unit = lastChar
- amount, _ = strconv.Atoi(recStr[:len(recStr)-1])
- } else {
- unit = "d"
- amount, _ = strconv.Atoi(recStr)
+ amountStr = recStr[:len(recStr)-1]
+ }
+
+ amount, err := strconv.Atoi(amountStr)
+ if err != nil || amount <= 0 {
+ return "", fmt.Errorf("invalid recurrence amount: %q", t.Recurrence)
}
baseDate := now
- if strings.Contains(t.Recurrence, "+") && t.HasDueDate {
+ if fromDue && t.HasDueDate {
baseDate = t.DueDate
}
diff --git a/actions_test.go b/actions_test.go
@@ -6,6 +6,7 @@ import (
"slices"
"strings"
"testing"
+ "time"
)
// newTestConfig writes lines to a throwaway todo file and points a config at
@@ -126,3 +127,150 @@ func TestCompleteTaskNotFound(t *testing.T) {
t.Error("CompleteTask on a missing ID returned no error")
}
}
+
+func TestCompileSmartDate(t *testing.T) {
+ now := time.Date(2026, 1, 15, 12, 0, 0, 0, time.UTC)
+
+ tests := []struct {
+ val string
+ want string
+ }{
+ {"today", "2026-01-15"},
+ {"TODAY", "2026-01-15"},
+ {"tod", "2026-01-15"},
+ {"tomorrow", "2026-01-16"},
+ {"tom", "2026-01-16"},
+ {"+3d", "2026-01-18"},
+ {"+2w", "2026-01-29"},
+ {"+1m", "2026-02-15"},
+ {"+1y", "2027-01-15"},
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.val, func(t *testing.T) {
+ got, err := compileSmartDate(tt.val, now)
+ if err != nil {
+ t.Fatalf("compileSmartDate(%q): %v", tt.val, err)
+ }
+ if got != tt.want {
+ t.Errorf("compileSmartDate(%q) = %q, want %q", tt.val, got, tt.want)
+ }
+ })
+ }
+}
+
+func TestCompileSmartDateRejectsGarbage(t *testing.T) {
+ now := time.Date(2026, 1, 15, 12, 0, 0, 0, time.UTC)
+
+ for _, val := range []string{"notadate", "+1x", "+1", "+d", "due:2026-01-02"} {
+ t.Run(val, func(t *testing.T) {
+ if _, err := compileSmartDate(val, now); err == nil {
+ t.Errorf("compileSmartDate(%q) returned no error", val)
+ }
+ })
+ }
+}
+
+func TestGenerateRecurringTask(t *testing.T) {
+ now := time.Date(2026, 1, 15, 0, 0, 0, 0, time.UTC)
+ due := func(y int, m time.Month, d int) time.Time {
+ return time.Date(y, m, d, 0, 0, 0, 0, time.UTC)
+ }
+
+ tests := []struct {
+ name string
+ task Task
+ wantDue string
+ }{
+ {
+ "bare number means days",
+ Task{RawLine: "water plants +home rec:3", Recurrence: "[3]"},
+ "2026-01-18",
+ },
+ {
+ "no + counts from today",
+ Task{RawLine: "water plants +home rec:1w", Recurrence: "[1w]"},
+ "2026-01-22",
+ },
+ {
+ "+ counts from the due date",
+ Task{RawLine: "water plants +home due:2026-01-01 rec:+1w", Recurrence: "[+1w]", DueDate: due(2026, 1, 1), HasDueDate: true},
+ "2026-01-08",
+ },
+ {
+ "monthly from the due date",
+ Task{RawLine: "pay rent +home due:2026-01-15 rec:+1m", Recurrence: "[+1m]", DueDate: due(2026, 1, 15), HasDueDate: true},
+ "2026-02-15",
+ },
+ {
+ "yearly from the due date",
+ Task{RawLine: "renew passport +self due:2026-03-01 rec:+1y", Recurrence: "[+1y]", DueDate: due(2026, 3, 1), HasDueDate: true},
+ "2027-03-01",
+ },
+ {
+ // AddDate normalises out-of-range days, so the 31st rolls into March
+ "month end normalises forward",
+ Task{RawLine: "pay rent +home due:2026-01-31 rec:+1m", Recurrence: "[+1m]", DueDate: due(2026, 1, 31), HasDueDate: true},
+ "2026-03-03",
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ got, err := generateRecurringTask(tt.task, now)
+ if err != nil {
+ t.Fatalf("generateRecurringTask: %v", err)
+ }
+ if !strings.Contains(got, "due:"+tt.wantDue) {
+ t.Errorf("next line = %q, want due:%s", got, tt.wantDue)
+ }
+ if tag := "rec:" + strings.Trim(tt.task.Recurrence, "[]"); !strings.Contains(got, tag) {
+ t.Errorf("next line lost its %s tag: %q", tag, got)
+ }
+ })
+ }
+}
+
+// the old code threw away the Atoi error, so a bad rec: quietly produced a
+// "next" task with an unchanged due date. it has to fail instead.
+func TestGenerateRecurringTaskRejectsBadInput(t *testing.T) {
+ now := time.Date(2026, 1, 15, 0, 0, 0, 0, time.UTC)
+
+ tests := []struct {
+ name string
+ recurrence string
+ }{
+ {"empty", ""},
+ {"no amount", "[w]"},
+ {"garbage amount", "[abc]"},
+ {"zero", "[0d]"},
+ {"negative", "[-1w]"},
+ {"lone plus", "[+]"},
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ task := Task{RawLine: "water plants +home rec" + tt.recurrence, Recurrence: tt.recurrence}
+ if _, err := generateRecurringTask(task, now); err == nil {
+ t.Errorf("generateRecurringTask(%q) returned no error", tt.recurrence)
+ }
+ })
+ }
+}
+
+// a malformed rec: must not half-complete the task: no archive entry, no
+// rewritten todo file.
+func TestCompleteTasksRejectsBadRecurrence(t *testing.T) {
+ cfg, todo := newTestConfig(t, "water plants +home rec:abc")
+ before := readLines(t, todo)
+
+ if _, err := CompleteTasks(cfg, []int{1}); err == nil {
+ t.Fatal("CompleteTasks accepted a malformed recurrence")
+ }
+ if after := readLines(t, todo); !slices.Equal(before, after) {
+ t.Errorf("todo file changed despite the error:\nbefore %v\nafter %v", before, after)
+ }
+ if _, err := os.Stat(cfg.DoneFile); !os.IsNotExist(err) {
+ t.Errorf("done file was touched despite the error (stat err = %v)", err)
+ }
+}
diff --git a/task_test.go b/task_test.go
@@ -0,0 +1,114 @@
+package main
+
+import (
+ "slices"
+ "testing"
+ "time"
+)
+
+func TestParseTask(t *testing.T) {
+ tests := []struct {
+ name string
+ line string
+ desc string
+ projects []string
+ contexts []string
+ due string
+ recurrence string
+ pinned bool
+ waiting bool
+ valid bool
+ }{
+ {"plain description", "buy catnip", "buy catnip", nil, nil, "", "", false, false, true},
+ {"projects and contexts", "buy catnip +metsa @chores", "buy catnip", []string{"metsa"}, []string{"chores"}, "", "", false, false, true},
+ {"several tags", "call mom +self +home @phone @urgent", "call mom", []string{"self", "home"}, []string{"phone", "urgent"}, "", "", false, false, true},
+ {"due date", "pay rent due:2026-01-02", "pay rent", nil, nil, "2026-01-02", "", false, false, true},
+ {"malformed due is ignored", "pay rent due:tomorrow", "pay rent", nil, nil, "", "", false, false, true},
+ {"recurrence", "water plants due:2026-01-01 rec:+1w", "water plants", nil, nil, "2026-01-01", "[+1w]", false, false, true},
+ {"priority tag stays in the description", "(A) buy catnip +metsa", "(A) buy catnip", []string{"metsa"}, nil, "", "", false, false, true},
+ {"pinned modifier", "* buy catnip +metsa", "buy catnip", []string{"metsa"}, nil, "", "", true, false, true},
+ {"pinned modifier attached", "*buy catnip", "buy catnip", nil, nil, "", "", true, false, true},
+ {"waiting modifier", "~ call mom @phone", "call mom", nil, []string{"phone"}, "", "", false, true, true},
+ {"modifier after priority", "(A) * buy catnip", "(A) buy catnip", nil, nil, "", "", true, false, true},
+ {"blank line is invalid", "", "", nil, nil, "", "", false, false, false},
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ got := ParseTask(1, tt.line)
+
+ if got.Description != tt.desc {
+ t.Errorf("description = %q, want %q", got.Description, tt.desc)
+ }
+ if !slices.Equal(got.Projects, tt.projects) {
+ t.Errorf("projects = %v, want %v", got.Projects, tt.projects)
+ }
+ if !slices.Equal(got.Contexts, tt.contexts) {
+ t.Errorf("contexts = %v, want %v", got.Contexts, tt.contexts)
+ }
+
+ gotDue := ""
+ if got.HasDueDate {
+ gotDue = got.DueDate.Format("2006-01-02")
+ }
+ if gotDue != tt.due {
+ t.Errorf("due = %q, want %q", gotDue, tt.due)
+ }
+
+ if got.Recurrence != tt.recurrence {
+ t.Errorf("recurrence = %q, want %q", got.Recurrence, tt.recurrence)
+ }
+ if got.IsPinned != tt.pinned {
+ t.Errorf("pinned = %v, want %v", got.IsPinned, tt.pinned)
+ }
+ if got.IsWaiting != tt.waiting {
+ t.Errorf("waiting = %v, want %v", got.IsWaiting, tt.waiting)
+ }
+ if got.IsValid != tt.valid {
+ t.Errorf("valid = %v, want %v", got.IsValid, tt.valid)
+ }
+ })
+ }
+}
+
+// IDs are line numbers, so a blank line in the middle must not consume one.
+func TestReadTasksSkipsBlankLines(t *testing.T) {
+ _, todo := newTestConfig(t, "first task +home", "", "second task +home")
+
+ tasks, err := ReadTasks(todo)
+ if err != nil {
+ t.Fatalf("ReadTasks: %v", err)
+ }
+ if len(tasks) != 2 {
+ t.Fatalf("read %d tasks, want 2", len(tasks))
+ }
+ if tasks[0].ID != 1 || tasks[1].ID != 2 {
+ t.Errorf("IDs = %d, %d; want 1, 2", tasks[0].ID, tasks[1].ID)
+ }
+ if tasks[0].Description != "first task" || tasks[1].Description != "second task" {
+ t.Errorf("descriptions = %q, %q", tasks[0].Description, tasks[1].Description)
+ }
+}
+
+func TestDaysUntilDue(t *testing.T) {
+ now := time.Now()
+
+ tests := []struct {
+ name string
+ task Task
+ want int
+ }{
+ {"today", Task{HasDueDate: true, DueDate: now}, 0},
+ {"tomorrow", Task{HasDueDate: true, DueDate: now.AddDate(0, 0, 1)}, 1},
+ {"two days ago", Task{HasDueDate: true, DueDate: now.AddDate(0, 0, -2)}, -2},
+ {"no due date", Task{}, 9999},
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ if got := tt.task.DaysUntilDue(); got != tt.want {
+ t.Errorf("DaysUntilDue() = %d, want %d", got, tt.want)
+ }
+ })
+ }
+}