Merge PR #1593: tools: Add code complexity linter, gocyclo

CHANGELOG.md

@@ -70,6 +70,7 @@ FEATURES
   * ineffassign
   * errcheck
   * unparam
+  * gocyclo
 * [tools] Add `make format` command to automate fixing misspell and gofmt errors.
 * [server] Default config now creates a profiler at port 6060, and increase p2p send/recv rates
 * [tests] Add WaitForNextNBlocksTM helper method

baseapp/baseapp.go

@@ -324,16 +324,39 @@ func (app *BaseApp) FilterPeerByPubKey(info string) abci.ResponseQuery {
 	return abci.ResponseQuery{}
 }
 
-// Implements ABCI.
-// Delegates to CommitMultiStore if it implements Queryable
-func (app *BaseApp) Query(req abci.RequestQuery) (res abci.ResponseQuery) {
-	path := strings.Split(req.Path, "/")
+func splitPath(requestPath string) (path []string) {
+	path = strings.Split(requestPath, "/")
 	// first element is empty string
 	if len(path) > 0 && path[0] == "" {
 		path = path[1:]
 	}
+	return path
+}
+
+// Implements ABCI.
+// Delegates to CommitMultiStore if it implements Queryable
+func (app *BaseApp) Query(req abci.RequestQuery) (res abci.ResponseQuery) {
+	path := splitPath(req.Path)
+	if len(path) == 0 {
+		msg := "no query path provided"
+		return sdk.ErrUnknownRequest(msg).QueryResult()
+	}
+	switch path[0] {
 	// "/app" prefix for special application queries
-	if len(path) >= 2 && path[0] == "app" {
+	case "app":
+		return handleQueryApp(app, path, req)
+	case "store":
+		return handleQueryStore(app, path, req)
+	case "p2p":
+		return handleQueryP2P(app, path, req)
+	}
+
+	msg := "unknown query path"
+	return sdk.ErrUnknownRequest(msg).QueryResult()
+}
+
+func handleQueryApp(app *BaseApp, path []string, req abci.RequestQuery) (res abci.ResponseQuery) {
+	if len(path) >= 2 {
 		var result sdk.Result
 		switch path[1] {
 		case "simulate":
@@ -358,18 +381,24 @@ func (app *BaseApp) Query(req abci.RequestQuery) (res abci.ResponseQuery) {
 			Value: value,
 		}
 	}
+	msg := "Expected second parameter to be either simulate or version, neither was present"
+	return sdk.ErrUnknownRequest(msg).QueryResult()
+}
+
+func handleQueryStore(app *BaseApp, path []string, req abci.RequestQuery) (res abci.ResponseQuery) {
 	// "/store" prefix for store queries
-	if len(path) >= 1 && path[0] == "store" {
-		queryable, ok := app.cms.(sdk.Queryable)
-		if !ok {
-			msg := "multistore doesn't support queries"
-			return sdk.ErrUnknownRequest(msg).QueryResult()
-		}
-		req.Path = "/" + strings.Join(path[1:], "/")
-		return queryable.Query(req)
+	queryable, ok := app.cms.(sdk.Queryable)
+	if !ok {
+		msg := "multistore doesn't support queries"
+		return sdk.ErrUnknownRequest(msg).QueryResult()
 	}
+	req.Path = "/" + strings.Join(path[1:], "/")
+	return queryable.Query(req)
+}
+
+func handleQueryP2P(app *BaseApp, path []string, req abci.RequestQuery) (res abci.ResponseQuery) {
 	// "/p2p" prefix for p2p queries
-	if len(path) >= 4 && path[0] == "p2p" {
+	if len(path) >= 4 {
 		if path[1] == "filter" {
 			if path[2] == "addr" {
 				return app.FilterPeerByAddrPort(path[3])
@@ -379,9 +408,13 @@ func (app *BaseApp) Query(req abci.RequestQuery) (res abci.ResponseQuery) {
 				// NOTE: this changed in tendermint and we didn't notice...
 				return app.FilterPeerByPubKey(path[3])
 			}
+		} else {
+			msg := "Expected second parameter to be filter"
+			return sdk.ErrUnknownRequest(msg).QueryResult()
 		}
 	}
-	msg := "unknown query path"
+
+	msg := "Expected path is p2p filter <addr|pubkey> <parameter>"
 	return sdk.ErrUnknownRequest(msg).QueryResult()
 }
 
@@ -472,8 +505,34 @@ func (app *BaseApp) Deliver(tx sdk.Tx) (result sdk.Result) {
 	return app.runTx(runTxModeDeliver, nil, tx)
 }
 
+func validateBasicTxMsgs(msgs []sdk.Msg) sdk.Error {
+	if msgs == nil || len(msgs) == 0 {
+		// TODO: probably shouldn't be ErrInternal. Maybe new ErrInvalidMessage, or ?
+		return sdk.ErrInternal("Tx.GetMsgs() must return at least one message in list")
+	}
+
+	for _, msg := range msgs {
+		// Validate the Msg.
+		err := msg.ValidateBasic()
+		if err != nil {
+			err = err.WithDefaultCodespace(sdk.CodespaceRoot)
+			return err
+		}
+	}
+	return nil
+}
+
+// Returns deliverState if app is in runTxModeDeliver, otherwhise returns checkstate
+func getState(app *BaseApp, mode runTxMode) *state {
+	if mode == runTxModeCheck || mode == runTxModeSimulate {
+		return app.checkState
+	}
+	return app.deliverState
+}
+
 // txBytes may be nil in some cases, eg. in tests.
 // Also, in the future we may support "internal" transactions.
+// nolint: gocyclo
 func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk.Result) {
 	//NOTE: GasWanted should be returned by the AnteHandler.
 	// GasUsed is determined by the GasMeter.
@@ -500,18 +559,9 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk
 
 	// Get the Msg.
 	var msgs = tx.GetMsgs()
-	if msgs == nil || len(msgs) == 0 {
-		// TODO: probably shouldn't be ErrInternal. Maybe new ErrInvalidMessage, or ?
-		return sdk.ErrInternal("Tx.GetMsgs() must return at least one message in list").Result()
-	}
-
-	for _, msg := range msgs {
-		// Validate the Msg.
-		err := msg.ValidateBasic()
-		if err != nil {
-			err = err.WithDefaultCodespace(sdk.CodespaceRoot)
-			return err.Result()
-		}
+	err := validateBasicTxMsgs(msgs)
+	if err != nil {
+		return err.Result()
 	}
 
 	// Run the ante handler.
@@ -526,17 +576,9 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk
 		gasWanted = anteResult.GasWanted
 	}
 
-	// Get the correct cache
-	var msCache sdk.CacheMultiStore
-	if mode == runTxModeCheck || mode == runTxModeSimulate {
-		// CacheWrap app.checkState.ms in case it fails.
-		msCache = app.checkState.CacheMultiStore()
-		ctx = ctx.WithMultiStore(msCache)
-	} else {
-		// CacheWrap app.deliverState.ms in case it fails.
-		msCache = app.deliverState.CacheMultiStore()
-		ctx = ctx.WithMultiStore(msCache)
-	}
+	// Get the correct cache, CacheWrap app.checkState.ms in case it fails.
+	msCache := getState(app, mode).CacheMultiStore()
+	ctx = ctx.WithMultiStore(msCache)
 
 	// accumulate results
 	logs := make([]string, 0, len(msgs))

client/keys/add.go

@@ -46,6 +46,8 @@ phrase, otherwise, a new key will be generated.`,
 	return cmd
 }
 
+// nolint: gocyclo
+// TODO remove the above when addressing #1446
 func runAddCmd(cmd *cobra.Command, args []string) error {
 	var kb keys.Keybase
 	var err error

server/testnet.go

@@ -60,10 +60,11 @@ Example:
 }
 
 func testnetWithConfig(config *cfg.Config, cdc *wire.Codec, appInit AppInit) error {
-
 	outDir := viper.GetString(outputDir)
+	numValidators := viper.GetInt(nValidators)
+
 	// Generate private key, node ID, initial transaction
-	for i := 0; i < viper.GetInt(nValidators); i++ {
+	for i := 0; i < numValidators; i++ {
 		nodeDirName := fmt.Sprintf("%s%d", viper.GetString(nodeDirPrefix), i)
 		nodeDir := filepath.Join(outDir, nodeDirName, "gaiad")
 		clientDir := filepath.Join(outDir, nodeDirName, "gaiacli")
@@ -83,18 +84,9 @@ func testnetWithConfig(config *cfg.Config, cdc *wire.Codec, appInit AppInit) err
 		}
 
 		config.Moniker = nodeDirName
-
-		ip := viper.GetString(startingIPAddress)
-		if len(ip) == 0 {
-			ip, err = externalIP()
-			if err != nil {
-				return err
-			}
-		} else {
-			ip, err = calculateIP(ip, i)
-			if err != nil {
-				return err
-			}
+		ip, err := getIP(i)
+		if err != nil {
+			return err
 		}
 
 		genTxConfig := gc.GenTx{
@@ -112,35 +104,22 @@ func testnetWithConfig(config *cfg.Config, cdc *wire.Codec, appInit AppInit) err
 
 		// Save private key seed words
 		name := fmt.Sprintf("%v.json", "key_seed")
-		writePath := filepath.Join(clientDir)
-		file := filepath.Join(writePath, name)
-		err = cmn.EnsureDir(writePath, 0700)
-		if err != nil {
-			return err
-		}
-		err = cmn.WriteFile(file, cliPrint, 0600)
+		err = writeFile(name, clientDir, cliPrint)
 		if err != nil {
 			return err
 		}
 
 		// Gather gentxs folder
 		name = fmt.Sprintf("%v.json", nodeDirName)
-		writePath = filepath.Join(gentxsDir)
-		file = filepath.Join(writePath, name)
-		err = cmn.EnsureDir(writePath, 0700)
-		if err != nil {
-			return err
-		}
-		err = cmn.WriteFile(file, genTxFile, 0644)
+		err = writeFile(name, gentxsDir, genTxFile)
 		if err != nil {
 			return err
 		}
-
 	}
 
 	// Generate genesis.json and config.toml
 	chainID := "chain-" + cmn.RandStr(6)
-	for i := 0; i < viper.GetInt(nValidators); i++ {
+	for i := 0; i < numValidators; i++ {
 
 		nodeDirName := fmt.Sprintf("%s%d", viper.GetString(nodeDirPrefix), i)
 		nodeDir := filepath.Join(outDir, nodeDirName, "gaiad")
@@ -165,6 +144,36 @@ func testnetWithConfig(config *cfg.Config, cdc *wire.Codec, appInit AppInit) err
 	return nil
 }
 
+func getIP(i int) (ip string, err error) {
+	ip = viper.GetString(startingIPAddress)
+	if len(ip) == 0 {
+		ip, err = externalIP()
+		if err != nil {
+			return "", err
+		}
+	} else {
+		ip, err = calculateIP(ip, i)
+		if err != nil {
+			return "", err
+		}
+	}
+	return ip, nil
+}
+
+func writeFile(name string, dir string, contents []byte) error {
+	writePath := filepath.Join(dir)
+	file := filepath.Join(writePath, name)
+	err := cmn.EnsureDir(writePath, 0700)
+	if err != nil {
+		return err
+	}
+	err = cmn.WriteFile(file, contents, 0600)
+	if err != nil {
+		return err
+	}
+	return nil
+}
+
 func calculateIP(ip string, i int) (string, error) {
 	ipv4 := net.ParseIP(ip).To4()
 	if ipv4 == nil {

server/util.go

@@ -149,24 +149,15 @@ func externalIP() (string, error) {
 		return "", err
 	}
 	for _, iface := range ifaces {
-		if iface.Flags&net.FlagUp == 0 {
-			continue // interface down
-		}
-		if iface.Flags&net.FlagLoopback != 0 {
-			continue // loopback interface
+		if skipInterface(iface) {
+			continue
 		}
 		addrs, err := iface.Addrs()
 		if err != nil {
 			return "", err
 		}
 		for _, addr := range addrs {
-			var ip net.IP
-			switch v := addr.(type) {
-			case *net.IPNet:
-				ip = v.IP
-			case *net.IPAddr:
-				ip = v.IP
-			}
+			ip := addrToIP(addr)
 			if ip == nil || ip.IsLoopback() {
 				continue
 			}
@@ -179,3 +170,24 @@ func externalIP() (string, error) {
 	}
 	return "", errors.New("are you connected to the network?")
 }
+
+func skipInterface(iface net.Interface) bool {
+	if iface.Flags&net.FlagUp == 0 {
+		return true // interface down
+	}
+	if iface.Flags&net.FlagLoopback != 0 {
+		return true // loopback interface
+	}
+	return false
+}
+
+func addrToIP(addr net.Addr) net.IP {
+	var ip net.IP
+	switch v := addr.(type) {
+	case *net.IPNet:
+		ip = v.IP
+	case *net.IPAddr:
+		ip = v.IP
+	}
+	return ip
+}