Chore: Enable goprintffuncname and nakedret linters (#26376)

* Chore: Enable goprintffuncname linter
* Chore: Enable nakedret linter

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

.circleci/config.yml

@@ -814,8 +814,9 @@ jobs:
           command: |
             # To save memory, run in two batches
             golangci-lint run -v -j 4 --config scripts/go/configs/ci/.golangci.toml -E deadcode -E depguard -E dogsled \
-              -E errcheck -E goconst -E golint -E gosec -E gosimple -E govet -E exportloopref -E whitespace ./pkg/...
-            golangci-lint run -v -j 4 --config scripts/go/configs/ci/.golangci.toml -E ineffassign -E gocritic \
+              -E errcheck -E goconst -E golint -E gosec -E gosimple -E govet -E exportloopref -E whitespace \
+              -E goprintffuncname ./pkg/...
+            golangci-lint run -v -j 4 --config scripts/go/configs/ci/.golangci.toml -E ineffassign -E gocritic -E nakedret \
               -E rowserrcheck -E staticcheck -E structcheck -E typecheck -E unconvert -E unused -E varcheck ./pkg/...
             ./scripts/go/bin/revive -formatter stylish -config ./scripts/go/configs/revive.toml ./pkg/...
             ./scripts/go/bin/revive -formatter stylish -config ./scripts/go/configs/revive-strict.toml \

pkg/api/app_routes.go

@@ -48,7 +48,7 @@ func (hs *HTTPServer) initAppPluginRoutes(r *macaron.Macaron) {
 			}
 			handlers = append(handlers, AppPluginRoute(route, plugin.Id, hs))
 			r.Route(url, route.Method, handlers...)
-			log.Debug("Plugins: Adding proxy route %s", url)
+			log.Debugf("Plugins: Adding proxy route %s", url)
 		}
 	}
 }

pkg/api/avatar/avatar.go

