Skip to content

Commit

Permalink
go: Fix more linter problems
Browse files Browse the repository at this point in the history
  • Loading branch information
2opremio committed Jun 20, 2024
1 parent 6f92bbf commit 837dc79
Show file tree
Hide file tree
Showing 53 changed files with 262 additions and 241 deletions.
18 changes: 10 additions & 8 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,10 @@ linters-settings:
linters:
enable-all: true
disable:
# deprecated:
- gomnd
- execinquery

- depguard
- nlreturn
- godox
Expand All @@ -101,23 +103,23 @@ linters:
- wsl
- wrapcheck
- testpackage
# TODO: I didn't manage to make it accept short parameter names
# TODO: varnamelen: I didn't manage to make it accept short parameter names
- varnamelen
# TODO: consider enabling it later on
# TODO: ireturn: consider enabling it later on
- ireturn
- godot
presets: [ ]
fast: false

issues:
# Excluding configuration per-path, per-linter, per-text and per-source
# Exclude certain checks in test files
exclude-rules:
- path: _test\.go
linters:
- gosec
- path: cmd/soroban-rpc/internal/integrationtest/infrastructure/
- path: '^(.*_test\.go|cmd/soroban-rpc/internal/integrationtest/infrastructure/.*)$'
linters:
- mnd
- gosec

- gochecknoglobals
- nosprintfhostport
run:
timeout: 10m
tests: false
11 changes: 5 additions & 6 deletions cmd/soroban-rpc/internal/config/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func (cfg *Config) AddFlags(cmd *cobra.Command) error {
}

