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
94 changes: 93 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,97 @@ required := RequiredKey.MustGet(ctx) // Panics with clear message if not set
6. **Boolean keys**: Special `BoolKey` type with Enabled/Disabled/ExplicitlyDisabled methods
7. **Three-state logic**: Distinguish between unset, explicitly true, and explicitly false

## Background: Go Official Proposal

A similar idea was proposed to the Go team in 2021:

- [proposal: context: add generic key and value type #49189](https://github.com/golang/go/issues/49189)

The proposal by [@dsnet](https://github.com/dsnet) (Joe Tsai, a Go team member) suggests:

```go
type Key[Value any] struct { name *string }

func NewKey[Value any](name string) Key[Value] {
return Key[Value]{&name} // Uses argument address for uniqueness
}

func (k Key[V]) WithValue(ctx Context, val V) Context
func (k Key[V]) Value(ctx Context) (V, bool)
```

This package implements essentially the same concept. However, the official proposal has been on hold for over 3 years, primarily because:

1. **Standard library generics policy is undecided** - [Discussion #48287](https://github.com/golang/go/discussions/48287) is still ongoing about how to add generics to existing packages
2. **Migration path unclear** - Whether to deprecate `context.WithValue`/`context.Value` or keep both APIs
3. **Alternative proposals being considered** - Multiple approaches are being evaluated in parallel

This package provides an immediate, production-ready solution while the Go team deliberates.

## Design Decisions

### Sealed Interface Pattern

Unlike the proposal's struct-based approach, this package uses the **Sealed Interface** pattern:

```go
type Key[V any] interface {
WithValue(ctx context.Context, value V) context.Context
Get(ctx context.Context) V
TryGet(ctx context.Context) (V, bool)
// ... other methods
downcast() key[V] // unexported method prevents external implementation
}

type key[V any] struct { // unexported implementation
name string
ident *opaque
}
```

**Why this matters:**

The struct-based approach has a subtle vulnerability. In Go, you can bypass constructor functions and directly initialize structs with zero values for unexported fields:

```go
// With struct-based design:
type Key[V any] struct { name *string }

// This compiles! Both keys have nil name pointer
badKeyX := Key[int]{}
badKeyY := Key[string]{}
// These will COLLIDE because both use (*string)(nil) as identity
```

Note: `(*T)(nil)` doesn't panic like `nil` does - it silently uses the zero value as the key, making collisions hard to detect.

With the Sealed Interface pattern:
- The implementation struct `key[V]` is unexported, preventing direct initialization
- The interface contains an unexported method `downcast()`, preventing external implementations
- Users **must** use `feature.New()` or `feature.NewBool()` to create keys

**Additional benefit:** `BoolKey` can be used anywhere `Key[bool]` is expected, providing better interoperability than struct embedding would allow.

### Empty Struct Optimization Avoidance

The internal `opaque` type that provides pointer identity includes a byte field:

```go
type opaque struct {
_ byte // Prevents address optimization
}
```

Without this, Go's compiler optimization would give all zero-size struct pointers the same address:

```go
type empty struct{}

a := new(empty)
b := new(empty)
fmt.Printf("%p %p\n", a, b) // Same address! Keys would collide.
```

## Best Practices

### 1. Define Keys as Package-Level Variables
Expand Down Expand Up @@ -298,4 +389,5 @@ Contributions are welcome! Please feel free to submit a Pull Request.

## Related Projects

- [context](https://pkg.go.dev/context) - Go's official context package
- [context](https://pkg.go.dev/context) - Go's official context package
- [proposal: context: add generic key and value type #49189](https://github.com/golang/go/issues/49189) - Go official proposal
6 changes: 0 additions & 6 deletions feature.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,12 +303,6 @@ func (k key[V]) String() string {
return k.name
}

// GoString returns a Go syntax representation of the key.
// This implements fmt.GoStringer.
func (k key[V]) GoString() string {
return fmt.Sprintf("feature.Key[%T]{name: %q}", *new(V), k.name)
}

// Inspect retrieves the value from the context and returns an Inspection.
func (k key[V]) Inspect(ctx context.Context) Inspection[V] {
val, ok := k.TryGet(ctx)
Expand Down
50 changes: 0 additions & 50 deletions feature_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -491,56 +491,6 @@ func TestString(t *testing.T) {
})
}

// TestGoString tests the GoString method for keys.
func TestGoString(t *testing.T) {
t.Parallel()

t.Run("Key GoString includes package name and type", func(t *testing.T) {
t.Parallel()

key := feature.NewNamed[string]("test-key")
goStr := key.GoString()

if !strings.Contains(goStr, "feature.Key[string]") {
t.Errorf("GoString() = %q, want to contain %q", goStr, "feature.Key[string]")
}

if !strings.Contains(goStr, "name:") {
t.Errorf("GoString() = %q, want to contain field name %q", goStr, "name:")
}

if !strings.Contains(goStr, "test-key") {
t.Errorf("GoString() = %q, want to contain %q", goStr, "test-key")
}
})

t.Run("Key GoString with int type", func(t *testing.T) {
t.Parallel()

key := feature.NewNamed[int]("max-retries")
goStr := key.GoString()

if !strings.Contains(goStr, "feature.Key[int]") {
t.Errorf("GoString() = %q, want to contain %q", goStr, "feature.Key[int]")
}
})

t.Run("BoolKey GoString includes bool type", func(t *testing.T) {
t.Parallel()

flag := feature.NewNamedBool("my-feature")
goStr := flag.GoString()

if !strings.Contains(goStr, "feature.Key[bool]") {
t.Errorf("GoString() = %q, want to contain %q", goStr, "feature.Key[bool]")
}

if !strings.Contains(goStr, "my-feature") {
t.Errorf("GoString() = %q, want to contain %q", goStr, "my-feature")
}
})
}

// TestNewNamed tests the NewNamed constructor.
func TestNewNamed(t *testing.T) {
t.Parallel()
Expand Down
24 changes: 24 additions & 0 deletions gostring.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package feature

import (
"fmt"
"reflect"
)

// GoString returns a Go syntax representation of the key.
// The output is a valid Go expression that creates an equivalent key
// (though with a different identity).
// This implements fmt.GoStringer.
func (k key[V]) GoString() string {
typeName := reflect.TypeOf((*V)(nil)).Elem().String()

return fmt.Sprintf("feature.New[%s](feature.WithName(%q))", typeName, k.name)
}

// GoString returns a Go syntax representation of the bool key.
// The output is a valid Go expression that creates an equivalent key
// (though with a different identity).
// This implements fmt.GoStringer.
func (k boolKey) GoString() string {
return fmt.Sprintf("feature.NewBool(feature.WithName(%q))", k.name)
}
Loading
Loading