From 2dcb3a11a5df46d13952f9f2f975da0eba3f5ae8 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 5 Sep 2017 17:40:46 +0100 Subject: [PATCH] Use gometalinter (#210) * Remove unused struct field * Ignore unused test data * Remove unused variables * Remove deadcode * Fix up vetshadow warnings * Convert to using gometalinter * Update travis * Use vendored versions of gometalinter * Make gometalinter install its stuff * Vendor misspell --- .travis.yml | 4 --- hooks/pre-commit | 34 +++++++++---------- linter.json | 17 ++++++++++ .../storage/accounts/account_data_table.go | 3 -- .../auth/storage/accounts/membership_table.go | 1 - .../dendrite/clientapi/readers/login.go | 5 --- .../dendrite/cmd/create-room-events/main.go | 7 ---- .../cmd/dendrite-monolith-server/main.go | 2 +- .../cmd/roomserver-integration-tests/main.go | 15 ++------ .../syncserver-integration-tests/testdata.go | 1 + .../roomserver/input/latest_events.go | 8 ++--- .../dendrite/syncapi/sync/notifier.go | 14 ++++---- 12 files changed, 48 insertions(+), 63 deletions(-) create mode 100644 linter.json diff --git a/.travis.yml b/.travis.yml index 27f6cb90..ab2492d2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -18,10 +18,6 @@ services: install: - go get github.com/constabulary/gb/... - - go get github.com/golang/lint/golint - - go get github.com/fzipp/gocyclo - - go get github.com/client9/misspell/... - - go get github.com/gordonklaus/ineffassign # Generate a self-signed X.509 certificate for TLS. before_script: diff --git a/hooks/pre-commit b/hooks/pre-commit index abce0f59..8d71b1c6 100755 --- a/hooks/pre-commit +++ b/hooks/pre-commit @@ -2,29 +2,27 @@ set -eu -golint src/... +export GOPATH="$(pwd):$(pwd)/vendor" +export PATH="$PATH:$(pwd)/vendor/bin:$(pwd)/bin" + +echo "Installing lint search engine..." +go install github.com/alecthomas/gometalinter/ +gometalinter --config=linter.json ./... --install + +echo "Looking for lint..." +gometalinter --config=linter.json ./... + +echo "Double checking spelling..." misspell -error src *.md -# gofmt doesn't exit with an error code if the files don't match the expected -# format. So we have to run it and see if it outputs anything. -if gofmt -l -s ./src/ 2>&1 | read -then - echo "Error: not all code had been formatted with gofmt." - echo "Fixing the following files" - gofmt -s -w -l ./src/ - echo - echo "Please add them to the commit" - git status --short - exit 1 -fi - -ineffassign ./src/ -go tool vet --all --shadow ./src -gocyclo -over 12 src/ +echo "Testing..." gb test # Check that all the packages can build. # When `go build` is given multiple packages it won't output anything, and just # checks that everything builds. This seems to do a better job of handling # missing imports than `gb build` does. -GOPATH=$(pwd):$(pwd)/vendor go build github.com/matrix-org/dendrite/cmd/... +echo "Double checking it builds..." +go build github.com/matrix-org/dendrite/cmd/... + +echo "Done!" diff --git a/linter.json b/linter.json new file mode 100644 index 00000000..c2a3d80e --- /dev/null +++ b/linter.json @@ -0,0 +1,17 @@ +{ + "Vendor": true, + "Cyclo": 12, + "Enable": [ + "vetshadow", + "gotype", + "deadcode", + "gocyclo", + "golint", + "varcheck", + "structcheck", + "aligncheck", + "ineffassign", + "gas", + "misspell" + ] +} diff --git a/src/github.com/matrix-org/dendrite/clientapi/auth/storage/accounts/account_data_table.go b/src/github.com/matrix-org/dendrite/clientapi/auth/storage/accounts/account_data_table.go index 0c1fc0ff..27ca5b61 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/auth/storage/accounts/account_data_table.go +++ b/src/github.com/matrix-org/dendrite/clientapi/auth/storage/accounts/account_data_table.go @@ -47,9 +47,6 @@ const selectAccountDataSQL = "" + const selectAccountDataByTypeSQL = "" + "SELECT content FROM account_data WHERE localpart = $1 AND room_id = $2 AND type = $3" -const deleteAccountDataSQL = "" + - "DELETE FROM account_data WHERE localpart = $1 AND room_id = $2 AND type = $3" - type accountDataStatements struct { insertAccountDataStmt *sql.Stmt selectAccountDataStmt *sql.Stmt diff --git a/src/github.com/matrix-org/dendrite/clientapi/auth/storage/accounts/membership_table.go b/src/github.com/matrix-org/dendrite/clientapi/auth/storage/accounts/membership_table.go index 9652cdac..590c5ed8 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/auth/storage/accounts/membership_table.go +++ b/src/github.com/matrix-org/dendrite/clientapi/auth/storage/accounts/membership_table.go @@ -56,7 +56,6 @@ const updateMembershipByEventIDSQL = "" + type membershipStatements struct { deleteMembershipsByEventIDsStmt *sql.Stmt insertMembershipStmt *sql.Stmt - selectMembershipByEventIDStmt *sql.Stmt selectMembershipsByLocalpartStmt *sql.Stmt updateMembershipByEventIDStmt *sql.Stmt } diff --git a/src/github.com/matrix-org/dendrite/clientapi/readers/login.go b/src/github.com/matrix-org/dendrite/clientapi/readers/login.go index 270b2e5a..1f6b5071 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/readers/login.go +++ b/src/github.com/matrix-org/dendrite/clientapi/readers/login.go @@ -15,7 +15,6 @@ package readers import ( - "fmt" "net/http" "github.com/matrix-org/dendrite/clientapi/auth" @@ -121,7 +120,3 @@ func Login( JSON: jsonerror.NotFound("Bad method"), } } - -func makeUserID(localpart string, domain gomatrixserverlib.ServerName) string { - return fmt.Sprintf("@%s:%s", localpart, domain) -} diff --git a/src/github.com/matrix-org/dendrite/cmd/create-room-events/main.go b/src/github.com/matrix-org/dendrite/cmd/create-room-events/main.go index a37c7685..c7b9d953 100644 --- a/src/github.com/matrix-org/dendrite/cmd/create-room-events/main.go +++ b/src/github.com/matrix-org/dendrite/cmd/create-room-events/main.go @@ -49,13 +49,6 @@ var ( format = flag.String("Format", "InputRoomEvent", "The output format to use for the messages: InputRoomEvent or Event") ) -func defaulting(value, defaultValue string) string { - if value == "" { - return defaultValue - } - return value -} - // By default we use a private key of 0. const defaultKey = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" diff --git a/src/github.com/matrix-org/dendrite/cmd/dendrite-monolith-server/main.go b/src/github.com/matrix-org/dendrite/cmd/dendrite-monolith-server/main.go index 8cc1ccbb..9b6422c5 100644 --- a/src/github.com/matrix-org/dendrite/cmd/dendrite-monolith-server/main.go +++ b/src/github.com/matrix-org/dendrite/cmd/dendrite-monolith-server/main.go @@ -197,7 +197,6 @@ func (m *monolith) setupFederation() { } func (m *monolith) setupKafka() { - var err error if m.cfg.Kafka.UseNaffka { naff, err := naffka.New(&naffka.MemoryDatabase{}) if err != nil { @@ -208,6 +207,7 @@ func (m *monolith) setupKafka() { m.naffka = naff m.kafkaProducer = naff } else { + var err error m.kafkaProducer, err = sarama.NewSyncProducer(m.cfg.Kafka.Addresses, nil) if err != nil { log.WithFields(log.Fields{ diff --git a/src/github.com/matrix-org/dendrite/cmd/roomserver-integration-tests/main.go b/src/github.com/matrix-org/dendrite/cmd/roomserver-integration-tests/main.go index 43305c2f..a95156f3 100644 --- a/src/github.com/matrix-org/dendrite/cmd/roomserver-integration-tests/main.go +++ b/src/github.com/matrix-org/dendrite/cmd/roomserver-integration-tests/main.go @@ -221,14 +221,14 @@ func testRoomserver(input []string, wantOutput []string, checkQueries func(api.R if err != nil { panic(err) } - if err := test.WriteConfig(cfg, dir); err != nil { + if err = test.WriteConfig(cfg, dir); err != nil { panic(err) } outputTopic := string(cfg.Kafka.Topics.OutputRoomEvent) exe.DeleteTopic(outputTopic) - if err := exe.CreateTopic(outputTopic); err != nil { + if err = exe.CreateTopic(outputTopic); err != nil { panic(err) } @@ -271,17 +271,6 @@ func testRoomserver(input []string, wantOutput []string, checkQueries func(api.R } } -func canonicalJSONInput(jsonData []string) []string { - for i := range jsonData { - jsonBytes, err := gomatrixserverlib.CanonicalJSON([]byte(jsonData[i])) - if err != nil { - panic(err) - } - jsonData[i] = string(jsonBytes) - } - return jsonData -} - func equalJSON(a, b string) bool { canonicalA, err := gomatrixserverlib.CanonicalJSON([]byte(a)) if err != nil { diff --git a/src/github.com/matrix-org/dendrite/cmd/syncserver-integration-tests/testdata.go b/src/github.com/matrix-org/dendrite/cmd/syncserver-integration-tests/testdata.go index 14c5dd32..dfbcc6a5 100644 --- a/src/github.com/matrix-org/dendrite/cmd/syncserver-integration-tests/testdata.go +++ b/src/github.com/matrix-org/dendrite/cmd/syncserver-integration-tests/testdata.go @@ -14,6 +14,7 @@ package main +// nolint: varcheck, deadcode const ( i0StateRoomCreate = iota i1StateAliceJoin diff --git a/src/github.com/matrix-org/dendrite/roomserver/input/latest_events.go b/src/github.com/matrix-org/dendrite/roomserver/input/latest_events.go index d9aa2b45..fe485607 100644 --- a/src/github.com/matrix-org/dendrite/roomserver/input/latest_events.go +++ b/src/github.com/matrix-org/dendrite/roomserver/input/latest_events.go @@ -99,14 +99,14 @@ type latestEventsUpdater struct { } func (u *latestEventsUpdater) doUpdateLatestEvents() error { - var err error var prevEvents []gomatrixserverlib.EventReference prevEvents = u.event.PrevEvents() oldLatest := u.updater.LatestEvents() u.lastEventIDSent = u.updater.LastEventIDSent() u.oldStateNID = u.updater.CurrentStateSnapshotNID() - if hasBeenSent, err := u.updater.HasEventBeenSent(u.stateAtEvent.EventNID); err != nil { + hasBeenSent, err := u.updater.HasEventBeenSent(u.stateAtEvent.EventNID) + if err != nil { return err } else if hasBeenSent { // Already sent this event so we can stop processing @@ -119,8 +119,8 @@ func (u *latestEventsUpdater) doUpdateLatestEvents() error { eventReference := u.event.EventReference() // Check if this event is already referenced by another event in the room. - var alreadyReferenced bool - if alreadyReferenced, err = u.updater.IsReferenced(eventReference); err != nil { + alreadyReferenced, err := u.updater.IsReferenced(eventReference) + if err != nil { return err } diff --git a/src/github.com/matrix-org/dendrite/syncapi/sync/notifier.go b/src/github.com/matrix-org/dendrite/syncapi/sync/notifier.go index 2fa4279c..49eed5eb 100644 --- a/src/github.com/matrix-org/dendrite/syncapi/sync/notifier.go +++ b/src/github.com/matrix-org/dendrite/syncapi/sync/notifier.go @@ -67,7 +67,7 @@ func (n *Notifier) OnNewEvent(ev *gomatrixserverlib.Event, userID string, pos ty userIDs := n.joinedUsers(ev.RoomID()) // If this is an invite, also add in the invitee to this list. if ev.Type() == "m.room.member" && ev.StateKey() != nil { - userID := *ev.StateKey() + targetUserID := *ev.StateKey() membership, err := ev.Membership() if err != nil { log.WithError(err).WithField("event_id", ev.EventID()).Errorf( @@ -77,22 +77,22 @@ func (n *Notifier) OnNewEvent(ev *gomatrixserverlib.Event, userID string, pos ty // Keep the joined user map up-to-date switch membership { case "invite": - userIDs = append(userIDs, userID) + userIDs = append(userIDs, targetUserID) case "join": // Manually append the new user's ID so they get notified // along all members in the room - userIDs = append(userIDs, userID) - n.addJoinedUser(ev.RoomID(), userID) + userIDs = append(userIDs, targetUserID) + n.addJoinedUser(ev.RoomID(), targetUserID) case "leave": fallthrough case "ban": - n.removeJoinedUser(ev.RoomID(), userID) + n.removeJoinedUser(ev.RoomID(), targetUserID) } } } - for _, userID := range userIDs { - n.wakeupUser(userID, pos) + for _, toNotifyUserID := range userIDs { + n.wakeupUser(toNotifyUserID, pos) } } else if len(userID) > 0 { n.wakeupUser(userID, pos)