Run staticcheck in CI (#1453)

Many of the code issues can be found using static analysis. This PR adds
staticcheck to CI code checks.

.circleci/config.yml

@@ -22,6 +22,14 @@ commands:
  name: Run govet
  command: ./govet.sh
 
+ # staticcheck runs staticcheck in the entire codebase.
+ staticcheck:
+ steps:
+ - checkout
+ - run:
+ name: Run staticcheck
+ command: ./staticcheck.sh
+
  # check_deprecations ensures a release is actually removing deprecated fields
  # that were supposed to be discontinued in said release.
  check_deprecations:
@@ -109,8 +117,10 @@ jobs:
  docker:
  - image: circleci/golang:1.12
  steps:
+ - install_go_deps
  - gofmt
  - govet
+ - staticcheck
 
  # test_code_1_10 performs all package tests using Go 1.10.
  test_code_1_10:

build/change_trust.go

@@ -93,9 +93,7 @@ func RemoveTrust(code, issuer string, args ...interface{}) (result ChangeTrustBu
  Limit("0"),
  }
 
- for _, mut := range args {
- mutators = append(mutators, mut)
- }
+ mutators = append(mutators, args...)
 
  return ChangeTrust(mutators...)
 }

clients/horizon/internal.go

@@ -28,12 +28,3 @@ func decodeResponse(resp *http.Response, object interface{}) (err error) {
  }
  return
 }
-
-func loadMemo(p *Payment) error {
- res, err := http.Get(p.Links.Transaction.Href)
- if err != nil {
- return err
- }
- defer res.Body.Close()
- return json.NewDecoder(res.Body).Decode(&p.Memo)
-}

clients/horizon/mocks.go

@@ -41,9 +41,7 @@ func (m *MockClient) LoadAccountOffers(
  //
  // Go errors with: "too many arguments in call to m.Mock.Called"
  args := []interface{}{accountID}
- for _, param := range params {
- args = append(args, param)
- }
+ args = append(args, params...)
  a := m.Called(args...)
  return a.Get(0).(OffersPage), a.Error(1)
 }

clients/horizonclient/client.go

