refactor/inline some type conversion and start returning error lists …

…instead of logging. also testing that linter ci should fail

.golangci.yml

@@ -0,0 +1,169 @@
+# Config for golangci-lint
+
+# output configuration options
+
+# all available settings of specific linters
+linters-settings:
+  gocritic:
+    disabled-checks:
+      - ifElseChain
+  dupl:
+    # tokens count to trigger issue, 150 by default
+    threshold: 100
+  exhaustive:
+    # indicates that switch statements are to be considered exhaustive if a
+    # 'default' case is present, even if all enum members aren't listed in the
+    # switch
+    default-signifies-exhaustive: false
+  funlen:
+    lines: 140
+    statements: 70
+  gocognit:
+    # minimal code complexity to report, 30 by default (but we recommend 10-20)
+    min-complexity: 42
+  nestif:
+    # minimal complexity of if statements to report, 5 by default
+    min-complexity: 4
+  gocyclo:
+    # minimal code complexity to report, 30 by default (but we recommend 10-20)
+    min-complexity: 30
+  godot:
+    # check all top-level comments, not only declarations
+    check-all: false
+  govet:
+    # report about shadowed variables
+    check-shadowing: true
+    # settings per analyzer
+    settings:
+      printf: # analyzer name, run `go tool vet help` to see all analyzers
+        funcs: # run `go tool vet help printf` to see available settings for `printf` analyzer
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Printf
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).FErrf
+    enable-all: true
+    disable-all: false
+  lll:
+    # max line length, lines longer will be reported. Default is 120.
+    # '\t' is counted as 1 character by default, and can be changed with the tab-width option
+    line-length: 132
+    # tab width in spaces. Default to 1.
+    tab-width: 1
+  misspell:
+    # Correct spellings using locale preferences for US or UK.
+    # Default is to use a neutral variety of English.
+    # Setting locale to US will correct the British spelling of 'colour' to 'color'.
+    locale: US
+    ignore-words:
+      - fortio
+  nakedret:
+    # make an issue if func has more lines of code than this setting and it has naked returns; default is 30
+    max-func-lines: 30
+  nolintlint:
+    require-specific: true
+  whitespace:
+    multi-if: false   # Enforces newlines (or comments) after every multi-line if statement
+    multi-func: false # Enforces newlines (or comments) after every multi-line function signature
+  gofumpt:
+    # Choose whether or not to use the extra rules that are disabled
+    # by default
+    extra-rules: false
+
+
+linters:
+  disable:
+    # bad ones:
+    - musttag
+    # Deprecated ones:
+    - scopelint
+    - golint
+    - interfacer
+    - maligned
+    - varcheck
+    - structcheck
+    - nosnakecase
+    - deadcode
+    # Weird/bad ones:
+    - wsl
+    - nlreturn
+    - gochecknoinits
+    - gochecknoglobals
+    - gomnd
+    - testpackage
+    - wrapcheck
+    - exhaustivestruct
+    - tagliatelle
+    - nonamedreturns
+    - varnamelen
+    - exhaustruct # seems like a good idea at first but actually a pain and go does have zero values for a reason.
+# TODO consider putting these back, when they stop being bugged (ifshort, wastedassign,...)
+    - paralleltest
+    - thelper
+    - forbidigo
+    - ifshort
+    - wastedassign
+    - cyclop
+    - forcetypeassert
+    - ireturn
+    - depguard
+  enable-all: true
+  disable-all: false
+  # Must not use fast: true in newer golangci-lint or it'll just skip a bunch of linter instead of doing caching like before (!)
+  fast: false
+
+
+issues:
+  # Excluding configuration per-path, per-linter, per-text and per-source
+  exclude-rules:
+    # Exclude some linters from running on tests files.
+    - path: _test\.go
+      linters:
+        - gocyclo
+        - errcheck
+        - dupl
+        - gosec
+        - gochecknoinits
+        - gochecknoglobals
+        - forcetypeassert
+        - nosnakecase
+        - noctx
+
+    # Exclude lll issues for long lines with go:generate
+    - linters:
+        - lll
+      source: "^//go:generate "
+    - linters:
+        - goerr113
+      text: "do not define dynamic errors"
+    - linters:
+        - govet
+      text: "fieldalignment:"
+    - linters:
+        - godox
+      text: "TODO"
+    - linters:
+        - nosnakecase
+      text: "grpc_|_SERVING|O_"
+
+  # Maximum issues count per one linter. Set to 0 to disable. Default is 50.
+  max-issues-per-linter: 0
+
+  # Maximum count of issues with the same text. Set to 0 to disable. Default is 3.
+  max-same-issues: 0
+
+severity:
+  # Default value is empty string.
+  # Set the default severity for issues. If severity rules are defined and the issues
+  # do not match or no severity is provided to the rule this will be the default
+  # severity applied. Severities should match the supported severity names of the
+  # selected out format.
+  # - Code climate: https://docs.codeclimate.com/docs/issues#issue-severity
+  # -   Checkstyle: https://checkstyle.sourceforge.io/property_types.html#severity
+  # -       Github: https://help.github.com/en/actions/reference/workflow-commands-for-github-actions#setting-an-error-message
+  default-severity: error
+
+  # The default value is false.
+  # If set to true severity-rules regular expressions become case sensitive.
+  case-sensitive: false

