Browse Source

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.
pull/350/head
Daniele Sluijters 4 years ago
parent
commit
6879960d22
  1. 4
      Dockerfile
  2. 4
      README.md
  3. 22
      api/handlers/auth.go
  4. 4
      api/handlers/service.go
  5. 12
      clients/bot_client.go
  6. 2
      goneb.go
  7. 2
      hooks/pre-commit
  8. 4
      polling/polling.go
  9. 3
      services/giphy/giphy.go
  10. 4
      services/github/client/client.go
  11. 31
      services/github/github_webhook.go
  12. 1
      services/github/webhook/webhook_test.go
  13. 18
      services/rssbot/rssbot.go
  14. 6
      services/slackapi/message.go
  15. 1
      staticcheck.conf
  16. 2
      types/actions.go

4
Dockerfile

@ -9,8 +9,8 @@ RUN git clone https://gitlab.matrix.org/matrix-org/olm.git /tmp/libolm \
COPY . /tmp/go-neb COPY . /tmp/go-neb
WORKDIR /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 && go build github.com/matrix-org/go-neb
# Ensures we're lint-free # Ensures we're lint-free

4
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: are not. You should install the ones which are not:
```bash ```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: You can then install the pre-commit hook:

22
api/handlers/auth.go

@ -18,7 +18,7 @@ import (
// RequestAuthSession represents an HTTP handler capable of processing /admin/requestAuthSession requests. // RequestAuthSession represents an HTTP handler capable of processing /admin/requestAuthSession requests.
type RequestAuthSession struct { type RequestAuthSession struct {
Db *database.ServiceDB
DB *database.ServiceDB
} }
// OnIncomingRequest handles POST requests to /admin/requestAuthSession. The HTTP body MUST be // 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()) return util.MessageResponse(400, err.Error())
} }
realm, err := h.Db.LoadAuthRealm(body.RealmID)
realm, err := h.DB.LoadAuthRealm(body.RealmID)
if err != nil { if err != nil {
logger.WithError(err).Info("Failed to LoadAuthRealm") logger.WithError(err).Info("Failed to LoadAuthRealm")
return util.MessageResponse(400, "Unknown RealmID") 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. // RemoveAuthSession represents an HTTP handler capable of processing /admin/removeAuthSession requests.
type RemoveAuthSession struct { type RemoveAuthSession struct {
Db *database.ServiceDB
DB *database.ServiceDB
} }
// OnIncomingRequest handles POST requests to /admin/removeAuthSession. // 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"`) 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 { if err != nil {
return util.MessageResponse(400, "Unknown RealmID") 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") logger.WithError(err).Error("Failed to RemoveAuthSession")
return util.MessageResponse(500, "Failed to remove auth session") 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. // RealmRedirect represents an HTTP handler which can process incoming redirects for auth realms.
type RealmRedirect struct { type RealmRedirect struct {
Db *database.ServiceDB
DB *database.ServiceDB
} }
// Handle requests for an auth realm. // Handle requests for an auth realm.
@ -158,7 +158,7 @@ func (rh *RealmRedirect) Handle(w http.ResponseWriter, req *http.Request) {
return return
} }
realm, err := rh.Db.LoadAuthRealm(realmID)
realm, err := rh.DB.LoadAuthRealm(realmID)
if err != nil { if err != nil {
log.WithError(err).WithField("realm_id", realmID).Print("Failed to load realm") log.WithError(err).WithField("realm_id", realmID).Print("Failed to load realm")
w.WriteHeader(404) 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. // ConfigureAuthRealm represents an HTTP handler capable of processing /admin/configureAuthRealm requests.
type ConfigureAuthRealm struct { type ConfigureAuthRealm struct {
Db *database.ServiceDB
DB *database.ServiceDB
} }
// OnIncomingRequest handles POST requests to /admin/configureAuthRealm. The JSON object // 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") return util.MessageResponse(400, "Error registering auth realm")
} }
oldRealm, err := h.Db.StoreAuthRealm(realm)
oldRealm, err := h.DB.StoreAuthRealm(realm)
if err != nil { if err != nil {
logger.WithError(err).Error("Failed to StoreAuthRealm") logger.WithError(err).Error("Failed to StoreAuthRealm")
return util.MessageResponse(500, "Error storing realm") 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. // GetSession represents an HTTP handler capable of processing /admin/getSession requests.
type GetSession struct { type GetSession struct {
Db *database.ServiceDB
DB *database.ServiceDB
} }
// OnIncomingRequest handles POST requests to /admin/getSession. // 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"`) 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 { if err != nil && err != sql.ErrNoRows {
logger.WithError(err).WithField("body", body).Error("Failed to LoadAuthSessionByUser") logger.WithError(err).WithField("body", body).Error("Failed to LoadAuthSessionByUser")
return util.MessageResponse(500, `Failed to load session`) return util.MessageResponse(500, `Failed to load session`)

4
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. // GetService represents an HTTP handler which can process /admin/getService requests.
type GetService struct { type GetService struct {
Db *database.ServiceDB
DB *database.ServiceDB
} }
// OnIncomingRequest handles POST requests to /admin/getService. // 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"`) 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 != nil {
if err == sql.ErrNoRows { if err == sql.ErrNoRows {
return util.MessageResponse(404, `Service not found`) return util.MessageResponse(404, `Service not found`)

12
clients/bot_client.go

@ -14,7 +14,6 @@ import (
"golang.org/x/net/context" "golang.org/x/net/context"
"maunium.net/go/mautrix" "maunium.net/go/mautrix"
"maunium.net/go/mautrix/crypto" "maunium.net/go/mautrix/crypto"
"maunium.net/go/mautrix/event"
mevt "maunium.net/go/mautrix/event" mevt "maunium.net/go/mautrix/event"
"maunium.net/go/mautrix/id" "maunium.net/go/mautrix/id"
) )
@ -57,6 +56,7 @@ func (botClient *BotClient) InitOlmMachine(client *mautrix.Client, nebStore *mat
if deviceID == "" { if deviceID == "" {
deviceID = "_empty_device_id" deviceID = "_empty_device_id"
} }
//lint:ignore SA1019 old code, unsure what happens when we change it
cryptoStore, err = crypto.NewGobStore(deviceID + ".gob") cryptoStore, err = crypto.NewGobStore(deviceID + ".gob")
if err != nil { if err != nil {
return return
@ -187,7 +187,7 @@ func (botClient *BotClient) VerifySASMatch(otherDevice *crypto.DeviceIdentity, s
"otherUser": otherDevice.UserID, "otherUser": otherDevice.UserID,
"otherDevice": otherDevice.DeviceID, "otherDevice": otherDevice.DeviceID,
}).Infof("Waiting for SAS") }).Infof("Waiting for SAS")
if sas.Type() != event.SASDecimal {
if sas.Type() != mevt.SASDecimal {
log.Warnf("Unsupported SAS type: %v", sas.Type()) log.Warnf("Unsupported SAS type: %v", sas.Type())
return false return false
} }
@ -239,7 +239,7 @@ func (botClient *BotClient) VerificationMethods() []crypto.VerificationMethod {
} }
// OnCancel is called when a SAS verification is canceled. // 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) atomic.AddInt32(&botClient.ongoingVerificationCount, -1)
log.Tracef("Verification cancelled with reason: %v", reason) log.Tracef("Verification cancelled with reason: %v", reason)
} }
@ -300,9 +300,9 @@ func (botClient *BotClient) ForwardRoomKeyToDevice(userID id.UserID, deviceID id
return err return err
} }
forwardedRoomKey := event.Content{
Parsed: &event.ForwardedRoomKeyEventContent{
RoomKeyEventContent: event.RoomKeyEventContent{
forwardedRoomKey := mevt.Content{
Parsed: &mevt.ForwardedRoomKeyEventContent{
RoomKeyEventContent: mevt.RoomKeyEventContent{
Algorithm: id.AlgorithmMegolmV1, Algorithm: id.AlgorithmMegolmV1,
RoomID: igs.RoomID, RoomID: igs.RoomID,
SessionID: igs.ID(), SessionID: igs.ID(),

2
goneb.go

@ -1,5 +1,7 @@
package main package main
//lint:file-ignore SA1019 need to fix our prometheus package usage
import ( import (
"encoding/json" "encoding/json"
"fmt" "fmt"

2
hooks/pre-commit

@ -2,8 +2,8 @@
set -eu set -eu
golint ./...
go fmt ./... go fmt ./...
go vet -composites=false ./... go vet -composites=false ./...
staticcheck ./...
gocyclo -over 12 . gocyclo -over 12 .
go test ./... go test ./...

4
polling/polling.go

@ -118,10 +118,10 @@ func pollLoop(service types.Service, ts int64) {
} }
// setPollStartTime clobbers the current poll time // setPollStartTime clobbers the current poll time
func setPollStartTime(service types.Service, startTs int64) {
func setPollStartTime(service types.Service, startTS int64) {
pollMutex.Lock() pollMutex.Lock()
defer pollMutex.Unlock() 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. // pollTimeChanged returns true if the poll start time for this service ID is different to the one supplied.

3
services/giphy/giphy.go

@ -11,7 +11,6 @@ import (
"github.com/matrix-org/go-neb/types" "github.com/matrix-org/go-neb/types"
log "github.com/sirupsen/logrus" log "github.com/sirupsen/logrus"
"maunium.net/go/mautrix/event"
mevt "maunium.net/go/mautrix/event" mevt "maunium.net/go/mautrix/event"
"maunium.net/go/mautrix/id" "maunium.net/go/mautrix/id"
) )
@ -93,7 +92,7 @@ func (s *Service) cmdGiphy(client types.MatrixClient, roomID id.RoomID, userID i
} }
return mevt.MessageEventContent{ return mevt.MessageEventContent{
MsgType: event.MsgImage,
MsgType: mevt.MsgImage,
Body: gifResult.Slug, Body: gifResult.Slug,
URL: resUpload.ContentURI.CUString(), URL: resUpload.ContentURI.CUString(),
Info: &mevt.FileInfo{ Info: &mevt.FileInfo{

4
services/github/client/client.go

@ -1,6 +1,8 @@
package client package client
import ( import (
"context"
"github.com/google/go-github/github" "github.com/google/go-github/github"
"golang.org/x/oauth2" "golang.org/x/oauth2"
) )
@ -46,6 +48,6 @@ func New(token string) *github.Client {
&oauth2.Token{AccessToken: token}, &oauth2.Token{AccessToken: token},
) )
} }
httpCli := oauth2.NewClient(oauth2.NoContext, tokenSource)
httpCli := oauth2.NewClient(context.TODO(), tokenSource)
return github.NewClient(httpCli) return github.NewClient(httpCli)
} }

31
services/github/github_webhook.go

@ -368,37 +368,6 @@ func (s *WebhookService) deleteHook(owner, repo string) error {
return err 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 // 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 elements that are only in the second. As a side-effect this sorts
// the input lists in-place. // the input lists in-place.

1
services/github/webhook/webhook_test.go

@ -1518,6 +1518,7 @@ func TestParseGithubEvent(t *testing.T) {
if outRepo == nil { if outRepo == nil {
t.Errorf("ParseGithubEvent(%s) => Repo is nil", gh.eventType) t.Errorf("ParseGithubEvent(%s) => Repo is nil", gh.eventType)
} }
//lint:ignore SA5011 caught by the previous check
if *outRepo.FullName != gh.outFullRepo { if *outRepo.FullName != gh.outFullRepo {
t.Errorf("ParseGithubEvent(%s) => Repo: Want %s got %s", gh.eventType, gh.outFullRepo, *outRepo.FullName) t.Errorf("ParseGithubEvent(%s) => Repo: Want %s got %s", gh.eventType, gh.outFullRepo, *outRepo.FullName)
} }

18
services/rssbot/rssbot.go

@ -243,21 +243,21 @@ func incrementMetrics(urlStr string, err error) {
func (s *Service) nextTimestamp() time.Time { func (s *Service) nextTimestamp() time.Time {
// return the earliest next poll ts // return the earliest next poll ts
var earliestNextTs int64
var earliestNextTS int64
for _, feedInfo := range s.Feeds { 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 // Don't allow times in the past. Set a min re-poll threshold of 60s to avoid
// tight-looping on feeds which 500. // tight-looping on feeds which 500.
now := time.Now().Unix() 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 // 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 now := time.Now().Unix() // Second resolution
// Work out when to next poll this feed // Work out when to next poll this feed
nextPollTsSec := now + minPollingIntervalSeconds
nextPollTSSec := now + minPollingIntervalSeconds
if s.Feeds[feedURL].PollIntervalMins > int(minPollingIntervalSeconds/60) { 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. // 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/ // 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 // Update the service config to persist the new times
f.NextPollTimestampSecs = nextPollTsSec
f.NextPollTimestampSecs = nextPollTSSec
f.FeedUpdatedTimestampSecs = now f.FeedUpdatedTimestampSecs = now
f.RecentGUIDs = guids f.RecentGUIDs = guids
f.IsFailing = false f.IsFailing = false

6
services/slackapi/message.go

@ -37,7 +37,7 @@ type slackAttachment struct {
TextRendered template.HTML TextRendered template.HTML
MrkdwnIn []string `json:"mrkdwn_in"` MrkdwnIn []string `json:"mrkdwn_in"`
Ts *int64 `json:"ts"`
TS *int64 `json:"ts"`
} }
type slackMessage struct { type slackMessage struct {
@ -86,7 +86,7 @@ var netClient = &http.Client{
} }
// TODO: What does this do? // TODO: What does this do?
var linkRegex, _ = regexp.Compile("<([^|]+)(\\|([^>]+))?>")
var linkRegex, _ = regexp.Compile(`<([^|]+)(\|([^>]+))?>`)
func getSlackMessage(req http.Request) (message slackMessage, err error) { func getSlackMessage(req http.Request) (message slackMessage, err error) {
ct := req.Header.Get("Content-Type") ct := req.Header.Get("Content-Type")
@ -197,7 +197,7 @@ func renderSlackAttachment(attachment *slackAttachment) {
func slackMessageToHTMLMessage(message slackMessage) (html mevt.MessageEventContent, err error) { func slackMessageToHTMLMessage(message slackMessage) (html mevt.MessageEventContent, err error) {
text := linkifyString(message.Text) 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))) message.TextRendered = template.HTML(blackfriday.MarkdownBasic([]byte(text)))
} }

1
staticcheck.conf

@ -0,0 +1 @@
checks = ["all", "-ST1000", "-ST1005"]

2
types/actions.go

@ -33,7 +33,7 @@ func (command *Command) Matches(arguments []string) bool {
return false return false
} }
for i, segment := range command.Path { for i, segment := range command.Path {
if strings.ToLower(segment) != strings.ToLower(arguments[i]) {
if !strings.EqualFold(segment, arguments[i]) {
return false return false
} }
} }

Loading…
Cancel
Save