diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 0000000..74b5497 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,26 @@ +# Copilot Instructions + +Project purpose +- Provide a small utility to bind struct fields to pflag flags for CLI programs. + +Design principles +- Small, dependency-minimal, and well-tested. +- Keep public API stable and backward-compatible. + +Error handling policy +- Use sentinel errors where appropriate and ensure `errors.Is` works. +- Guard nil-sensitive inputs and return clear errors. + +Testing / validation +- Local validation command: `task test` +- Prefer deterministic tests using `httptest` or mocked `http.RoundTripper`. + +Release process +- Tag from `main`/`master` with semver `vMAJOR.MINOR.PATCH`. +- Annotated tags: `git tag -a vX.Y.Z -m "Release vX.Y.Z"`. +- Push tag and create GitHub release with `gh release create`. + +Operational pattern +1. Create focused branch per topic. +2. Make small, scoped changes and run `task test`. +3. Push branch, open PR, and merge after CI passes. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..dc0d6f1 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,44 @@ +name: ci + +on: + push: + branches: + - master + pull_request: + +permissions: + contents: read + +jobs: + test-and-lint: + name: lint and test + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + + - uses: actions/setup-go@v6 + with: + go-version-file: go.mod + cache-dependency-path: go.sum + + - name: golangci-lint + uses: golangci/golangci-lint-action@v9 + with: + version: latest + args: --enable gosec + + - name: Test module + run: go test -shuffle on ./... + + govulncheck: + name: govulncheck + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + + - name: Run govulncheck + uses: golang/govulncheck-action@v1 + with: + go-version-file: go.mod + go-package: ./... + repo-checkout: false diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml new file mode 100644 index 0000000..28b32f6 --- /dev/null +++ b/.github/workflows/codeql.yml @@ -0,0 +1,35 @@ +name: CodeQL + +on: + push: + branches: + - master + pull_request: + branches: + - master + schedule: + - cron: "0 20 * * 0" + +permissions: + actions: read + contents: read + security-events: write + +jobs: + analyze: + name: Analyze + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v6 + + - name: Initialize CodeQL + uses: github/codeql-action/init@v3 + with: + languages: go + + - name: Autobuild + uses: github/codeql-action/autobuild@v3 + + - name: Perform CodeQL analysis + uses: github/codeql-action/analyze@v3 diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml deleted file mode 100644 index 26b63ce..0000000 --- a/.github/workflows/lint.yml +++ /dev/null @@ -1,50 +0,0 @@ -name: lint -on: - push: - branches: - - master - pull_request: - -permissions: - contents: read - # Optional: allow read access to pull request. Use with `only-new-issues` option. - # pull-requests: read -jobs: - golangci: - name: lint - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v5 - - uses: actions/setup-go@v6 - with: - go-version-file: 'go.mod' - - name: golangci-lint - uses: golangci/golangci-lint-action@v9 - with: - # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version - version: latest - - # Optional: working directory, useful for monorepos - # working-directory: somedir - - # Optional: golangci-lint command line arguments. - args: --enable gosec - - # Optional: show only new issues if it's a pull request. The default value is `false`. - # only-new-issues: true - - # Optional: if set to true then the all caching functionality will be complete disabled, - # takes precedence over all other caching options. - # skip-cache: true - - # Optional: if set to true then the action don't cache or restore ~/go/pkg. - # skip-pkg-cache: true - - # Optional: if set to true then the action don't cache or restore ~/.cache/go-build. - # skip-build-cache: true - - name: testing - run: go test -shuffle on ./... - - name: install govulncheck - run: go install golang.org/x/vuln/cmd/govulncheck@latest - - name: running govulncheck - run: govulncheck ./... diff --git a/README.md b/README.md index b4b51b0..387c0c3 100644 --- a/README.md +++ b/README.md @@ -1,14 +1,34 @@ # [struct2pflag] -[![lint status](https://github.com/goark/struct2pflag/workflows/lint/badge.svg)](https://github.com/goark/struct2pflag/actions) +[![ci status](https://github.com/goark/struct2pflag/actions/workflows/ci.yml/badge.svg)](https://github.com/goark/struct2pflag/actions/workflows/ci.yml) +[![codeql status](https://github.com/goark/struct2pflag/actions/workflows/codeql.yml/badge.svg)](https://github.com/goark/struct2pflag/actions/workflows/codeql.yml) [![GitHub license](http://img.shields.io/badge/license-MIT-blue.svg)](https://raw.githubusercontent.com/goark/struct2pflag/master/LICENSE) -[![GitHub release](http://img.shields.io/github/release/goark/struct2pflag.svg)](https://github.com/goark/mt/releases/latest) +[![GitHub release](http://img.shields.io/github/release/goark/struct2pflag.svg)](https://github.com/goark/struct2pflag/releases/latest) [![Go reference](https://pkg.go.dev/badge/github.com/goark/struct2pflag.svg)](https://pkg.go.dev/github.com/goark/struct2pflag) +Design goals + +- Provide a minimal, dependency-light helper to bind struct fields to pflag flags. +- Maintain backwards compatibility for public API and use clear sentinel errors. + +Development + +- Local validation: `task test` (preferred). Include `gofmt`, `go vet`, and `go test` in CI. +- Keep changes small and topic-focused; open one PR per change. + +CI Workflows + +- Keep two primary workflows in `.github/workflows/`: `ci.yml` and `codeql.yml`. +- Update README badges if workflow names change. + +Usage + [`struct2pflag`][struct2pflag] automatically registers struct fields as flags for your Go command-line programs. +Supported types in `Bind` include `bool`, `int`, `uint`, `float32`, `float64`, `string`, and `time.Duration`. + (forked from [hymkor/struct2flag](https://github.com/hymkor/struct2flag)) Minimal example @@ -71,6 +91,32 @@ N=1 S="foo" ``` +Environment variable binding +---------------------------- + +`struct2pflag.BindEnv` can apply environment variables to your config struct before CLI parsing. + +```go +type Env struct { + TopN int `env:"TOP_N" pflag:"top-n,n,number of top tags"` + Wait time.Duration `env:"WAIT" pflag:"wait,w,wait duration"` +} + +func main() { + env := Env{TopN: 10, Wait: time.Second} + + _ = struct2pflag.BindEnv(&env) + struct2pflag.BindDefault(&env) + pflag.Parse() +} +``` + +Recommended precedence is: + +1. CLI flags +2. Environment variables +3. Struct default values + Reading default values from JSON and overriding them with command-line flags ---------------------------------------------------------------------------- diff --git a/Taskfile.yml b/Taskfile.yml index c70e045..a0b44b8 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -5,7 +5,7 @@ tasks: cmds: - task: prepare - task: test - # - task: nancy + - task: govulncheck - task: graph test: @@ -18,10 +18,10 @@ tasks: - ./go.mod - '**/*.go' - nancy: - desc: Check vulnerability of external packages with Nancy. + govulncheck: + desc: Check reachable vulnerabilities with latest govulncheck. cmds: - - depm list -j | nancy sleuth -n + - go run golang.org/x/vuln/cmd/govulncheck@latest ./... sources: - ./go.mod - '**/*.go' diff --git a/dependency.png b/dependency.png index 82d12ea..0bcec86 100644 Binary files a/dependency.png and b/dependency.png differ diff --git a/docs/struct2pflag-env-bind-spec.md b/docs/struct2pflag-env-bind-spec.md new file mode 100644 index 0000000..79937dc --- /dev/null +++ b/docs/struct2pflag-env-bind-spec.md @@ -0,0 +1,139 @@ +# struct2pflag: Additional Env Binding Specification + +## Background + +- We want to support environment variable-based configuration such as `TOP_N` in `tagtools`. +- Currently, `github.com/goark/struct2pflag` binds struct fields to CLI flags. +- To enable reuse across multiple tools, environment variable binding should also be implemented in `struct2pflag`. + +## Goals + +- Add support in `struct2pflag` for binding environment variables to struct fields. +- Standardize conversion rules and unify the behavior among flags, environment variables, and struct defaults. +- Include `time.Duration` conversion in the initial implementation. + +## Scope + +- Add functionality to `struct2pflag` itself: + - API to apply environment variables to struct fields + - Conversion error handling + - Implementation of supported types + - Unit tests +- Integrating this in `tagtools` will be handled in a separate PR/session. + +## Out of Scope + +- Full application on the `tagtools` side (not implemented in this session) +- Breaking compatibility of the existing `Bind` API +- Expanding to complex nested struct rules or custom tag specifications beyond the minimum needed + +## Precedence Rules (Consumer Guide) + +Recommended precedence: + +1. CLI flags +2. Environment variables +3. Struct default values + +Note: env binding in `struct2pflag` is responsible only for applying values to the struct. Final precedence is controlled by call order. +Example: `default -> env bind -> flag parse` + +## Proposed API + +### Minimal API + +- `func BindEnv(v any, opts ...EnvOpt) error` + +`v` must be a pointer to a struct. + +### Option Proposals + +- `WithEnvPrefix(prefix string)` + - Example: prepend `APP_` +- `WithEnvTag(tag string)` + - Default tag key: `env` +- `WithStrict(bool)` + - When `strict=true`, unsupported types and invalid values become errors + +## Tag Specification Proposal + +- Explicit declaration such as `env:"TOP_N"` +- When no tag is specified, either: + - A: Ignore the field (safe) + - B: Auto-generate from the field name (`TopN` -> `TOP_N`) + +Start with A (explicit tags) as the recommended policy. + +## Supported Types (Initial) + +- `string` +- `bool` +- `int`, `int8`, `int16`, `int32`, `int64` +- `uint`, `uint8`, `uint16`, `uint32`, `uint64` +- `float32`, `float64` +- `time.Duration` + +### Duration Conversion + +- Use `time.ParseDuration` +- Examples: `100ms`, `5s`, `3m`, `1h30m` + +## Error Handling + +- Environment variable is unset: skip (no error) +- Environment variable is set but conversion fails: return error +- Error messages should include at least: + - Environment variable name + - Field name + - Expected type + - Original raw value + +## Implementation Strategy (Recommended) + +- Iterate struct fields with reflection +- Process only settable/exported fields +- Organize per-type conversion handlers in an internal table-like structure +- Keep room for future converter-registration API extensions + +## Test Cases (struct2pflag side) + +### Success Cases + +- Correctly set values for each supported type +- Preserve defaults when env variables are unset +- Correct duration conversion behavior + +### Failure Cases + +- Error when assigning non-numeric string to int +- Error for invalid bool value +- Error for invalid duration value +- Error when input is not a struct pointer + +### Boundary Cases + +- Behavior for empty-string env values (documented per type) +- Behavior differences with and without strict option + +## tagtools Integration Image (Next Session) + +- Target: `TopN` in `toptags` +- Example: + - `TopN int `env:"TOP_N" pflag:"top-n,n,number of top tags"`` + - `default -> BindEnv(&cfg) -> struct2pflag.Bind(fs, &cfg) -> fs.Parse(args)` +- Expected behavior: + - `TOP_N` overrides defaults when set + - `--top-n` takes precedence over `TOP_N` when provided + +## Compatibility Considerations + +- No impact on existing `Bind` call sites +- New API is additive only +- Existing users see no behavior change unless env binding is explicitly used + +## Definition of Done + +- Implement env binding API and duration conversion in `struct2pflag` +- Unit tests pass +- Add usage notes to README/package docs +- Verify PoC integration (`TOP_N`) in `tagtools` with a separate PR diff --git a/env.go b/env.go new file mode 100644 index 0000000..69ba0e7 --- /dev/null +++ b/env.go @@ -0,0 +1,191 @@ +package struct2pflag + +import ( + "errors" + "fmt" + "os" + "reflect" + "strconv" + "time" +) + +var ( + // ErrInvalidEnvTarget indicates BindEnv received a non-struct pointer target. + ErrInvalidEnvTarget = errors.New("invalid env bind target") + // ErrEnvParse indicates a conversion error while applying an environment variable. + ErrEnvParse = errors.New("failed to parse environment variable") + // ErrUnsupportedEnvType indicates the target field type is not supported by BindEnv. + ErrUnsupportedEnvType = errors.New("unsupported env field type") +) + +// EnvOpt configures BindEnv behavior. +type EnvOpt func(*envOptions) + +type envOptions struct { + prefix string + tag string + strict bool +} + +var durationType = reflect.TypeOf(time.Duration(0)) + +func defaultEnvOptions() envOptions { + return envOptions{tag: "env"} +} + +// WithEnvPrefix adds a prefix to environment variable names, such as APP_. +func WithEnvPrefix(prefix string) EnvOpt { + return func(o *envOptions) { + o.prefix = prefix + } +} + +// WithEnvTag changes the struct tag key used to resolve environment names. +func WithEnvTag(tag string) EnvOpt { + return func(o *envOptions) { + if tag != "" { + o.tag = tag + } + } +} + +// WithStrict enables errors for unsupported tagged field types. +func WithStrict(strict bool) EnvOpt { + return func(o *envOptions) { + o.strict = strict + } +} + +// BindEnv applies environment variables to exported fields of a struct pointer. +// +// By default, BindEnv reads the env tag (for example, env:"TOP_N"). If the +// environment variable exists, it is converted and assigned to the field. +// Unset environment variables are ignored. +func BindEnv(v any, opts ...EnvOpt) error { + if v == nil { + return fmt.Errorf("%w: expected pointer to struct", ErrInvalidEnvTarget) + } + rv := reflect.ValueOf(v) + if rv.Kind() != reflect.Pointer || rv.IsNil() || rv.Elem().Kind() != reflect.Struct { + return fmt.Errorf("%w: expected pointer to struct", ErrInvalidEnvTarget) + } + + config := defaultEnvOptions() + for _, opt := range opts { + if opt != nil { + opt(&config) + } + } + + return bindEnvStruct(rv.Elem(), config) +} + +func bindEnvStruct(v reflect.Value, opts envOptions) error { + t := v.Type() + for i := range v.NumField() { + f := v.Field(i) + field := t.Field(i) + if !field.IsExported() { + continue + } + + switch f.Kind() { + case reflect.Struct: + if err := bindEnvStruct(f, opts); err != nil { + return err + } + case reflect.Pointer: + if f.Type().Elem().Kind() == reflect.Struct && !f.IsNil() { + if err := bindEnvStruct(f.Elem(), opts); err != nil { + return err + } + } + } + + envName, ok := field.Tag.Lookup(opts.tag) + if !ok || envName == "" { + continue + } + envName = opts.prefix + envName + + raw, found := os.LookupEnv(envName) + if !found { + continue + } + if err := assignFromEnv(f, field.Name, envName, raw, opts.strict); err != nil { + return err + } + } + return nil +} + +func assignFromEnv(f reflect.Value, fieldName, envName, raw string, strict bool) error { + if !f.CanSet() { + if strict { + return fmt.Errorf("%w: env %q for field %q cannot be assigned", ErrUnsupportedEnvType, envName, fieldName) + } + return nil + } + + // time.Duration has underlying kind int64, so we must detect it by exact type + // before the generic integer conversion path. + if f.Type() == durationType { + d, err := time.ParseDuration(raw) + if err != nil { + return parseError(envName, fieldName, "time.Duration", raw, err) + } + f.SetInt(int64(d)) + return nil + } + + switch f.Kind() { + case reflect.String: + f.SetString(raw) + return nil + case reflect.Bool: + value, err := strconv.ParseBool(raw) + if err != nil { + return parseError(envName, fieldName, "bool", raw, err) + } + f.SetBool(value) + return nil + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + value, err := strconv.ParseInt(raw, 10, f.Type().Bits()) + if err != nil { + return parseError(envName, fieldName, f.Type().String(), raw, err) + } + f.SetInt(value) + return nil + case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: + value, err := strconv.ParseUint(raw, 10, f.Type().Bits()) + if err != nil { + return parseError(envName, fieldName, f.Type().String(), raw, err) + } + f.SetUint(value) + return nil + case reflect.Float32, reflect.Float64: + value, err := strconv.ParseFloat(raw, f.Type().Bits()) + if err != nil { + return parseError(envName, fieldName, f.Type().String(), raw, err) + } + f.SetFloat(value) + return nil + default: + if strict { + return fmt.Errorf("%w: env %q for field %q has unsupported type %s", ErrUnsupportedEnvType, envName, fieldName, f.Type()) + } + return nil + } +} + +func parseError(envName, fieldName, expected, raw string, err error) error { + return fmt.Errorf( + "%w: env %q for field %q expects %s (value=%q): %v", + ErrEnvParse, + envName, + fieldName, + expected, + raw, + err, + ) +} diff --git a/env_test.go b/env_test.go new file mode 100644 index 0000000..d44b9b8 --- /dev/null +++ b/env_test.go @@ -0,0 +1,156 @@ +package struct2pflag_test + +import ( + "errors" + "testing" + "time" + + "github.com/goark/struct2pflag" +) + +type envNested struct { + Name string `env:"APP_NAME"` +} + +type envConfig struct { + Enabled bool `env:"ENABLED"` + Count int `env:"COUNT"` + Rate float64 `env:"RATE"` + Wait time.Duration `env:"WAIT"` + Label string `env:"LABEL"` + Nested envNested +} + +func TestBindEnvBasic(t *testing.T) { + t.Setenv("ENABLED", "true") + t.Setenv("COUNT", "12") + t.Setenv("RATE", "1.25") + t.Setenv("WAIT", "3s") + t.Setenv("LABEL", "alpha") + t.Setenv("APP_NAME", "demo") + + cfg := envConfig{} + if err := struct2pflag.BindEnv(&cfg); err != nil { + t.Fatalf("BindEnv() error = %v", err) + } + + if cfg.Enabled != true { + t.Fatalf("Enabled = %v, want true", cfg.Enabled) + } + if cfg.Count != 12 { + t.Fatalf("Count = %v, want 12", cfg.Count) + } + if cfg.Rate != 1.25 { + t.Fatalf("Rate = %v, want 1.25", cfg.Rate) + } + if cfg.Wait != 3*time.Second { + t.Fatalf("Wait = %v, want 3s", cfg.Wait) + } + if cfg.Label != "alpha" { + t.Fatalf("Label = %q, want alpha", cfg.Label) + } + if cfg.Nested.Name != "demo" { + t.Fatalf("Nested.Name = %q, want demo", cfg.Nested.Name) + } +} + +func TestBindEnvUnsetPreservesDefaults(t *testing.T) { + cfg := envConfig{ + Enabled: true, + Count: 5, + Rate: 2.0, + Wait: time.Minute, + Label: "default", + } + if err := struct2pflag.BindEnv(&cfg); err != nil { + t.Fatalf("BindEnv() error = %v", err) + } + if cfg.Enabled != true || cfg.Count != 5 || cfg.Rate != 2.0 || cfg.Wait != time.Minute || cfg.Label != "default" { + t.Fatalf("defaults were unexpectedly changed: %#v", cfg) + } +} + +func TestBindEnvOptions(t *testing.T) { + t.Setenv("APP_PORT", "8080") + t.Setenv("APP_DEBUG", "true") + type cfg struct { + Port int `conf:"PORT"` + Debug bool `conf:"DEBUG"` + } + + c := cfg{} + err := struct2pflag.BindEnv(&c, + struct2pflag.WithEnvTag("conf"), + struct2pflag.WithEnvPrefix("APP_"), + ) + if err != nil { + t.Fatalf("BindEnv() error = %v", err) + } + if c.Port != 8080 || c.Debug != true { + t.Fatalf("BindEnv() applied unexpected values: %#v", c) + } +} + +func TestBindEnvParseErrors(t *testing.T) { + type cfg struct { + N int `env:"N"` + B bool `env:"B"` + D time.Duration `env:"D"` + } + + t.Run("int", func(t *testing.T) { + t.Setenv("N", "oops") + c := cfg{} + err := struct2pflag.BindEnv(&c) + if err == nil || !errors.Is(err, struct2pflag.ErrEnvParse) { + t.Fatalf("BindEnv() error = %v, want ErrEnvParse", err) + } + }) + + t.Run("bool", func(t *testing.T) { + t.Setenv("B", "oops") + c := cfg{} + err := struct2pflag.BindEnv(&c) + if err == nil || !errors.Is(err, struct2pflag.ErrEnvParse) { + t.Fatalf("BindEnv() error = %v, want ErrEnvParse", err) + } + }) + + t.Run("duration", func(t *testing.T) { + t.Setenv("D", "oops") + c := cfg{} + err := struct2pflag.BindEnv(&c) + if err == nil || !errors.Is(err, struct2pflag.ErrEnvParse) { + t.Fatalf("BindEnv() error = %v, want ErrEnvParse", err) + } + }) +} + +func TestBindEnvInvalidTarget(t *testing.T) { + type cfg struct { + Value int `env:"VALUE"` + } + var c cfg + err := struct2pflag.BindEnv(c) + if err == nil || !errors.Is(err, struct2pflag.ErrInvalidEnvTarget) { + t.Fatalf("BindEnv() error = %v, want ErrInvalidEnvTarget", err) + } +} + +func TestBindEnvStrictUnsupportedType(t *testing.T) { + t.Setenv("ITEMS", "a,b,c") + type cfg struct { + Items []string `env:"ITEMS"` + } + + c1 := cfg{} + if err := struct2pflag.BindEnv(&c1); err != nil { + t.Fatalf("BindEnv() strict=false error = %v", err) + } + + c2 := cfg{} + err := struct2pflag.BindEnv(&c2, struct2pflag.WithStrict(true)) + if err == nil || !errors.Is(err, struct2pflag.ErrUnsupportedEnvType) { + t.Fatalf("BindEnv() strict=true error = %v, want ErrUnsupportedEnvType", err) + } +} diff --git a/examples/example3.go b/examples/example3.go index 390ba67..1672be5 100644 --- a/examples/example3.go +++ b/examples/example3.go @@ -28,7 +28,9 @@ func main() { var env Env if data, err := os.ReadFile("example3.json"); err == nil { - _ = json.Unmarshal(data, &env) + if err := json.Unmarshal(data, &env); err != nil { + fmt.Fprintf(os.Stderr, "warning: failed to parse example3.json: %v\n", err) + } } struct2pflag.BindDefault(&env) pflag.Parse() diff --git a/flag_test.go b/flag_test.go index f650fb0..1bfd588 100644 --- a/flag_test.go +++ b/flag_test.go @@ -3,6 +3,7 @@ package struct2pflag_test import ( "os" "testing" + "time" "github.com/goark/struct2pflag" "github.com/spf13/pflag" @@ -20,6 +21,12 @@ type ts struct { B1 bool `flag:"boolean option(1)"` B2 bool `flag:"B2,boolean option(2)"` B3 bool `flag:"B3,boolean option(3)"` + + F1 float32 `flag:"float option(1)"` + F2 float64 `flag:"F2,float option(2)"` + + D1 time.Duration `flag:"duration option(1)"` + D2 time.Duration `flag:"D2,duration option(2)"` } func TestBind(t *testing.T) { @@ -34,6 +41,10 @@ func TestBind(t *testing.T) { "--I2", "8", "--b1", "--B2", + "--f1", "1.5", + "--F2", "2.5", + "--d1", "3s", + "--D2", "4s", }, ) @@ -71,6 +82,20 @@ func TestBind(t *testing.T) { t.Fatalf("expect %#v,but %#v", ts1.B3, expect) } + if expect := float32(1.5); ts1.F1 != expect { + t.Fatalf("expect %#v,but %#v", ts1.F1, expect) + } + if expect := 2.5; ts1.F2 != expect { + t.Fatalf("expect %#v,but %#v", ts1.F2, expect) + } + + if expect := 3 * time.Second; ts1.D1 != expect { + t.Fatalf("expect %#v,but %#v", ts1.D1, expect) + } + if expect := 4 * time.Second; ts1.D2 != expect { + t.Fatalf("expect %#v,but %#v", ts1.D2, expect) + } + ts1 = &ts{} flagSet = pflag.NewFlagSet("", pflag.ContinueOnError) struct2pflag.Bind(flagSet, ts1) @@ -81,6 +106,10 @@ func TestBind(t *testing.T) { {"--i2", "should be upper case"}, {"--B1"}, {"--b2"}, + {"--F1", "should be lower case"}, + {"--f2", "should be upper case"}, + {"--D1", "should be lower case"}, + {"--d2", "should be upper case"}, } stderrSaved := os.Stderr devnull, err := os.Create(os.DevNull) diff --git a/main.go b/main.go index 22d73b8..a15d038 100644 --- a/main.go +++ b/main.go @@ -3,6 +3,7 @@ package struct2pflag import ( "reflect" "strings" + "time" "github.com/spf13/pflag" ) @@ -36,7 +37,7 @@ const ( // pointer is non-nil (nil pointers are skipped). // // Supported field kinds: -// - bool, int, uint, string +// - bool, int, uint, float32, float64, string, time.Duration // // For each supported field Bind registers the corresponding fs.*VarP binding using the field's // address and the field's current value as the flag default. @@ -95,6 +96,13 @@ func Bind(fs *pflag.FlagSet, cfg interface{}) { } } + // time.Duration has underlying kind int64, so we must detect it by exact type + // and bind with DurationVarP before the generic integer path. + if f.Type() == durationType { + fs.DurationVarP(f.Addr().Interface().(*time.Duration), longname, shortname, time.Duration(f.Int()), usage) + continue + } + switch f.Kind() { case reflect.Bool: fs.BoolVarP(f.Addr().Interface().(*bool), longname, shortname, f.Bool(), usage) @@ -102,6 +110,10 @@ func Bind(fs *pflag.FlagSet, cfg interface{}) { fs.IntVarP(f.Addr().Interface().(*int), longname, shortname, int(f.Int()), usage) case reflect.Uint: fs.UintVarP(f.Addr().Interface().(*uint), longname, shortname, uint(f.Uint()), usage) + case reflect.Float32: + fs.Float32VarP(f.Addr().Interface().(*float32), longname, shortname, float32(f.Float()), usage) + case reflect.Float64: + fs.Float64VarP(f.Addr().Interface().(*float64), longname, shortname, f.Float(), usage) case reflect.String: fs.StringVarP(f.Addr().Interface().(*string), longname, shortname, f.String(), usage) } diff --git a/pflag_test.go b/pflag_test.go index 4fe9b02..46c169d 100644 --- a/pflag_test.go +++ b/pflag_test.go @@ -3,6 +3,7 @@ package struct2pflag_test import ( "os" "testing" + "time" "github.com/goark/struct2pflag" "github.com/spf13/pflag" @@ -22,6 +23,12 @@ type tsPflag struct { B1 bool `pflag:"boolean option(1)"` // name omitted B2 bool `pflag:"B2,boolean option(2)"` // long name only B3 bool `pflag:"B3,b,boolean option(3)"` // long and short names + + F1 float32 `pflag:"F1,f,float option(1)"` // long and short names + F2 float64 `pflag:"F2,float option(2)"` // long name only + + D1 time.Duration `pflag:"D1,d,duration option(1)"` // long and short names + D2 time.Duration `pflag:"D2,duration option(2)"` // long name only } func TestPflagBindLongname(t *testing.T) { @@ -36,6 +43,10 @@ func TestPflagBindLongname(t *testing.T) { "--UI2", "8", "--b1", "--B2", + "--F1", "1.25", + "--F2", "2.5", + "--D1", "1500ms", + "--D2", "2s", }, ) if err != nil { @@ -74,6 +85,18 @@ func TestPflagBindLongname(t *testing.T) { if expect := false; ts1.B3 != expect { t.Errorf("expect %#v,but %#v", ts1.B3, expect) } + if expect := float32(1.25); ts1.F1 != expect { + t.Errorf("expect %#v,but %#v", ts1.F1, expect) + } + if expect := 2.5; ts1.F2 != expect { + t.Errorf("expect %#v,but %#v", ts1.F2, expect) + } + if expect := 1500 * time.Millisecond; ts1.D1 != expect { + t.Errorf("expect %#v,but %#v", ts1.D1, expect) + } + if expect := 2 * time.Second; ts1.D2 != expect { + t.Errorf("expect %#v,but %#v", ts1.D2, expect) + } } func TestPflagBindShortname(t *testing.T) { @@ -85,6 +108,8 @@ func TestPflagBindShortname(t *testing.T) { "-s", "foo", "-i", "9", "-b", + "-f", "1.5", + "-d", "750ms", }, ) if err != nil { @@ -123,6 +148,18 @@ func TestPflagBindShortname(t *testing.T) { if expect := true; ts2.B3 != expect { t.Errorf("expect %#v,but %#v", ts2.B3, expect) } + if expect := float32(1.5); ts2.F1 != expect { + t.Errorf("expect %#v,but %#v", ts2.F1, expect) + } + if expect := 0.0; ts2.F2 != expect { + t.Errorf("expect %#v,but %#v", ts2.F2, expect) + } + if expect := 750 * time.Millisecond; ts2.D1 != expect { + t.Errorf("expect %#v,but %#v", ts2.D1, expect) + } + if expect := time.Duration(0); ts2.D2 != expect { + t.Errorf("expect %#v,but %#v", ts2.D2, expect) + } } func TestPflagBindErr(t *testing.T) { @@ -141,6 +178,12 @@ func TestPflagBindErr(t *testing.T) { {"--B1"}, {"--b2"}, {"-B"}, + {"--f1", "should be upper case"}, + {"--f2", "should be upper case"}, + {"--d1", "should be upper case"}, + {"--d2", "should be upper case"}, + {"-F", "should be lower case"}, + {"-D", "should be lower case"}, } stderrSaved := os.Stderr devnull, err := os.Create(os.DevNull)