Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

apply the megacheck code vetting tool for idiomatic go #3949

Merged
merged 7 commits into from
Jun 7, 2017
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .codeclimate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ exclude_paths:
engines:
fixme:
enabled: true
config:
strings:
- FIXME
- HACK
- XXX
- BUG
golint:
enabled: true
govet:
Expand Down
8 changes: 1 addition & 7 deletions Rules.mk
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,6 @@ include $(dir)/Rules.mk
# -------------------- #
# core targets #
# -------------------- #
PACKAGES_NOVENDOR := $(shell go list github.com/ipfs/go-ipfs/... | grep -v /Godeps/)

megacheck:
@go get honnef.co/go/tools/cmd/megacheck
@for pkg in ${PACKAGES_NOVENDOR}; do megacheck "$$pkg"; done

.PHONY: megacheck

build: $(TGT_BIN)
.PHONY: build
Expand Down Expand Up @@ -149,6 +142,7 @@ help:
@echo ' test_go_short'
@echo ' test_go_expensive'
@echo ' test_go_race'
@echo ' test_go_megacheck' - Run the `megacheck` vetting tool
@echo ' test_sharness_short'
@echo ' test_sharness_expensive'
@echo ' test_sharness_race'
Expand Down
9 changes: 2 additions & 7 deletions blocks/blockstore/blockstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,6 @@ func NewBlockstore(d ds.Batching) Blockstore {
type blockstore struct {
datastore ds.Batching

lk sync.RWMutex
gcreq int32
gcreqlk sync.Mutex

rehash bool
}

Expand Down Expand Up @@ -246,9 +242,8 @@ func NewGCLocker() GCLocker {
}

type gclocker struct {
lk sync.RWMutex
gcreq int32
gcreqlk sync.Mutex
lk sync.RWMutex
gcreq int32
}

// Unlocker represents an object which can Unlock
Expand Down
7 changes: 1 addition & 6 deletions cmd/ipfs/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,12 @@ import (
// log is the command logger
var log = logging.Logger("cmd/ipfs")

var (
// errUnexpectedApiOutput = errors.New("api returned unexpected output")
// errApiVersionMismatch = errors.New("api version mismatch")
errRequestCanceled = errors.New("request canceled")
)
var errRequestCanceled = errors.New("request canceled")

const (
EnvEnableProfiling = "IPFS_PROF"
cpuProfile = "ipfs.cpuprof"
heapProfile = "ipfs.memprof"
// errorFormat = "ERROR: %v\n\n"
)

type cmdInvocation struct {
Expand Down
1 change: 0 additions & 1 deletion commands/files/multipartfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (

const (
multipartFormdataType = "multipart/form-data"
// multipartMixedType = "multipart/mixed"

applicationDirectory = "application/x-directory"
applicationSymlink = "application/symlink"
Expand Down
9 changes: 3 additions & 6 deletions commands/http/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,9 @@ const (
extraContentLengthHeader = "X-Content-Length"
uaHeader = "User-Agent"
contentTypeHeader = "Content-Type"
// contentDispHeader = "Content-Disposition"
// transferEncodingHeader = "Transfer-Encoding"
applicationJson = "application/json"
applicationOctetStream = "application/octet-stream"
plainText = "text/plain"
// originHeader = "origin"
applicationJson = "application/json"
applicationOctetStream = "application/octet-stream"
plainText = "text/plain"
)

var AllowedExposedHeadersArr = []string{streamHeader, channelHeader, extraContentLengthHeader}
Expand Down
5 changes: 0 additions & 5 deletions core/commands/files/files.go
Original file line number Diff line number Diff line change
Expand Up @@ -664,11 +664,6 @@ stat' on the file or any of its ancestors.
return
}

var r io.Reader = input
if countfound {
r = io.LimitReader(r, int64(count))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrm... Maybe you discovered a bug here. I think the copy below was supposed to be from 'r' instead of 'input'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense. The tests are happy with the fix.

}

n, err := io.Copy(wfd, input)
if err != nil {
res.SetError(err, cmds.ErrNormal)
Expand Down
4 changes: 1 addition & 3 deletions core/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ import (

const IpnsValidatorTag = "ipns"

// const kSizeBlockstoreWriteCache = 100
const kReprovideFrequency = time.Hour * 12
const discoveryConnTimeout = time.Second * 30

Expand All @@ -84,8 +83,7 @@ type mode int

const (
// zero value is not a valid mode, must be explicitly set
invalidMode mode = iota
localMode
localMode mode = iota
offlineMode
onlineMode
)
Expand Down
7 changes: 1 addition & 6 deletions core/corerouting/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,7 @@ import (
// default and 2) to avoid a circular dependency (it needs to be referenced in
// the core if it's going to be the default)

var (
// errHostMissing = errors.New("supernode routing client requires a Host component")
// errIdentityMissing = errors.New("supernode routing server requires a peer ID identity")
// errPeerstoreMissing = errors.New("supernode routing server requires a peerstore")
errServersMissing = errors.New("supernode routing client requires at least 1 server peer")
)
var errServersMissing = errors.New("supernode routing client requires at least 1 server peer")

// SupernodeServer returns a configuration for a routing server that stores
// routing records to the provided datastore. Only routing records are store in
Expand Down
5 changes: 2 additions & 3 deletions exchange/bitswap/bitswap.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,8 @@ const (
// TODO: if a 'non-nice' strategy is implemented, consider increasing this value
maxProvidersPerRequest = 3
providerRequestTimeout = time.Second * 10
// hasBlockTimeout = time.Second * 15
provideTimeout = time.Second * 15
sizeBatchRequestChan = 32
provideTimeout = time.Second * 15
sizeBatchRequestChan = 32
// kMaxPriority is the max priority as defined by the bitswap protocol
kMaxPriority = math.MaxInt32
)
Expand Down
4 changes: 2 additions & 2 deletions exchange/bitswap/bitswap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -601,14 +601,14 @@ func TestBitswapLedgerTwoWay(t *testing.T) {

ctx, cancel := context.WithTimeout(context.Background(), time.Second*5)
defer cancel()
blk, err := instances[1].Exchange.GetBlock(ctx, blocks[0].Cid())
_, err = instances[1].Exchange.GetBlock(ctx, blocks[0].Cid())
if err != nil {
t.Fatal(err)
}

ctx, cancel = context.WithTimeout(context.Background(), time.Second*5)
defer cancel()
blk, err = instances[0].Exchange.GetBlock(ctx, blocks[1].Cid())
blk, err := instances[0].Exchange.GetBlock(ctx, blocks[1].Cid())
if err != nil {
t.Fatal(err)
}
Expand Down
3 changes: 0 additions & 3 deletions exchange/bitswap/decision/ledger.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ type ledger struct {
// Accounting tracks bytes sent and recieved.
Accounting debtRatio

// firstExchnage is the time of the first data exchange.
firstExchange time.Time

// lastExchange is the time of the last data exchange.
lastExchange time.Time

Expand Down
2 changes: 0 additions & 2 deletions exchange/bitswap/testutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,6 @@ func (i *Instance) SetBlockstoreLatency(t time.Duration) time.Duration {
// just a much better idea.
func Session(ctx context.Context, net tn.Network, p testutil.Identity) Instance {
bsdelay := delay.Fixed(0)
// const bloomSize = 512
// const writeCacheElems = 100

adapter := net.Adapter(p)
dstore := ds_sync.MutexWrap(datastore2.WithDelay(ds.NewMapDatastore(), bsdelay))
Expand Down
10 changes: 0 additions & 10 deletions exchange/bitswap/wantmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,6 @@ func NewWantManager(ctx context.Context, network bsnet.BitSwapNetwork) *WantMana
}
}

/*type msgPair struct {
to peer.ID
msg bsmsg.BitSwapMessage
}

type cancellation struct {
who peer.ID
blk *cid.Cid
}*/

type msgQueue struct {
p peer.ID

Expand Down
5 changes: 0 additions & 5 deletions fuse/node/mount_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"errors"
"fmt"
"strings"
"time"

core "github.com/ipfs/go-ipfs/core"
ipns "github.com/ipfs/go-ipfs/fuse/ipns"
Expand All @@ -18,10 +17,6 @@ import (

var log = logging.Logger("node")

// amount of time to wait for mount errors
// TODO is this non-deterministic?
const mountTimeout = time.Second

// fuseNoDirectory used to check the returning fuse error
const fuseNoDirectory = "fusermount: failed to access mountpoint"

Expand Down
1 change: 0 additions & 1 deletion fuse/readonly/readonly_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ func (*Root) ReadDirAll(ctx context.Context) ([]fuse.Dirent, error) {
type Node struct {
Ipfs *core.IpfsNode
Nd *mdag.ProtoNode
fd *uio.DagReader
cached *ftpb.Data
}

Expand Down
6 changes: 0 additions & 6 deletions importer/balanced/balanced_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
h "github.com/ipfs/go-ipfs/importer/helpers"
dag "github.com/ipfs/go-ipfs/merkledag"
mdtest "github.com/ipfs/go-ipfs/merkledag/test"
pin "github.com/ipfs/go-ipfs/pin"
uio "github.com/ipfs/go-ipfs/unixfs/io"

"context"
Expand Down Expand Up @@ -124,11 +123,6 @@ func dagrArrComp(t *testing.T, r io.Reader, should []byte) {
}
}

type dagservAndPinner struct {
ds dag.DAGService
mp pin.Pinner
}

func TestIndirectBlocks(t *testing.T) {
ds := mdtest.Mock()
dag, buf := getTestDag(t, ds, 1024*1024, 512)
Expand Down
2 changes: 0 additions & 2 deletions importer/helpers/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"fmt"
"os"

chunk "github.com/ipfs/go-ipfs/importer/chunk"
dag "github.com/ipfs/go-ipfs/merkledag"
pi "github.com/ipfs/go-ipfs/thirdparty/posinfo"
ft "github.com/ipfs/go-ipfs/unixfs"
Expand All @@ -18,7 +17,6 @@ import (
var BlockSizeLimit = 1048576 // 1 MB

// rough estimates on expected sizes
var roughDataBlockSize = chunk.DefaultBlockSize
var roughLinkBlockSize = 1 << 13 // 8KB
var roughLinkSize = 34 + 8 + 5 // sha256 multihash + size + no name + protobuf framing

Expand Down
34 changes: 0 additions & 34 deletions importer/trickle/trickle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
h "github.com/ipfs/go-ipfs/importer/helpers"
merkledag "github.com/ipfs/go-ipfs/merkledag"
mdtest "github.com/ipfs/go-ipfs/merkledag/test"
pin "github.com/ipfs/go-ipfs/pin"
ft "github.com/ipfs/go-ipfs/unixfs"
uio "github.com/ipfs/go-ipfs/unixfs/io"

Expand Down Expand Up @@ -125,11 +124,6 @@ func arrComp(a, b []byte) error {
return nil
}

type dagservAndPinner struct {
ds merkledag.DAGService
mp pin.Pinner
}

func TestIndirectBlocks(t *testing.T) {
splitter := chunk.SizeSplitterGen(512)
nbytes := 1024 * 1024
Expand Down Expand Up @@ -565,31 +559,3 @@ func TestAppendSingleBytesToEmpty(t *testing.T) {
t.Fatal(err)
}
}

func printDag(nd *merkledag.ProtoNode, ds merkledag.DAGService, indent int) {
pbd, err := ft.FromBytes(nd.Data())
if err != nil {
panic(err)
}

for i := 0; i < indent; i++ {
fmt.Print(" ")
}
fmt.Printf("{size = %d, type = %s, nc = %d", pbd.GetFilesize(), pbd.GetType().String(), len(pbd.GetBlocksizes()))
if len(nd.Links()) > 0 {
fmt.Println()
}
for _, lnk := range nd.Links() {
child, err := lnk.GetNode(context.Background(), ds)
if err != nil {
panic(err)
}
printDag(child.(*merkledag.ProtoNode), ds, indent+1)
}
if len(nd.Links()) > 0 {
for i := 0; i < indent; i++ {
fmt.Print(" ")
}
}
fmt.Println("}")
}
6 changes: 6 additions & 0 deletions mk/golang.mk
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ CHECK_GO :=
go-pkg-name=$(shell go list $(go-tags) github.com/ipfs/go-ipfs/$(1))
go-main-name=$(notdir $(call go-pkg-name,$(1)))$(?exe)
go-curr-pkg-tgt=$(d)/$(call go-main-name,$(d))
go-pkgs-novendor=$(shell go list github.com/ipfs/go-ipfs/... | grep -v /Godeps/)

go-tags=$(if $(GOTAGS), -tags="$(call join-with,$(space),$(GOTAGS))")
go-flags-with-tags=$(GOFLAGS)$(go-tags)
Expand All @@ -39,6 +40,11 @@ test_go_fmt:
.PHONY: test_go_fmt
TEST_GO += test_go_fmt

test_go_megacheck:
@go get honnef.co/go/tools/cmd/megacheck
@for pkg in $(go-pkgs-novendor); do megacheck "$$pkg"; done
.PHONY: megacheck

test_go: $(TEST_GO)

check_go_version:
Expand Down
17 changes: 3 additions & 14 deletions namesys/publisher.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,17 +160,11 @@ func PutRecordToRouting(ctx context.Context, k ci.PrivKey, value path.Path, seqn
errs <- PublishPublicKey(ctx, r, namekey, k.GetPublic())
}()

err = waitOnErrChan(ctx, errs)
if err != nil {
return err
}

err = waitOnErrChan(ctx, errs)
if err != nil {
if err := waitOnErrChan(ctx, errs); err != nil {
return err
}

return nil
return waitOnErrChan(ctx, errs)
}

func waitOnErrChan(ctx context.Context, errs chan error) error {
Expand Down Expand Up @@ -340,12 +334,7 @@ func InitializeKeyspace(ctx context.Context, ds dag.DAGService, pub Publisher, p
return err
}

err = pub.Publish(ctx, key, path.FromCid(nodek))
if err != nil {
return err
}

return nil
return pub.Publish(ctx, key, path.FromCid(nodek))
}

func IpnsKeysForID(id peer.ID) (name, ipns string) {
Expand Down
7 changes: 0 additions & 7 deletions repo/config/supernode.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,6 @@ var DefaultSNRServers = []string{
"/ip4/178.62.61.185/tcp/4002/ipfs/QmVw6fGNqBixZE4bewRLT2VXX7fAHUHs8JyidDiJ1P7RUN",
}

func initSNRConfig() (*SupernodeClientConfig, error) {
// TODO perform validation
return &SupernodeClientConfig{
Servers: DefaultSNRServers,
}, nil
}

func (gcr *SupernodeClientConfig) ServerIPFSAddrs() ([]ipfsaddr.IPFSAddr, error) {
var addrs []ipfsaddr.IPFSAddr
for _, server := range gcr.Servers {
Expand Down
Loading