lint: fix all lint errors as reported by golangci-lint. (#49)

This PR updates many packages to pass lint checks implemented using golangci-lint. The changes are mostly cosmetic except for: - netconfig/osconfig: required the use golang.org/x/net/route rather than raw system calls. - nsync: tests were refactored to avoid using testing.T from a goroutine. - ibe: needs to transition from the bn256 elliptic curve, a // nolint comment was used to defer this. - // nolint: gocyclo was used to avoid having to refactor overly complex functions in multiple packages.
vanadium · May 14, 2020 · 2f9767f · 2f9767f
1 parent b2730bc
commit 2f9767f
Show file tree

Hide file tree

Showing 117 changed files with 810 additions and 642 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -1,13 +1,43 @@
-version: 2
 jobs:
-  build:
-    docker:
-      - image: circleci/golang:1.14
+  test:
+    executor:
+      name: go/default
+      tag: "1.14"
     steps:
       - checkout
+      - go/load-cache
+      - go/mod-download
+      - go/save-cache
+      - go/test:
+          covermode: atomic
+          failfast: true
+          race: true
+
+  lint:
+    executor:
+      name: go/default
+      tag: "1.14"
+    steps:
+      - checkout
+      - go/load-cache
+      - go/mod-download
+      - run:
+          name: downloads
+          command: |
+            go get github.com/golangci/golangci-lint/cmd/golangci-lint@v1.27.0
+            go get github.com/matthewloring/validjson/cmd/validjson
       - run:
-          name: build
+          name: lint
           command: |
-            set -eux -o pipefail 
-            go get -v -t -d ./...
-            go test -v ./...
+            go run github.com/golangci/golangci-lint/cmd/golangci-lint run ./...
+            go run github.com/matthewloring/validjson/cmd/validjson ./...
+      - go/save-cache
+
+version: 2.1
+orbs:
+  go: circleci/go@1.1.2
+workflows:
+  main:
+    jobs:
+      - test
+      - lint
diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
diff --git a/.travis.yml b/.travis.yml
diff --git a/cmd/flagvar/flagvar.go b/cmd/flagvar/flagvar.go
@@ -9,6 +9,9 @@
 // This makes it convenient to colocate flags with related data structures and
 // to avoid large numbers of global variables as are often encountered with
 // complex, multi-level command structures.
+//
+// NOTE: this package is no longer under active development, the
+// cloudeng.io/cmdutil/flags supercedes it.
 package flagvar
 
 import (
@@ -119,15 +122,31 @@ func ParseFlagTag(t string) (name, value, usage string, err error) {
 	if err != nil {
 		return
 	}
-	usage, remaining, err = parseField(remaining, "<usage>", false, false)
-	if err != nil {
-		return
-	}
-
+	usage, _, err = parseField(remaining, "<usage>", false, false)
 	return
 }
 
-func literalDefault(typeName, literal string, initialValue interface{}) (value interface{}, usage string, err error) {
+func defaultLiteralValue(typeName string) interface{} {
+	switch typeName {
+	case "int":
+		return int(0)
+	case "int64", "time.Duration":
+		return int64(0)
+	case "uint":
+		return uint(0)
+	case "uint64":
+		return uint64(0)
+	case "bool":
+		return bool(false)
+	case "float64":
+		return float64(0)
+	case "string":
+		return ""
+	}
+	return nil
+}
+
+func literalDefault(typeName, literal string, initialValue interface{}) (value interface{}, usageDefault string, err error) {
 	if initialValue != nil {
 		switch v := initialValue.(type) {
 		case int, int64, uint, uint64, bool, float64, string, time.Duration:
@@ -136,26 +155,11 @@ func literalDefault(typeName, literal string, initialValue interface{}) (value i
 		}
 	}
 	if len(literal) == 0 {
-		switch typeName {
-		case "int":
-			value = int(0)
-		case "int64", "time.Duration":
-			value = int64(0)
-		case "uint":
-			value = uint(0)
-		case "uint64":
-			value = uint64(0)
-		case "bool":
-			value = bool(false)
-		case "float64":
-			value = float64(0)
-		case "string":
-			value = ""
-		}
+		value = defaultLiteralValue(typeName)
 		return
 	}
 	if tmp := os.ExpandEnv(literal); tmp != literal {
-		usage = literal
+		usageDefault = literal
 		literal = tmp
 	}
 	var tmp int64
@@ -232,19 +236,80 @@ func RegisterFlagsInStruct(fs *flag.FlagSet, tag string, structWithFlags interfa
 	return nil
 }
 
-func registerFlagsInStruct(fs *flag.FlagSet, tag string, structWithFlags interface{}, valueDefaults map[string]interface{}, usageDefaults map[string]string) error {
+func createVarFlag(fs *flag.FlagSet, fieldValue reflect.Value, name, value, description string, usageDefaults map[string]string) error {
+	addr := fieldValue.Addr()
+	if !addr.Type().Implements(flagValueType) {
+		return fmt.Errorf("does not implement flag.Value")
+	}
+	dv := addr.Interface().(flag.Value)
+	fs.Var(dv, name, description)
+	if len(value) > 0 {
+		if err := dv.Set(value); err != nil {
+			return fmt.Errorf("failed to set initial default value for flag.Value: %v", err)
+		}
+	}
+	if ud, ok := usageDefaults[name]; ok {
+		fs.Lookup(name).DefValue = ud
+	} else {
+		fs.Lookup(name).DefValue = value
+	}
+	return nil
+}
+
+func createFlagsBasedOnValue(fs *flag.FlagSet, initialValue interface{}, fieldValue reflect.Value, name, description string) bool {
+
+	switch dv := initialValue.(type) {
+	case int:
+		ptr := (*int)(unsafe.Pointer(fieldValue.Addr().Pointer()))
+		fs.IntVar(ptr, name, dv, description)
+	case int64:
+		ptr := (*int64)(unsafe.Pointer(fieldValue.Addr().Pointer()))
+		fs.Int64Var(ptr, name, dv, description)
+	case uint:
+		ptr := (*uint)(unsafe.Pointer(fieldValue.Addr().Pointer()))
+		fs.UintVar(ptr, name, dv, description)
+	case uint64:
+		ptr := (*uint64)(unsafe.Pointer(fieldValue.Addr().Pointer()))
+		fs.Uint64Var(ptr, name, dv, description)
+	case bool:
+		ptr := (*bool)(unsafe.Pointer(fieldValue.Addr().Pointer()))
+		fs.BoolVar(ptr, name, dv, description)
+	case float64:
+		ptr := (*float64)(unsafe.Pointer(fieldValue.Addr().Pointer()))
+		fs.Float64Var(ptr, name, dv, description)
+	case string:
+		ptr := (*string)(unsafe.Pointer(fieldValue.Addr().Pointer()))
+		fs.StringVar(ptr, name, dv, description)
+	case time.Duration:
+		ptr := (*time.Duration)(unsafe.Pointer(fieldValue.Addr().Pointer()))
+		fs.DurationVar(ptr, name, dv, description)
+	default:
+		return false
+	}
+	return true
+}
+
+func getTypeVal(structWithFlags interface{}) (reflect.Type, reflect.Value, error) {
 	typ := reflect.TypeOf(structWithFlags)
 	val := reflect.ValueOf(structWithFlags)
 	if typ.Kind() == reflect.Ptr {
 		typ = typ.Elem()
 		val = reflect.Indirect(val)
 	}
 	if !val.CanAddr() {
-		return fmt.Errorf("%T is not addressable", structWithFlags)
+		return nil, reflect.Value{}, fmt.Errorf("%T is not addressable", structWithFlags)
 	}
 
 	if typ.Kind() != reflect.Struct {
-		return fmt.Errorf("%T is not a pointer to a struct", structWithFlags)
+		return nil, reflect.Value{}, fmt.Errorf("%T is not a pointer to a struct", structWithFlags)
+	}
+	return typ, val, nil
+}
+
+func registerFlagsInStruct(fs *flag.FlagSet, tag string, structWithFlags interface{}, valueDefaults map[string]interface{}, usageDefaults map[string]string) error {
+	typ, val, err := getTypeVal(structWithFlags)
+	if err != nil {
+		return err
 	}
 
 	for i := 0; i < typ.NumField(); i++ {
@@ -280,62 +345,23 @@ func registerFlagsInStruct(fs *flag.FlagSet, tag string, structWithFlags interfa
 			return fmt.Errorf("%v: field can't be a pointer", errPrefix())
 		}
 
-		initialValue, defaultUsageValue, err := literalDefault(fieldTypeName, value, valueDefaults[name])
+		initialValue, usageDefault, err := literalDefault(fieldTypeName, value, valueDefaults[name])
 		if err != nil {
 			return fmt.Errorf("%v: failed to set initial default value: %v", errPrefix(), err)
 		}
 
 		if initialValue == nil {
-			addr := fieldValue.Addr()
-			if !addr.Type().Implements(flagValueType) {
-				return fmt.Errorf("%v: does not implement flag.Value", errPrefix())
-			}
-			dv := addr.Interface().(flag.Value)
-			fs.Var(dv, name, description)
-			if len(value) > 0 {
-				if err := dv.Set(value); err != nil {
-					return fmt.Errorf("%v: failed to set initial default value for flag.Value: %v", errPrefix(), err)
-				}
-			}
-			if ud, ok := usageDefaults[name]; ok {
-				fs.Lookup(name).DefValue = ud
-			} else {
-				fs.Lookup(name).DefValue = value
+			if err := createVarFlag(fs, fieldValue, name, value, description, usageDefaults); err != nil {
+				return fmt.Errorf("%v: %v", errPrefix(), err)
 			}
 			continue
 		}
-
-		switch dv := initialValue.(type) {
-		case int:
-			ptr := (*int)(unsafe.Pointer(fieldValue.Addr().Pointer()))
-			fs.IntVar(ptr, name, dv, description)
-		case int64:
-			ptr := (*int64)(unsafe.Pointer(fieldValue.Addr().Pointer()))
-			fs.Int64Var(ptr, name, dv, description)
-		case uint:
-			ptr := (*uint)(unsafe.Pointer(fieldValue.Addr().Pointer()))
-			fs.UintVar(ptr, name, dv, description)
-		case uint64:
-			ptr := (*uint64)(unsafe.Pointer(fieldValue.Addr().Pointer()))
-			fs.Uint64Var(ptr, name, dv, description)
-		case bool:
-			ptr := (*bool)(unsafe.Pointer(fieldValue.Addr().Pointer()))
-			fs.BoolVar(ptr, name, dv, description)
-		case float64:
-			ptr := (*float64)(unsafe.Pointer(fieldValue.Addr().Pointer()))
-			fs.Float64Var(ptr, name, dv, description)
-		case string:
-			ptr := (*string)(unsafe.Pointer(fieldValue.Addr().Pointer()))
-			fs.StringVar(ptr, name, dv, description)
-		case time.Duration:
-			ptr := (*time.Duration)(unsafe.Pointer(fieldValue.Addr().Pointer()))
-			fs.DurationVar(ptr, name, dv, description)
-		default:
+		if !createFlagsBasedOnValue(fs, initialValue, fieldValue, name, description) {
 			// should never reach here.
 			panic(fmt.Sprintf("%v flag: field %v, flag %v: unsupported type %T", fieldTypeName, fieldName, name, initialValue))
 		}
-		if len(defaultUsageValue) > 0 {
-			fs.Lookup(name).DefValue = defaultUsageValue
+		if len(usageDefault) > 0 {
+			fs.Lookup(name).DefValue = usageDefault
 		}
 	}
 	return nil

diff --git a/cmd/flagvar/flagvar_test.go b/cmd/flagvar/flagvar_test.go
@@ -104,8 +104,6 @@ func TestTags(t *testing.T) {
 	}
 }
 
-type dummy struct{}
-
 func allFlags(fs *flag.FlagSet) string {
 	out := []string{}
 	fs.VisitAll(func(f *flag.Flag) {
@@ -124,19 +122,6 @@ func allFlags(fs *flag.FlagSet) string {
 	return strings.Join(out, "\n")
 }
 
-func MaxProcs() int {
-	return runtime.GOMAXPROCS(-1)
-}
-
-func CurrentDirectory() string {
-	d, _ := os.Getwd()
-	return d
-}
-
-func OneHour() time.Duration {
-	return time.Hour
-}
-
 func TestRegister(t *testing.T) {
 	assert := func(got, want interface{}) {
 		_, file, line, _ := runtime.Caller(1)