From 7a8b0dfe4d85f94633e52114557a96882676193b Mon Sep 17 00:00:00 2001 From: rn404 Date: Wed, 8 Jul 2026 19:11:16 +0900 Subject: [PATCH] quality: add LICENSE, strengthen linting, and close coverage gaps - Add MIT LICENSE (required by pkg.go.dev redistributability and most quality scorers) - Enable gocritic/godot/gosec/misspell/nilnil/revive/unconvert/unparam in golangci-lint (gosec excluded for tests writing into t.TempDir) - logfile.Stat: replace nil,nil contract with ErrNotFound sentinel - Add root-command tests: cmd/sava coverage 0% -> 83.9% - Add logfile error-path tests (broken JSON, invalid date): 77.8% -> 81% - Fix revive/funcorder findings (package comment, unused params, constructor placement) Co-Authored-By: Claude Fable 5 --- .golangci.yml | 16 ++++++ LICENSE | 21 ++++++++ cmd/sava/commands.go | 6 +-- cmd/sava/main.go | 2 + cmd/sava/root_test.go | 87 ++++++++++++++++++++++++++++++++ internal/command/command.go | 3 +- internal/command/command_test.go | 7 +-- internal/logfile/logfile.go | 24 +++++---- internal/logfile/logfile_test.go | 21 +++++++- internal/model/model.go | 43 +++++++++------- 10 files changed, 193 insertions(+), 37 deletions(-) create mode 100644 LICENSE create mode 100644 cmd/sava/root_test.go diff --git a/.golangci.yml b/.golangci.yml index 0ba2457..28b0ee5 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,6 +1,15 @@ version: "2" linters: + enable: + - gocritic + - godot + - gosec + - misspell + - nilnil + - revive + - unconvert + - unparam settings: errcheck: # CLI output to stdout/stderr: write errors are not actionable. @@ -8,3 +17,10 @@ linters: - fmt.Fprint - fmt.Fprintf - fmt.Fprintln + exclusions: + rules: + # Tests read fixtures and write into t.TempDir(); the file + # permission and path-from-variable findings do not apply. + - path: _test\.go + linters: + - gosec diff --git a/LICENSE b/LICENSE new file mode 100644 index 0000000..c99661b --- /dev/null +++ b/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2026 rn404 + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/cmd/sava/commands.go b/cmd/sava/commands.go index 1b30baa..ed95319 100644 --- a/cmd/sava/commands.go +++ b/cmd/sava/commands.go @@ -13,7 +13,7 @@ func newAddCommand() *cobra.Command { Use: "add ", Short: "Add contents to nippo log.", Args: cobra.ExactArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(_ *cobra.Command, args []string) error { return command.Add(logfile.Dir(), args[0], memo) }, } @@ -37,7 +37,7 @@ func newDelCommand() *cobra.Command { Use: "del ", Short: "delete task.", Args: cobra.ExactArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(_ *cobra.Command, args []string) error { return command.Del(logfile.Dir(), args[0]) }, } @@ -68,7 +68,7 @@ func newClearCommand() *cobra.Command { Use: "clear", Short: "delete log", Args: cobra.NoArgs, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { return command.Clear(cmd.OutOrStdout(), cmd.InOrStdin(), logfile.Dir(), all, yes) }, } diff --git a/cmd/sava/main.go b/cmd/sava/main.go index a41a262..3bbb2f3 100644 --- a/cmd/sava/main.go +++ b/cmd/sava/main.go @@ -1,3 +1,5 @@ +// Command sava is a console memo tool that helps writing daily +// reports (nippo) without leaving the terminal. package main import ( diff --git a/cmd/sava/root_test.go b/cmd/sava/root_test.go new file mode 100644 index 0000000..5cbc9c1 --- /dev/null +++ b/cmd/sava/root_test.go @@ -0,0 +1,87 @@ +package main + +import ( + "strings" + "testing" +) + +// execute runs the root command with args and returns combined output. +func execute(t *testing.T, args ...string) (string, error) { + t.Helper() + + root := newRootCommand() + var buf strings.Builder + root.SetOut(&buf) + root.SetErr(&buf) + root.SetArgs(args) + + err := root.Execute() + return buf.String(), err +} + +func mustExecute(t *testing.T, args ...string) string { + t.Helper() + + out, err := execute(t, args...) + if err != nil { + t.Fatalf("sava %s: %v", strings.Join(args, " "), err) + } + return out +} + +func TestVersion(t *testing.T) { + out := mustExecute(t, "--version") + if !strings.Contains(out, version) { + t.Errorf("version output = %q, want to contain %q", out, version) + } + if version == "" { + t.Error("embedded version should not be empty") + } +} + +func TestAddListFlow(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + mustExecute(t, "add", "buy cabbage") + mustExecute(t, "add", "-m", "shrimp memo") + + out := mustExecute(t, "list") + for _, want := range []string{"Task ->", "buy cabbage", "Memo ->", "shrimp memo"} { + if !strings.Contains(out, want) { + t.Errorf("list output should contain %q:\n%s", want, out) + } + } + + stat := mustExecute(t, "list", "-s") + if !strings.Contains(stat, "Task: 1 (unfinished: 1), Memo: 1") { + t.Errorf("list -s output = %q", stat) + } +} + +func TestClearAllWithYes(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + mustExecute(t, "add", "temporary") + mustExecute(t, "clear", "-a", "--yes") + + out := mustExecute(t, "list", "-a") + if strings.Contains(out, "-Task") || strings.Count(out, "\n- ") > 0 { + t.Errorf("no log files should remain after clear -a --yes:\n%s", out) + } +} + +func TestInvalidDateFails(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + if _, err := execute(t, "list", "not-a-date"); err == nil { + t.Error("list with an invalid date should fail") + } +} + +func TestUnknownHashFails(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + if _, err := execute(t, "end", "no-such-hash"); err == nil { + t.Error("end with an unknown hash should fail") + } +} diff --git a/internal/command/command.go b/internal/command/command.go index 103fb42..fa6be4a 100644 --- a/internal/command/command.go +++ b/internal/command/command.go @@ -5,6 +5,7 @@ package command import ( "bufio" + "errors" "fmt" "io" "strings" @@ -113,7 +114,7 @@ func List(w io.Writer, r io.Reader, dir string, opts ListOptions) error { func listOneDay(w io.Writer, dir string, opts ListOptions) error { file, err := logfile.Stat(dir, opts.Date) - if err != nil { + if err != nil && !errors.Is(err, logfile.ErrNotFound) { return err } diff --git a/internal/command/command_test.go b/internal/command/command_test.go index 7bcd6c5..3ea23b9 100644 --- a/internal/command/command_test.go +++ b/internal/command/command_test.go @@ -1,6 +1,7 @@ package command import ( + "errors" "fmt" "os" "path/filepath" @@ -14,12 +15,12 @@ import ( func todayItems(t *testing.T, dir string) []model.Item { t.Helper() file, err := logfile.Stat(dir, "") + if errors.Is(err, logfile.ErrNotFound) { + return nil + } if err != nil { t.Fatal(err) } - if file == nil { - return nil - } return file.Body.Items } diff --git a/internal/logfile/logfile.go b/internal/logfile/logfile.go index a6c5786..e6e9104 100644 --- a/internal/logfile/logfile.go +++ b/internal/logfile/logfile.go @@ -19,8 +19,12 @@ const ( fileExt = ".json" ) -// ErrFreezed is returned when attempting to update a frozen log file. -var ErrFreezed = errors.New("this log file is freezed, no updates") +var ( + // ErrFreezed is returned when attempting to update a frozen log file. + ErrFreezed = errors.New("this log file is freezed, no updates") + // ErrNotFound is returned by Stat when the day has no log file. + ErrNotFound = errors.New("log file not found") +) // LogFile is a loaded daily log file. type LogFile struct { @@ -61,8 +65,8 @@ func resolveName(day string) (string, error) { return day, nil } -// Stat loads the log file for day (today if empty). It returns nil -// without error when the file does not exist. +// Stat loads the log file for day (today if empty). It returns an +// error wrapping ErrNotFound when the file does not exist. func Stat(dir, day string) (*LogFile, error) { name, err := resolveName(day) if err != nil { @@ -70,9 +74,9 @@ func Stat(dir, day string) (*LogFile, error) { } path := pathFor(dir, name) - data, err := os.ReadFile(path) + data, err := os.ReadFile(path) //nolint:gosec // path is the log dir joined with a strictly validated date if errors.Is(err, os.ErrNotExist) { - return nil, nil + return nil, fmt.Errorf("%s: %w", name, ErrNotFound) } if err != nil { return nil, err @@ -90,12 +94,12 @@ func Stat(dir, day string) (*LogFile, error) { // one when it does not exist yet. func Get(dir, day string) (*LogFile, error) { file, err := Stat(dir, day) - if err != nil { - return nil, err - } - if file != nil { + if err == nil { return file, nil } + if !errors.Is(err, ErrNotFound) { + return nil, err + } name, err := resolveName(day) if err != nil { diff --git a/internal/logfile/logfile_test.go b/internal/logfile/logfile_test.go index a2440bc..065a0a3 100644 --- a/internal/logfile/logfile_test.go +++ b/internal/logfile/logfile_test.go @@ -12,14 +12,31 @@ import ( func TestStatMissingFile(t *testing.T) { file, err := Stat(t.TempDir(), "2026-07-05") - if err != nil { - t.Fatal(err) + if !errors.Is(err, ErrNotFound) { + t.Fatalf("err = %v, want ErrNotFound", err) } if file != nil { t.Errorf("Stat for missing file should return nil, got %+v", file) } } +func TestStatBrokenFile(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "2026-07-05.json"), []byte("{broken"), 0o600); err != nil { + t.Fatal(err) + } + + if _, err := Stat(dir, "2026-07-05"); err == nil || errors.Is(err, ErrNotFound) { + t.Errorf("broken JSON should surface a parse error, got %v", err) + } +} + +func TestUpdateInvalidDate(t *testing.T) { + if err := Update(t.TempDir(), "not-a-date", model.NewLog()); err == nil { + t.Error("Update with an invalid date should fail") + } +} + func TestGetCreatesNewFile(t *testing.T) { dir := t.TempDir() file, err := Get(dir, "2026-07-05") diff --git a/internal/model/model.go b/internal/model/model.go index 60e5a8d..7e97652 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -16,6 +16,9 @@ const DateLayout = "2006-01-02" // isoLayout mirrors JavaScript's Date.toISOString() (UTC, milliseconds). const isoLayout = "2006-01-02T15:04:05.000Z" +// idBytes is the entropy of an item/file ID (hex-encoded to 8 chars). +const idBytes = 4 + // Item is a single log entry. The presence of Closed distinguishes a // task (non-nil) from a memo (nil). type Item struct { @@ -26,27 +29,11 @@ type Item struct { Closed *bool `json:"closed,omitempty"` } -// Log is the body of one daily log file. -type Log struct { - Hash string `json:"hash"` - Freezed bool `json:"freezed"` - Items []Item `json:"items"` -} - -// IsTask reports whether the item is a task (has a closed flag). -func (i Item) IsTask() bool { - return i.Closed != nil -} - -// IsClosed reports whether the item is a finished task. -func (i Item) IsClosed() bool { - return i.Closed != nil && *i.Closed -} - // NewTaskItem creates an open task with a fresh ID and timestamps. func NewTaskItem(content string) Item { now := NowISO() closed := false + return Item{ Hash: NewID(), Content: content, @@ -59,6 +46,7 @@ func NewTaskItem(content string) Item { // NewMemoItem creates a memo with a fresh ID and timestamps. func NewMemoItem(content string) Item { now := NowISO() + return Item{ Hash: NewID(), Content: content, @@ -67,6 +55,23 @@ func NewMemoItem(content string) Item { } } +// IsTask reports whether the item is a task (has a closed flag). +func (i Item) IsTask() bool { + return i.Closed != nil +} + +// IsClosed reports whether the item is a finished task. +func (i Item) IsClosed() bool { + return i.Closed != nil && *i.Closed +} + +// Log is the body of one daily log file. +type Log struct { + Hash string `json:"hash"` + Freezed bool `json:"freezed"` + Items []Item `json:"items"` +} + // NewLog creates an empty, unfrozen log body. func NewLog() Log { return Log{ @@ -78,11 +83,12 @@ func NewLog() Log { // NewID returns a random 8-character hex ID used as item/file hash. func NewID() string { - buf := make([]byte, 4) + buf := make([]byte, idBytes) if _, err := rand.Read(buf); err != nil { // crypto/rand never fails on supported platforms; fall back to time. return fmt.Sprintf("%08x", time.Now().UnixNano()&0xffffffff) } + return hex.EncodeToString(buf) } @@ -97,6 +103,7 @@ func ParseDate(value string) (time.Time, error) { if err != nil { return time.Time{}, fmt.Errorf("invalid date %q: expected format yyyy-MM-dd", value) } + return t, nil }