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
37 changes: 5 additions & 32 deletions MODERNIZATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,36 +16,9 @@ Replaced all 8 occurrences of `multierr.Append` with `errors.Join` in `proteus.g

---

## 3. Replace `jonbodner/stackerr` with `fmt.Errorf` and `errors.New`
## ~~3. Replace `jonbodner/stackerr` with `fmt.Errorf` and `errors.New`~~ (DONE)

The `stackerr` package provides stack-trace-annotated errors — a pattern from before Go 1.13 added `%w` wrapping. Modern Go uses:

- `errors.New("message")` for simple sentinel errors
- `fmt.Errorf("context: %w", err)` for wrapping with context

**Files affected:** `proteus.go`, `builder.go`, `runner.go`, `mapper/mapper.go`, `mapper/extract.go`

**Before:**
```go
return stackerr.New("not a pointer")
return stackerr.Errorf("no query found for name %s", name)
```

**After:**
```go
return errors.New("not a pointer")
return fmt.Errorf("no query found for name %s", name)
```

For places where `stackerr` wraps an existing error, use `%w`:
```go
// Before
return nil, stackerr.Errorf("invalid index: %s :%w", path[1], err)
// After
return nil, fmt.Errorf("invalid index: %s: %w", path[1], err)
```

This removes the `jonbodner/stackerr` dependency entirely.
Replaced all `stackerr.New(...)` calls with `errors.New(...)` and all `stackerr.Errorf(...)` calls with `fmt.Errorf(...)` across 11 files (6 production, 5 test). Removed the `jonbodner/stackerr` dependency from `go.mod`.

---

Expand Down Expand Up @@ -198,10 +171,10 @@ Several dependencies are significantly out of date:
| `google/go-cmp` | v0.4.0 | v0.6+ | Test-only dep |
| `jonbodner/dbtimer` | 2017 commit | - | Pinned to a 2017 commit hash |
| ~~`jonbodner/multierr`~~ | ~~v1.0.0~~ | - | ~~Replaced with `errors.Join` (see #2)~~ *(DONE)* |
| `jonbodner/stackerr` | v1.0.0 | - | Replace with stdlib (see #3) |
| ~~`jonbodner/stackerr`~~ | ~~v1.0.0~~ | - | ~~Replaced with stdlib (see #3)~~ *(DONE)* |
| `pkg/profile` | v1.7.0 | - | Only used in `speed/speed.go`; consider removing or moving to a build-tagged file |

After removing `multierr` and `stackerr`, the dependency list shrinks significantly.
After removing `multierr` and `stackerr` (both done), the dependency list has shrunk significantly.

---

Expand Down Expand Up @@ -391,7 +364,7 @@ If `Build` returns an error, `productDao` will have nil function fields. Subsequ
**Medium priority (idiomatic modernization):**
- ~~#1 — `interface{}` to `any`~~ *(DONE)*
- ~~#2 — Replace `multierr` with `errors.Join`~~ *(DONE)*
- #3 — Replace `stackerr` with stdlib error handling
- ~~#3 — Replace `stackerr` with stdlib error handling~~ *(DONE)*
- ~~#4 — Fix slog usage for proper structured logging~~ *(DONE)*
- #11 — Update dependencies

Expand Down
27 changes: 13 additions & 14 deletions builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"database/sql/driver"

"github.com/jonbodner/proteus/mapper"
"github.com/jonbodner/stackerr"
)

func buildNameOrderMap(paramOrder string, startPos int) map[string]int {
Expand Down Expand Up @@ -87,7 +86,7 @@ func buildFixedQueryAndParamOrder(ctx context.Context, query string, nameOrderMa
if inVar {
if curVar.Len() == 0 {
//error! must have a something
return nil, nil, stackerr.Errorf("empty variable declaration at position %d", k)
return nil, nil, fmt.Errorf("empty variable declaration at position %d", k)
}
curVarS := curVar.String()
id, err := validIdentifier(ctx, curVarS)
Expand All @@ -110,13 +109,13 @@ func buildFixedQueryAndParamOrder(ctx context.Context, query string, nameOrderMa
paramType := funcType.In(paramPos)
if len(path) > 1 {
if paramType == nil {
return nil, nil, stackerr.Errorf("query Parameter %s has a path, but the incoming parameter is nil", paramName)
return nil, nil, fmt.Errorf("query Parameter %s has a path, but the incoming parameter is nil", paramName)
}
switch paramType.Kind() {
case reflect.Map, reflect.Struct:
//do nothing
default:
return nil, nil, stackerr.Errorf("query Parameter %s has a path, but the incoming parameter is not a map or a struct it is %s", paramName, paramType.Kind())
return nil, nil, fmt.Errorf("query Parameter %s has a path, but the incoming parameter is not a map or a struct it is %s", paramName, paramType.Kind())
}
}
pathType, err := mapper.ExtractType(ctx, paramType, path)
Expand All @@ -132,7 +131,7 @@ func buildFixedQueryAndParamOrder(ctx context.Context, query string, nameOrderMa
}
paramOrder = append(paramOrder, paramInfo{id, paramPos, isSlice})
} else {
return nil, nil, stackerr.Errorf("query Parameter %s cannot be found in the incoming parameters", paramName)
return nil, nil, fmt.Errorf("query Parameter %s cannot be found in the incoming parameters", paramName)
}

inVar = false
Expand All @@ -149,7 +148,7 @@ func buildFixedQueryAndParamOrder(ctx context.Context, query string, nameOrderMa
}
}
if inVar {
return nil, nil, stackerr.Errorf("missing a closing : somewhere: %s", query)
return nil, nil, fmt.Errorf("missing a closing : somewhere: %s", query)
}

queryString := out.String()
Expand Down Expand Up @@ -233,7 +232,7 @@ func addSlice(sliceName string) string {

func validIdentifier(ctx context.Context, curVar string) (string, error) {
if strings.Contains(curVar, ";") {
return "", stackerr.Errorf("; is not allowed in an identifier: %s", curVar)
return "", fmt.Errorf("; is not allowed in an identifier: %s", curVar)
}
curVarB := []byte(curVar)

Expand All @@ -254,7 +253,7 @@ loop:
switch tok {
case token.EOF:
if first || lastPeriod {
return "", stackerr.Errorf("identifiers cannot be empty or end with a .: %s", curVar)
return "", fmt.Errorf("identifiers cannot be empty or end with a .: %s", curVar)
}
break loop
case token.SEMICOLON:
Expand All @@ -263,15 +262,15 @@ loop:
continue
case token.IDENT:
if !first && !lastPeriod && !lastFloat {
return "", stackerr.Errorf(". missing between parts of an identifier: %s", curVar)
return "", fmt.Errorf(". missing between parts of an identifier: %s", curVar)
}
first = false
lastPeriod = false
lastFloat = false
identifier += lit
case token.PERIOD:
if first || lastPeriod {
return "", stackerr.Errorf("identifier cannot start with . or have two . in a row: %s", curVar)
return "", fmt.Errorf("identifier cannot start with . or have two . in a row: %s", curVar)
}
lastPeriod = true
identifier += "."
Expand All @@ -283,10 +282,10 @@ loop:
first = false
continue
}
return "", stackerr.Errorf("invalid character found in identifier: %s", curVar)
return "", fmt.Errorf("invalid character found in identifier: %s", curVar)
case token.INT:
if !dollar || first {
return "", stackerr.Errorf("invalid character found in identifier: %s", curVar)
return "", fmt.Errorf("invalid character found in identifier: %s", curVar)
}
identifier += lit
if dollar {
Expand All @@ -300,7 +299,7 @@ loop:
// returns .0 as the lit value
//Only valid for $ notation and array/slice references.
if first {
return "", stackerr.Errorf("invalid character found in identifier: %s", curVar)
return "", fmt.Errorf("invalid character found in identifier: %s", curVar)
}
identifier += lit
if dollar {
Expand All @@ -311,7 +310,7 @@ loop:
lastPeriod = true
}
default:
return "", stackerr.Errorf("invalid character found in identifier: %s", curVar)
return "", fmt.Errorf("invalid character found in identifier: %s", curVar)
}
}
return identifier, nil
Expand Down
1 change: 0 additions & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
version: "3"
services:
db:
image: "postgres:11"
Expand Down
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ require (
github.com/go-sql-driver/mysql v1.5.0
github.com/google/go-cmp v0.6.0
github.com/jonbodner/dbtimer v0.0.0-20170410163237-7002f3758ae1
github.com/jonbodner/stackerr v1.0.0
github.com/lib/pq v1.10.9
github.com/pkg/profile v1.7.0
github.com/rickar/props v1.0.0
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ github.com/google/pprof v0.0.0-20230429030804-905365eefe3e/go.mod h1:79YE0hCXdHa
github.com/ianlancetaylor/demangle v0.0.0-20210905161508-09a460cdf81d/go.mod h1:aYm2/VgdVmcIU8iMfdMvDMsRAQjcfZSKFby6HOFvi/w=
github.com/jonbodner/dbtimer v0.0.0-20170410163237-7002f3758ae1 h1:mgFL7UFb88FOlSVgVoIRGJ4yKlkfp8KcXHqy7no+lEU=
github.com/jonbodner/dbtimer v0.0.0-20170410163237-7002f3758ae1/go.mod h1:PjOlFbeJKs+4b2CvuN9FFF8Ed8cZ6FHWPb5tLK2QKOM=
github.com/jonbodner/stackerr v1.0.0 h1:rAe+Fh13cfC9IGuKE4YWiVCzwt9zce9Saldpc8fYEIM=
github.com/jonbodner/stackerr v1.0.0/go.mod h1:In1ShJr570PDuDHbYfymEQle+H7PgY9KpT+alyk0nEM=
github.com/lib/pq v1.10.9 h1:YXG7RB+JIjhP29X+OtkiDnYaXQwpS4JEWq7dtCCRUEw=
github.com/lib/pq v1.10.9/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
github.com/pkg/profile v1.7.0 h1:hnbDkaNWPCLMO9wGLdBFTIZvzDrDfBM2072E1S9gJkA=
Expand Down
27 changes: 14 additions & 13 deletions mapper/extract.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,18 @@ package mapper
import (
"context"
"database/sql/driver"
"fmt"
"log/slog"
"reflect"
"strconv"

"github.com/jonbodner/stackerr"
"errors"
)

func ExtractType(ctx context.Context, curType reflect.Type, path []string) (reflect.Type, error) {
// error case path length == 0
if len(path) == 0 {
return nil, stackerr.New("cannot extract type; no path remaining")
return nil, errors.New("cannot extract type; no path remaining")
}
ss := fromPtrType(curType)
// base case path length == 1
Expand All @@ -22,7 +23,7 @@ func ExtractType(ctx context.Context, curType reflect.Type, path []string) (refl
}
// length > 1, find a match for path[1], and recurse
if ss == nil {
return nil, stackerr.New("cannot find the type for the subfield of a nil")
return nil, errors.New("cannot find the type for the subfield of a nil")
}
switch ss.Kind() {
case reflect.Map:
Expand All @@ -33,23 +34,23 @@ func ExtractType(ctx context.Context, curType reflect.Type, path []string) (refl
if f, exists := ss.FieldByName(path[1]); exists {
return ExtractType(ctx, f.Type, path[1:])
}
return nil, stackerr.New("cannot find the type; no such field " + path[1])
return nil, errors.New("cannot find the type; no such field " + path[1])
case reflect.Array, reflect.Slice:
// handle slices and arrays
_, err := strconv.Atoi(path[1])
if err != nil {
return nil, stackerr.Errorf("invalid index: %s :%w", path[1], err)
return nil, fmt.Errorf("invalid index: %s :%w", path[1], err)
}
return ExtractType(ctx, ss.Elem(), path[1:])
default:
return nil, stackerr.New("cannot find the type for the subfield of anything other than a map, struct, slice, or array")
return nil, errors.New("cannot find the type for the subfield of anything other than a map, struct, slice, or array")
}
}

func Extract(ctx context.Context, s any, path []string) (any, error) {
// error case path length == 0
if len(path) == 0 {
return nil, stackerr.New("cannot extract value; no path remaining")
return nil, errors.New("cannot extract value; no path remaining")
}
// base case path length == 1
if len(path) == 1 {
Expand All @@ -65,19 +66,19 @@ func Extract(ctx context.Context, s any, path []string) (any, error) {
switch sv.Kind() {
case reflect.Map:
if sv.Type().Key().Kind() != reflect.String {
return nil, stackerr.New("cannot extract value; map does not have a string key")
return nil, errors.New("cannot extract value; map does not have a string key")
}
slog.DebugContext(ctx, "map extract", "key", path[1], "availableKeys", sv.MapKeys())
v := sv.MapIndex(reflect.ValueOf(path[1]))
slog.DebugContext(ctx, "map extract result", "value", v)
if !v.IsValid() {
return nil, stackerr.New("cannot extract value; no such map key " + path[1])
return nil, errors.New("cannot extract value; no such map key " + path[1])
}
return Extract(ctx, v.Interface(), path[1:])
case reflect.Struct:
//make sure the field exists
if _, exists := sv.Type().FieldByName(path[1]); !exists {
return nil, stackerr.New("cannot extract value; no such field " + path[1])
return nil, errors.New("cannot extract value; no such field " + path[1])
}

v := sv.FieldByName(path[1])
Expand All @@ -86,15 +87,15 @@ func Extract(ctx context.Context, s any, path []string) (any, error) {
// handle slices and arrays
pos, err := strconv.Atoi(path[1])
if err != nil {
return nil, stackerr.Errorf("invalid index: %s :%w", path[1], err)
return nil, fmt.Errorf("invalid index: %s :%w", path[1], err)
}
if pos < 0 || pos >= sv.Len() {
return nil, stackerr.Errorf("invalid index: %s", path[1])
return nil, fmt.Errorf("invalid index: %s", path[1])
}
v := sv.Index(pos)
return Extract(ctx, v.Interface(), path[1:])
default:
return nil, stackerr.New("cannot extract value; only maps and structs can have contained values")
return nil, errors.New("cannot extract value; only maps and structs can have contained values")
}
}

Expand Down
5 changes: 3 additions & 2 deletions mapper/extract_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ import (
"reflect"
"testing"

"errors"

"github.com/jonbodner/proteus/cmp"
"github.com/jonbodner/stackerr"
)

func TestExtractPointer(t *testing.T) {
Expand Down Expand Up @@ -123,7 +124,7 @@ func TestExtractFail(t *testing.T) {
if err == nil {
t.Errorf("Expected an error %s, got none", msg)
}
eExp := stackerr.New(msg)
eExp := errors.New(msg)
if !cmp.Errors(err, eExp) {
t.Errorf("Expected error %s, got %s", eExp, err)
}
Expand Down
Loading
Loading