From 6879960d2242820612395a3a42c150b2c960b317 Mon Sep 17 00:00:00 2001 From: Daniele Sluijters Date: Tue, 18 May 2021 13:48:04 +0200 Subject: [PATCH] Replace golint with staticcheck golint has been deprecated and archived. There's no 1:1 match for it, but staticcheck is a checker the golint README recommends and has tons of useful checks. This commit enables all checks, and then disables: * ST1000: Incorrect or missing package comment * ST1005: Incorrectly formatted error string, necessary due to the fact that all our error strings are capitalised, which they should not be * SA1019: Locally in goneb.go because we have to rework how we use the prometheus package there It also fixes a number of other errors surfaced by staticheck. --- Dockerfile | 4 ++-- README.md | 4 ++-- api/handlers/auth.go | 22 +++++++++--------- api/handlers/service.go | 4 ++-- clients/bot_client.go | 12 +++++----- goneb.go | 2 ++ hooks/pre-commit | 2 +- polling/polling.go | 4 ++-- services/giphy/giphy.go | 3 +-- services/github/client/client.go | 4 +++- services/github/github_webhook.go | 31 ------------------------- services/github/webhook/webhook_test.go | 1 + services/rssbot/rssbot.go | 18 +++++++------- services/slackapi/message.go | 6 ++--- staticcheck.conf | 1 + types/actions.go | 2 +- 16 files changed, 47 insertions(+), 73 deletions(-) create mode 100644 staticcheck.conf diff --git a/Dockerfile b/Dockerfile index bb827b5..3cdf577 100644 --- a/Dockerfile +++ b/Dockerfile @@ -9,8 +9,8 @@ RUN git clone https://gitlab.matrix.org/matrix-org/olm.git /tmp/libolm \ COPY . /tmp/go-neb WORKDIR /tmp/go-neb -RUN go get golang.org/x/lint/golint \ - && go get github.com/fzipp/gocyclo/cmd/gocyclo \ +RUN go install honnef.co/go/tools/cmd/staticcheck@latest \ + && go install github.com/fzipp/gocyclo/cmd/gocyclo@latest \ && go build github.com/matrix-org/go-neb # Ensures we're lint-free diff --git a/README.md b/README.md index f3478d1..f2f0db5 100644 --- a/README.md +++ b/README.md @@ -212,8 +212,8 @@ things like linting. Some of them are bundled with go (fmt and vet) but some are not. You should install the ones which are not: ```bash -go get golang.org/x/lint/golint -go get github.com/fzipp/gocyclo +go install honnef.co/go/tools/cmd/staticcheck@latest +go install github.com/fzipp/gocyclo/cmd/gocyclo@latest ``` You can then install the pre-commit hook: diff --git a/api/handlers/auth.go b/api/handlers/auth.go index d6835d9..15dd77e 100644 --- a/api/handlers/auth.go +++ b/api/handlers/auth.go @@ -18,7 +18,7 @@ import ( // RequestAuthSession represents an HTTP handler capable of processing /admin/requestAuthSession requests. type RequestAuthSession struct { - Db *database.ServiceDB + DB *database.ServiceDB } // OnIncomingRequest handles POST requests to /admin/requestAuthSession. The HTTP body MUST be @@ -60,7 +60,7 @@ func (h *RequestAuthSession) OnIncomingRequest(req *http.Request) util.JSONRespo return util.MessageResponse(400, err.Error()) } - realm, err := h.Db.LoadAuthRealm(body.RealmID) + realm, err := h.DB.LoadAuthRealm(body.RealmID) if err != nil { logger.WithError(err).Info("Failed to LoadAuthRealm") return util.MessageResponse(400, "Unknown RealmID") @@ -82,7 +82,7 @@ func (h *RequestAuthSession) OnIncomingRequest(req *http.Request) util.JSONRespo // RemoveAuthSession represents an HTTP handler capable of processing /admin/removeAuthSession requests. type RemoveAuthSession struct { - Db *database.ServiceDB + DB *database.ServiceDB } // OnIncomingRequest handles POST requests to /admin/removeAuthSession. @@ -119,12 +119,12 @@ func (h *RemoveAuthSession) OnIncomingRequest(req *http.Request) util.JSONRespon return util.MessageResponse(400, `Must supply a "UserID", a "RealmID"`) } - _, err := h.Db.LoadAuthRealm(body.RealmID) + _, err := h.DB.LoadAuthRealm(body.RealmID) if err != nil { return util.MessageResponse(400, "Unknown RealmID") } - if err := h.Db.RemoveAuthSession(body.RealmID, body.UserID); err != nil { + if err := h.DB.RemoveAuthSession(body.RealmID, body.UserID); err != nil { logger.WithError(err).Error("Failed to RemoveAuthSession") return util.MessageResponse(500, "Failed to remove auth session") } @@ -137,7 +137,7 @@ func (h *RemoveAuthSession) OnIncomingRequest(req *http.Request) util.JSONRespon // RealmRedirect represents an HTTP handler which can process incoming redirects for auth realms. type RealmRedirect struct { - Db *database.ServiceDB + DB *database.ServiceDB } // Handle requests for an auth realm. @@ -158,7 +158,7 @@ func (rh *RealmRedirect) Handle(w http.ResponseWriter, req *http.Request) { return } - realm, err := rh.Db.LoadAuthRealm(realmID) + realm, err := rh.DB.LoadAuthRealm(realmID) if err != nil { log.WithError(err).WithField("realm_id", realmID).Print("Failed to load realm") w.WriteHeader(404) @@ -172,7 +172,7 @@ func (rh *RealmRedirect) Handle(w http.ResponseWriter, req *http.Request) { // ConfigureAuthRealm represents an HTTP handler capable of processing /admin/configureAuthRealm requests. type ConfigureAuthRealm struct { - Db *database.ServiceDB + DB *database.ServiceDB } // OnIncomingRequest handles POST requests to /admin/configureAuthRealm. The JSON object @@ -222,7 +222,7 @@ func (h *ConfigureAuthRealm) OnIncomingRequest(req *http.Request) util.JSONRespo return util.MessageResponse(400, "Error registering auth realm") } - oldRealm, err := h.Db.StoreAuthRealm(realm) + oldRealm, err := h.DB.StoreAuthRealm(realm) if err != nil { logger.WithError(err).Error("Failed to StoreAuthRealm") return util.MessageResponse(500, "Error storing realm") @@ -241,7 +241,7 @@ func (h *ConfigureAuthRealm) OnIncomingRequest(req *http.Request) util.JSONRespo // GetSession represents an HTTP handler capable of processing /admin/getSession requests. type GetSession struct { - Db *database.ServiceDB + DB *database.ServiceDB } // OnIncomingRequest handles POST requests to /admin/getSession. @@ -287,7 +287,7 @@ func (h *GetSession) OnIncomingRequest(req *http.Request) util.JSONResponse { return util.MessageResponse(400, `Must supply a "RealmID" and "UserID"`) } - session, err := h.Db.LoadAuthSessionByUser(body.RealmID, body.UserID) + session, err := h.DB.LoadAuthSessionByUser(body.RealmID, body.UserID) if err != nil && err != sql.ErrNoRows { logger.WithError(err).WithField("body", body).Error("Failed to LoadAuthSessionByUser") return util.MessageResponse(500, `Failed to load session`) diff --git a/api/handlers/service.go b/api/handlers/service.go index cedcaef..f5a0939 100644 --- a/api/handlers/service.go +++ b/api/handlers/service.go @@ -168,7 +168,7 @@ func (s *ConfigureService) createService(req *http.Request) (types.Service, *uti // GetService represents an HTTP handler which can process /admin/getService requests. type GetService struct { - Db *database.ServiceDB + DB *database.ServiceDB } // OnIncomingRequest handles POST requests to /admin/getService. @@ -205,7 +205,7 @@ func (h *GetService) OnIncomingRequest(req *http.Request) util.JSONResponse { return util.MessageResponse(400, `Must supply a "ID"`) } - srv, err := h.Db.LoadService(body.ID) + srv, err := h.DB.LoadService(body.ID) if err != nil { if err == sql.ErrNoRows { return util.MessageResponse(404, `Service not found`) diff --git a/clients/bot_client.go b/clients/bot_client.go index 6dbaea2..a298197 100644 --- a/clients/bot_client.go +++ b/clients/bot_client.go @@ -14,7 +14,6 @@ import ( "golang.org/x/net/context" "maunium.net/go/mautrix" "maunium.net/go/mautrix/crypto" - "maunium.net/go/mautrix/event" mevt "maunium.net/go/mautrix/event" "maunium.net/go/mautrix/id" ) @@ -57,6 +56,7 @@ func (botClient *BotClient) InitOlmMachine(client *mautrix.Client, nebStore *mat if deviceID == "" { deviceID = "_empty_device_id" } + //lint:ignore SA1019 old code, unsure what happens when we change it cryptoStore, err = crypto.NewGobStore(deviceID + ".gob") if err != nil { return @@ -187,7 +187,7 @@ func (botClient *BotClient) VerifySASMatch(otherDevice *crypto.DeviceIdentity, s "otherUser": otherDevice.UserID, "otherDevice": otherDevice.DeviceID, }).Infof("Waiting for SAS") - if sas.Type() != event.SASDecimal { + if sas.Type() != mevt.SASDecimal { log.Warnf("Unsupported SAS type: %v", sas.Type()) return false } @@ -239,7 +239,7 @@ func (botClient *BotClient) VerificationMethods() []crypto.VerificationMethod { } // OnCancel is called when a SAS verification is canceled. -func (botClient *BotClient) OnCancel(cancelledByUs bool, reason string, reasonCode event.VerificationCancelCode) { +func (botClient *BotClient) OnCancel(cancelledByUs bool, reason string, reasonCode mevt.VerificationCancelCode) { atomic.AddInt32(&botClient.ongoingVerificationCount, -1) log.Tracef("Verification cancelled with reason: %v", reason) } @@ -300,9 +300,9 @@ func (botClient *BotClient) ForwardRoomKeyToDevice(userID id.UserID, deviceID id return err } - forwardedRoomKey := event.Content{ - Parsed: &event.ForwardedRoomKeyEventContent{ - RoomKeyEventContent: event.RoomKeyEventContent{ + forwardedRoomKey := mevt.Content{ + Parsed: &mevt.ForwardedRoomKeyEventContent{ + RoomKeyEventContent: mevt.RoomKeyEventContent{ Algorithm: id.AlgorithmMegolmV1, RoomID: igs.RoomID, SessionID: igs.ID(), diff --git a/goneb.go b/goneb.go index 831d924..8d793a8 100644 --- a/goneb.go +++ b/goneb.go @@ -1,5 +1,7 @@ package main +//lint:file-ignore SA1019 need to fix our prometheus package usage + import ( "encoding/json" "fmt" diff --git a/hooks/pre-commit b/hooks/pre-commit index 41f871a..9d07653 100755 --- a/hooks/pre-commit +++ b/hooks/pre-commit @@ -2,8 +2,8 @@ set -eu -golint ./... go fmt ./... go vet -composites=false ./... +staticcheck ./... gocyclo -over 12 . go test ./... diff --git a/polling/polling.go b/polling/polling.go index fa5f04d..c5fa822 100644 --- a/polling/polling.go +++ b/polling/polling.go @@ -118,10 +118,10 @@ func pollLoop(service types.Service, ts int64) { } // setPollStartTime clobbers the current poll time -func setPollStartTime(service types.Service, startTs int64) { +func setPollStartTime(service types.Service, startTS int64) { pollMutex.Lock() defer pollMutex.Unlock() - startPollTime[service.ServiceID()] = startTs + startPollTime[service.ServiceID()] = startTS } // pollTimeChanged returns true if the poll start time for this service ID is different to the one supplied. diff --git a/services/giphy/giphy.go b/services/giphy/giphy.go index 583b0fb..6662183 100644 --- a/services/giphy/giphy.go +++ b/services/giphy/giphy.go @@ -11,7 +11,6 @@ import ( "github.com/matrix-org/go-neb/types" log "github.com/sirupsen/logrus" - "maunium.net/go/mautrix/event" mevt "maunium.net/go/mautrix/event" "maunium.net/go/mautrix/id" ) @@ -93,7 +92,7 @@ func (s *Service) cmdGiphy(client types.MatrixClient, roomID id.RoomID, userID i } return mevt.MessageEventContent{ - MsgType: event.MsgImage, + MsgType: mevt.MsgImage, Body: gifResult.Slug, URL: resUpload.ContentURI.CUString(), Info: &mevt.FileInfo{ diff --git a/services/github/client/client.go b/services/github/client/client.go index 2dcfb55..6a239b5 100644 --- a/services/github/client/client.go +++ b/services/github/client/client.go @@ -1,6 +1,8 @@ package client import ( + "context" + "github.com/google/go-github/github" "golang.org/x/oauth2" ) @@ -46,6 +48,6 @@ func New(token string) *github.Client { &oauth2.Token{AccessToken: token}, ) } - httpCli := oauth2.NewClient(oauth2.NoContext, tokenSource) + httpCli := oauth2.NewClient(context.TODO(), tokenSource) return github.NewClient(httpCli) } diff --git a/services/github/github_webhook.go b/services/github/github_webhook.go index 6e43b00..672be6c 100644 --- a/services/github/github_webhook.go +++ b/services/github/github_webhook.go @@ -368,37 +368,6 @@ func (s *WebhookService) deleteHook(owner, repo string) error { return err } -func sameRepos(a *WebhookService, b *WebhookService) bool { - getRepos := func(s *WebhookService) []string { - r := make(map[string]bool) - for _, roomConfig := range s.Rooms { - for ownerRepo := range roomConfig.Repos { - r[ownerRepo] = true - } - } - var rs []string - for k := range r { - rs = append(rs, k) - } - return rs - } - aRepos := getRepos(a) - bRepos := getRepos(b) - - if len(aRepos) != len(bRepos) { - return false - } - - sort.Strings(aRepos) - sort.Strings(bRepos) - for i := 0; i < len(aRepos); i++ { - if aRepos[i] != bRepos[i] { - return false - } - } - return true -} - // difference returns the elements that are only in the first list and // the elements that are only in the second. As a side-effect this sorts // the input lists in-place. diff --git a/services/github/webhook/webhook_test.go b/services/github/webhook/webhook_test.go index e8e6616..02a0062 100644 --- a/services/github/webhook/webhook_test.go +++ b/services/github/webhook/webhook_test.go @@ -1518,6 +1518,7 @@ func TestParseGithubEvent(t *testing.T) { if outRepo == nil { t.Errorf("ParseGithubEvent(%s) => Repo is nil", gh.eventType) } + //lint:ignore SA5011 caught by the previous check if *outRepo.FullName != gh.outFullRepo { t.Errorf("ParseGithubEvent(%s) => Repo: Want %s got %s", gh.eventType, gh.outFullRepo, *outRepo.FullName) } diff --git a/services/rssbot/rssbot.go b/services/rssbot/rssbot.go index c339975..d7e3472 100644 --- a/services/rssbot/rssbot.go +++ b/services/rssbot/rssbot.go @@ -243,21 +243,21 @@ func incrementMetrics(urlStr string, err error) { func (s *Service) nextTimestamp() time.Time { // return the earliest next poll ts - var earliestNextTs int64 + var earliestNextTS int64 for _, feedInfo := range s.Feeds { - if earliestNextTs == 0 || feedInfo.NextPollTimestampSecs < earliestNextTs { - earliestNextTs = feedInfo.NextPollTimestampSecs + if earliestNextTS == 0 || feedInfo.NextPollTimestampSecs < earliestNextTS { + earliestNextTS = feedInfo.NextPollTimestampSecs } } // Don't allow times in the past. Set a min re-poll threshold of 60s to avoid // tight-looping on feeds which 500. now := time.Now().Unix() - if earliestNextTs <= now { - earliestNextTs = now + 60 + if earliestNextTS <= now { + earliestNextTS = now + 60 } - return time.Unix(earliestNextTs, 0) + return time.Unix(earliestNextTS, 0) } // Query the given feed, update relevant timestamps and return NEW items @@ -291,9 +291,9 @@ func (s *Service) queryFeed(feedURL string) (*gofeed.Feed, []gofeed.Item, error) now := time.Now().Unix() // Second resolution // Work out when to next poll this feed - nextPollTsSec := now + minPollingIntervalSeconds + nextPollTSSec := now + minPollingIntervalSeconds if s.Feeds[feedURL].PollIntervalMins > int(minPollingIntervalSeconds/60) { - nextPollTsSec = now + int64(s.Feeds[feedURL].PollIntervalMins*60) + nextPollTSSec = now + int64(s.Feeds[feedURL].PollIntervalMins*60) } // TODO: Handle the 'sy' Syndication extension to control update interval. // See http://www.feedforall.com/syndication.htm and http://web.resource.org/rss/1.0/modules/syndication/ @@ -323,7 +323,7 @@ func (s *Service) queryFeed(feedURL string) (*gofeed.Feed, []gofeed.Item, error) } // Update the service config to persist the new times - f.NextPollTimestampSecs = nextPollTsSec + f.NextPollTimestampSecs = nextPollTSSec f.FeedUpdatedTimestampSecs = now f.RecentGUIDs = guids f.IsFailing = false diff --git a/services/slackapi/message.go b/services/slackapi/message.go index 54099bf..e8d9f9c 100644 --- a/services/slackapi/message.go +++ b/services/slackapi/message.go @@ -37,7 +37,7 @@ type slackAttachment struct { TextRendered template.HTML MrkdwnIn []string `json:"mrkdwn_in"` - Ts *int64 `json:"ts"` + TS *int64 `json:"ts"` } type slackMessage struct { @@ -86,7 +86,7 @@ var netClient = &http.Client{ } // TODO: What does this do? -var linkRegex, _ = regexp.Compile("<([^|]+)(\\|([^>]+))?>") +var linkRegex, _ = regexp.Compile(`<([^|]+)(\|([^>]+))?>`) func getSlackMessage(req http.Request) (message slackMessage, err error) { ct := req.Header.Get("Content-Type") @@ -197,7 +197,7 @@ func renderSlackAttachment(attachment *slackAttachment) { func slackMessageToHTMLMessage(message slackMessage) (html mevt.MessageEventContent, err error) { text := linkifyString(message.Text) - if message.Mrkdwn == nil || *message.Mrkdwn == true { + if message.Mrkdwn == nil || *message.Mrkdwn { message.TextRendered = template.HTML(blackfriday.MarkdownBasic([]byte(text))) } diff --git a/staticcheck.conf b/staticcheck.conf new file mode 100644 index 0000000..0931ed7 --- /dev/null +++ b/staticcheck.conf @@ -0,0 +1 @@ +checks = ["all", "-ST1000", "-ST1005"] \ No newline at end of file diff --git a/types/actions.go b/types/actions.go index dadcfc5..8592b58 100644 --- a/types/actions.go +++ b/types/actions.go @@ -33,7 +33,7 @@ func (command *Command) Matches(arguments []string) bool { return false } for i, segment := range command.Path { - if strings.ToLower(segment) != strings.ToLower(arguments[i]) { + if !strings.EqualFold(segment, arguments[i]) { return false } }