From 2c20e42a21aaef4248901adc8a80afc2ef4c8506 Mon Sep 17 00:00:00 2001 From: eikendev Date: Sat, 1 Apr 2023 20:00:58 +0200 Subject: [PATCH] Add misspell, gocritic, and revive --- Makefile | 10 +++++++--- cmd/pushbits/main.go | 1 + internal/api/alertmanager/handler.go | 13 ++++++++----- internal/api/application.go | 2 +- internal/api/application_test.go | 2 +- internal/api/interfaces.go | 3 ++- internal/api/middleware.go | 10 ++++++---- internal/api/notification.go | 2 +- internal/api/util.go | 5 +++-- internal/assert/assert.go | 1 + internal/authentication/authentication.go | 1 + .../authentication/credentials/credentials.go | 1 + internal/authentication/token_test.go | 2 +- internal/configuration/configuration.go | 1 + internal/database/application.go | 6 +++--- internal/database/database.go | 1 + internal/database/user.go | 6 +++--- internal/dispatcher/application.go | 2 +- internal/dispatcher/dispatcher.go | 1 + internal/dispatcher/notification.go | 10 +++++----- internal/log/ginlogrus.go | 1 + internal/log/log.go | 3 +++ internal/model/alertmanager.go | 3 +++ internal/model/notification.go | 5 +++-- internal/pberrors/errors.go | 1 + internal/router/router.go | 3 ++- internal/runner/runner.go | 1 + tests/mockups/application.go | 1 + tests/mockups/dispatcher.go | 15 ++++++++++----- tests/request.go | 9 +++++---- 30 files changed, 79 insertions(+), 43 deletions(-) diff --git a/Makefile b/Makefile index d27877d..dc542fb 100644 --- a/Makefile +++ b/Makefile @@ -21,11 +21,13 @@ clean: .PHONY: test test: - stdout=$$(gofumpt -l $(GO_FILES) 2>&1); if [ "$$stdout" ]; then exit 1; fi + stdout=$$(gofumpt -l . 2>&1); if [ "$$stdout" ]; then exit 1; fi go vet ./... - gocyclo -over 10 $(GO_FILES) + misspell -error $(GO_FILES) staticcheck ./... errcheck -exclude errcheck_excludes.txt ./... + gocritic check -disable='#experimental,#opinionated' -@ifElseChain.minThreshold 3 ./... + revive -set_exit_status -exclude ./docs ./... go test -v -cover ./... gosec -exclude-dir=tests ./... govulncheck ./... @@ -33,8 +35,10 @@ test: .PHONY: setup setup: - go install github.com/fzipp/gocyclo/cmd/gocyclo@latest + go install github.com/client9/misspell/cmd/misspell@latest + go install github.com/go-critic/go-critic/cmd/gocritic@latest go install github.com/kisielk/errcheck@latest + go install github.com/mgechev/revive@latest go install github.com/securego/gosec/v2/cmd/gosec@latest go install github.com/swaggo/swag/cmd/swag@latest go install golang.org/x/vuln/cmd/govulncheck@latest diff --git a/cmd/pushbits/main.go b/cmd/pushbits/main.go index db6d4bb..1d10352 100644 --- a/cmd/pushbits/main.go +++ b/cmd/pushbits/main.go @@ -1,3 +1,4 @@ +// Package main provides the main function as a starting point of this tool. package main import ( diff --git a/internal/api/alertmanager/handler.go b/internal/api/alertmanager/handler.go index b5f73b1..bfc45f0 100644 --- a/internal/api/alertmanager/handler.go +++ b/internal/api/alertmanager/handler.go @@ -1,3 +1,4 @@ +// Package alertmanager provides definitions and functionality related to Alertmanager notifications. package alertmanager import ( @@ -11,12 +12,14 @@ import ( "github.com/pushbits/server/internal/model" ) -type AlertmanagerHandler struct { +// Handler holds information for processing alerts received via Alertmanager. +type Handler struct { DP api.NotificationDispatcher - Settings AlertmanagerHandlerSettings + Settings HandlerSettings } -type AlertmanagerHandlerSettings struct { +// HandlerSettings represents the settings for processing alerts received via Alertmanager. +type HandlerSettings struct { TitleAnnotation string MessageAnnotation string } @@ -33,7 +36,7 @@ type AlertmanagerHandlerSettings struct { // @Success 200 {object} []model.Notification // @Failure 500,404,403 "" // @Router /alert [post] -func (h *AlertmanagerHandler) CreateAlert(ctx *gin.Context) { +func (h *Handler) CreateAlert(ctx *gin.Context) { application := authentication.GetApplication(ctx) log.L.Printf("Sending alert notification for application %s.", application.Name) @@ -52,7 +55,7 @@ func (h *AlertmanagerHandler) CreateAlert(ctx *gin.Context) { } notification.ID = messageID - notification.UrlEncodedID = url.QueryEscape(messageID) + notification.URLEncodedID = url.QueryEscape(messageID) notifications[i] = notification } ctx.JSON(http.StatusOK, ¬ifications) diff --git a/internal/api/application.go b/internal/api/application.go index 435909f..774bb01 100644 --- a/internal/api/application.go +++ b/internal/api/application.go @@ -30,7 +30,7 @@ func (h *ApplicationHandler) generateToken(compat bool) string { func (h *ApplicationHandler) registerApplication(ctx *gin.Context, a *model.Application, u *model.User) error { log.L.Printf("Registering application %s.", a.Name) - channelID, err := h.DP.RegisterApplication(a.ID, a.Name, a.Token, u.MatrixID) + channelID, err := h.DP.RegisterApplication(a.ID, a.Name, u.MatrixID) if success := SuccessOrAbort(ctx, http.StatusInternalServerError, err); !success { return err } diff --git a/internal/api/application_test.go b/internal/api/application_test.go index 58376c3..eefaf53 100644 --- a/internal/api/application_test.go +++ b/internal/api/application_test.go @@ -323,7 +323,7 @@ func TestApi_DeleteApplication(t *testing.T) { } // GetApplicationHandler creates and returns an application handler -func getApplicationHandler(c *configuration.Configuration) (*ApplicationHandler, error) { +func getApplicationHandler(_ *configuration.Configuration) (*ApplicationHandler, error) { dispatcher := &mockups.MockDispatcher{} return &ApplicationHandler{ diff --git a/internal/api/interfaces.go b/internal/api/interfaces.go index 0de2a32..865629a 100644 --- a/internal/api/interfaces.go +++ b/internal/api/interfaces.go @@ -1,3 +1,4 @@ +// Package api provides definitions and functionality related to the API. package api import ( @@ -27,7 +28,7 @@ type Database interface { // The Dispatcher interface for relaying notifications. type Dispatcher interface { - RegisterApplication(id uint, name, token, user string) (string, error) + RegisterApplication(id uint, name, user string) (string, error) DeregisterApplication(a *model.Application, u *model.User) error UpdateApplication(a *model.Application, behavior *configuration.RepairBehavior) error } diff --git a/internal/api/middleware.go b/internal/api/middleware.go index 3d84840..d94887d 100644 --- a/internal/api/middleware.go +++ b/internal/api/middleware.go @@ -4,18 +4,20 @@ import ( "github.com/gin-gonic/gin" ) -type idInURI struct { +// IDInURI is used to retrieve an ID from a context. +type IDInURI struct { ID uint `uri:"id" binding:"required"` } -type messageIdInURI struct { +// messageIDInURI is used to retrieve an message ID from a context. +type messageIDInURI struct { MessageID string `uri:"messageid" binding:"required"` } // RequireIDInURI returns a Gin middleware which requires an ID to be supplied in the URI of the request. func RequireIDInURI() gin.HandlerFunc { return func(ctx *gin.Context) { - var requestModel idInURI + var requestModel IDInURI if err := ctx.BindUri(&requestModel); err != nil { return @@ -28,7 +30,7 @@ func RequireIDInURI() gin.HandlerFunc { // RequireMessageIDInURI returns a Gin middleware which requires an messageID to be supplied in the URI of the request. func RequireMessageIDInURI() gin.HandlerFunc { return func(ctx *gin.Context) { - var requestModel messageIdInURI + var requestModel messageIDInURI if err := ctx.BindUri(&requestModel); err != nil { return diff --git a/internal/api/notification.go b/internal/api/notification.go index 83f8e59..53ca239 100644 --- a/internal/api/notification.go +++ b/internal/api/notification.go @@ -59,7 +59,7 @@ func (h *NotificationHandler) CreateNotification(ctx *gin.Context) { } notification.ID = messageID - notification.UrlEncodedID = url.QueryEscape(messageID) + notification.URLEncodedID = url.QueryEscape(messageID) ctx.JSON(http.StatusOK, ¬ification) } diff --git a/internal/api/util.go b/internal/api/util.go index 9c5ed86..546aa5a 100644 --- a/internal/api/util.go +++ b/internal/api/util.go @@ -10,6 +10,7 @@ import ( "github.com/gin-gonic/gin" ) +// SuccessOrAbort is a convenience function to write a HTTP status code based on a given error. func SuccessOrAbort(ctx *gin.Context, code int, err error) bool { if err != nil { // If we know the error force error code @@ -24,13 +25,13 @@ func SuccessOrAbort(ctx *gin.Context, code int, err error) bool { return err == nil } -func isCurrentUser(ctx *gin.Context, ID uint) bool { +func isCurrentUser(ctx *gin.Context, id uint) bool { user := authentication.GetUser(ctx) if user == nil { return false } - if user.ID != ID { + if user.ID != id { ctx.AbortWithError(http.StatusForbidden, errors.New("only owner can delete application")) return false } diff --git a/internal/assert/assert.go b/internal/assert/assert.go index f13c30c..8c6b910 100644 --- a/internal/assert/assert.go +++ b/internal/assert/assert.go @@ -1,3 +1,4 @@ +// Package assert provides convenience function to make assertions at runtime. package assert import ( diff --git a/internal/authentication/authentication.go b/internal/authentication/authentication.go index 8cd0635..42d71cb 100644 --- a/internal/authentication/authentication.go +++ b/internal/authentication/authentication.go @@ -1,3 +1,4 @@ +// Package authentication provides definitions and functionality related to user authentication. package authentication import ( diff --git a/internal/authentication/credentials/credentials.go b/internal/authentication/credentials/credentials.go index 1833054..4de7606 100644 --- a/internal/authentication/credentials/credentials.go +++ b/internal/authentication/credentials/credentials.go @@ -1,3 +1,4 @@ +// Package credentials provides definitions and functionality related to credential management. package credentials import ( diff --git a/internal/authentication/token_test.go b/internal/authentication/token_test.go index 2993e09..b3490ee 100644 --- a/internal/authentication/token_test.go +++ b/internal/authentication/token_test.go @@ -11,7 +11,7 @@ const ( minRandomChars = 14 ) -func isGoodToken(assert *assert.Assertions, require *require.Assertions, token string, compat bool) { +func isGoodToken(assert *assert.Assertions, _ *require.Assertions, token string, compat bool) { tokenLength := len(token) if compat { diff --git a/internal/configuration/configuration.go b/internal/configuration/configuration.go index f57e341..be38f41 100644 --- a/internal/configuration/configuration.go +++ b/internal/configuration/configuration.go @@ -1,3 +1,4 @@ +// Package configuration provides definitions and functionality related to the configuration. package configuration import ( diff --git a/internal/database/application.go b/internal/database/application.go index 9a45d75..8ff9cbd 100644 --- a/internal/database/application.go +++ b/internal/database/application.go @@ -25,16 +25,16 @@ func (d *Database) UpdateApplication(application *model.Application) error { } // GetApplicationByID returns the application with the given ID or nil. -func (d *Database) GetApplicationByID(ID uint) (*model.Application, error) { +func (d *Database) GetApplicationByID(id uint) (*model.Application, error) { var application model.Application - err := d.gormdb.First(&application, ID).Error + err := d.gormdb.First(&application, id).Error if errors.Is(err, gorm.ErrRecordNotFound) { return nil, err } - assert.Assert(application.ID == ID) + assert.Assert(application.ID == id) return &application, err } diff --git a/internal/database/database.go b/internal/database/database.go index c05cba7..a4555f3 100644 --- a/internal/database/database.go +++ b/internal/database/database.go @@ -1,3 +1,4 @@ +// Package database provides definitions and functionality related to the database. package database import ( diff --git a/internal/database/user.go b/internal/database/user.go index 3c2c136..54e495f 100644 --- a/internal/database/user.go +++ b/internal/database/user.go @@ -34,16 +34,16 @@ func (d *Database) UpdateUser(user *model.User) error { } // GetUserByID returns the user with the given ID or nil. -func (d *Database) GetUserByID(ID uint) (*model.User, error) { +func (d *Database) GetUserByID(id uint) (*model.User, error) { var user model.User - err := d.gormdb.First(&user, ID).Error + err := d.gormdb.First(&user, id).Error if errors.Is(err, gorm.ErrRecordNotFound) { return nil, err } - assert.Assert(user.ID == ID) + assert.Assert(user.ID == id) return &user, err } diff --git a/internal/dispatcher/application.go b/internal/dispatcher/application.go index cceb471..b1b9e4e 100644 --- a/internal/dispatcher/application.go +++ b/internal/dispatcher/application.go @@ -17,7 +17,7 @@ func buildRoomTopic(id uint) string { } // RegisterApplication creates a channel for an application. -func (d *Dispatcher) RegisterApplication(id uint, name, token, user string) (string, error) { +func (d *Dispatcher) RegisterApplication(id uint, name, user string) (string, error) { log.L.Printf("Registering application %s, notifications will be relayed to user %s.\n", name, user) resp, err := d.mautrixClient.CreateRoom(&mautrix.ReqCreateRoom{ diff --git a/internal/dispatcher/dispatcher.go b/internal/dispatcher/dispatcher.go index 05d557b..1bce2d4 100644 --- a/internal/dispatcher/dispatcher.go +++ b/internal/dispatcher/dispatcher.go @@ -1,3 +1,4 @@ +// Package dispatcher provides definitions and functionality related to executing Matrix requests. package dispatcher import ( diff --git a/internal/dispatcher/notification.go b/internal/dispatcher/notification.go index 9fe1d45..3708876 100644 --- a/internal/dispatcher/notification.go +++ b/internal/dispatcher/notification.go @@ -52,8 +52,8 @@ type NewContent struct { Format MessageFormat `json:"format"` } -// SendNotification sends a notification to the specified user. -func (d *Dispatcher) SendNotification(a *model.Application, n *model.Notification) (eventId string, err error) { +// SendNotification sends a notification to a given user. +func (d *Dispatcher) SendNotification(a *model.Application, n *model.Notification) (eventID string, err error) { log.L.Printf("Sending notification to room %s.", a.MatrixID) plainMessage := strings.TrimSpace(n.Message) @@ -80,7 +80,7 @@ func (d *Dispatcher) SendNotification(a *model.Application, n *model.Notificatio return evt.EventID.String(), nil } -// DeleteNotification sends a notification to the specified user that another notificaion is deleted +// DeleteNotification sends a notification to a given user that another notification is deleted func (d *Dispatcher) DeleteNotification(a *model.Application, n *model.DeleteNotification) error { log.L.Printf("Sending delete notification to room %s", a.MatrixID) var oldFormattedBody string @@ -128,7 +128,7 @@ func (d *Dispatcher) getFormattedTitle(n *model.Notification) string { // Converts different syntaxes to a HTML-formatted message func (d *Dispatcher) getFormattedMessage(n *model.Notification) string { trimmedMessage := strings.TrimSpace(n.Message) - message := strings.Replace(html.EscapeString(trimmedMessage), "\n", "
", -1) // default to text/plain + message := strings.ReplaceAll(html.EscapeString(trimmedMessage), "\n", "
") // default to text/plain if optionsDisplayRaw, ok := n.Extras["client::display"]; ok { optionsDisplay, ok := optionsDisplayRaw.(map[string]interface{}) @@ -140,7 +140,7 @@ func (d *Dispatcher) getFormattedMessage(n *model.Notification) string { switch contentType { case "html", "text/html": - message = strings.Replace(trimmedMessage, "\n", "
", -1) + message = strings.ReplaceAll(trimmedMessage, "\n", "
") case "markdown", "md", "text/md", "text/markdown": // Allow HTML in Markdown message = string(markdown.ToHTML([]byte(trimmedMessage), nil, nil)) diff --git a/internal/log/ginlogrus.go b/internal/log/ginlogrus.go index ace201b..6284467 100644 --- a/internal/log/ginlogrus.go +++ b/internal/log/ginlogrus.go @@ -1,5 +1,6 @@ // Source: https://github.com/toorop/gin-logrus +// Package log provides a connector between gin and logrus. package log import ( diff --git a/internal/log/log.go b/internal/log/log.go index 322b18d..5598d31 100644 --- a/internal/log/log.go +++ b/internal/log/log.go @@ -1,3 +1,4 @@ +// Package log provides functionality to configure the logger. package log import ( @@ -6,6 +7,7 @@ import ( log "github.com/sirupsen/logrus" ) +// L is the global logger instance for PushBits. var L *log.Logger func init() { @@ -17,6 +19,7 @@ func init() { }) } +// SetDebug sets the logger to output debug information. func SetDebug() { L.SetLevel(log.DebugLevel) } diff --git a/internal/model/alertmanager.go b/internal/model/alertmanager.go index 9013cea..b2af075 100644 --- a/internal/model/alertmanager.go +++ b/internal/model/alertmanager.go @@ -2,6 +2,7 @@ package model import "strings" +// AlertmanagerWebhook is used to pass notifications over webhook pushes. type AlertmanagerWebhook struct { Version string `json:"version"` GroupKey string `json:"groupKey"` @@ -13,6 +14,7 @@ type AlertmanagerWebhook struct { Alerts []AlertmanagerAlert `json:"alerts"` } +// AlertmanagerAlert holds information related to a single alert in a notification. type AlertmanagerAlert struct { Labels map[string]string `json:"labels"` Annotations map[string]string `json:"annotations"` @@ -21,6 +23,7 @@ type AlertmanagerAlert struct { Status string `json:"status"` } +// ToNotification converts an Alertmanager alert into a Notification func (alert *AlertmanagerAlert) ToNotification(titleAnnotation, messageAnnotation string) Notification { title := strings.Builder{} message := strings.Builder{} diff --git a/internal/model/notification.go b/internal/model/notification.go index 3fbb44f..0553f95 100644 --- a/internal/model/notification.go +++ b/internal/model/notification.go @@ -1,3 +1,4 @@ +// Package model contains structs used in the PushBits API and across the application. package model import ( @@ -8,7 +9,7 @@ import ( // Notification holds information like the message, the title, and the priority of a notification. type Notification struct { ID string `json:"id"` - UrlEncodedID string `json:"id_url_encoded"` + URLEncodedID string `json:"id_url_encoded"` ApplicationID uint `json:"appid"` Message string `json:"message" form:"message" query:"message" binding:"required"` Title string `json:"title" form:"title" query:"title"` @@ -20,7 +21,7 @@ type Notification struct { // Sanitize sets explicit defaults for a notification. func (n *Notification) Sanitize(application *Application) { n.ID = "" - n.UrlEncodedID = "" + n.URLEncodedID = "" n.ApplicationID = application.ID if strings.TrimSpace(n.Title) == "" { n.Title = application.Name diff --git a/internal/pberrors/errors.go b/internal/pberrors/errors.go index ff31d0c..f3255aa 100644 --- a/internal/pberrors/errors.go +++ b/internal/pberrors/errors.go @@ -1,3 +1,4 @@ +// Package pberrors defines errors specific to PushBits package pberrors import "errors" diff --git a/internal/router/router.go b/internal/router/router.go index 7c4d92d..87f53f3 100644 --- a/internal/router/router.go +++ b/internal/router/router.go @@ -1,3 +1,4 @@ +// Package router provides functions to configure the web server. package router import ( @@ -28,7 +29,7 @@ func Create(debug bool, trustedProxies []string, cm *credentials.Manager, db *da healthHandler := api.HealthHandler{DB: db} notificationHandler := api.NotificationHandler{DB: db, DP: dp} userHandler := api.UserHandler{AH: &applicationHandler, CM: cm, DB: db, DP: dp} - alertmanagerHandler := alertmanager.AlertmanagerHandler{DP: dp, Settings: alertmanager.AlertmanagerHandlerSettings{ + alertmanagerHandler := alertmanager.Handler{DP: dp, Settings: alertmanager.HandlerSettings{ TitleAnnotation: alertmanagerConfig.AnnotationTitle, MessageAnnotation: alertmanagerConfig.AnnotationMessage, }} diff --git a/internal/runner/runner.go b/internal/runner/runner.go index 911f778..c0cf8c3 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -1,3 +1,4 @@ +// Package runner provides functions to run the web server. package runner import ( diff --git a/tests/mockups/application.go b/tests/mockups/application.go index 754b37e..be46f41 100644 --- a/tests/mockups/application.go +++ b/tests/mockups/application.go @@ -1,3 +1,4 @@ +// Package mockups contains mockup objects and functions for tests. package mockups import "github.com/pushbits/server/internal/model" diff --git a/tests/mockups/dispatcher.go b/tests/mockups/dispatcher.go index c21af04..72eeafe 100644 --- a/tests/mockups/dispatcher.go +++ b/tests/mockups/dispatcher.go @@ -10,22 +10,27 @@ import ( // MockDispatcher is a dispatcher used for testing - it does not need any storage interface type MockDispatcher struct{} -func (d *MockDispatcher) RegisterApplication(id uint, name, token, user string) (string, error) { +// RegisterApplication mocks a functions to create a channel for an application. +func (*MockDispatcher) RegisterApplication(id uint, name, _ string) (string, error) { return fmt.Sprintf("%d-%s", id, name), nil } -func (d *MockDispatcher) DeregisterApplication(a *model.Application, u *model.User) error { +// DeregisterApplication mocks a function to delete a channel for an application. +func (*MockDispatcher) DeregisterApplication(_ *model.Application, _ *model.User) error { return nil } -func (d *MockDispatcher) UpdateApplication(a *model.Application, behavior *configuration.RepairBehavior) error { +// UpdateApplication mocks a function to update a channel for an application. +func (*MockDispatcher) UpdateApplication(_ *model.Application, _ *configuration.RepairBehavior) error { return nil } -func (d *MockDispatcher) SendNotification(a *model.Application, n *model.Notification) (id string, err error) { +// SendNotification mocks a function to send a notification to a given user. +func (*MockDispatcher) SendNotification(_ *model.Application, _ *model.Notification) (id string, err error) { return randStr(15), nil } -func (d *MockDispatcher) DeleteNotification(a *model.Application, n *model.DeleteNotification) error { +// DeleteNotification mocks a function to send a notification to a given user that another notification is deleted +func (*MockDispatcher) DeleteNotification(_ *model.Application, _ *model.DeleteNotification) error { return nil } diff --git a/tests/request.go b/tests/request.go index 40c659e..72db78b 100644 --- a/tests/request.go +++ b/tests/request.go @@ -1,3 +1,4 @@ +// Package tests provides definitions and functionality related to unit and integration tests. package tests import ( @@ -23,14 +24,14 @@ type Request struct { // GetRequest returns a ResponseRecorder and gin context according to the data set in the Request. // String data is passed as is, all other data types are marshaled before. func (r *Request) GetRequest() (w *httptest.ResponseRecorder, c *gin.Context, err error) { - var body io.Reader w = httptest.NewRecorder() + var body io.Reader - switch r.Data.(type) { + switch data := r.Data.(type) { case string: - body = strings.NewReader(r.Data.(string)) + body = strings.NewReader(data) default: - dataMarshaled, err := json.Marshal(r.Data) + dataMarshaled, err := json.Marshal(data) if err != nil { return nil, nil, err }