diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 18f39a1..f37a51b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,6 +48,8 @@ jobs: with: go-version: 1.26.x cache: true + - name: Check Go modernizations + run: go fix -diff ./... - name: Run golangci-lint uses: golangci/golangci-lint-action@v9 with: diff --git a/README.md b/README.md index bf9239e..4fafd47 100644 --- a/README.md +++ b/README.md @@ -6,10 +6,11 @@ Stacktrace adds compact, contextual call-site information to Go errors. -This repository is an actively maintained fork of -[palantir/stacktrace](https://github.com/palantir/stacktrace), which has been -inactive for many years. The fork preserves the original Apache-2.0-licensed -API while adding current Go tooling and error-chain support. +> [!NOTE] +> This repository is an actively maintained fork of +> [palantir/stacktrace](https://github.com/palantir/stacktrace), which has been +> inactive for many years. The fork preserves the original Apache-2.0-licensed +> API while adding current Go tooling and error-chain support. ## Fork features @@ -22,7 +23,7 @@ API while adding current Go tooling and error-chain support. ## Installation -```sh +```bash go get github.com/NdoleStudio/stacktrace@latest ``` @@ -30,13 +31,13 @@ go get github.com/NdoleStudio/stacktrace@latest This is difficult to debug: -``` +```text Inverse tachyon pulse failed ``` This gives the full story and is easier to debug: -``` +```text Failed to register for villain discovery --- at github.com/palantir/shield/agent/discovery.go:265 (ShieldAgent.reallyRegister) --- --- at github.com/palantir/shield/connector/impl.go:89 (Connector.Register) --- @@ -65,23 +66,22 @@ maximally useful. ## Example Usage - -
+```go
 func WriteAll(baseDir string, entities []Entity) error {
     err := os.MkdirAll(baseDir, 0755)
     if err != nil {
-        return stacktrace.Propagate(err, "Failed to create base directory")
+        return stacktrace.Propagate(err, "Failed to create base directory")
     }
     for _, ent := range entities {
         path := filepath.Join(baseDir, fileNameForEntity(ent))
         err = Write(path, ent)
         if err != nil {
-            return stacktrace.Propagate(err, "Failed to write %v to %s", ent, path)
+            return stacktrace.Propagate(err, "Failed to write %v to %s", ent, path)
         }
     }
     return nil
 }
-
+``` ## Functions @@ -95,12 +95,12 @@ As in all of these functions, the `format` and `args` work like `fmt.Errorf`. The message passed to Propagate should describe the action that failed, resulting in `err`. The canonical call looks like this: -
+```go
 result, err := process(arg)
 if err != nil {
-    return nil, stacktrace.Propagate(err, "Failed to process %v", arg)
+    return nil, stacktrace.Propagate(err, "Failed to process %v", arg)
 }
-
+``` To write the message, ask yourself "what does this call do?" What does `process(arg)` do? It processes ${arg}, so the message is that we failed to @@ -115,15 +115,15 @@ corresponding to the "base directory" so we propagate with that information. If it is not possible to add any useful contextual information beyond what is already included in an error, `format` can be an empty string: -
+```go
 func Something() error {
     mutex.Lock()
     defer mutex.Unlock()
 
     err := reallySomething()
-    return stacktrace.Propagate(err, "")
+    return stacktrace.Propagate(err, "")
 }
-
+``` The purpose of `""` as opposed to a separate function is to make you feel a little guilty every time you do this. @@ -136,11 +136,11 @@ This example also illustrates the behavior of Propagate when `err` is nil NewError is a drop-in replacement for `fmt.Errorf` that includes line number information. The canonical call looks like this: -
+```go
 if !IsOkay(arg) {
-    return stacktrace.NewError("Expected %v to be okay", arg)
+    return stacktrace.NewError("Expected %v to be okay", arg)
 }
-
+``` ### Error Codes @@ -151,7 +151,7 @@ code. The type `stacktrace.ErrorCode` is a typedef for uint16. You name the set of error codes relevant to your application. -``` +```go const ( EcodeManifestNotFound = stacktrace.ErrorCode(iota) EcodeBadInput @@ -171,12 +171,12 @@ An ordinary `stacktrace.Propagate` preserves the error code of an error. PropagateWithCode and NewErrorWithCode are analogous to Propagate and NewError but also attach an error code. -
+```go
 _, err := os.Stat(manifestPath)
 if os.IsNotExist(err) {
-    return stacktrace.PropagateWithCode(err, EcodeManifestNotFound, "")
+    return stacktrace.PropagateWithCode(err, EcodeManifestNotFound, "")
 }
-
+``` #### `stacktrace.NewMessageWithCode(code ErrorCode, format string, args ...any) error` @@ -184,27 +184,27 @@ The error code mechanism can be useful by itself even where stack traces with line numbers are not required. NewMessageWithCode returns an error that prints just like `fmt.Errorf` with no line number, but including a code. -
+```go
 ttl := req.URL.Query().Get("ttl")
 if ttl == "" {
-    return 0, stacktrace.NewMessageWithCode(EcodeBadInput, "Missing ttl query parameter")
+    return 0, stacktrace.NewMessageWithCode(EcodeBadInput, "Missing ttl query parameter")
 }
-
+``` #### `stacktrace.GetCode(err error) ErrorCode` GetCode extracts the error code from an error. -
+```go
 for i := 0; i < attempts; i++ {
     err := Do()
-    if stacktrace.GetCode(err) != EcodeTimeout {
+    if stacktrace.GetCode(err) != EcodeTimeout {
         return err
     }
     // try a few more times
 }
 return stacktrace.NewError("timed out after %d attempts", attempts)
-
+``` GetCode returns the special value `stacktrace.NoCode` if `err` is nil or if there is no error code attached to `err`. @@ -225,7 +225,7 @@ retains the original Palantir copyright notices and attribution. Contributions are welcome. -```sh +```bash go test -race -cover ./... go vet ./... golangci-lint run diff --git a/cleanpath/gopath_test.go b/cleanpath/gopath_test.go index f77bd3b..f08a477 100644 --- a/cleanpath/gopath_test.go +++ b/cleanpath/gopath_test.go @@ -1,7 +1,6 @@ package cleanpath_test import ( - "os" "path/filepath" "strings" "testing" @@ -49,8 +48,7 @@ func TestRemoveGoPath(t *testing.T) { }, } { gopath := strings.Join(testcase.gopath, string(filepath.ListSeparator)) - err := os.Setenv("GOPATH", gopath) - assert.NoError(t, err, "error setting gopath") + t.Setenv("GOPATH", gopath) cleaned := cleanpath.RemoveGoPath(testcase.path) assert.Equal(t, testcase.expected, cleaned, "testcase: %+v", testcase) diff --git a/docs/superpowers/plans/2026-07-18-go-modernization-audit.md b/docs/superpowers/plans/2026-07-18-go-modernization-audit.md new file mode 100644 index 0000000..fb86801 --- /dev/null +++ b/docs/superpowers/plans/2026-07-18-go-modernization-audit.md @@ -0,0 +1,313 @@ +# Go Modernization Audit Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Apply the justified results of a complete repository audit while preserving Go 1.18 compatibility and all public behavior. + +**Architecture:** Keep the linked stacktrace implementation and public API unchanged. Apply the Go 1.26 official modernizer's behavior-preserving formatter rewrite, modernize test environment handling, complete the pending README formatting work, and enforce future official modernizer checks in the existing lint job. + +**Tech Stack:** Go 1.18+, Go 1.26 `go fix`, `testify/assert`, GitHub Actions, golangci-lint v2.12.2, pre-commit + +## Global Constraints + +- Keep `go 1.18` as the module minimum. +- Preserve every exported name, signature, formatting rule, stack frame, error-code rule, nil-propagation behavior, and error-chain behavior. +- Keep path-sensitive tests portable across Linux, Windows, and macOS. +- Add no dependencies. +- Do not overwrite unrelated user changes. + +--- + +### Task 1: Complete README formatting + +**Files:** +- Modify: `README.md` +- Reference: `docs/superpowers/specs/2026-07-18-readme-formatting-design.md` + +**Interfaces:** +- Consumes: Existing README prose, examples, and contributor commands. +- Produces: GitHub-renderable language-specific examples without changing their content. + +- [ ] **Step 1: Record the current formatting gaps** + +Run: + +```bash +rg -n '```$|```sh|
|
|||actively maintained fork' README.md +``` + +Expected: unlabeled or `sh` fences, HTML example tags, and the unquoted fork notice are found. + +- [ ] **Step 2: Apply the approved Markdown containers** + +Change the two-sentence fork notice to: + +```markdown +> [!NOTE] +> This repository is an actively maintained fork of +> [palantir/stacktrace](https://github.com/palantir/stacktrace), which has been +> inactive for many years. The fork preserves the original Apache-2.0-licensed +> API while adding current Go tooling and error-chain support. +``` + +Use `bash` for command blocks, `text` for output, and `go` for Go snippets. +Replace each `
`/`` example with the same source in a fenced `go` block.
+
+- [ ] **Step 3: Verify README structure**
+
+Run:
+
+```bash
+rg -n '```$|```sh|
|
||' README.md +``` + +Expected: no output. Every opening fence is `bash`, `text`, or `go`. + +- [ ] **Step 4: Commit the documentation change** + +```bash +git add README.md +git commit -m "docs: modernize README formatting" +``` + +--- + +### Task 2: Apply the official formatter modernization + +**Files:** +- Modify: `format.go:60-78` +- Test: `format_test.go` + +**Interfaces:** +- Consumes: `fmt.State`, the format verb rune, and the existing rendered stacktrace text. +- Produces: The exact same dynamic format directive passed to `fmt.Fprintf`. + +- [ ] **Step 1: Establish formatter behavior** + +Run: + +```bash +go test -run '^TestFormat$' . +``` + +Expected: PASS. + +- [ ] **Step 2: Preview the official modernization** + +Run: + +```bash +go fix -diff ./... +``` + +Expected: a patch replacing `formatString` concatenation in `format.go` with +`strings.Builder`; exit status is nonzero because a diff exists. + +- [ ] **Step 3: Apply and inspect the official modernization** + +Run: + +```bash +go fix ./... +git diff -- format.go +``` + +Expected: only the dynamic format directive construction changes. It starts +with `var formatString strings.Builder`, writes the same flags, width, +precision, and verb in the same order, and passes `formatString.String()` to +`fmt.Fprintf`. + +- [ ] **Step 4: Verify formatter behavior and modernizer cleanliness** + +Run: + +```bash +go test -run '^TestFormat$' . +go fix -diff ./... +``` + +Expected: the test passes and `go fix` prints no patch. + +- [ ] **Step 5: Commit the formatter modernization** + +```bash +git add format.go +git commit -m "perf: modernize format construction" +``` + +--- + +### Task 3: Modernize clean-path test environment handling + +**Files:** +- Modify: `cleanpath/gopath_test.go:3-52` +- Test: `cleanpath/gopath_test.go` + +**Interfaces:** +- Consumes: Each table row's synthetic `GOPATH`. +- Produces: The same `RemoveGoPath` assertions with automatic environment restoration. + +- [ ] **Step 1: Establish clean-path behavior** + +Run: + +```bash +go test -run '^TestRemoveGoPath$' ./cleanpath +``` + +Expected: PASS. + +- [ ] **Step 2: Replace manual environment mutation** + +Remove the `os` import and replace: + +```go +err := os.Setenv("GOPATH", gopath) +assert.NoError(t, err, "error setting gopath") +``` + +with: + +```go +t.Setenv("GOPATH", gopath) +``` + +- [ ] **Step 3: Format and verify the focused test** + +Run: + +```bash +gofmt -w cleanpath/gopath_test.go +go test -run '^TestRemoveGoPath$' ./cleanpath +``` + +Expected: PASS. + +- [ ] **Step 4: Commit the test modernization** + +```bash +git add cleanpath/gopath_test.go +git commit -m "test: restore GOPATH automatically" +``` + +--- + +### Task 4: Enforce official modernizers in CI + +**Files:** +- Modify: `.github/workflows/ci.yml:39-49` + +**Interfaces:** +- Consumes: The existing Go 1.26 lint environment. +- Produces: A CI failure whenever `go fix -diff ./...` finds a compatible modernization. + +- [ ] **Step 1: Add the modernizer check** + +After the lint job's `Set up Go` step, add: + +```yaml + - name: Check Go modernizations + run: go fix -diff ./... +``` + +Keep the existing golangci-lint action unchanged. + +- [ ] **Step 2: Verify the command and workflow diff** + +Run: + +```bash +go fix -diff ./... +git diff --check +git diff -- .github/workflows/ci.yml +``` + +Expected: `go fix` prints no patch, `git diff --check` is clean, and the +workflow diff contains only the new step. + +- [ ] **Step 3: Commit the CI guard** + +```bash +git add .github/workflows/ci.yml +git commit -m "ci: enforce Go modernizers" +``` + +--- + +### Task 5: Validate and review the complete audit + +**Files:** +- Review: All tracked files +- Modify only if a validation failure is directly caused by Tasks 1-4 + +**Interfaces:** +- Consumes: The complete branch diff and repository quality gates. +- Produces: A clean, review-ready branch. + +- [ ] **Step 1: Verify module and formatting stability** + +Run: + +```bash +go mod tidy +git diff --exit-code -- go.mod go.sum +golangci-lint fmt --diff +go fix -diff ./... +``` + +Expected: all commands succeed without changes. + +- [ ] **Step 2: Run Go validation** + +Run: + +```bash +go test -cover ./... +go vet ./... +golangci-lint run +``` + +Expected: all commands pass. + +- [ ] **Step 3: Run the repository hooks** + +Run: + +```bash +pre-commit run --all-files +``` + +Expected: every hook passes. + +- [ ] **Step 4: Attempt race validation** + +Run: + +```bash +go test -race -cover ./... +``` + +Expected on supported hosts: PASS. On Windows/ARM64, record the Go tool's +documented `-race is not supported on windows/arm64` limitation; the existing +GitHub Actions matrix runs this command on supported hosted runners. + +- [ ] **Step 5: Review the final diff** + +Run: + +```bash +git diff --check origin/main...HEAD +git status --short +git diff --stat origin/main...HEAD +``` + +Expected: no whitespace errors, a clean worktree, and only the planned files. + +- [ ] **Step 6: Push and open the pull request** + +```bash +git push -u origin copilot/go-modernization-audit +gh pr create --base main --head copilot/go-modernization-audit +``` + +Expected: GitHub returns the new pull request URL. diff --git a/docs/superpowers/specs/2026-07-18-go-modernization-audit-design.md b/docs/superpowers/specs/2026-07-18-go-modernization-audit-design.md new file mode 100644 index 0000000..5756ec7 --- /dev/null +++ b/docs/superpowers/specs/2026-07-18-go-modernization-audit-design.md @@ -0,0 +1,72 @@ +# Go Modernization Audit Design + +## Goal + +Perform a fresh line-by-line audit of every tracked repository file and apply +only evidence-backed, backward-compatible modernization changes while +preserving Go 1.18 support. + +## Compatibility + +- Keep `go 1.18` as the module minimum. +- Preserve every exported name, signature, formatting rule, stack frame, + error-code rule, nil-propagation behavior, and error-chain behavior. +- Keep path-sensitive tests portable across Linux, Windows, and macOS. +- Do not introduce dependencies or adopt language or library features newer + than Go 1.18 in production or test code. + +## Approach + +Use Go 1.26.4's official `go fix -diff ./...` modernizers as the primary source +of code changes, then verify each suggestion against the compatibility +constraints and existing tests. Review all other source, test, workflow, +configuration, documentation, dependency, and licensing files manually. +Files without a justified change remain untouched. + +The audit identified four changes: + +1. Apply the official modernizer's `strings.Builder` rewrite to the dynamic + format directive assembled by `(*stacktrace).Format`. +2. Replace manual `os.Setenv` plus assertion handling in the clean-path test + with Go 1.17's `t.Setenv`, which restores `GOPATH` automatically. +3. Implement the already-committed README formatting design so every example + has a language-specific fence and the fork notice uses a GitHub note alert. +4. Run `go fix -diff ./...` in the Go 1.26 CI lint job so future compatible + modernizations cannot silently accumulate. + +## Testing + +- Use the existing formatter table as the regression suite for the + behavior-preserving builder rewrite. +- Run `TestRemoveGoPath` after changing environment setup. +- Run `go fix -diff ./...` and require an empty diff. +- Run `go test -cover ./...`, `go vet ./...`, golangci-lint formatting and + analysis, and pre-commit. +- Attempt the repository's race-enabled suite. On hosts where the Go race + detector is unsupported, rely on the existing GitHub Actions matrix for + race execution and run the complete suite locally without `-race`. + +## File Impact + +Modify: + +- `README.md` +- `format.go` +- `cleanpath/gopath_test.go` +- `.github/workflows/ci.yml` + +Create: + +- `docs/superpowers/specs/2026-07-18-go-modernization-audit-design.md` +- `docs/superpowers/plans/2026-07-18-go-modernization-audit.md` + +All other tracked files were reviewed and require no change. + +## Acceptance Checks + +1. `go fix -diff ./...` reports no patch with Go 1.26. +2. `go test -cover ./...` and `go vet ./...` pass. +3. `golangci-lint fmt --diff` and `golangci-lint run` pass. +4. `pre-commit run --all-files` passes. +5. README fences and the fork note match the existing formatting design. +6. The final diff contains no exported API or observable formatting changes. diff --git a/format.go b/format.go index 51816d8..7b566dd 100644 --- a/format.go +++ b/format.go @@ -57,22 +57,23 @@ func (st *stacktrace) Format(f fmt.State, c rune) { }[DefaultFormat](st) } - formatString := "%" + var formatString strings.Builder + formatString.WriteString("%") // keep the flags recognized by fmt package for _, flag := range "-+# 0" { if f.Flag(int(flag)) { - formatString += string(flag) + formatString.WriteString(string(flag)) } } if width, has := f.Width(); has { - formatString += fmt.Sprint(width) + _, _ = fmt.Fprint(&formatString, width) } if precision, has := f.Precision(); has { - formatString += "." - formatString += fmt.Sprint(precision) + formatString.WriteString(".") + _, _ = fmt.Fprint(&formatString, precision) } - formatString += string(c) - _, _ = fmt.Fprintf(f, formatString, text) + formatString.WriteString(string(c)) + _, _ = fmt.Fprintf(f, formatString.String(), text) } func formatFull(st *stacktrace) string {