Merge pull request #11705 from mjtrangoni/fix-ineffassign-issues

Fix ineffassign issues

.circleci/config.yml

@@ -27,12 +27,13 @@ jobs:
     steps:
       - checkout
       - run: 'go get -u gopkg.in/alecthomas/gometalinter.v2'
+      - run: 'go get -u github.com/gordonklaus/ineffassign'
       - run: 'go get -u github.com/opennota/check/cmd/structcheck'
       - run: 'go get -u github.com/mdempsky/unconvert'
       - run: 'go get -u github.com/opennota/check/cmd/varcheck'
       - run:
           name: run linters
-          command: 'gometalinter.v2 --enable-gc --vendor --deadline 10m --disable-all --enable=structcheck --enable=unconvert --enable=varcheck ./...'
+          command: 'gometalinter.v2 --enable-gc --vendor --deadline 10m --disable-all --enable=ineffassign --enable=structcheck --enable=unconvert --enable=varcheck ./...'
 
   test-frontend:
     docker:

pkg/cmd/grafana-server/main.go

@@ -100,9 +100,9 @@ func main() {
 }
 
 func listenToSystemSignals(server *GrafanaServerImpl, shutdownCompleted chan int) {
+	var code int
 	signalChan := make(chan os.Signal, 1)
 	ignoreChan := make(chan os.Signal, 1)
-	code := 0
 
 	signal.Notify(ignoreChan, syscall.SIGHUP)
 	signal.Notify(signalChan, os.Interrupt, os.Kill, syscall.SIGTERM)

pkg/components/dashdiffs/compare.go

@@ -141,5 +141,9 @@ func getDiff(baseData, newData *simplejson.Json) (interface{}, diff.Diff, error)
 
 	left := make(map[string]interface{})
 	err = json.Unmarshal(leftBytes, &left)
+	if err != nil {
+		return nil, nil, err
+	}
+
 	return left, jsonDiff, nil
 }

pkg/components/dynmap/dynmap_test.go

