diff --git a/.github/workflows/commit-lint.yml b/.github/workflows/commit-lint.yml new file mode 100644 index 0000000..5a2d3c1 --- /dev/null +++ b/.github/workflows/commit-lint.yml @@ -0,0 +1,25 @@ +name: Commit Lint + +on: + pull_request: + branches: [main] + +jobs: + commit-lint: + name: Commit messages + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Setup Go + uses: actions/setup-go@v5 + with: + go-version: '1.23' + + - name: Lint commit messages added by this PR + run: | + go run ./cmd/commitmsg lint \ + --range "${{ github.event.pull_request.base.sha }}..${{ github.event.pull_request.head.sha }}" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..58dd714 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,57 @@ +# Contributing to Nightshift + +Thanks for helping out. This is a short guide; the README covers installation +and configuration, and `docs/` covers individual subsystems. + +## Getting set up + +```bash +go build ./... # or: make build +make test # go test ./... +make lint # golangci-lint, if installed +make install-hooks # opt-in git hooks (recommended) +``` + +`make install-hooks` symlinks the repo's hooks into `.git/hooks/`: + +* **pre-commit** — gofmt, `go vet`, `go build` on staged Go files +* **commit-msg** — normalizes and validates the commit message + +Nothing is installed automatically; run the command when you want the hooks. +Bypass them once with `git commit --no-verify`. + +## Commit messages + +Nightshift uses [Conventional Commits](https://www.conventionalcommits.org/): + +``` +type(scope): subject line + +Optional body, wrapped at 72 columns (the linter's hard limit is 80). + +Optional-Trailer: value +``` + +Allowed types are `feat`, `fix`, `docs`, `style`, `refactor`, `perf`, `test`, +`build`, `ci`, `chore`, and `revert`. Subjects are imperative, lowercase, have +no trailing period, and stay within 72 characters. + +The full standard, with examples and the exact rules the linter enforces, is in +[docs/commit-messages.md](docs/commit-messages.md). + +Check your messages before pushing: + +```bash +make commit-lint # origin/main..HEAD +make commit-lint RANGE=HEAD~20..HEAD # any range +``` + +CI runs the same check over the commits a pull request adds. Existing history is +not retroactively enforced. + +## Pull requests + +* Keep changes focused; separate refactors from behaviour changes. +* Add or update tests for behaviour you change — `make test` must pass. +* Run `gofmt -w .` (the pre-commit hook checks this). +* Update the docs when you change user-visible behaviour. diff --git a/Makefile b/Makefile index 088be01..9061e36 100644 --- a/Makefile +++ b/Makefile @@ -1,9 +1,12 @@ -.PHONY: build test test-verbose test-race coverage coverage-html lint clean deps check install calibrate-providers install-hooks help +.PHONY: build test test-verbose test-race coverage coverage-html lint clean deps check install calibrate-providers install-hooks commit-lint help # Binary name BINARY=nightshift PKG=./cmd/nightshift +# Commit range checked by `make commit-lint` +RANGE?=origin/main..HEAD + # Build the binary build: go build -o $(BINARY) $(PKG) @@ -75,10 +78,17 @@ help: @echo " check - Run tests and lint" @echo " install - Build and install to Go bin directory" @echo " calibrate-providers - Compare local Claude/Codex session usage for calibration" - @echo " install-hooks - Install git pre-commit hook" + @echo " install-hooks - Install git pre-commit and commit-msg hooks" + @echo " commit-lint - Lint commit messages in RANGE (default: origin/main..HEAD)" @echo " help - Show this help" -# Install git pre-commit hook +# Install git hooks install-hooks: @ln -sf ../../scripts/pre-commit.sh .git/hooks/pre-commit @echo "✓ pre-commit hook installed (.git/hooks/pre-commit → scripts/pre-commit.sh)" + @ln -sf ../../scripts/commit-msg.sh .git/hooks/commit-msg + @echo "✓ commit-msg hook installed (.git/hooks/commit-msg → scripts/commit-msg.sh)" + +# Lint commit messages in RANGE (default: commits not yet on main) +commit-lint: + go run ./cmd/commitmsg lint --range $(RANGE) diff --git a/README.md b/README.md index 84f92cd..105dcdb 100644 --- a/README.md +++ b/README.md @@ -258,21 +258,36 @@ Each task has a default cooldown interval to prevent the same task from running ## Development -### Pre-commit hooks +### Git hooks -Install the git pre-commit hook to catch formatting and vet issues before pushing: +Install the git hooks to catch formatting, vet, and commit message issues before pushing: ```bash make install-hooks ``` -This symlinks `scripts/pre-commit.sh` into `.git/hooks/pre-commit`. The hook runs: +This symlinks `scripts/pre-commit.sh` into `.git/hooks/pre-commit` and +`scripts/commit-msg.sh` into `.git/hooks/commit-msg`. The pre-commit hook runs: - **gofmt** — flags any staged `.go` files that need formatting - **go vet** — catches common correctness issues - **go build** — ensures the project compiles +The commit-msg hook normalizes the commit message and checks it against the +standard. + To bypass in a pinch: `git commit --no-verify` +### Commit messages + +Nightshift uses Conventional Commits (`type(scope): subject`). See +[docs/commit-messages.md](docs/commit-messages.md) for the full standard, and +[CONTRIBUTING.md](CONTRIBUTING.md) for how to contribute. + +```bash +make commit-lint # check origin/main..HEAD +make commit-lint RANGE=HEAD~20..HEAD # check any range +``` + ## Uninstalling ```bash diff --git a/cmd/commitmsg/main.go b/cmd/commitmsg/main.go new file mode 100644 index 0000000..691bce8 --- /dev/null +++ b/cmd/commitmsg/main.go @@ -0,0 +1,211 @@ +// Command commitmsg normalizes and lints commit messages against the nightshift +// commit message standard documented in docs/commit-messages.md. +// +// Usage: +// +// commitmsg normalize rewrite the message file in place +// commitmsg lint validate one message file ("-" for stdin) +// commitmsg lint --range validate every commit in a range +// commitmsg lint --range --report-only +// report violations but always exit 0 +// +// It exits 1 when a message violates the standard and 2 on usage or I/O errors. +package main + +import ( + "flag" + "fmt" + "io" + "os" + "os/exec" + "strings" + + "github.com/marcus/nightshift/internal/commitmsg" +) + +const ( + exitViolation = 1 + exitUsage = 2 +) + +func main() { + if len(os.Args) < 2 { + usage() + os.Exit(exitUsage) + } + + switch os.Args[1] { + case "normalize": + os.Exit(runNormalize(os.Args[2:])) + case "lint": + os.Exit(runLint(os.Args[2:])) + case "-h", "--help", "help": + usage() + os.Exit(0) + default: + fmt.Fprintf(os.Stderr, "commitmsg: unknown command %q\n\n", os.Args[1]) + usage() + os.Exit(exitUsage) + } +} + +func usage() { + fmt.Fprint(os.Stderr, `commitmsg — normalize and lint commit messages + +Usage: + commitmsg normalize rewrite a message file in place + commitmsg lint [file] validate a message file ("-" or + omitted reads stdin) + commitmsg lint --range validate every commit in a range + commitmsg lint --range --report-only report but always exit 0 + +The standard is documented in docs/commit-messages.md. +`) +} + +func runNormalize(args []string) int { + fs := flag.NewFlagSet("normalize", flag.ContinueOnError) + if err := fs.Parse(args); err != nil { + return exitUsage + } + if fs.NArg() != 1 { + fmt.Fprintln(os.Stderr, "commitmsg normalize: expected exactly one message file") + return exitUsage + } + + path := fs.Arg(0) + raw, err := os.ReadFile(path) + if err != nil { + fmt.Fprintf(os.Stderr, "commitmsg normalize: %v\n", err) + return exitUsage + } + + normalized := commitmsg.Normalize(string(raw)) + if normalized == string(raw) { + return 0 + } + info, err := os.Stat(path) + mode := os.FileMode(0o644) + if err == nil { + mode = info.Mode().Perm() + } + if err := os.WriteFile(path, []byte(normalized), mode); err != nil { + fmt.Fprintf(os.Stderr, "commitmsg normalize: %v\n", err) + return exitUsage + } + return 0 +} + +func runLint(args []string) int { + fs := flag.NewFlagSet("lint", flag.ContinueOnError) + revRange := fs.String("range", "", "validate every commit in this git rev-range instead of a file") + reportOnly := fs.Bool("report-only", false, "print violations but always exit 0") + if err := fs.Parse(args); err != nil { + return exitUsage + } + + if *revRange != "" { + if fs.NArg() > 0 { + fmt.Fprintln(os.Stderr, "commitmsg lint: --range cannot be combined with a file argument") + return exitUsage + } + return lintRange(*revRange, *reportOnly) + } + + msg, err := readMessage(fs.Arg(0)) + if err != nil { + fmt.Fprintf(os.Stderr, "commitmsg lint: %v\n", err) + return exitUsage + } + issues := commitmsg.Lint(msg) + if len(issues) == 0 { + return 0 + } + report(os.Stderr, firstLine(msg), issues) + if *reportOnly { + return 0 + } + return exitViolation +} + +func readMessage(path string) (string, error) { + if path == "" || path == "-" { + raw, err := io.ReadAll(os.Stdin) + return string(raw), err + } + raw, err := os.ReadFile(path) + return string(raw), err +} + +func lintRange(revRange string, reportOnly bool) int { + shas, err := gitLines("rev-list", "--no-merges", revRange) + if err != nil { + fmt.Fprintf(os.Stderr, "commitmsg lint: %v\n", err) + return exitUsage + } + if len(shas) == 0 { + fmt.Printf("commitmsg: no commits in range %s\n", revRange) + return 0 + } + + bad := 0 + for _, sha := range shas { + out, err := exec.Command("git", "log", "-1", "--pretty=%B", sha).Output() + if err != nil { + fmt.Fprintf(os.Stderr, "commitmsg lint: reading %s: %v\n", sha, err) + return exitUsage + } + msg := string(out) + issues := commitmsg.Lint(msg) + if len(issues) == 0 { + continue + } + bad++ + report(os.Stderr, fmt.Sprintf("%s %s", shortSHA(sha), firstLine(msg)), issues) + } + + fmt.Printf("commitmsg: checked %d commit(s) in %s, %d with violations\n", len(shas), revRange, bad) + if bad > 0 && !reportOnly { + fmt.Fprintln(os.Stderr, "\nSee docs/commit-messages.md for the standard.") + return exitViolation + } + return 0 +} + +func report(w io.Writer, header string, issues []commitmsg.Issue) { + fmt.Fprintf(w, "✗ %s\n", header) + for _, issue := range issues { + fmt.Fprintf(w, " %s\n", issue) + } +} + +func gitLines(args ...string) ([]string, error) { + out, err := exec.Command("git", args...).Output() + if err != nil { + return nil, fmt.Errorf("git %s: %w", strings.Join(args, " "), err) + } + var lines []string + for _, line := range strings.Split(strings.TrimSpace(string(out)), "\n") { + if line != "" { + lines = append(lines, line) + } + } + return lines, nil +} + +func firstLine(msg string) string { + for _, line := range strings.Split(msg, "\n") { + line = strings.TrimSpace(line) + if line != "" && !strings.HasPrefix(line, "#") { + return line + } + } + return "(empty message)" +} + +func shortSHA(sha string) string { + if len(sha) > 8 { + return sha[:8] + } + return sha +} diff --git a/cmd/commitmsg/main_test.go b/cmd/commitmsg/main_test.go new file mode 100644 index 0000000..aaeae1e --- /dev/null +++ b/cmd/commitmsg/main_test.go @@ -0,0 +1,95 @@ +package main + +import ( + "os" + "path/filepath" + "testing" +) + +func writeMsg(t *testing.T, content string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "COMMIT_EDITMSG") + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("writing message file: %v", err) + } + return path +} + +func readMsgFile(t *testing.T, path string) string { + t.Helper() + raw, err := os.ReadFile(path) + if err != nil { + t.Fatalf("reading message file: %v", err) + } + return string(raw) +} + +func TestRunNormalizeRewritesFileInPlace(t *testing.T) { + path := writeMsg(t, "Feat(cli): Add the --timeout flag. \nBody line.\n# a comment\n") + + if code := runNormalize([]string{path}); code != 0 { + t.Fatalf("runNormalize exited %d, want 0", code) + } + + want := "feat(cli): add the --timeout flag\n\nBody line.\n" + if got := readMsgFile(t, path); got != want { + t.Fatalf("normalized file =\n%q\nwant\n%q", got, want) + } + + // Running again must not change the file. + if code := runNormalize([]string{path}); code != 0 { + t.Fatalf("second runNormalize exited %d, want 0", code) + } + if got := readMsgFile(t, path); got != want { + t.Fatalf("normalize is not idempotent through the CLI: %q", got) + } +} + +func TestRunNormalizeMissingArgs(t *testing.T) { + if code := runNormalize(nil); code != exitUsage { + t.Fatalf("runNormalize with no file exited %d, want %d", code, exitUsage) + } + if code := runNormalize([]string{filepath.Join(t.TempDir(), "nope")}); code != exitUsage { + t.Fatalf("runNormalize with a missing file exited %d, want %d", code, exitUsage) + } +} + +func TestRunLintExitCodes(t *testing.T) { + tests := []struct { + name string + msg string + want int + }{ + {"valid", "feat(cli): add the --timeout flag\n", 0}, + {"invalid", "Fixed the thing.\n", exitViolation}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if code := runLint([]string{writeMsg(t, tt.msg)}); code != tt.want { + t.Fatalf("runLint exited %d, want %d", code, tt.want) + } + }) + } +} + +func TestRunLintReportOnlyAlwaysSucceeds(t *testing.T) { + path := writeMsg(t, "Fixed the thing.\n") + if code := runLint([]string{"--report-only", path}); code != 0 { + t.Fatalf("runLint --report-only exited %d, want 0", code) + } +} + +func TestRunLintRejectsRangeWithFile(t *testing.T) { + if code := runLint([]string{"--range", "HEAD~1..HEAD", "some-file"}); code != exitUsage { + t.Fatalf("runLint exited %d, want %d", code, exitUsage) + } +} + +func TestFirstLineSkipsCommentsAndBlanks(t *testing.T) { + if got := firstLine("\n# comment\nfeat: a thing\n"); got != "feat: a thing" { + t.Fatalf("firstLine() = %q", got) + } + if got := firstLine("# only comments\n"); got != "(empty message)" { + t.Fatalf("firstLine() = %q", got) + } +} diff --git a/docs/commit-messages.md b/docs/commit-messages.md new file mode 100644 index 0000000..c4b2845 --- /dev/null +++ b/docs/commit-messages.md @@ -0,0 +1,148 @@ +# Commit messages + +Nightshift uses [Conventional Commits](https://www.conventionalcommits.org/) for +commit subjects. The format is machine-readable, so it can drive the changelog +and release tooling, and it is already what most of this repository's history +looks like. + +Tooling lives in `cmd/commitmsg` (the CLI) and `internal/commitmsg` (the rules). +The `commit-msg` hook normalizes what it can and rejects what it cannot fix, and +CI checks the commits a pull request adds. + +## Format + +``` +type(scope)!: subject line + +Body paragraph, wrapped at 72 columns. Explain why the change is being +made, not what the diff already shows. + +Trailer-Key: value +``` + +* `type` — required, from the table below. +* `(scope)` — optional, lowercase, the area touched (`budget`, `cli`, + `providers`, `tasks`, …). +* `!` — optional, marks a breaking change. +* `: ` — a colon *and* a space. +* `subject` — required. + +## Types + +| Type | Meaning | +| --- | --- | +| `feat` | a user-visible feature | +| `fix` | a bug fix | +| `docs` | documentation only | +| `style` | formatting only, no behaviour change | +| `refactor` | restructuring without behaviour change | +| `perf` | a performance improvement | +| `test` | adding or fixing tests | +| `build` | build system, dependencies, release packaging | +| `ci` | CI configuration and workflows | +| `chore` | maintenance that fits nowhere else | +| `revert` | reverts a previous commit | + +## Rules + +Subject: + +* imperative mood — "add the flag", not "added" or "adds" +* starts lowercase; acronyms (`API`) and identifiers (`README.md`) keep their + case +* no trailing period +* 72 characters or fewer, including the `type(scope): ` prefix + +Body: + +* separated from the subject by exactly one blank line +* optional — small changes do not need one +* wrapped at 72 columns; the linter's hard limit is 80, so a slightly long line + is tolerated rather than rejected + +The width limit is not applied to fenced code blocks, indented blocks, trailers, +or lines containing a token (a URL or path) that is itself longer than the +limit. + +## Trailers + +The final block of `Key: value` lines is a trailer block, and only when at least +one of its keys is one git and this project recognize — `Co-Authored-By`, +`Signed-off-by`, `Reviewed-by`, `Refs`, `Fixes`, `Closes`, `BREAKING CHANGE`, +`Nightshift-*` and friends. A paragraph that merely ends in a line like +`Before: 2.1s.` or `Note: this is opt-in` is prose, and is left exactly where the +author put it. A trailer block is never rewritten, reflowed, or width-checked; +prose always is. Nightshift-generated commits carry: + +``` +Nightshift-Task: commit-normalize +Nightshift-Ref: https://github.com/marcus/nightshift +``` + +Once a block is anchored by a recognized key, any hyphenated key beside it +(`Reviewed-by:`, `Reported-by:`) is kept with the block. + +## Exemptions + +Messages that git itself writes or rewrites are not checked: merge commits +(`Merge …`), reverts (`Revert "…"`), and autosquash messages (`fixup! …`, +`squash! …`). + +## Examples + +Good: + +``` +feat(budget): add monthly rollover for unused credit +fix(cli): apply --max-projects after the processed-today filter +docs: README.md gains a hook install section +feat(config)!: rename provider.key to provider.api_key +chore: wire up the commit-msg hook +``` + +Bad: + +``` +Fixed the bug. → no type; past tense; trailing period +feat:add the flag → missing space after the colon +feet: add the flag → "feet" is not an allowed type +feat: Add the flag → subject starts with a capital +feat(): add the flag → empty scope +update stuff → no type, and says nothing +``` + +## Tooling + +Install the hooks (opt-in, one command): + +```bash +make install-hooks +``` + +That symlinks `scripts/pre-commit.sh` and `scripts/commit-msg.sh` into +`.git/hooks/`. The `commit-msg` hook runs the normalizer and then the linter on +every commit. Bypass it once with `git commit --no-verify`. + +Run the tools directly: + +```bash +go run ./cmd/commitmsg normalize .git/COMMIT_EDITMSG # rewrite in place +go run ./cmd/commitmsg lint .git/COMMIT_EDITMSG # validate one message +echo "feat: add a thing" | go run ./cmd/commitmsg lint # validate stdin +make commit-lint # origin/main..HEAD +make commit-lint RANGE=HEAD~20..HEAD # any range +go run ./cmd/commitmsg lint --range HEAD~50..HEAD --report-only +``` + +The normalizer only fixes mechanical problems — casing, the space after the +colon, trailing periods, whitespace, blank-line placement. It never rewraps or +rewrites prose, so a too-long subject or an unwrapped body is reported by the +linter for a human to fix. + +Normalization is idempotent: running it twice produces the same output. + +## Scope of enforcement + +History is not rewritten and existing commits are not retroactively enforced. +CI (`.github/workflows/commit-lint.yml`) checks only the commits a pull request +adds. diff --git a/internal/commitmsg/commitmsg.go b/internal/commitmsg/commitmsg.go new file mode 100644 index 0000000..82c5955 --- /dev/null +++ b/internal/commitmsg/commitmsg.go @@ -0,0 +1,500 @@ +// Package commitmsg implements the nightshift commit message standard: a +// Conventional Commits variant documented in docs/commit-messages.md. +// +// It provides two operations used by the commit-msg hook and by CI: +// +// - Normalize rewrites a message into canonical form without changing intent. +// It is idempotent: Normalize(Normalize(x)) == Normalize(x). +// - Lint reports the violations that Normalize cannot fix on its own, with a +// specific, actionable message per violation. +package commitmsg + +import ( + "fmt" + "regexp" + "sort" + "strings" + "unicode" +) + +// Limits for the standard, measured in runes. +// +// Bodies should be wrapped at WrapBodyAt; MaxBodyLineLen is the hard limit the +// linter enforces. The slack between the two keeps the rule from rejecting the +// many otherwise-good commits already in this repository's history. +const ( + MaxSubjectLen = 72 + WrapBodyAt = 72 + MaxBodyLineLen = 80 +) + +// Types are the allowed commit types, mapped to a one-line meaning. The same +// table is rendered in docs/commit-messages.md. +var Types = map[string]string{ + "feat": "a user-visible feature", + "fix": "a bug fix", + "docs": "documentation only", + "style": "formatting only, no behaviour change", + "refactor": "restructuring without behaviour change", + "perf": "a performance improvement", + "test": "adding or fixing tests", + "build": "build system, dependencies, release packaging", + "ci": "CI configuration and workflows", + "chore": "maintenance that fits nowhere else", + "revert": "reverts a previous commit", +} + +// TypeList returns the allowed types in a stable, alphabetical order. +func TypeList() []string { + out := make([]string, 0, len(Types)) + for t := range Types { + out = append(out, t) + } + sort.Strings(out) + return out +} + +var ( + // subjectRe matches "type(scope)!: description". Scope and "!" are optional. + subjectRe = regexp.MustCompile(`^([A-Za-z]+)(\(([^()]*)\))?(!)?: (.*)$`) + // looseColonRe matches anything that at least looks like it is trying to be + // a conventional subject, so we can produce a targeted error. + looseColonRe = regexp.MustCompile(`^([A-Za-z]+)(\(([^()]*)\))?(!)?:(.*)$`) + // trailerRe matches a line shaped like a trailer, e.g. "Nightshift-Task: foo". + // Matching the shape is not enough to call a line a trailer: ordinary prose + // produces lines like "Before: 2.1s." too. See isTrailerLine. + trailerRe = regexp.MustCompile(`^([A-Za-z][A-Za-z0-9-]*|BREAKING CHANGE): .+$`) + // scissorsRe matches git's --verbose cut line; everything below it is a diff. + scissorsRe = regexp.MustCompile(`^#* *-+ >8 -+`) + // fixupRe matches messages git itself will rewrite during an autosquash. + fixupRe = regexp.MustCompile(`^(fixup|squash|amend)! `) + // revertRe matches git's default revert subject. + revertRe = regexp.MustCompile(`^Revert "`) + // mergeRe matches git's default merge subject. + mergeRe = regexp.MustCompile(`^Merge `) +) + +// knownTrailerKeys are the trailer keys this project recognizes by name, +// lowercased for case-insensitive lookup. A trailing block is only treated as a +// trailer block when at least one of its lines uses one of these keys, which is +// what keeps a body paragraph ending in "Note: …" or "Before: 2.1s." from being +// mistaken for trailers. Keys containing a hyphen ("Reviewed-by") are also +// accepted inside a block anchored by a known key, since prose never has that +// shape; see isTrailerLine. +var knownTrailerKeys = map[string]bool{ + "acked-by": true, + "breaking change": true, + "bug": true, + "cc": true, + "change-id": true, + "closes": true, + "co-authored-by": true, + "fixes": true, + "helped-by": true, + "link": true, + "reported-by": true, + "resolves": true, + "reviewed-by": true, + "refs": true, + "signed-off-by": true, + "suggested-by": true, + "tested-by": true, +} + +// trailerKey returns the key of a trailer-shaped line, lowercased, and whether +// the line is trailer-shaped at all. +func trailerKey(line string) (string, bool) { + m := trailerRe.FindStringSubmatch(line) + if m == nil { + return "", false + } + return strings.ToLower(m[1]), true +} + +// isKnownTrailer reports whether the line names a trailer key this project +// recognizes, or a Nightshift-* trailer. +func isKnownTrailer(line string) bool { + key, ok := trailerKey(line) + if !ok { + return false + } + return knownTrailerKeys[key] || strings.HasPrefix(key, "nightshift-") +} + +// isTrailerLine reports whether the line may appear inside a trailer block. A +// hyphenated key is accepted on shape alone; a single-word key must be one we +// know, so prose such as "Note: be careful" is not swallowed. +func isTrailerLine(line string) bool { + key, ok := trailerKey(line) + if !ok { + return false + } + return strings.Contains(key, "-") || knownTrailerKeys[key] +} + +// Issue is a single lint violation. +type Issue struct { + Line int // 1-based line within the message, or 0 when not line-specific + Rule string // stable identifier, e.g. "subject-too-long" + Msg string // actionable, human-readable description +} + +func (i Issue) String() string { + if i.Line > 0 { + return fmt.Sprintf("line %d: %s (%s)", i.Line, i.Msg, i.Rule) + } + return fmt.Sprintf("%s (%s)", i.Msg, i.Rule) +} + +// Exempt reports whether a message is one git generates or rewrites itself, and +// which therefore is not held to the standard. +func Exempt(msg string) bool { + subject := firstLine(msg) + return subject == "" || + fixupRe.MatchString(subject) || + revertRe.MatchString(subject) || + mergeRe.MatchString(subject) +} + +func firstLine(msg string) string { + for _, line := range strings.Split(msg, "\n") { + line = strings.TrimSpace(line) + if line != "" && !strings.HasPrefix(line, "#") { + return line + } + } + return "" +} + +// Normalize rewrites raw into canonical form. It never reflows or rewraps text, +// so prose, code fences and trailers survive untouched; it only fixes the +// mechanical parts of the format. The result always ends in a single newline +// (or is empty, when the message had no content at all). +func Normalize(raw string) string { + lines := stripComments(raw) + lines = trimTrailingWS(lines) + lines = collapseBlanks(lines) + if len(lines) == 0 { + return "" + } + + if !Exempt(strings.Join(lines, "\n")) { + lines[0] = normalizeSubject(lines[0]) + } + + // Ensure exactly one blank line between the subject and the body. + if len(lines) > 1 && lines[1] != "" { + rest := append([]string{""}, lines[1:]...) + lines = append(lines[:1], rest...) + } + + // Ensure a blank line before the trailer block. + if start := trailerStart(lines); start > 1 && lines[start-1] != "" { + rest := append([]string{""}, lines[start:]...) + lines = append(lines[:start], rest...) + } + + return strings.Join(lines, "\n") + "\n" +} + +// stripComments drops git comment lines and everything below the --verbose +// scissors marker. +func stripComments(raw string) []string { + raw = strings.ReplaceAll(raw, "\r\n", "\n") + var out []string + for _, line := range strings.Split(raw, "\n") { + if scissorsRe.MatchString(line) { + break + } + if strings.HasPrefix(line, "#") { + continue + } + out = append(out, line) + } + return out +} + +func trimTrailingWS(lines []string) []string { + out := make([]string, len(lines)) + for i, line := range lines { + out[i] = strings.TrimRight(line, " \t") + } + return out +} + +// collapseBlanks removes leading and trailing blank lines and squeezes runs of +// blank lines down to one, except inside fenced code blocks. +func collapseBlanks(lines []string) []string { + var out []string + inFence := false + prevBlank := false + for _, line := range lines { + if isFence(line) { + inFence = !inFence + } + if !inFence && line == "" { + if len(out) == 0 || prevBlank { + continue + } + prevBlank = true + out = append(out, line) + continue + } + prevBlank = false + out = append(out, line) + } + for len(out) > 0 && out[len(out)-1] == "" { + out = out[:len(out)-1] + } + return out +} + +func isFence(line string) bool { + return strings.HasPrefix(strings.TrimSpace(line), "```") +} + +// normalizeSubject trims the subject, collapses internal whitespace, removes +// trailing periods and lowercases the first word of the description when doing +// so is unambiguously safe. +func normalizeSubject(subject string) string { + subject = strings.Join(strings.Fields(subject), " ") + if subject == "" { + return "" + } + + m := subjectRe.FindStringSubmatch(subject) + if m == nil { + if lm := looseColonRe.FindStringSubmatch(subject); lm != nil { + // "feat:no space" -> "feat: no space" + m = lm + m[5] = strings.TrimSpace(m[5]) + } else { + // Not conventional at all; Lint reports it. Still drop trailing dots. + return trimTrailingPeriod(subject) + } + } + + typ, scope, bang, desc := strings.ToLower(m[1]), m[3], m[4], m[5] + desc = trimTrailingPeriod(strings.TrimSpace(desc)) + desc = lowerFirstWord(desc) + + prefix := typ + if m[2] != "" { + prefix += "(" + strings.TrimSpace(scope) + ")" + } + return prefix + bang + ": " + desc +} + +func trimTrailingPeriod(s string) string { + for strings.HasSuffix(s, ".") { + s = strings.TrimSuffix(s, ".") + s = strings.TrimRight(s, " \t") + } + return s +} + +// lowerFirstWord lowercases the first character of the description, but leaves +// acronyms ("API"), identifiers ("GoModule") and qualified names ("README.md") +// alone. +func lowerFirstWord(desc string) string { + if desc == "" { + return desc + } + word := desc + if i := strings.IndexAny(desc, " \t"); i >= 0 { + word = desc[:i] + } + runes := []rune(word) + if !unicode.IsUpper(runes[0]) { + return desc + } + if strings.ContainsAny(word, "./_-") { + return desc + } + for _, r := range runes[1:] { + if unicode.IsUpper(r) { + return desc // acronym or CamelCase identifier + } + } + runes[0] = unicode.ToLower(runes[0]) + return string(runes) + desc[len(word):] +} + +// trailerStart returns the index of the first line of the trailing trailer +// block, or -1 when the message has no trailer block. The trailer block is the +// longest run of trailer (and continuation) lines at the end of the message, +// and it only counts as one when at least one of those lines uses a key we +// recognize. Requiring a known key is what stops a body paragraph whose last +// lines happen to read "Before: 2.1s." from being treated as trailers and split +// off with a blank line. The subject line is never treated as a trailer, even +// though it looks like one. +func trailerStart(lines []string) int { + end := len(lines) + for end > 0 && lines[end-1] == "" { + end-- + } + start := end + sawKnown := false + for start > 1 && lines[start-1] != "" { + line := lines[start-1] + switch { + case isTrailerLine(line): + sawKnown = sawKnown || isKnownTrailer(line) + case strings.HasPrefix(line, " ") || strings.HasPrefix(line, "\t"): + // continuation of the trailer above it + default: + // A non-trailer line ends the block. + if sawKnown { + return start + } + return -1 + } + start-- + } + if !sawKnown || start == 0 { + return -1 + } + return start +} + +// Lint validates a message against the standard and returns every violation it +// finds. An empty result means the message is valid. +func Lint(msg string) []Issue { + lines := trimTrailingWS(stripComments(msg)) + for len(lines) > 0 && lines[len(lines)-1] == "" { + lines = lines[:len(lines)-1] + } + if len(lines) == 0 || strings.TrimSpace(strings.Join(lines, "")) == "" { + return []Issue{{Rule: "empty-message", Msg: "commit message is empty"}} + } + if Exempt(strings.Join(lines, "\n")) { + return nil + } + + issues := lintSubject(lines[0]) + if len(lines) > 1 && lines[1] != "" { + issues = append(issues, Issue{ + Line: 2, + Rule: "missing-blank-line", + Msg: "the subject must be followed by a blank line before the body", + }) + } + issues = append(issues, lintBody(lines)...) + return issues +} + +func lintSubject(subject string) []Issue { + var issues []Issue + + if strings.TrimSpace(subject) != subject { + issues = append(issues, Issue{ + Line: 1, Rule: "subject-whitespace", + Msg: "the subject has leading or trailing whitespace", + }) + subject = strings.TrimSpace(subject) + } + + m := subjectRe.FindStringSubmatch(subject) + if m == nil { + if lm := looseColonRe.FindStringSubmatch(subject); lm != nil { + if strings.TrimSpace(lm[5]) != "" { + issues = append(issues, Issue{ + Line: 1, Rule: "missing-space-after-colon", + Msg: fmt.Sprintf("the subject needs a space after the colon: %q", lm[1]+lm[2]+lm[4]+": "+strings.TrimSpace(lm[5])), + }) + } + m = lm + m[5] = strings.TrimSpace(m[5]) + } else { + return append(issues, Issue{ + Line: 1, Rule: "missing-type", + Msg: fmt.Sprintf("the subject must start with %q or %q; allowed types: %s", + "type: ", "type(scope): ", strings.Join(TypeList(), ", ")), + }) + } + } + + typ, scopeGroup, scope, desc := m[1], m[2], m[3], m[5] + if _, ok := Types[typ]; !ok { + issues = append(issues, Issue{ + Line: 1, Rule: "unknown-type", + Msg: fmt.Sprintf("%q is not an allowed type; use one of: %s", typ, strings.Join(TypeList(), ", ")), + }) + } + if scopeGroup != "" && strings.TrimSpace(scope) == "" { + issues = append(issues, Issue{ + Line: 1, Rule: "empty-scope", + Msg: "the scope is empty; write \"type: subject\" or give the scope a name", + }) + } + if strings.TrimSpace(desc) == "" { + issues = append(issues, Issue{ + Line: 1, Rule: "empty-subject", + Msg: "the subject text after the type is empty", + }) + return issues + } + if n := len([]rune(subject)); n > MaxSubjectLen { + issues = append(issues, Issue{ + Line: 1, Rule: "subject-too-long", + Msg: fmt.Sprintf("the subject is %d characters; the limit is %d", n, MaxSubjectLen), + }) + } + if strings.HasSuffix(desc, ".") { + issues = append(issues, Issue{ + Line: 1, Rule: "subject-trailing-period", + Msg: "the subject must not end with a period", + }) + } + if first := []rune(desc)[0]; unicode.IsUpper(first) && lowerFirstWord(desc) != desc { + issues = append(issues, Issue{ + Line: 1, Rule: "subject-capitalized", + Msg: "the subject must start with a lowercase word (acronyms and identifiers are fine)", + }) + } + return issues +} + +func lintBody(lines []string) []Issue { + var issues []Issue + trailerAt := trailerStart(lines) + inFence := false + for i := 1; i < len(lines); i++ { + line := lines[i] + if isFence(line) { + inFence = !inFence + continue + } + if inFence || (trailerAt >= 0 && i >= trailerAt) { + continue + } + if strings.HasPrefix(line, " ") || strings.HasPrefix(line, "\t") { + continue // indented code block + } + if len([]rune(line)) <= MaxBodyLineLen { + continue + } + // A line holding a token that is itself over the limit (a long URL or + // path) cannot be wrapped, so it is allowed. + if hasUnwrappableWord(line, MaxBodyLineLen) { + continue + } + issues = append(issues, Issue{ + Line: i + 1, Rule: "body-line-too-long", + Msg: fmt.Sprintf("the body line is %d characters; wrap the body at %d (hard limit %d)", + len([]rune(line)), WrapBodyAt, MaxBodyLineLen), + }) + } + return issues +} + +// hasUnwrappableWord reports whether the line contains a single word longer +// than n runes, which makes wrapping the line at n impossible. +func hasUnwrappableWord(line string, n int) bool { + for _, word := range strings.Fields(line) { + if len([]rune(word)) > n { + return true + } + } + return false +} diff --git a/internal/commitmsg/commitmsg_test.go b/internal/commitmsg/commitmsg_test.go new file mode 100644 index 0000000..403e90c --- /dev/null +++ b/internal/commitmsg/commitmsg_test.go @@ -0,0 +1,280 @@ +package commitmsg + +import ( + "strings" + "testing" +) + +func TestNormalize(t *testing.T) { + tests := []struct { + name string + in string + want string + }{ + { + name: "already canonical is unchanged", + in: "feat(budget): add monthly rollover\n", + want: "feat(budget): add monthly rollover\n", + }, + { + name: "drops trailing period and capital", + in: "fix: Correct the budget rollover.\n", + want: "fix: correct the budget rollover\n", + }, + { + name: "drops multiple trailing periods", + in: "chore: tidy up the makefile...\n", + want: "chore: tidy up the makefile\n", + }, + { + name: "lowercases the type", + in: "Fix: broken parser\n", + want: "fix: broken parser\n", + }, + { + name: "inserts the space after the colon", + in: "docs:explain the hook\n", + want: "docs: explain the hook\n", + }, + { + name: "collapses internal whitespace and trims", + in: " feat(cli): add --timeout flag \n", + want: "feat(cli): add --timeout flag\n", + }, + { + name: "preserves acronyms", + in: "feat: API key rotation\n", + want: "feat: API key rotation\n", + }, + { + name: "preserves identifiers with dots", + in: "docs: README.md gains a hook section\n", + want: "docs: README.md gains a hook section\n", + }, + { + name: "preserves the breaking change marker", + in: "feat(config)!: Rename the provider key.\n", + want: "feat(config)!: rename the provider key\n", + }, + { + name: "strips comments and the verbose diff", + in: "fix: drop the stray log line\n" + + "# Please enter the commit message for your changes.\n" + + "# On branch main\n" + + "# ------------------------ >8 ------------------------\n" + + "diff --git a/x.go b/x.go\n", + want: "fix: drop the stray log line\n", + }, + { + name: "inserts the blank line after the subject", + in: "fix: drop the stray log line\nIt was left over from debugging.\n", + want: "fix: drop the stray log line\n\nIt was left over from debugging.\n", + }, + { + name: "squeezes repeated blank lines", + in: "fix: a\n\n\n\nbody text\n\n\n", + want: "fix: a\n\nbody text\n", + }, + { + name: "preserves trailers verbatim", + in: "chore: wire up the hook\n\nbody text\n" + + "Nightshift-Task: commit-normalize\n" + + "Nightshift-Ref: https://github.com/marcus/nightshift\n" + + "Co-Authored-By: Someone \n", + want: "chore: wire up the hook\n\nbody text\n\n" + + "Nightshift-Task: commit-normalize\n" + + "Nightshift-Ref: https://github.com/marcus/nightshift\n" + + "Co-Authored-By: Someone \n", + }, + { + name: "does not split a paragraph whose last lines look like trailers", + in: "fix: speed up parsing\n\n" + + "The parser rescanned the buffer on every token.\n" + + "Before: 2.1s on the fixture corpus.\n" + + "After: 0.3s.\n", + want: "fix: speed up parsing\n\n" + + "The parser rescanned the buffer on every token.\n" + + "Before: 2.1s on the fixture corpus.\n" + + "After: 0.3s.\n", + }, + { + name: "does not treat a closing prose line as a trailer", + in: "docs: describe the hook\n\nInstall it with make hooks.\nNote: it is opt-in.\n", + want: "docs: describe the hook\n\nInstall it with make hooks.\nNote: it is opt-in.\n", + }, + { + name: "keeps an unknown-key trailer with the block it belongs to", + in: "chore: record the review\n\nbody text\n" + + "Reviewed-by: Someone \n" + + "Co-Authored-By: Someone Else \n", + want: "chore: record the review\n\nbody text\n\n" + + "Reviewed-by: Someone \n" + + "Co-Authored-By: Someone Else \n", + }, + { + name: "leaves fenced code untouched", + in: "docs: show the hook output\n\n```\n\n spaced out\n\n```\n", + want: "docs: show the hook output\n\n```\n\n spaced out\n\n```\n", + }, + { + name: "leaves merge subjects alone", + in: "Merge pull request #17 from someone/branch\n", + want: "Merge pull request #17 from someone/branch\n", + }, + { + name: "leaves revert subjects alone", + in: "Revert \"feat: add the thing.\"\n", + want: "Revert \"feat: add the thing.\"\n", + }, + { + name: "leaves fixup subjects alone", + in: "fixup! feat: add the thing\n", + want: "fixup! feat: add the thing\n", + }, + { + name: "handles CRLF line endings", + in: "fix: windows line endings.\r\n\r\nbody\r\n", + want: "fix: windows line endings\n\nbody\n", + }, + { + name: "an all-comment message normalizes to empty", + in: "# nothing here\n", + want: "", + }, + { + name: "does not rewrap a long body line", + in: "docs: a\n\n" + strings.Repeat("word ", 30) + "\n", + want: "docs: a\n\n" + strings.TrimSpace(strings.Repeat("word ", 30)) + "\n", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := Normalize(tt.in) + if got != tt.want { + t.Errorf("Normalize()\n got: %q\nwant: %q", got, tt.want) + } + if again := Normalize(got); again != got { + t.Errorf("Normalize is not idempotent\nfirst: %q\nsecond: %q", got, again) + } + }) + } +} + +func TestNormalizeIsIdempotentOnLintedOutput(t *testing.T) { + msg := Normalize("Feat(cli): Add the --timeout flag. \n\n\nSome body text.\nNightshift-Task: commit-normalize\n") + if issues := Lint(msg); len(issues) != 0 { + t.Fatalf("normalized message should lint clean, got %v", issues) + } + if got := Normalize(msg); got != msg { + t.Fatalf("not idempotent: %q vs %q", got, msg) + } +} + +func TestLintValid(t *testing.T) { + valid := []string{ + "feat: add the budget rollover\n", + "feat(cli): add the --timeout flag\n", + "fix(config)!: rename the provider key\n", + "docs: README.md gains a hook section\n", + "feat: API key rotation\n", + "chore: wire up the hook\n\nA body paragraph that stays well under the limit.\n\nNightshift-Task: commit-normalize\nNightshift-Ref: https://github.com/marcus/nightshift\n", + "docs: link to the standard\n\nSee\nhttps://github.com/marcus/nightshift/blob/main/docs/commit-messages.md#the-trailer-block\nfor the full write-up.\n", + "docs: show a snippet\n\n```\nthis fenced line is deliberately far longer than the seventy-two column limit\n```\n", + "Merge pull request #17 from someone/branch\n", + "Revert \"feat: add the thing\"\n", + } + for _, msg := range valid { + t.Run(strings.SplitN(msg, "\n", 2)[0], func(t *testing.T) { + if issues := Lint(msg); len(issues) != 0 { + t.Errorf("expected no issues, got %v", issues) + } + }) + } +} + +func TestLintInvalid(t *testing.T) { + tests := []struct { + name string + in string + rule string + }{ + {"empty", "\n#comment\n", "empty-message"}, + {"no type", "just do the thing\n", "missing-type"}, + {"unknown type", "feet: add the thing\n", "unknown-type"}, + {"missing space after colon", "feat:add the thing\n", "missing-space-after-colon"}, + {"empty scope", "feat(): add the thing\n", "empty-scope"}, + {"empty subject", "feat: \n", "empty-subject"}, + {"trailing period", "feat: add the thing.\n", "subject-trailing-period"}, + {"capitalized", "feat: Add the thing\n", "subject-capitalized"}, + {"too long", "feat: " + strings.Repeat("a", MaxSubjectLen) + "\n", "subject-too-long"}, + {"no blank line", "feat: add the thing\nbody starts immediately\n", "missing-blank-line"}, + {"body line too long", "feat: add the thing\n\n" + strings.Repeat("word ", 30) + "\n", "body-line-too-long"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + issues := Lint(tt.in) + if len(issues) == 0 { + t.Fatalf("expected issue %q, got none", tt.rule) + } + for _, issue := range issues { + if issue.Rule == tt.rule { + if issue.Msg == "" { + t.Errorf("issue %q has an empty message", tt.rule) + } + return + } + } + t.Errorf("expected issue %q, got %v", tt.rule, issues) + }) + } +} + +func TestTrailersAreNotLengthChecked(t *testing.T) { + msg := "chore: record the task\n\nNightshift-Ref: https://github.com/marcus/nightshift/blob/main/docs/commit-messages.md#trailers\n" + if issues := Lint(msg); len(issues) != 0 { + t.Fatalf("trailer lines must be exempt from the width limit, got %v", issues) + } + if got := Normalize(msg); got != msg { + t.Fatalf("trailers must survive normalization: %q", got) + } +} + +// A prose line that merely looks like a trailer must not inherit the trailer +// block's exemption from the width limit. +func TestProseLooksLikeTrailerIsStillWidthChecked(t *testing.T) { + long := "Note: " + strings.TrimSpace(strings.Repeat("word ", 30)) + msg := "fix: a thing\n\nsome body text\n" + long + "\n" + issues := Lint(msg) + if len(issues) != 1 || issues[0].Rule != "body-line-too-long" { + t.Fatalf("expected body-line-too-long for a long %q line, got %v", "Note:", issues) + } +} + +// The gap between the recommended wrap width and the enforced limit is +// deliberate; pin it so it is not narrowed by accident. +func TestBodyWidthSlack(t *testing.T) { + body := func(n int) string { + // n runes made of wrappable words. + line := strings.TrimSpace(strings.Repeat("word ", n/5+2)) + return "feat: a thing\n\n" + string([]rune(line)[:n]) + "\n" + } + if issues := Lint(body(WrapBodyAt + 1)); len(issues) != 0 { + t.Errorf("a line just over the wrap width should be tolerated, got %v", issues) + } + if issues := Lint(body(MaxBodyLineLen)); len(issues) != 0 { + t.Errorf("a line at the hard limit should be accepted, got %v", issues) + } + if issues := Lint(body(MaxBodyLineLen + 1)); len(issues) == 0 { + t.Error("a line over the hard limit should be rejected") + } +} + +func TestTypeListIsStable(t *testing.T) { + got := strings.Join(TypeList(), ",") + want := "build,chore,ci,docs,feat,fix,perf,refactor,revert,style,test" + if got != want { + t.Fatalf("TypeList() = %q, want %q", got, want) + } +} diff --git a/scripts/commit-msg.sh b/scripts/commit-msg.sh new file mode 100755 index 0000000..266e529 --- /dev/null +++ b/scripts/commit-msg.sh @@ -0,0 +1,44 @@ +#!/usr/bin/env bash +# commit-msg hook for nightshift +# +# Normalizes the commit message in place, then validates it against the +# standard in docs/commit-messages.md. +# +# Install: make install-hooks +# (or: ln -sf ../../scripts/commit-msg.sh .git/hooks/commit-msg) +# Skip once: git commit --no-verify +set -euo pipefail + +MSG_FILE="${1:?commit-msg hook: expected a message file path}" + +REPO_ROOT="$(git rev-parse --show-toplevel)" +cd "$REPO_ROOT" + +if command -v commitmsg >/dev/null 2>&1; then + run_commitmsg() { command commitmsg "$@"; } +elif command -v go >/dev/null 2>&1; then + # Drop `go run`'s own "exit status N" line so the hook output stays clean. + run_commitmsg() { + local out rc=0 + out=$(go run ./cmd/commitmsg "$@" 2>&1) || rc=$? + if [[ -n "$out" ]]; then + grep -v '^exit status [0-9]*$' <<<"$out" >&2 || true + fi + return $rc + } +else + echo "🪡 commit-msg: neither 'commitmsg' nor 'go' found — skipping the check" >&2 + exit 0 +fi + +run_commitmsg normalize "$MSG_FILE" + +if ! run_commitmsg lint "$MSG_FILE"; then + cat >&2 <<'EOF' + +The commit message does not follow the nightshift standard. +See docs/commit-messages.md, fix the message, and commit again. +To bypass this check once: git commit --no-verify +EOF + exit 1 +fi