// AddFlag adds a CLI flag for this option to the given flagset.
func (co *ConfigOption) AddFlag(flagset *pflag.FlagSet) error {
func (co *Option) AddFlag(flagset *pflag.FlagSet) error {
// config options that has no names do not represent a valid flag.
if len(co.Name) == 0 {
return nil
Expand Down Expand Up @@ -100,7 +100,7 @@ func (co *ConfigOption) AddFlag(flagset *pflag.FlagSet) error {
return nil
}

func (co *ConfigOption) GetFlag(flagset *pflag.FlagSet) (interface{}, error) {
func (co *Option) GetFlag(flagset *pflag.FlagSet) (interface{}, error) {

Check failure on line 103 in cmd/soroban-rpc/internal/config/flags.go

View workflow job for this annotation

GitHub Actions / golangci-lint

calculated cyclomatic complexity for function GetFlag is 25, max is 15 (cyclop)
// Treat any option with a custom parser as a string option.
if co.CustomSetValue != nil {
return flagset.GetString(co.Name)
Expand Down Expand Up @@ -160,13 +160,12 @@ func (co *ConfigOption) GetFlag(flagset *pflag.FlagSet) (interface{}, error) {
}

// UsageText returns the string to use for the usage text of the option. The
// string returned will be the Usage defined on the ConfigOption, along with
// string returned will be the Usage defined on the Option, along with
// the environment variable.
func (co *ConfigOption) UsageText() string {
func (co *Option) UsageText() string {
envVar, hasEnvVar := co.getEnvKey()
if hasEnvVar {
return fmt.Sprintf("%s (%s)", co.Usage, envVar)
} else {
return co.Usage
}
return co.Usage
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ type Config struct {
MaxGetFeeStatsExecutionDuration time.Duration

// We memoize these, so they bind to pflags correctly
optionsCache *ConfigOptions
optionsCache *Options
flagset *pflag.FlagSet
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ import (
"github.com/stellar/go/support/strutils"
)

// ConfigOptions is a group of ConfigOptions that can be for convenience
// Options is a group of Options that can be for convenience
// initialized and set at the same time.
type ConfigOptions []*ConfigOption
type Options []*Option

// Validate all the config options.
func (options ConfigOptions) Validate() error {
func (options Options) Validate() error {
var missingOptions []errMissingRequiredOption
for _, option := range options {
if option.Validate != nil {
Expand All @@ -44,23 +44,23 @@ func (options ConfigOptions) Validate() error {
return nil
}

// ConfigOption is a complete description of the configuration of a command line option
type ConfigOption struct {
Name string // e.g. "database-url"
EnvVar string // e.g. "DATABASE_URL". Defaults to uppercase/underscore representation of name
TomlKey string // e.g. "DATABASE_URL". Defaults to uppercase/underscore representation of name. - to omit from toml
Usage string // Help text
DefaultValue interface{} // A default if no option is provided. Omit or set to `nil` if no default
ConfigKey interface{} // Pointer to the final key in the linked Config struct
CustomSetValue func(*ConfigOption, interface{}) error // Optional function for custom validation/transformation
Validate func(*ConfigOption) error // Function called after loading all options, to validate the configuration
MarshalTOML func(*ConfigOption) (interface{}, error)
// Option is a complete description of the configuration of a command line option
type Option struct {
Name string // e.g. "database-url"
EnvVar string // e.g. "DATABASE_URL". Defaults to uppercase/underscore representation of name
TomlKey string // e.g. "DATABASE_URL". Defaults to uppercase/underscore representation of name. - to omit from toml
Usage string // Help text
DefaultValue interface{} // A default if no option is provided. Omit or set to `nil` if no default
ConfigKey interface{} // Pointer to the final key in the linked Config struct
CustomSetValue func(*Option, interface{}) error // Optional function for custom validation/transformation
Validate func(*Option) error // Function called after loading all options, to validate the configuration
MarshalTOML func(*Option) (interface{}, error)

flag *pflag.Flag // The persistent flag that the config option is attached to
}

// Returns false if this option is omitted in the toml
func (o ConfigOption) getTomlKey() (string, bool) {
func (o Option) getTomlKey() (string, bool) {
if o.TomlKey == "-" || o.TomlKey == "_" {
return "", false
}
Expand All @@ -74,7 +74,7 @@ func (o ConfigOption) getTomlKey() (string, bool) {
}

// Returns false if this option is omitted in the env
func (o ConfigOption) getEnvKey() (string, bool) {
func (o Option) getEnvKey() (string, bool) {
if o.EnvVar == "-" || o.EnvVar == "_" {
return "", false
}
Expand All @@ -85,7 +85,7 @@ func (o ConfigOption) getEnvKey() (string, bool) {
}

// TODO: See if we can remove CustomSetValue into just SetValue/ParseValue
func (o *ConfigOption) setValue(i interface{}) (err error) {
func (o *Option) setValue(i interface{}) (err error) {
if o.CustomSetValue != nil {
return o.CustomSetValue(o, i)
}
Expand All @@ -99,7 +99,7 @@ func (o *ConfigOption) setValue(i interface{}) (err error) {
err = fmt.Errorf("config option setting error ('%s') %v", o.Name, recoverRes)
}
}()
parser := func(option *ConfigOption, i interface{}) error {
parser := func(option *Option, i interface{}) error {

Check failure on line 102 in cmd/soroban-rpc/internal/config/option.go

View workflow job for this annotation

GitHub Actions / golangci-lint

unused-parameter: parameter 'option' seems to be unused, consider removing or renaming it as _ (revive)
return fmt.Errorf("no parser for flag %s", o.Name)
}
switch o.ConfigKey.(type) {
Expand All @@ -124,7 +124,7 @@ func (o *ConfigOption) setValue(i interface{}) (err error) {
return parser(o, i)
}

func (o *ConfigOption) marshalTOML() (interface{}, error) {
func (o *Option) marshalTOML() (interface{}, error) {
if o.MarshalTOML != nil {
return o.MarshalTOML(o)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,39 +12,39 @@ import (

func TestConfigOptionGetTomlKey(t *testing.T) {
// Explicitly set toml key
key, ok := ConfigOption{TomlKey: "TOML_KEY"}.getTomlKey()
key, ok := Option{TomlKey: "TOML_KEY"}.getTomlKey()
assert.Equal(t, "TOML_KEY", key)
assert.True(t, ok)

// Explicitly disabled toml key via `-`
key, ok = ConfigOption{TomlKey: "-"}.getTomlKey()
key, ok = Option{TomlKey: "-"}.getTomlKey()
assert.Equal(t, "", key)
assert.False(t, ok)

// Explicitly disabled toml key via `_`
key, ok = ConfigOption{TomlKey: "_"}.getTomlKey()
key, ok = Option{TomlKey: "_"}.getTomlKey()
assert.Equal(t, "", key)
assert.False(t, ok)

// Fallback to env var
key, ok = ConfigOption{EnvVar: "ENV_VAR"}.getTomlKey()
key, ok = Option{EnvVar: "ENV_VAR"}.getTomlKey()
assert.Equal(t, "ENV_VAR", key)
assert.True(t, ok)

// Env-var disabled, autogenerate from name
key, ok = ConfigOption{Name: "test-flag", EnvVar: "-"}.getTomlKey()
key, ok = Option{Name: "test-flag", EnvVar: "-"}.getTomlKey()
assert.Equal(t, "TEST_FLAG", key)
assert.True(t, ok)

// Env-var not set, autogenerate from name
key, ok = ConfigOption{Name: "test-flag"}.getTomlKey()
key, ok = Option{Name: "test-flag"}.getTomlKey()
assert.Equal(t, "TEST_FLAG", key)
assert.True(t, ok)
}

func TestValidateRequired(t *testing.T) {
var strVal string
o := &ConfigOption{
o := &Option{
Name: "required-option",
ConfigKey: &strVal,
Validate: required,
Expand All @@ -64,7 +64,7 @@ func TestValidateRequired(t *testing.T) {

func TestValidatePositiveUint32(t *testing.T) {
var val uint32
o := &ConfigOption{
o := &Option{
Name: "positive-option",
ConfigKey: &val,
Validate: positive,
Expand All @@ -84,7 +84,7 @@ func TestValidatePositiveUint32(t *testing.T) {

func TestValidatePositiveInt(t *testing.T) {
var val int
o := &ConfigOption{
o := &Option{
Name: "positive-option",
ConfigKey: &val,
Validate: positive,
Expand All @@ -107,7 +107,7 @@ func TestValidatePositiveInt(t *testing.T) {
}

func TestUnassignableField(t *testing.T) {
var co ConfigOption
var co Option
var b bool
co.Name = "mykey"
co.ConfigKey = &b
Expand All @@ -117,7 +117,7 @@ func TestUnassignableField(t *testing.T) {
}

func TestNoParserForFlag(t *testing.T) {
var co ConfigOption
var co Option
var invalidKey []time.Duration
co.Name = "mykey"
co.ConfigKey = &invalidKey
Expand Down Expand Up @@ -256,7 +256,7 @@ func TestSetValue(t *testing.T) {
},
} {
t.Run(scenario.name, func(t *testing.T) {
co := ConfigOption{
co := Option{
Name: scenario.name,
ConfigKey: scenario.key,
}
Expand Down
26 changes: 13 additions & 13 deletions cmd/soroban-rpc/internal/config/options.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//nolint:mnd // lots of magic constants due to default values
//nolint:mnd,lll // lots of magic constants and long lines due to default values and help strings
package config

import (
Expand All @@ -20,12 +20,12 @@ import (

const defaultHTTPEndpoint = "localhost:8000"

func (cfg *Config) options() ConfigOptions {
func (cfg *Config) options() Options {
if cfg.optionsCache != nil {
return *cfg.optionsCache
}
defaultStellarCoreBinaryPath, _ := exec.LookPath("stellar-core")
cfg.optionsCache = &ConfigOptions{
cfg.optionsCache = &Options{
{
Name: "config-path",
EnvVar: "SOROBAN_RPC_CONFIG_PATH",
Expand Down Expand Up @@ -56,7 +56,7 @@ func (cfg *Config) options() ConfigOptions {
Name: "stellar-core-url",
Usage: "URL used to query Stellar Core (local captive core by default)",
ConfigKey: &cfg.StellarCoreURL,
Validate: func(co *ConfigOption) error {
Validate: func(co *Option) error {

Check failure on line 59 in cmd/soroban-rpc/internal/config/options.go

View workflow job for this annotation

GitHub Actions / golangci-lint

unused-parameter: parameter 'co' seems to be unused, consider removing or renaming it as _ (revive)
// This is a bit awkward. We're actually setting a default, but we
// can't do that until the config is fully parsed, so we do it as a
// validator here.
Expand All @@ -83,7 +83,7 @@ func (cfg *Config) options() ConfigOptions {
Usage: "minimum log severity (debug, info, warn, error) to log",
ConfigKey: &cfg.LogLevel,
DefaultValue: logrus.InfoLevel,
CustomSetValue: func(option *ConfigOption, i interface{}) error {
CustomSetValue: func(option *Option, i interface{}) error {
switch v := i.(type) {
case nil:
return nil
Expand All @@ -102,7 +102,7 @@ func (cfg *Config) options() ConfigOptions {
}
return nil
},
MarshalTOML: func(option *ConfigOption) (interface{}, error) {
MarshalTOML: func(option *Option) (interface{}, error) {

Check failure on line 105 in cmd/soroban-rpc/internal/config/options.go

View workflow job for this annotation

GitHub Actions / golangci-lint

unused-parameter: parameter 'option' seems to be unused, consider removing or renaming it as _ (revive)
return cfg.LogLevel.String(), nil
},
},
Expand All @@ -111,7 +111,7 @@ func (cfg *Config) options() ConfigOptions {
Usage: "format used for output logs (json or text)",
ConfigKey: &cfg.LogFormat,
DefaultValue: LogFormatText,
CustomSetValue: func(option *ConfigOption, i interface{}) error {
CustomSetValue: func(option *Option, i interface{}) error {
switch v := i.(type) {
case nil:
return nil
Expand All @@ -130,7 +130,7 @@ func (cfg *Config) options() ConfigOptions {
}
return nil
},
MarshalTOML: func(option *ConfigOption) (interface{}, error) {
MarshalTOML: func(option *Option) (interface{}, error) {

Check failure on line 133 in cmd/soroban-rpc/internal/config/options.go

View workflow job for this annotation

GitHub Actions / golangci-lint

unused-parameter: parameter 'option' seems to be unused, consider removing or renaming it as _ (revive)
return cfg.LogFormat.String()
},
},
Expand All @@ -151,7 +151,7 @@ func (cfg *Config) options() ConfigOptions {
Name: "captive-core-storage-path",
Usage: "Storage location for Captive Core bucket data",
ConfigKey: &cfg.CaptiveCoreStoragePath,
CustomSetValue: func(option *ConfigOption, i interface{}) error {
CustomSetValue: func(option *Option, i interface{}) error {
switch v := i.(type) {
case string:
if v == "" || v == "." {
Expand Down Expand Up @@ -255,7 +255,7 @@ func (cfg *Config) options() ConfigOptions {
Usage: "Default cap on the amount of events included in a single getEvents response",
ConfigKey: &cfg.DefaultEventsLimit,
DefaultValue: uint(100),
Validate: func(co *ConfigOption) error {
Validate: func(co *Option) error {

Check failure on line 258 in cmd/soroban-rpc/internal/config/options.go

View workflow job for this annotation

GitHub Actions / golangci-lint

unused-parameter: parameter 'co' seems to be unused, consider removing or renaming it as _ (revive)
if cfg.DefaultEventsLimit > cfg.MaxEventsLimit {
return fmt.Errorf(
"default-events-limit (%v) cannot exceed max-events-limit (%v)",
Expand All @@ -277,7 +277,7 @@ func (cfg *Config) options() ConfigOptions {
Usage: "Default cap on the amount of transactions included in a single getTransactions response",
ConfigKey: &cfg.DefaultTransactionsLimit,
DefaultValue: uint(50),
Validate: func(co *ConfigOption) error {
Validate: func(co *Option) error {

Check failure on line 280 in cmd/soroban-rpc/internal/config/options.go

View workflow job for this annotation

GitHub Actions / golangci-lint

unused-parameter: parameter 'co' seems to be unused, consider removing or renaming it as _ (revive)
if cfg.DefaultTransactionsLimit > cfg.MaxTransactionsLimit {
return fmt.Errorf(
"default-transactions-limit (%v) cannot exceed max-transactions-limit (%v)",
Expand Down Expand Up @@ -490,7 +490,7 @@ func (e errMissingRequiredOption) Error() string {
return e.strErr
}

func required(option *ConfigOption) error {
func required(option *Option) error {
switch reflect.ValueOf(option.ConfigKey).Elem().Kind() {
case reflect.Slice:
if reflect.ValueOf(option.ConfigKey).Elem().Len() > 0 {
Expand Down Expand Up @@ -527,7 +527,7 @@ func required(option *ConfigOption) error {
return errMissingRequiredOption{strErr: fmt.Sprintf("%s is required.%s", option.Name, advice), usage: option.Usage}
}

func positive(option *ConfigOption) error {
func positive(option *Option) error {
switch v := option.ConfigKey.(type) {
case *int, *int8, *int16, *int32, *int64:
if reflect.ValueOf(v).Elem().Int() <= 0 {
Expand Down
Loading

0 comments on commit 837dc79

Please sign in to comment.