@@ -60,6 +60,7 @@ func TestFirst(t *testing.T) {
   }`
 
 	j, err := NewObjectFromBytes([]byte(testJSON))
+	assert.True(err == nil, "failed to create new object from bytes")
 
 	a, err := j.GetObject("address")
 	assert.True(a != nil && err == nil, "failed to create json from string")
@@ -108,6 +109,7 @@ func TestFirst(t *testing.T) {
 	//log.Println("address: ", address)
 
 	s, err = address.GetString("street")
+	assert.True(s == "Street 42" && err == nil, "street mismatching")
 
 	addressAsString, err := j.GetString("address")
 	assert.True(addressAsString == "" && err != nil, "address should not be an string")
@@ -148,6 +150,7 @@ func TestFirst(t *testing.T) {
 		//assert.True(element.IsObject() == true, "first fail")
 
 		element, err := elementValue.Object()
+		assert.True(err == nil, "create element fail")
 
 		s, err = element.GetString("street")
 		assert.True(s == "Street 42" && err == nil, "second fail")
@@ -232,6 +235,7 @@ func TestSecond(t *testing.T) {
 			assert.True(fromName == "Tom Brady" && err == nil, "fromName mismatch")
 
 			actions, err := dataItem.GetObjectArray("actions")
+			assert.True(err == nil, "get object from array failed")
 
 			for index, action := range actions {
 

pkg/components/imguploader/azureblobuploader_test.go

@@ -13,6 +13,7 @@ func TestUploadToAzureBlob(t *testing.T) {
 		err := setting.NewConfigContext(&setting.CommandLineArgs{
 			HomePath: "../../../",
 		})
+		So(err, ShouldBeNil)
 
 		uploader, _ := NewImageUploader()
 

pkg/components/imguploader/imguploader_test.go

@@ -19,6 +19,7 @@ func TestImageUploaderFactory(t *testing.T) {
 
 			Convey("with bucket url https://foo.bar.baz.s3-us-east-2.amazonaws.com", func() {
 				s3sec, err := setting.Cfg.GetSection("external_image_storage.s3")
+				So(err, ShouldBeNil)
 				s3sec.NewKey("bucket_url", "https://foo.bar.baz.s3-us-east-2.amazonaws.com")
 				s3sec.NewKey("access_key", "access_key")
 				s3sec.NewKey("secret_key", "secret_key")
@@ -37,6 +38,7 @@ func TestImageUploaderFactory(t *testing.T) {
 
 			Convey("with bucket url https://s3.amazonaws.com/mybucket", func() {
 				s3sec, err := setting.Cfg.GetSection("external_image_storage.s3")
+				So(err, ShouldBeNil)
 				s3sec.NewKey("bucket_url", "https://s3.amazonaws.com/my.bucket.com")
 				s3sec.NewKey("access_key", "access_key")
 				s3sec.NewKey("secret_key", "secret_key")
@@ -55,15 +57,15 @@ func TestImageUploaderFactory(t *testing.T) {
 
 			Convey("with bucket url https://s3-us-west-2.amazonaws.com/mybucket", func() {
 				s3sec, err := setting.Cfg.GetSection("external_image_storage.s3")
+				So(err, ShouldBeNil)
 				s3sec.NewKey("bucket_url", "https://s3-us-west-2.amazonaws.com/my.bucket.com")
 				s3sec.NewKey("access_key", "access_key")
 				s3sec.NewKey("secret_key", "secret_key")
 
 				uploader, err := NewImageUploader()
-
 				So(err, ShouldBeNil)
-				original, ok := uploader.(*S3Uploader)
 
+				original, ok := uploader.(*S3Uploader)
 				So(ok, ShouldBeTrue)
 				So(original.region, ShouldEqual, "us-west-2")
 				So(original.bucket, ShouldEqual, "my.bucket.com")
@@ -82,6 +84,7 @@ func TestImageUploaderFactory(t *testing.T) {
 			setting.ImageUploadProvider = "webdav"
 
 			webdavSec, err := setting.Cfg.GetSection("external_image_storage.webdav")
+			So(err, ShouldBeNil)
 			webdavSec.NewKey("url", "webdavUrl")
 			webdavSec.NewKey("username", "username")
 			webdavSec.NewKey("password", "password")
@@ -107,14 +110,14 @@ func TestImageUploaderFactory(t *testing.T) {
 			setting.ImageUploadProvider = "gcs"
 
 			gcpSec, err := setting.Cfg.GetSection("external_image_storage.gcs")
+			So(err, ShouldBeNil)
 			gcpSec.NewKey("key_file", "/etc/secrets/project-79a52befa3f6.json")
 			gcpSec.NewKey("bucket", "project-grafana-east")
 
 			uploader, err := NewImageUploader()
-
 			So(err, ShouldBeNil)
-			original, ok := uploader.(*GCSUploader)
 
+			original, ok := uploader.(*GCSUploader)
 			So(ok, ShouldBeTrue)
 			So(original.keyFile, ShouldEqual, "/etc/secrets/project-79a52befa3f6.json")
 			So(original.bucket, ShouldEqual, "project-grafana-east")
@@ -128,15 +131,15 @@ func TestImageUploaderFactory(t *testing.T) {
 
 			Convey("with container name", func() {
 				azureBlobSec, err := setting.Cfg.GetSection("external_image_storage.azure_blob")
+				So(err, ShouldBeNil)
 				azureBlobSec.NewKey("account_name", "account_name")
 				azureBlobSec.NewKey("account_key", "account_key")
 				azureBlobSec.NewKey("container_name", "container_name")
 
 				uploader, err := NewImageUploader()
-
 				So(err, ShouldBeNil)
-				original, ok := uploader.(*AzureBlobUploader)
 
+				original, ok := uploader.(*AzureBlobUploader)
 				So(ok, ShouldBeTrue)
 				So(original.account_name, ShouldEqual, "account_name")
 				So(original.account_key, ShouldEqual, "account_key")

pkg/components/imguploader/webdavuploader.go

@@ -41,14 +41,20 @@ func (u *WebdavUploader) Upload(ctx context.Context, pa string) (string, error)
 	url.Path = path.Join(url.Path, filename)
 
 	imgData, err := ioutil.ReadFile(pa)
+	if err != nil {
+		return "", err
+	}
+
 	req, err := http.NewRequest("PUT", url.String(), bytes.NewReader(imgData))
+	if err != nil {
+		return "", err
+	}
 
 	if u.username != "" {
 		req.SetBasicAuth(u.username, u.password)
 	}
 
 	res, err := netClient.Do(req)
-
 	if err != nil {
 		return "", err
 	}

pkg/log/file_test.go

@@ -32,7 +32,9 @@ func TestLogFile(t *testing.T) {
 
 		Convey("Logging should add lines", func() {
 			err := fileLogWrite.WriteLine("test1\n")
+			So(err, ShouldBeNil)
 			err = fileLogWrite.WriteLine("test2\n")
+			So(err, ShouldBeNil)
 			err = fileLogWrite.WriteLine("test3\n")
 			So(err, ShouldBeNil)
 			So(fileLogWrite.maxlines_curlines, ShouldEqual, 3)

pkg/services/alerting/notifiers/telegram.go

@@ -3,13 +3,14 @@ package notifiers
 import (
 	"bytes"
 	"fmt"
+	"io"
+	"mime/multipart"
+	"os"
+
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/log"
 	m "github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/alerting"
-	"io"
-	"mime/multipart"
-	"os"
 )
 
 const (
@@ -133,6 +134,9 @@ func (this *TelegramNotifier) buildMessageInlineImage(evalContext *alerting.Eval
 	}
 
 	ruleUrl, err := evalContext.GetRuleUrl()
+	if err != nil {
+		return nil, err
+	}
 
 	metrics := generateMetricsMessage(evalContext)
 	message := generateImageCaption(evalContext, ruleUrl, metrics)

pkg/services/alerting/notifiers/victorops.go

@@ -83,27 +83,6 @@ func (this *VictoropsNotifier) Notify(evalContext *alerting.EvalContext) error {
 		return nil
 	}
 
-	fields := make([]map[string]interface{}, 0)
-	fieldLimitCount := 4
-	for index, evt := range evalContext.EvalMatches {
-		fields = append(fields, map[string]interface{}{
-			"title": evt.Metric,
-			"value": evt.Value,
-			"short": true,
-		})
-		if index > fieldLimitCount {
-			break
-		}
-	}
-
-	if evalContext.Error != nil {
-		fields = append(fields, map[string]interface{}{
-			"title": "Error message",
-			"value": evalContext.Error.Error(),
-			"short": false,
-		})
-	}
-
 	messageType := evalContext.Rule.State
 	if evalContext.Rule.State == models.AlertStateAlerting { // translate 'Alerting' to 'CRITICAL' (Victorops analog)
 		messageType = AlertStateCritical

pkg/services/guardian/guardian_test.go

@@ -649,6 +649,9 @@ func (sc *scenarioContext) verifyUpdateChildDashboardPermissionsWithOverrideShou
 			}
 
 			_, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, permissionList)
+			if err != nil {
+				sc.reportFailure(tc, nil, err)
+			}
 			sc.updatePermissions = permissionList
 			ok, err := sc.g.CheckPermissionBeforeUpdate(m.PERMISSION_ADMIN, permissionList)
 

pkg/services/provisioning/dashboards/file_reader.go

@@ -235,7 +235,6 @@ func getOrCreateFolderId(cfg *DashboardsAsConfig, service dashboards.DashboardPr
 func resolveSymlink(fileinfo os.FileInfo, path string) (os.FileInfo, error) {
 	checkFilepath, err := filepath.EvalSymlinks(path)
 	if path != checkFilepath {
-		path = checkFilepath
 		fi, err := os.Lstat(checkFilepath)
 		if err != nil {
 			return nil, err

pkg/services/sqlstore/annotation_test.go

@@ -256,6 +256,7 @@ func TestAnnotations(t *testing.T) {
 				annotationId := items[0].Id
 
 				err = repo.Delete(&annotations.DeleteParams{Id: annotationId})
+				So(err, ShouldBeNil)
 
 				items, err = repo.Find(query)
 				So(err, ShouldBeNil)

pkg/services/sqlstore/dashboard.go

@@ -77,7 +77,7 @@ func saveDashboard(sess *DBSession, cmd *m.SaveDashboardCommand) error {
 	}
 
 	parentVersion := dash.Version
-	affectedRows := int64(0)
+	var affectedRows int64
 	var err error
 
 	if dash.Id == 0 {

pkg/services/sqlstore/dashboard_acl_test.go

@@ -154,6 +154,7 @@ func TestDashboardAclDataAccess(t *testing.T) {
 					DashboardId: savedFolder.Id,
 					Permission:  m.PERMISSION_EDIT,
 				})
+				So(err, ShouldBeNil)
 
 				q1 := &m.GetDashboardAclInfoListQuery{DashboardId: savedFolder.Id, OrgId: 1}
 				err = GetDashboardAclInfoList(q1)

pkg/services/sqlstore/migrations/migrations_test.go

@@ -27,7 +27,7 @@ func TestMigrations(t *testing.T) {
 
 			sqlutil.CleanDB(x)
 
-			has, err := x.SQL(sql).Get(&r)
+			_, err = x.SQL(sql).Get(&r)
 			So(err, ShouldNotBeNil)
 
 			mg := NewMigrator(x)
@@ -36,7 +36,7 @@ func TestMigrations(t *testing.T) {
 			err = mg.Start()
 			So(err, ShouldBeNil)
 
-			has, err = x.SQL(sql).Get(&r)
+			has, err := x.SQL(sql).Get(&r)
 			So(err, ShouldBeNil)
 			So(has, ShouldBeTrue)
 			expectedMigrations := mg.MigrationsCount() - 2 //we currently skip to migrations. We should rewrite skipped migrations to write in the log as well. until then we have to keep this

pkg/services/sqlstore/playlist.go

@@ -22,6 +22,9 @@ func CreatePlaylist(cmd *m.CreatePlaylistCommand) error {
 	}
 
 	_, err := x.Insert(&playlist)
+	if err != nil {
+		return err
+	}
 
 	playlistItems := make([]m.PlaylistItem, 0)
 	for _, item := range cmd.Items {

pkg/services/sqlstore/plugin_setting.go

@@ -48,6 +48,9 @@ func UpdatePluginSetting(cmd *m.UpdatePluginSettingCmd) error {
 		var pluginSetting m.PluginSetting
 
 		exists, err := sess.Where("org_id=? and plugin_id=?", cmd.OrgId, cmd.PluginId).Get(&pluginSetting)
+		if err != nil {
+			return err
+		}
 		sess.UseBool("enabled")
 		sess.UseBool("pinned")
 		if !exists {

pkg/services/sqlstore/preferences.go

@@ -72,6 +72,9 @@ func SavePreferences(cmd *m.SavePreferencesCommand) error {
 
 		var prefs m.Preferences
 		exists, err := sess.Where("org_id=? AND user_id=?", cmd.OrgId, cmd.UserId).Get(&prefs)
+		if err != nil {
+			return err
+		}
 
 		if !exists {
 			prefs = m.Preferences{

pkg/services/sqlstore/team_test.go

@@ -74,6 +74,7 @@ func TestTeamCommandsAndQueries(t *testing.T) {
 			Convey("Should be able to return all teams a user is member of", func() {
 				groupId := group2.Result.Id
 				err := AddTeamMember(&m.AddTeamMemberCommand{OrgId: testOrgId, TeamId: groupId, UserId: userIds[0]})
+				So(err, ShouldBeNil)
 
 				query := &m.GetTeamsByUserQuery{OrgId: testOrgId, UserId: userIds[0]}
 				err = GetTeamsByUser(query)
@@ -103,7 +104,7 @@ func TestTeamCommandsAndQueries(t *testing.T) {
 				err = AddTeamMember(&m.AddTeamMemberCommand{OrgId: testOrgId, TeamId: groupId, UserId: userIds[2]})
 				So(err, ShouldBeNil)
 				err = testHelperUpdateDashboardAcl(1, m.DashboardAcl{DashboardId: 1, OrgId: testOrgId, Permission: m.PERMISSION_EDIT, TeamId: groupId})
-
+				So(err, ShouldBeNil)
 				err = DeleteTeam(&m.DeleteTeamCommand{OrgId: testOrgId, Id: groupId})
 				So(err, ShouldBeNil)
 

pkg/services/sqlstore/user.go

@@ -168,11 +168,9 @@ func GetUserByLogin(query *m.GetUserByLoginQuery) error {
 		return m.ErrUserNotFound
 	}
 
-	user := new(m.User)
-
 	// Try and find the user by login first.
 	// It's not sufficient to assume that a LoginOrEmail with an "@" is an email.
-	user = &m.User{Login: query.LoginOrEmail}
+	user := &m.User{Login: query.LoginOrEmail}
 	has, err := x.Get(user)
 
 	if err != nil {
@@ -202,9 +200,7 @@ func GetUserByEmail(query *m.GetUserByEmailQuery) error {
 		return m.ErrUserNotFound
 	}
 
-	user := new(m.User)
-
-	user = &m.User{Email: query.Email}
+	user := &m.User{Email: query.Email}
 	has, err := x.Get(user)
 
 	if err != nil {