@@ -94,7 +94,7 @@ func (this *CacheServer) Handler(ctx *models.ReqContext) {
 	if avatar.Expired() {
 		// The cache item is either expired or newly created, update it from the server
 		if err := avatar.Update(); err != nil {
-			log.Trace("avatar update error: %v", err)
+			log.Tracef("avatar update error: %v", err)
 			avatar = this.notFound
 		}
 	}
@@ -103,7 +103,7 @@ func (this *CacheServer) Handler(ctx *models.ReqContext) {
 		avatar = this.notFound
 	} else if !exists {
 		if err := this.cache.Add(hash, avatar, gocache.DefaultExpiration); err != nil {
-			log.Trace("Error adding avatar to cache: %s", err)
+			log.Tracef("Error adding avatar to cache: %s", err)
 		}
 	}
 
@@ -116,7 +116,7 @@ func (this *CacheServer) Handler(ctx *models.ReqContext) {
 	ctx.Resp.Header().Add("Cache-Control", "private, max-age=3600")
 
 	if err := avatar.Encode(ctx.Resp); err != nil {
-		log.Warn("avatar encode error: %v", err)
+		log.Warnf("avatar encode error: %v", err)
 		ctx.WriteHeader(500)
 	}
 }
@@ -135,7 +135,7 @@ func newNotFound() *Avatar {
 	path := filepath.Join(setting.StaticRootPath, "img", "user_profile.png")
 
 	if data, err := ioutil.ReadFile(path); err != nil {
-		log.Error(3, "Failed to read user_profile.png, %v", path)
+		log.Errorf(3, "Failed to read user_profile.png, %v", path)
 	} else {
 		avatar.data = bytes.NewBuffer(data)
 	}
@@ -208,7 +208,7 @@ var client = &http.Client{
 func (this *thunderTask) fetch() error {
 	this.Avatar.timestamp = time.Now()
 
-	log.Debug("avatar.fetch(fetch new avatar): %s", this.Url)
+	log.Debugf("avatar.fetch(fetch new avatar): %s", this.Url)
 	req, _ := http.NewRequest("GET", this.Url, nil)
 	req.Header.Set("Accept", "text/html,application/xhtml+xml,application/xml;q=0.9,image/jpeg,image/png,*/*;q=0.8")
 	req.Header.Set("Accept-Encoding", "deflate,sdch")

pkg/api/dashboard.go

@@ -321,7 +321,7 @@ func (hs *HTTPServer) GetHomeDashboard(c *models.ReqContext) Response {
 			dashRedirect := dtos.DashboardRedirect{RedirectUri: url}
 			return JSON(200, &dashRedirect)
 		}
-		log.Warn("Failed to get slug from database, %s", err.Error())
+		log.Warnf("Failed to get slug from database, %s", err.Error())
 	}
 
 	filePath := hs.Cfg.DefaultHomeDashboardPath

pkg/api/dtos/models.go

@@ -65,7 +65,7 @@ func GetGravatarUrl(text string) string {
 
 	hasher := md5.New()
 	if _, err := hasher.Write([]byte(strings.ToLower(text))); err != nil {
-		log.Warn("Failed to hash text: %s", err)
+		log.Warnf("Failed to hash text: %s", err)
 	}
 	return fmt.Sprintf(setting.AppSubUrl+"/avatar/%x", hasher.Sum(nil))
 }

pkg/api/frontendsettings.go

@@ -75,7 +75,7 @@ func (hs *HTTPServer) getFrontendSettingsMap(c *models.ReqContext) (map[string]i
 
 		meta, exists := enabledPlugins.DataSources[ds.Type]
 		if !exists {
-			log.Error(3, "Could not find plugin definition for data source: %v", ds.Type)
+			log.Errorf(3, "Could not find plugin definition for data source: %v", ds.Type)
 			continue
 		}
 

pkg/api/live/conn.go

@@ -76,14 +76,14 @@ func (c *connection) readPump() {
 func (c *connection) handleMessage(message []byte) {
 	json, err := simplejson.NewJson(message)
 	if err != nil {
-		log.Error(3, "Unreadable message on websocket channel. error: %v", err)
+		log.Errorf(3, "Unreadable message on websocket channel. error: %v", err)
 	}
 
 	msgType := json.Get("action").MustString()
 	streamName := json.Get("stream").MustString()
 
 	if len(streamName) == 0 {
-		log.Error(3, "Not allowed to subscribe to empty stream name")
+		log.Errorf(3, "Not allowed to subscribe to empty stream name")
 		return
 	}
 

pkg/api/live/stream_manager.go

@@ -27,11 +27,11 @@ func NewStreamManager() *StreamManager {
 }
 
 func (sm *StreamManager) Run(context context.Context) {
-	log.Debug("Initializing Stream Manager")
+	log.Debugf("Initializing Stream Manager")
 
 	go func() {
 		sm.hub.run(context)
-		log.Info("Stopped Stream Manager")
+		log.Infof("Stopped Stream Manager")
 	}()
 }
 

pkg/api/login.go

@@ -117,7 +117,7 @@ func (hs *HTTPServer) LoginView(c *models.ReqContext) {
 			if err := hs.ValidateRedirectTo(redirectTo); err != nil {
 				// the user is already logged so instead of rendering the login page with error
 				// it should be redirected to the home page.
-				log.Debug("Ignored invalid redirect_to cookie value: %v", redirectTo)
+				log.Debugf("Ignored invalid redirect_to cookie value: %v", redirectTo)
 				redirectTo = hs.Cfg.AppSubUrl + "/"
 			}
 			middleware.DeleteCookie(c.Resp, "redirect_to", hs.CookieOptionsFromCfg)
@@ -138,12 +138,12 @@ func tryOAuthAutoLogin(c *models.ReqContext) bool {
 	}
 	oauthInfos := setting.OAuthService.OAuthInfos
 	if len(oauthInfos) != 1 {
-		log.Warn("Skipping OAuth auto login because multiple OAuth providers are configured")
+		log.Warnf("Skipping OAuth auto login because multiple OAuth providers are configured")
 		return false
 	}
 	for key := range setting.OAuthService.OAuthInfos {
 		redirectUrl := setting.AppSubUrl + "/login/" + key
-		log.Info("OAuth auto login enabled. Redirecting to " + redirectUrl)
+		log.Infof("OAuth auto login enabled. Redirecting to " + redirectUrl)
 		c.Redirect(redirectUrl, 307)
 		return true
 	}
@@ -201,7 +201,7 @@ func (hs *HTTPServer) LoginPost(c *models.ReqContext, cmd dtos.LoginCommand) Res
 		if err := hs.ValidateRedirectTo(redirectTo); err == nil {
 			result["redirectUrl"] = redirectTo
 		} else {
-			log.Info("Ignored invalid redirect_to cookie value: %v", redirectTo)
+			log.Infof("Ignored invalid redirect_to cookie value: %v", redirectTo)
 		}
 		middleware.DeleteCookie(c.Resp, "redirect_to", hs.CookieOptionsFromCfg)
 	}

pkg/api/login_oauth.go

@@ -232,7 +232,7 @@ func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) {
 			ctx.Redirect(redirectTo)
 			return
 		}
-		log.Debug("Ignored invalid redirect_to cookie value: %v", redirectTo)
+		log.Debugf("Ignored invalid redirect_to cookie value: %v", redirectTo)
 	}
 
 	ctx.Redirect(setting.AppSubUrl + "/")

pkg/api/pluginproxy/pluginproxy.go

@@ -90,7 +90,7 @@ func NewApiPluginProxy(ctx *models.ReqContext, proxyPath string, route *plugins.
 			}
 
 			for key, value := range headers {
-				log.Trace("setting key %v value <redacted>", key)
+				log.Tracef("setting key %v value <redacted>", key)
 				req.Header.Set(key, value[0])
 			}
 		}
@@ -112,7 +112,7 @@ func NewApiPluginProxy(ctx *models.ReqContext, proxyPath string, route *plugins.
 		}
 
 		// reqBytes, _ := httputil.DumpRequestOut(req, true);
-		// log.Trace("Proxying plugin request: %s", string(reqBytes))
+		// log.Tracef("Proxying plugin request: %s", string(reqBytes))
 	}
 
 	return &httputil.ReverseProxy{Director: director}

pkg/components/imguploader/s3uploader.go

@@ -73,7 +73,7 @@ func (u *S3Uploader) Upload(ctx context.Context, imageDiskPath string) (string,
 		return "", err
 	}
 	key := u.path + rand + pngExt
-	log.Debug("Uploading image to s3. bucket = %s, path = %s", u.bucket, key)
+	log.Debugf("Uploading image to s3. bucket = %s, path = %s", u.bucket, key)
 
 	file, err := os.Open(imageDiskPath)
 	if err != nil {

pkg/components/securejsondata/securejsondata.go

@@ -16,7 +16,7 @@ func (s SecureJsonData) DecryptedValue(key string) (string, bool) {
 	if value, ok := s[key]; ok {
 		decryptedData, err := util.Decrypt(value, setting.SecretKey)
 		if err != nil {
-			log.Fatal(4, err.Error())
+			log.Fatalf(4, err.Error())
 		}
 		return string(decryptedData), true
 	}
@@ -30,7 +30,7 @@ func (s SecureJsonData) Decrypt() map[string]string {
 	for key, data := range s {
 		decryptedData, err := util.Decrypt(data, setting.SecretKey)
 		if err != nil {
-			log.Fatal(4, err.Error())
+			log.Fatalf(4, err.Error())
 		}
 
 		decrypted[key] = string(decryptedData)
@@ -44,7 +44,7 @@ func GetEncryptedJsonData(sjd map[string]string) SecureJsonData {
 	for key, data := range sjd {
 		encryptedData, err := util.Encrypt([]byte(data), setting.SecretKey)
 		if err != nil {
-			log.Fatal(4, err.Error())
+			log.Fatalf(4, err.Error())
 		}
 
 		encrypted[key] = encryptedData

pkg/infra/log/log.go

@@ -36,7 +36,7 @@ func New(logger string, ctx ...interface{}) Logger {
 	return Root.New(params...)
 }
 
-func Trace(format string, v ...interface{}) {
+func Tracef(format string, v ...interface{}) {
 	var message string
 	if len(v) > 0 {
 		message = fmt.Sprintf(format, v...)
@@ -47,7 +47,7 @@ func Trace(format string, v ...interface{}) {
 	Root.Debug(message)
 }
 
-func Debug(format string, v ...interface{}) {
+func Debugf(format string, v ...interface{}) {
 	var message string
 	if len(v) > 0 {
 		message = fmt.Sprintf(format, v...)
@@ -58,7 +58,7 @@ func Debug(format string, v ...interface{}) {
 	Root.Debug(message)
 }
 
-func Info(format string, v ...interface{}) {
+func Infof(format string, v ...interface{}) {
 	var message string
 	if len(v) > 0 {
 		message = fmt.Sprintf(format, v...)
@@ -69,7 +69,7 @@ func Info(format string, v ...interface{}) {
 	Root.Info(message)
 }
 
-func Warn(format string, v ...interface{}) {
+func Warnf(format string, v ...interface{}) {
 	var message string
 	if len(v) > 0 {
 		message = fmt.Sprintf(format, v...)
@@ -80,15 +80,15 @@ func Warn(format string, v ...interface{}) {
 	Root.Warn(message)
 }
 
-func Error(skip int, format string, v ...interface{}) {
+func Errorf(skip int, format string, v ...interface{}) {
 	Root.Error(fmt.Sprintf(format, v...))
 }
 
-func Critical(skip int, format string, v ...interface{}) {
+func Criticalf(skip int, format string, v ...interface{}) {
 	Root.Crit(fmt.Sprintf(format, v...))
 }
 
-func Fatal(skip int, format string, v ...interface{}) {
+func Fatalf(skip int, format string, v ...interface{}) {
 	Root.Crit(fmt.Sprintf(format, v...))
 	Close()
 	os.Exit(1)

pkg/login/social/common.go

@@ -64,7 +64,7 @@ func HttpGet(client *http.Client, url string) (response HttpGetResponse, err err
 		return
 	}
 
-	log.Trace("HTTP GET %s: %s %s", url, r.Status, string(response.Body))
+	log.Tracef("HTTP GET %s: %s %s", url, r.Status, string(response.Body))
 
 	err = nil
 	return

pkg/middleware/middleware.go

@@ -94,7 +94,7 @@ func initContextWithAnonymousUser(ctx *models.ReqContext) bool {
 
 	orgQuery := models.GetOrgByNameQuery{Name: setting.AnonymousOrgName}
 	if err := bus.Dispatch(&orgQuery); err != nil {
-		log.Error(3, "Anonymous access organization error: '%s': %s", setting.AnonymousOrgName, err)
+		log.Errorf(3, "Anonymous access organization error: '%s': %s", setting.AnonymousOrgName, err)
 		return false
 	}
 

pkg/plugins/update_checker.go

@@ -50,7 +50,7 @@ func (pm *PluginManager) checkForUpdates() {
 	resp, err := httpClient.Get("https://grafana.com/api/plugins/versioncheck?slugIn=" + pluginSlugs + "&grafanaVersion=" + setting.BuildVersion)
 
 	if err != nil {
-		log.Trace("Failed to get plugins repo from grafana.com, %v", err.Error())
+		log.Tracef("Failed to get plugins repo from grafana.com, %v", err.Error())
 		return
 	}
 
@@ -58,14 +58,14 @@ func (pm *PluginManager) checkForUpdates() {
 
 	body, err := ioutil.ReadAll(resp.Body)
 	if err != nil {
-		log.Trace("Update check failed, reading response from grafana.com, %v", err.Error())
+		log.Tracef("Update check failed, reading response from grafana.com, %v", err.Error())
 		return
 	}
 
 	gNetPlugins := []GrafanaNetPlugin{}
 	err = json.Unmarshal(body, &gNetPlugins)
 	if err != nil {
-		log.Trace("Failed to unmarshal plugin repo, reading response from grafana.com, %v", err.Error())
+		log.Tracef("Failed to unmarshal plugin repo, reading response from grafana.com, %v", err.Error())
 		return
 	}
 
@@ -88,21 +88,21 @@ func (pm *PluginManager) checkForUpdates() {
 
 	resp2, err := httpClient.Get("https://raw.githubusercontent.com/grafana/grafana/master/latest.json")
 	if err != nil {
-		log.Trace("Failed to get latest.json repo from github.com: %v", err.Error())
+		log.Tracef("Failed to get latest.json repo from github.com: %v", err.Error())
 		return
 	}
 
 	defer resp2.Body.Close()
 	body, err = ioutil.ReadAll(resp2.Body)
 	if err != nil {
-		log.Trace("Update check failed, reading response from github.com, %v", err.Error())
+		log.Tracef("Update check failed, reading response from github.com, %v", err.Error())
 		return
 	}
 
 	var githubLatest GithubLatest
 	err = json.Unmarshal(body, &githubLatest)
 	if err != nil {
-		log.Trace("Failed to unmarshal github.com latest, reading response from github.com: %v", err.Error())
+		log.Tracef("Failed to unmarshal github.com latest, reading response from github.com: %v", err.Error())
 		return
 	}
 

pkg/services/alerting/notifiers/telegram.go

@@ -265,7 +265,7 @@ func appendIfPossible(message string, extra string, sizeLimit int) string {
 	if len(extra)+len(message) <= sizeLimit {
 		return message + extra
 	}
-	log.Debug("Line too long for image caption. value: %s", extra)
+	log.Debugf("Line too long for image caption. value: %s", extra)
 	return message
 }
 

pkg/services/login/login.go

@@ -44,13 +44,13 @@ func (ls *LoginService) UpsertUser(cmd *models.UpsertUserCommand) error {
 			return err
 		}
 		if !cmd.SignupAllowed {
-			log.Warn("Not allowing %s login, user not found in internal user database and allow signup = false", extUser.AuthModule)
+			log.Warnf("Not allowing %s login, user not found in internal user database and allow signup = false", extUser.AuthModule)
 			return ErrInvalidCredentials
 		}
 
 		limitReached, err := ls.QuotaService.QuotaReached(cmd.ReqContext, "user")
 		if err != nil {
-			log.Warn("Error getting user quota. error: %v", err)
+			log.Warnf("Error getting user quota. error: %v", err)
 			return ErrGettingUserQuota
 		}
 		if limitReached {

pkg/services/sqlstore/transactions.go

@@ -66,7 +66,7 @@ func inTransactionWithRetryCtx(ctx context.Context, engine *xorm.Engine, callbac
 	if len(sess.events) > 0 {
 		for _, e := range sess.events {
 			if err = bus.Publish(e); err != nil {
-				log.Error(3, "Failed to publish event after commit. error: %v", err)
+				log.Errorf(3, "Failed to publish event after commit. error: %v", err)
 			}
 		}
 	}

pkg/setting/setting.go

@@ -336,7 +336,7 @@ func parseAppUrlAndSubUrl(section *ini.Section) (string, string, error) {
 	// Check if has app suburl.
 	url, err := url.Parse(appUrl)
 	if err != nil {
-		log.Fatal(4, "Invalid root_url(%s): %s", appUrl, err)
+		log.Fatalf(4, "Invalid root_url(%s): %s", appUrl, err)
 	}
 	appSubUrl := strings.TrimSuffix(url.Path, "/")
 
@@ -444,7 +444,7 @@ func getCommandLineProperties(args []string) map[string]string {
 		trimmed := strings.TrimPrefix(arg, "cfg:")
 		parts := strings.Split(trimmed, "=")
 		if len(parts) != 2 {
-			log.Fatal(3, "Invalid command line argument. argument: %v", arg)
+			log.Fatalf(3, "Invalid command line argument. argument: %v", arg)
 			return nil
 		}
 
@@ -533,7 +533,7 @@ func (cfg *Cfg) loadConfiguration(args *CommandLineArgs) (*ini.File, error) {
 		if err2 != nil {
 			return nil, err2
 		}
-		log.Fatal(3, err.Error())
+		log.Fatalf(3, err.Error())
 	}
 
 	// apply environment overrides
@@ -953,7 +953,7 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error {
 		}
 		_, err := url.Parse(cfg.RendererCallbackUrl)
 		if err != nil {
-			log.Fatal(4, "Invalid callback_url(%s): %s", cfg.RendererCallbackUrl, err)
+			log.Fatalf(4, "Invalid callback_url(%s): %s", cfg.RendererCallbackUrl, err)
 		}
 	}
 	cfg.RendererConcurrentRequestLimit = renderSec.Key("concurrent_render_request_limit").MustInt(30)
@@ -1036,7 +1036,7 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error {
 	cfg.readQuotaSettings()
 
 	if VerifyEmailEnabled && !cfg.Smtp.Enabled {
-		log.Warn("require_email_validation is enabled but smtp is disabled")
+		log.Warnf("require_email_validation is enabled but smtp is disabled")
 	}
 
 	// check old key  name

scripts/go/configs/.golangci.toml

@@ -21,13 +21,14 @@ enable = [
   "gocritic",
   "goimports",
   "golint",
-  # "goprintffuncname",
+  "goprintffuncname",
   "gosec",
   "gosimple",
   "govet",
   "ineffassign",
   # "interfacer",
   "misspell",
+  "nakedret",
   "rowserrcheck",
   "exportloopref",
   "staticcheck",