Chore: a bit of spring cleaning (#16710)

* Chore: use early return technic everywhere

And enable "indent-error-flow" revive rule

* Chore: remove if-return rule from revive config

* Chore: improve error messages

And enable "error-strings" revive rule

* Chore: enable "error-naming" revive rule

* Chore: make linter happy

* Chore: do not duplicate gofmt execution

* Chore: make linter happy

* Chore: address the pull review comments

pkg/cmd/grafana-cli/commands/install_command.go

@@ -14,6 +14,7 @@ import (
 	"strings"
 
 	"github.com/fatih/color"
+
 	"github.com/grafana/grafana/pkg/cmd/grafana-cli/logger"
 	m "github.com/grafana/grafana/pkg/cmd/grafana-cli/models"
 	s "github.com/grafana/grafana/pkg/cmd/grafana-cli/services"
@@ -135,7 +136,7 @@ func downloadFile(pluginName, filePath, url string) (err error) {
 			} else {
 				failure := fmt.Sprintf("%v", r)
 				if failure == "runtime error: makeslice: len out of range" {
-					err = fmt.Errorf("Corrupt http response from source. Please try again.\n")
+					err = fmt.Errorf("Corrupt http response from source. Please try again")
 				} else {
 					panic(r)
 				}

pkg/cmd/grafana-server/server.go

@@ -12,36 +12,33 @@ import (
 	"time"
 
 	"github.com/facebookgo/inject"
+	"golang.org/x/sync/errgroup"
+
 	"github.com/grafana/grafana/pkg/api"
 	"github.com/grafana/grafana/pkg/api/routing"
 	"github.com/grafana/grafana/pkg/bus"
-	"github.com/grafana/grafana/pkg/login"
-	"github.com/grafana/grafana/pkg/login/social"
-	"github.com/grafana/grafana/pkg/middleware"
-	"github.com/grafana/grafana/pkg/registry"
-
-	"golang.org/x/sync/errgroup"
-
-	"github.com/grafana/grafana/pkg/log"
-	"github.com/grafana/grafana/pkg/services/cache"
-	"github.com/grafana/grafana/pkg/setting"
-
-	// self registering services
 	_ "github.com/grafana/grafana/pkg/extensions"
 	_ "github.com/grafana/grafana/pkg/infra/metrics"
 	_ "github.com/grafana/grafana/pkg/infra/remotecache"
 	_ "github.com/grafana/grafana/pkg/infra/serverlock"
 	_ "github.com/grafana/grafana/pkg/infra/tracing"
 	_ "github.com/grafana/grafana/pkg/infra/usagestats"
+	"github.com/grafana/grafana/pkg/log"
+	"github.com/grafana/grafana/pkg/login"
+	"github.com/grafana/grafana/pkg/login/social"
+	"github.com/grafana/grafana/pkg/middleware"
 	_ "github.com/grafana/grafana/pkg/plugins"
+	"github.com/grafana/grafana/pkg/registry"
 	_ "github.com/grafana/grafana/pkg/services/alerting"
 	_ "github.com/grafana/grafana/pkg/services/auth"
+	"github.com/grafana/grafana/pkg/services/cache"
 	_ "github.com/grafana/grafana/pkg/services/cleanup"
 	_ "github.com/grafana/grafana/pkg/services/notifications"
 	_ "github.com/grafana/grafana/pkg/services/provisioning"
 	_ "github.com/grafana/grafana/pkg/services/rendering"
 	_ "github.com/grafana/grafana/pkg/services/search"
 	_ "github.com/grafana/grafana/pkg/services/sqlstore"
+	"github.com/grafana/grafana/pkg/setting"
 )
 
 func NewGrafanaServer() *GrafanaServerImpl {
@@ -238,7 +235,7 @@ func sendSystemdNotification(state string) error {
 	notifySocket := os.Getenv("NOTIFY_SOCKET")
 
 	if notifySocket == "" {
-		return fmt.Errorf("NOTIFY_SOCKET environment variable empty or unset.")
+		return fmt.Errorf("NOTIFY_SOCKET environment variable empty or unset")
 	}
 
 	socketAddr := &net.UnixAddr{

pkg/components/imguploader/webdavuploader.go

@@ -39,11 +39,11 @@ var netClient = &http.Client{
 func (u *WebdavUploader) PublicURL(filename string) string {
 	if strings.Contains(u.public_url, "${file}") {
 		return strings.Replace(u.public_url, "${file}", filename, -1)
-	} else {
-		publicURL, _ := url.Parse(u.public_url)
-		publicURL.Path = path.Join(publicURL.Path, filename)
-		return publicURL.String()
 	}
+
+	publicURL, _ := url.Parse(u.public_url)
+	publicURL.Path = path.Join(publicURL.Path, filename)
+	return publicURL.String()
 }
 
 func (u *WebdavUploader) Upload(ctx context.Context, pa string) (string, error) {

pkg/infra/usagestats/usage_stats.go

@@ -171,7 +171,6 @@ func (uss *UsageStatsService) updateTotalStats() {
 func getEdition() string {
 	if setting.IsEnterprise {
 		return "enterprise"
-	} else {
-		return "oss"
 	}
+	return "oss"
 }

pkg/log/file.go

@@ -150,7 +150,7 @@ func (w *FileLogWriter) initFd() error {
 	fd := w.mw.fd
 	finfo, err := fd.Stat()
 	if err != nil {
-		return fmt.Errorf("get stat: %s\n", err)
+		return fmt.Errorf("get stat: %s", err)
 	}
 	w.maxsize_cursize = int(finfo.Size())
 	w.daily_opendate = time.Now().Day()
@@ -180,7 +180,7 @@ func (w *FileLogWriter) DoRotate() error {
 		}
 		// return error if the last file checked still existed
 		if err == nil {
-			return fmt.Errorf("rotate: cannot find free log number to rename %s\n", w.Filename)
+			return fmt.Errorf("rotate: cannot find free log number to rename %s", w.Filename)
 		}
 
 		// block Logger's io.Writer
@@ -193,12 +193,12 @@ func (w *FileLogWriter) DoRotate() error {
 		// close fd before rename
 		// Rename the file to its newfound home
 		if err = os.Rename(w.Filename, fname); err != nil {
-			return fmt.Errorf("Rotate: %s\n", err)
+			return fmt.Errorf("Rotate: %s", err)
 		}
 
 		// re-start logger
 		if err = w.StartLogger(); err != nil {
-			return fmt.Errorf("Rotate StartLogger: %s\n", err)
+			return fmt.Errorf("Rotate StartLogger: %s", err)
 		}
 
 		go w.deleteOldLog()

pkg/login/auth.go

@@ -14,7 +14,7 @@ var (
 	ErrProviderDeniedRequest = errors.New("Login provider denied login request")
 	ErrSignUpNotAllowed      = errors.New("Signup is not allowed for this adapter")
 	ErrTooManyLoginAttempts  = errors.New("Too many consecutive incorrect login attempts for user. Login for user temporarily blocked")
-	ErrPasswordEmpty         = errors.New("No password provided.")
+	ErrPasswordEmpty         = errors.New("No password provided")
 	ErrUsersQuotaReached     = errors.New("Users quota reached")
 	ErrGettingUserQuota      = errors.New("Error getting user quota")
 )

pkg/models/alert.go

@@ -1,9 +1,8 @@
 package models
 
 import (
-	"time"
-
 	"fmt"
+	"time"
 
 	"github.com/grafana/grafana/pkg/components/simplejson"
 )
@@ -36,7 +35,7 @@ const (
 
 var (
 	ErrCannotChangeStateOnPausedAlert = fmt.Errorf("Cannot change state on pause alert")
-	ErrRequiresNewState               = fmt.Errorf("update alert state requires a new state.")
+	ErrRequiresNewState               = fmt.Errorf("update alert state requires a new state")
 )
 
 func (s AlertStateType) IsValid() bool {

pkg/models/alert_notifications.go

@@ -11,7 +11,7 @@ var (
 	ErrNotificationFrequencyNotFound            = errors.New("Notification frequency not specified")
 	ErrAlertNotificationStateNotFound           = errors.New("alert notification state not found")
 	ErrAlertNotificationStateVersionConflict    = errors.New("alert notification state update version conflict")
-	ErrAlertNotificationStateAlreadyExist       = errors.New("alert notification state already exists.")
+	ErrAlertNotificationStateAlreadyExist       = errors.New("alert notification state already exists")
 	ErrAlertNotificationFailedGenerateUniqueUid = errors.New("Failed to generate unique alert notification uid")
 )
 

pkg/models/dashboard_acl.go

@@ -24,10 +24,10 @@ func (p PermissionType) String() string {
 
 // Typed errors
 var (
-	ErrDashboardAclInfoMissing           = errors.New("User id and team id cannot both be empty for a dashboard permission.")
+	ErrDashboardAclInfoMissing           = errors.New("User id and team id cannot both be empty for a dashboard permission")
-	ErrDashboardPermissionDashboardEmpty = errors.New("Dashboard Id must be greater than zero for a dashboard permission.")
+	ErrDashboardPermissionDashboardEmpty = errors.New("Dashboard Id must be greater than zero for a dashboard permission")
-	ErrFolderAclInfoMissing              = errors.New("User id and team id cannot both be empty for a folder permission.")
+	ErrFolderAclInfoMissing              = errors.New("User id and team id cannot both be empty for a folder permission")
-	ErrFolderPermissionFolderEmpty       = errors.New("Folder Id must be greater than zero for a folder permission.")
+	ErrFolderPermissionFolderEmpty       = errors.New("Folder Id must be greater than zero for a folder permission")
 )
 
 // Dashboard ACL model

pkg/models/datasource.go

@@ -30,7 +30,7 @@ var (
 	ErrDataSourceNotFound           = errors.New("Data source not found")
 	ErrDataSourceNameExists         = errors.New("Data source with same name already exists")
 	ErrDataSourceUpdatingOldVersion = errors.New("Trying to update old version of datasource")
-	ErrDatasourceIsReadOnly         = errors.New("Data source is readonly. Can only be updated from configuration.")
+	ErrDatasourceIsReadOnly         = errors.New("Data source is readonly. Can only be updated from configuration")
 	ErrDataSourceAccessDenied       = errors.New("Data source access denied")
 )
 

pkg/models/notifications.go

@@ -3,7 +3,7 @@ package models
 import "errors"
 
 var ErrInvalidEmailCode = errors.New("Invalid or expired email code")
-var ErrSmtpNotEnabled = errors.New("SMTP not configured, check your grafana.ini config file's [smtp] section.")
+var ErrSmtpNotEnabled = errors.New("SMTP not configured, check your grafana.ini config file's [smtp] section")
 
 type SendEmailCommand struct {
 	To           []string

pkg/services/alerting/rule.go

@@ -130,11 +130,11 @@ func NewRuleFromDBAlert(ruleDef *m.Alert) (*Rule, error) {
 		if id, err := jsonModel.Get("id").Int64(); err == nil {
 			model.Notifications = append(model.Notifications, fmt.Sprintf("%09d", id))
 		} else {
-			if uid, err := jsonModel.Get("uid").String(); err != nil {
+			uid, err := jsonModel.Get("uid").String()
+			if err != nil {
 				return nil, ValidationError{Reason: "Neither id nor uid is specified, " + err.Error(), DashboardId: model.DashboardId, Alertid: model.Id, PanelId: model.PanelId}
-			} else {
-				model.Notifications = append(model.Notifications, uid)
 			}
+			model.Notifications = append(model.Notifications, uid)
 		}
 	}
 

pkg/services/rendering/rendering.go

@@ -105,9 +105,8 @@ func (rs *RenderingService) Render(ctx context.Context, opts Opts) (*RenderResul
 
 	if rs.renderAction != nil {
 		return rs.renderAction(ctx, opts)
-	} else {
-		return nil, fmt.Errorf("No renderer found")
 	}
+	return nil, fmt.Errorf("No renderer found")
 }
 
 func (rs *RenderingService) getFilePathForNewImage() string {

pkg/services/sqlstore/alert_notification.go

@@ -213,11 +213,12 @@ func getAlertNotificationWithUidInternal(query *m.GetAlertNotificationsWithUidQu
 func CreateAlertNotificationCommand(cmd *m.CreateAlertNotificationCommand) error {
 	return inTransaction(func(sess *DBSession) error {
 		if cmd.Uid == "" {
-			if uid, uidGenerationErr := generateNewAlertNotificationUid(sess, cmd.OrgId); uidGenerationErr != nil {
+			uid, uidGenerationErr := generateNewAlertNotificationUid(sess, cmd.OrgId)
+			if uidGenerationErr != nil {
 				return uidGenerationErr
-			} else {
-				cmd.Uid = uid
 			}
+
+			cmd.Uid = uid
 		}
 		existingQuery := &m.GetAlertNotificationsWithUidQuery{OrgId: cmd.OrgId, Uid: cmd.Uid}
 		err := getAlertNotificationWithUidInternal(existingQuery, sess)

pkg/services/sqlstore/tls_mysql.go

@@ -35,7 +35,7 @@ func makeCert(tlsPoolName string, config DatabaseConfig) (*tls.Config, error) {
 	}
 	// Return more meaningful error before it is too late
 	if config.ServerCertName == "" && !tlsConfig.InsecureSkipVerify {
-		return nil, fmt.Errorf("server_cert_name is missing. Consider using ssl_mode = skip-verify.")
+		return nil, fmt.Errorf("server_cert_name is missing. Consider using ssl_mode = skip-verify")
 	}
 	return tlsConfig, nil
 }

pkg/services/sqlstore/transactions_test.go

@@ -5,12 +5,12 @@ import (
 	"errors"
 	"testing"
 
-	"github.com/grafana/grafana/pkg/models"
-
 	. "github.com/smartystreets/goconvey/convey"
+
+	"github.com/grafana/grafana/pkg/models"
 )
 
-var ProvokedError = errors.New("testing error.")
+var ErrProvokedError = errors.New("testing error")
 
 func TestTransaction(t *testing.T) {
 	ss := InitTestDB(t)
@@ -42,10 +42,10 @@ func TestTransaction(t *testing.T) {
 					return err
 				}
 
-				return ProvokedError
+				return ErrProvokedError
 			})
 
-			So(err, ShouldEqual, ProvokedError)
+			So(err, ShouldEqual, ErrProvokedError)
 
 			query := &models.GetApiKeyByIdQuery{ApiKeyId: cmd.Result.Id}
 			err = GetApiKeyById(query)

pkg/services/sqlstore/user.go

@@ -2,12 +2,11 @@ package sqlstore
 
 import (
 	"context"
+	"fmt"
 	"strconv"
 	"strings"
 	"time"
 
-	"fmt"
-
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/events"
 	m "github.com/grafana/grafana/pkg/models"
@@ -48,16 +47,15 @@ func getOrgIdForNewUser(cmd *m.CreateUserCommand, sess *DBSession) (int64, error
 		}
 		if has {
 			return org.Id, nil
+		}
+		if setting.AutoAssignOrgId == 1 {
+			org.Name = "Main Org."
+			org.Id = int64(setting.AutoAssignOrgId)
 		} else {
-			if setting.AutoAssignOrgId == 1 {
+			sqlog.Info("Could not create user: organization id %v does not exist",
-				org.Name = "Main Org."
+				setting.AutoAssignOrgId)
-				org.Id = int64(setting.AutoAssignOrgId)
+			return 0, fmt.Errorf("Could not create user: organization id %v does not exist",
-			} else {
+				setting.AutoAssignOrgId)
-				sqlog.Info("Could not create user: organization id %v does not exist",
-					setting.AutoAssignOrgId)
-				return 0, fmt.Errorf("Could not create user: organization id %v does not exist",
-					setting.AutoAssignOrgId)
-			}
 		}
 	} else {
 		org.Name = cmd.OrgName

pkg/tsdb/influxdb/query.go

@@ -2,11 +2,10 @@ package influxdb
 
 import (
 	"fmt"
+	"regexp"
 	"strconv"
 	"strings"
 
-	"regexp"
-
 	"github.com/grafana/grafana/pkg/tsdb"
 )
 
@@ -160,7 +159,6 @@ func (query *Query) renderTz() string {
 	tz := query.Tz
 	if tz == "" {
 		return ""
-	} else {
-		return fmt.Sprintf(" tz('%s')", tz)
 	}
+	return fmt.Sprintf(" tz('%s')", tz)
 }

pkg/tsdb/postgres/macros.go

@@ -108,9 +108,13 @@ func (m *postgresMacroEngine) evaluateMacro(name string, args []string) (string,
 
 		if m.timescaledb {
 			return fmt.Sprintf("time_bucket('%vs',%s)", interval.Seconds(), args[0]), nil
-		} else {
-			return fmt.Sprintf("floor(extract(epoch from %s)/%v)*%v", args[0], interval.Seconds(), interval.Seconds()), nil
 		}
+
+		return fmt.Sprintf(
+			"floor(extract(epoch from %s)/%v)*%v", args[0],
+			interval.Seconds(),
+			interval.Seconds(),
+		), nil
 	case "__timeGroupAlias":
 		tg, err := m.evaluateMacro("__timeGroup", args)
 		if err == nil {

scripts/circle-test-backend.sh

@@ -10,9 +10,6 @@ function exit_if_fail {
     fi
 }
 
-echo "running go fmt"
-exit_if_fail test -z \"'$(gofmt -s -l ./pkg | tee /dev/stderr)'\"
-
 echo "building backend with install to cache pkgs"
 exit_if_fail time go install ./pkg/cmd/grafana-server
 

scripts/revive.toml

@@ -9,6 +9,9 @@ errorCode = 0
 [rule.range]
 [rule.superfluous-else]
 [rule.modifies-parameter]
+[rule.indent-error-flow]
+[rule.error-strings]
+[rule.error-naming]
 
 # This can be checked by other tools like megacheck
 [rule.unreachable-code]
@@ -17,11 +20,6 @@ errorCode = 0
 # [rule.unexported-return]
 # [rule.exported]
 # [rule.var-naming]
-# [error-naming]
 # [rule.dot-imports]
-# [blank-imports]
 # [rule.receiver-naming]
-# [error-strings]
-# [rule.if-return]
-# [rule.indent-error-flow]
 # [rule.blank-imports]