env.go

@@ -5,7 +5,7 @@
 // conventions, which is useful for generating or parsing environment variables,
 // JSON tags, or command line flags.
 //
-// The package defines several case conversion functions that aid in manipulating
+// The package also defines several case conversion functions that aid in manipulating
 // strings to fit conventional casing for various programming and configuration contexts.
 // Additionally, it provides functions to serialize structs into slices of key-value pairs
 // where the keys are derived from struct field names transformed to upper snake case by default,
@@ -195,37 +195,6 @@ func structToEnvVars(prefix string, s interface{}) []KeyValue {
 	return envVars
 }
 
-// TODO: consider returning errors or at least counting issues.
-
-// setInt sets an integer field from a string.
-func setInt(fieldValue reflect.Value, val string) {
-	intVal, err := strconv.ParseInt(val, 10, fieldValue.Type().Bits())
-	if err != nil {
-		log.Errf("Invalid integer value %q: %v", val, err)
-		return
-	}
-	fieldValue.SetInt(intVal)
-}
-
-// setFloat sets a float field from a string.
-func setFloat(fieldValue reflect.Value, val string) {
-	floatVal, err := strconv.ParseFloat(val, fieldValue.Type().Bits())
-	if err != nil {
-		log.Errf("Invalid float value %q: %v", val, err)
-		return
-	}
-	fieldValue.SetFloat(floatVal)
-}
-
-func setBool(fieldValue reflect.Value, val string) {
-	boolVal, err := strconv.ParseBool(val)
-	if err != nil {
-		log.Errf("Invalid bool value %q: %v", val, err)
-		return
-	}
-	fieldValue.SetBool(boolVal)
-}
-
 func setPointer(fieldValue reflect.Value) reflect.Value {
 	// Ensure we have a pointer to work with, allocate if nil.
 	if fieldValue.IsNil() {
@@ -249,15 +218,21 @@ func checkEnv(envName, fieldName string, fieldValue reflect.Value) *string {
 	return &val
 }
 
-func SetFromEnv(prefix string, s interface{}) {
+func SetFromEnv(prefix string, s interface{}) []error {
+	return setFromEnv(nil, prefix, s)
+}
+func setFromEnv(allErrors []error, prefix string, s interface{}) []error {
+	// TOOD: this is quite similar in structure to structToEnvVars() - can it be refactored with
+	// passing setter vs getter function and share the same iteration (yet a little bit of copy is the go way too)
 	v := reflect.ValueOf(s)
 	// if we're passed a pointer to a struct instead of the struct, let that work too
 	if v.Kind() == reflect.Ptr {
 		v = v.Elem()
 	}
 	if v.Kind() != reflect.Struct {
-		log.Errf("Unexpected kind %v, expected a struct", v.Kind())
-		return
+		err := fmt.Errorf("unexpected kind %v, expected a struct", v.Kind())
+		allErrors = append(allErrors, err)
+		return allErrors
 	}
 	t := v.Type()
 	for i := 0; i < t.NumField(); i++ {
@@ -295,18 +270,34 @@ func SetFromEnv(prefix string, s interface{}) {
 			kind = fieldValue.Type().Elem().Kind()
 			fieldValue = setPointer(fieldValue)
 		}
-
+		var err error
 		switch kind { //nolint: exhaustive // we have default: for the other cases
 		case reflect.String:
 			fieldValue.SetString(envVal)
 		case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
-			setInt(fieldValue, envVal)
+			var ev int64
+			ev, err = strconv.ParseInt(envVal, 10, fieldValue.Type().Bits())
+			if err == nil {
+				fieldValue.SetInt(ev)
+			}
 		case reflect.Float32, reflect.Float64:
-			setFloat(fieldValue, envVal)
+			var ev float64
+			ev, err = strconv.ParseFloat(envVal, fieldValue.Type().Bits())
+			if err == nil {
+				fieldValue.SetFloat(ev)
+			}
 		case reflect.Bool:
-			setBool(fieldValue, envVal)
+			var ev bool
+			ev, err = strconv.ParseBool(envVal)
+			if err == nil {
+				fieldValue.SetBool(ev)
+			}
 		default:
-			log.Warnf("Unsupported type %v to set from %s=%q", kind, envName, envVal)
+			err = fmt.Errorf("unsupported type %v to set from %s=%q", kind, envName, envVal)
+		}
+		if err != nil {
+			allErrors = append(allErrors, err)
 		}
 	}
+	return allErrors
 }