Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .claude-plugin/marketplace.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,14 +5,14 @@
},
"metadata": {
"description": "Coding guidelines, code review commands, and quality agents for Go and Python development",
"version": "0.9.12"
"version": "0.11.0"
},
"plugins": [
{
"name": "coding",
"description": "Coding guidelines, code review commands, and quality agents for Go and Python development",
"source": "./",
"version": "0.9.12",
"version": "0.11.0",
"strict": true
}
]
Expand Down
2 changes: 1 addition & 1 deletion .claude-plugin/plugin.json
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
{
"name": "coding",
"description": "Coding guidelines, code review commands, and quality agents for Go and Python development",
"version": "0.9.12",
"version": "0.11.0",
"author": {
"name": "Benjamin Borbe"
},
Expand Down
3 changes: 1 addition & 2 deletions .github/workflows/ci.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,8 +7,7 @@ on:
branches: [master]

jobs:
precommit:
name: Precommit
test:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
Expand Down
128 changes: 128 additions & 0 deletions docs/go-error-wrapping-guide.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -238,3 +238,131 @@ It("returns ErrNotFound for missing item", func() {
Expect(err).To(MatchError(ErrNotFound))
})
```

### RULE go-errors/no-fmt-errorf (MUST)

**Owner**: go-error-assistant
**Applies when**: any `*.go` file outside `main.go`, `*_test.go`, `vendor/` calls `fmt.Errorf(...)`.
**Enforcement**: `rules/go/no-fmt-errorf.yml`
**Why**: `fmt.Errorf` loses ctx-derived structured data and stack traces. Use `errors.Wrapf(ctx, err, "...")` (wrapping) or `errors.Errorf(ctx, "...", args...)` (new error) from `github.com/bborbe/errors` instead.

#### Bad

```go
return fmt.Errorf("fetch failed: %w", err)
```

#### Good

```go
return errors.Wrapf(ctx, err, "fetch failed")
```

### RULE go-errors/no-bare-return-err (MUST)

**Owner**: go-error-assistant
**Applies when**: a Go `return err` statement appears inside an `if err != nil { ... }` block, outside `*_test.go` and `vendor/`. Inner closures where the outer scope already wraps are an exception — see RULE `go-errors/inner-closure-no-double-wrap`.
**Enforcement**: `rules/go/no-bare-return-err.yml`
**Why**: bare `return err` propagates errors without context or stack trace. Wrap with `errors.Wrapf(ctx, err, "operation description")` at every layer that adds meaning.

#### Bad

```go
if err != nil {
return err // No context, no stack trace
}
```

#### Good

```go
result, err := s.repo.Fetch(ctx, id)
if err != nil {
return nil, errors.Wrapf(ctx, err, "fetch account %s", id)
}
```

### RULE go-errors/no-context-background-in-business-logic (MUST)

**Owner**: go-error-assistant
**Applies when**: a Go `context.Background()` call appears outside `main.go`, `cmd/**`, `*_test.go`, `vendor/`. Top-level goroutine spawners in `main` are exempt by path filter.
**Enforcement**: `rules/go/no-context-background-in-business-logic.yml`
**Why**: `context.Background()` discards any context data the caller added via `errors.AddToContext`, making subsequent wrapping pointless. Add `ctx context.Context` as a function parameter and propagate from callers.

#### Bad

```go
return errors.Wrapf(context.Background(), err, "failed")
// Loses all structured data from caller's context
```

#### Good

```go
func (s *svc) validate(ctx context.Context, input string) error {
return errors.Errorf(ctx, "invalid: %s", input)
}
```

### RULE go-errors/inner-closure-no-double-wrap (SHOULD)

**Owner**: go-error-assistant
**Applies when**: an inner closure (passed to `db.Update`, `filepath.WalkDir`, or similar callback APIs) calls `errors.Wrap`/`errors.Wrapf` while the surrounding function ALSO wraps the closure's return value.
**Enforcement**: judgment
**Why**: double-wrapping inflates error messages with redundant prefixes (`save data X: update: put: bolt: connection refused`) and doesn't add new information. The outer wrap is enough.

#### Bad

```go
err := s.db.Update(func(tx *bolt.Tx) error {
if err := put(tx); err != nil {
return errors.Wrapf(ctx, err, "put") // Redundant — outer wraps too
}
return nil
})
return errors.Wrapf(ctx, err, "update") // Double-wrapped
```

#### Good

```go
func (s *svc) Save(ctx context.Context, data Data) error {
err := s.db.Update(func(tx *bolt.Tx) error {
// Inner closure: bare return is OK here
// The outer Wrapf below will add context
return tx.Bucket(key).Put(id, encoded)
})
if err != nil {
return errors.Wrapf(ctx, err, "save data %s", data.ID)
}
return nil
}
```

### RULE go-errors/sentinel-err-prefix-naming (SHOULD)

**Owner**: go-error-assistant
**Applies when**: a package-level sentinel error variable uses the legacy `XxxError`/`XxxErr` naming convention (e.g. `BucketNotFoundErr`, `ConnectionError`) instead of the stdlib-style `ErrXxx` prefix (`ErrBucketNotFound`, `ErrConnection`).
**Enforcement**: judgment
**Why**: stdlib uses `Err` prefix (`io.EOF`, `sql.ErrNoRows`); matching it makes the convention discoverable and consistent. Legacy projects may keep the old name as a `Deprecated:` alias during transition.

#### Bad

```go
var BucketNotFoundErr = stderrors.New("bucket not found")
```

#### Good

```go
var ErrNotFound = stderrors.New("not found")
```

And during a rename transition:

```go
var ErrBucketNotFound = stderrors.New("bucket not found")

// Deprecated: use ErrBucketNotFound.
var BucketNotFoundErr = ErrBucketNotFound
```
Loading
Loading