@@ -88,7 +88,7 @@ func (c *Client) stream(
  for {
  // updates the url with new cursor
  su.RawQuery = query.Encode()
- req, err := http.NewRequest("GET", fmt.Sprintf("%s", su), nil)
+ req, err := http.NewRequest("GET", su.String(), nil)
  if err != nil {
  return errors.Wrap(err, "error creating HTTP request")
  }
@@ -280,7 +280,7 @@ func (c *Client) Ledgers(request LedgerRequest) (ledgers hProtocol.LedgersPage,
 // LedgerDetail returns information about a particular ledger for a given sequence number
 // See https://www.stellar.org/developers/horizon/reference/endpoints/ledgers-single.html
 func (c *Client) LedgerDetail(sequence uint32) (ledger hProtocol.Ledger, err error) {
- if sequence <= 0 {
+ if sequence == 0 {
  err = errors.New("invalid sequence number provided")
  }
 
@@ -289,7 +289,6 @@ func (c *Client) LedgerDetail(sequence uint32) (ledger hProtocol.Ledger, err err
  }
 
  request := LedgerRequest{forSequence: sequence}
-
  err = c.sendRequest(request, &ledger)
  return
 }

exp/crypto/derivation/main.go

@@ -31,7 +31,7 @@ var (
  ErrInvalidPath = errors.New("Invalid derivation path")
  ErrNoPublicDerivation = errors.New("No public derivation for ed25519")
 
- pathRegex = regexp.MustCompile("^m(\\/[0-9]+')+$")
+ pathRegex = regexp.MustCompile(`^m(\/[0-9]+')+$`)
 )
 
 type Key struct {

exp/ingest/adapters/ledger_backend_adapter.go

@@ -42,8 +42,5 @@ func (lba *LedgerBackendAdapter) Close() error {
 
 // hasBackend checks for the presence of LedgerBackendAdapter.Backend.
 func (lba *LedgerBackendAdapter) hasBackend() bool {
- if lba.Backend == nil {
- return false
- }
- return true
+ return lba.Backend != nil
 }

exp/ingest/cmd/main.go

@@ -87,51 +87,51 @@ func useAdapter() {
 }
 
 // Demos direct use of the DatabaseBackend
-func useBackend() {
- dbb := ledgerbackend.DatabaseBackend{
- DataSourceName: dbURI,
- }
- defer dbb.Close()
-
- ledgerSequence, err := dbb.GetLatestLedgerSequence()
- if err != nil {
- log.Fatal(err)
- }
-
- fmt.Println("Latest ledger =", ledgerSequence)
-
- exists, ledgerCloseMeta, err := dbb.GetLedger(ledgerSequence)
-
- if err != nil {
- log.Fatal("error reading ledger from backend: ", err)
- }
- if !exists {
- log.Fatalf("Ledger %d was not found", ledgerSequence)
- }
-
- fmt.Println("N transactions =", len(ledgerCloseMeta.TransactionEnvelope))
- fmt.Println("ledgerCloseMeta.Transaction:", ledgerCloseMeta.TransactionEnvelope)
-
- fmt.Println("N transactionReults =", len(ledgerCloseMeta.TransactionResult))
- fmt.Println("ledgerCloseMeta.TransactionResults:", ledgerCloseMeta.TransactionResult)
-
- fmt.Println("N transactionMeta =", len(ledgerCloseMeta.TransactionMeta))
- fmt.Println("ledgerCloseMeta.TransactionMeta:", ledgerCloseMeta.TransactionMeta)
-
- a := ledgerCloseMeta.TransactionResult[0]
- fmt.Println("TransactionResultPair:", a)
- fmt.Println(" fee charged", a.Result.FeeCharged)
- fmt.Println(" ext", a.Result.Ext)
- fmt.Println(" result", a.Result.Result)
-
- b := ledgerCloseMeta.TransactionEnvelope[0]
- fmt.Println("TransactionEnvelope:", b)
- fmt.Println("b", b.Tx)
-
- c := ledgerCloseMeta.TransactionMeta[0]
- fmt.Println("TransactionMeta", c.Operations)
- fmt.Println(" operations", c.Operations)
- fmt.Println(" V", c.V)
- fmt.Println(" V1.Operations", c.V1.Operations)
- fmt.Println(" V1.TxChanges", c.V1.TxChanges)
-}
+// func useBackend() {
+//  dbb := ledgerbackend.DatabaseBackend{
+//  DataSourceName: dbURI,
+//  }
+//  defer dbb.Close()
+
+//  ledgerSequence, err := dbb.GetLatestLedgerSequence()
+//  if err != nil {
+//  log.Fatal(err)
+//  }
+
+//  fmt.Println("Latest ledger =", ledgerSequence)
+
+//  exists, ledgerCloseMeta, err := dbb.GetLedger(ledgerSequence)
+
+//  if err != nil {
+//  log.Fatal("error reading ledger from backend: ", err)
+//  }
+//  if !exists {
+//  log.Fatalf("Ledger %d was not found", ledgerSequence)
+//  }
+
+//  fmt.Println("N transactions =", len(ledgerCloseMeta.TransactionEnvelope))
+//  fmt.Println("ledgerCloseMeta.Transaction:", ledgerCloseMeta.TransactionEnvelope)
+
+//  fmt.Println("N transactionReults =", len(ledgerCloseMeta.TransactionResult))
+//  fmt.Println("ledgerCloseMeta.TransactionResults:", ledgerCloseMeta.TransactionResult)
+
+//  fmt.Println("N transactionMeta =", len(ledgerCloseMeta.TransactionMeta))
+//  fmt.Println("ledgerCloseMeta.TransactionMeta:", ledgerCloseMeta.TransactionMeta)
+
+//  a := ledgerCloseMeta.TransactionResult[0]
+//  fmt.Println("TransactionResultPair:", a)
+//  fmt.Println(" fee charged", a.Result.FeeCharged)
+//  fmt.Println(" ext", a.Result.Ext)
+//  fmt.Println(" result", a.Result.Result)
+
+//  b := ledgerCloseMeta.TransactionEnvelope[0]
+//  fmt.Println("TransactionEnvelope:", b)
+//  fmt.Println("b", b.Tx)
+
+//  c := ledgerCloseMeta.TransactionMeta[0]
+//  fmt.Println("TransactionMeta", c.Operations)
+//  fmt.Println(" operations", c.Operations)
+//  fmt.Println(" V", c.V)
+//  fmt.Println(" V1.Operations", c.V1.Operations)
+//  fmt.Println(" V1.TxChanges", c.V1.TxChanges)
+// }

exp/ingest/ledgerbackend/ledger_backend.go

@@ -62,16 +62,16 @@ type txFeeHistory struct {
 }
 
 // scpHistory holds a row of data from the stellar-core `scphistory` table.
-type scpHistory struct {
- NodeID string `db:"nodeid"`
- LedgerSeq uint32 `db:"ledgerseq"`
- Envelope string `db:"envelope"`
-}
+// type scpHistory struct {
+//  NodeID string `db:"nodeid"`
+//  LedgerSeq uint32 `db:"ledgerseq"`
+//  Envelope string `db:"envelope"`
+// }
 
 // upgradeHistory holds a row of data from the stellar-core `upgradehistory` table.
-type upgradeHistory struct {
- LedgerSeq uint32 `db:"ledgerseq"`
- UpgradeIndex uint32 `db:"upgradeindex"`
- Upgrade string `db:"upgrade"`
- Changes string `db:"changes"`
-}
+// type upgradeHistory struct {
+//  LedgerSeq uint32 `db:"ledgerseq"`
+//  UpgradeIndex uint32 `db:"upgradeindex"`
+//  Upgrade string `db:"upgrade"`
+//  Changes string `db:"changes"`
+// }

exp/support/pipeline/main.go

@@ -3,7 +3,6 @@ package pipeline
 import (
  "context"
  "sync"
- "time"
 )
 
 // BufferedReadWriteCloser implements ReadCloser and WriteCloser and acts
@@ -51,7 +50,6 @@ type PipelineNode struct {
  Processor Processor
  Children []*PipelineNode
 
- duration time.Duration
  jobs int
  readEntries int
  readsPerSecond int

network/main.go

@@ -4,7 +4,6 @@ package network
 
 import (
  "bytes"
- "fmt"
 
  "strings"
 
@@ -38,7 +37,8 @@ func HashTransaction(tx *xdr.Transaction, passphrase string) ([32]byte, error) {
  return [32]byte{}, errors.New("empty network passphrase")
  }
 
- _, err := fmt.Fprintf(&txBytes, "%s", ID(passphrase))
+ id := ID(passphrase)
+ _, err := txBytes.Write(id[:])
  if err != nil {
  return [32]byte{}, errors.Wrap(err, "fprint network id failed")
  }

protocols/compliance/validator.go

@@ -19,9 +19,5 @@ func isStellarAddress(i interface{}, context interface{}) bool {
 
  _, _, err := address.Split(addr)
 
- if err == nil {
- return true
- }
-
- return false
+ return err == nil
 }

protocols/horizon/operations/main.go

@@ -55,8 +55,8 @@ type Base struct {
 }
 
 // PagingToken implements hal.Pageable
-func (this Base) PagingToken() string {
- return this.PT
+func (base Base) PagingToken() string {
+ return base.PT
 }
 
 // BumpSequence is the json resource representing a single operation whose type is
@@ -204,25 +204,25 @@ type Operation interface {
 }
 
 // GetType returns the type of operation
-func (this Base) GetType() string {
- return this.Type
+func (base Base) GetType() string {
+ return base.Type
 }
 
 // GetTypeI returns the ID of type of operation
-func (this Base) GetTypeI() int32 {
- return this.TypeI
+func (base Base) GetTypeI() int32 {
+ return base.TypeI
 }
 
-func (this Base) GetID() string {
- return this.ID
+func (base Base) GetID() string {
+ return base.ID
 }
 
-func (this Base) GetTransactionHash() string {
- return this.TransactionHash
+func (base Base) GetTransactionHash() string {
+ return base.TransactionHash
 }
 
-func (this Base) IsTransactionSuccessful() bool {
- return this.TransactionSuccessful
+func (base Base) IsTransactionSuccessful() bool {
+ return base.TransactionSuccessful
 }
 
 // OperationsPage is the json resource representing a page of operations.

services/bifrost/server/server.go

@@ -272,5 +272,4 @@ func (s *Server) HandlerRecoveryTransaction(w stdhttp.ResponseWriter, r *stdhttp
  }
 
  w.WriteHeader(stdhttp.StatusOK)
- return
 }

services/bifrost/stress/bitcoin.go

@@ -1,7 +1,6 @@
 package stress
 
 import (
- "encoding/hex"
  "errors"
  "math/rand"
  "time"
@@ -145,9 +144,3 @@ func (g *RandomBitcoinClient) randomAddress() btcutil.Address {
 func (g *RandomBitcoinClient) randomAmount() int64 {
  return rand.Int63n(100 * satsInBtc)
 }
-
-func (g *RandomBitcoinClient) randomHash() string {
- var hash [32]byte
- rand.Read(hash[:])
- return hex.EncodeToString(hash[:])
-}

services/bridge/internal/config/main.go

@@ -45,6 +45,8 @@ type Database struct {
  URL string `valid:"required"`
 }
 
+var assetCodeMatch = regexp.MustCompile("^[a-zA-Z0-9]{1,12}$")
+
 // Validate validates config and returns error if any of config values is incorrect
 func (c *Config) Validate() (err error) {
  if c.Port == nil {
@@ -84,12 +86,7 @@ func (c *Config) Validate() (err error) {
  }
  }
 
- var matched bool
- matched, err = regexp.MatchString("^[a-zA-Z0-9]{1,12}$", asset.Code)
- if err != nil {
- return err
- }
-
+ matched := assetCodeMatch.MatchString(asset.Code)
  if !matched {
  return errors.New("Invalid asset code: " + asset.Code)
  }

services/bridge/internal/handlers/main.go

@@ -22,12 +22,3 @@ type RequestHandler struct {
  TransactionSubmitter submitter.TransactionSubmitterInterface `inject:""`
  PaymentListener *listener.PaymentListener `inject:""`
 }
-
-func (rh *RequestHandler) isAssetAllowed(code string, issuer string) bool {
- for _, asset := range rh.Config.Assets {
- if asset.Code == code && asset.Issuer == issuer {
- return true
- }
- }
- return false
-}