add megacheck as makefile target (#9288)

Fix code issues in channel_test.go

Fix Channel Test Issues detected by Megacheck

Fix API Emoji Test Issues detected by Megacheck

Fixed API Issues Reported by Megacheck

Fixed App issues reported by megacheck

Remaining fixes

removed test added by mistake from old HEAD

gofmt

Store Fixes

simplified returns

Fix test for multi member channel delete

revert to delete unused function

13 files changed, 25 insertions, 43 deletions

diff --git a/app/app.go b/app/app.go
index b704bb449..2e0433d23 100644
--- a/app/app.go
+++ b/app/app.go
@@ -642,7 +642,6 @@ func (a *App) DoEmojisPermissionsMigration() {
 			mlog.Critical(err.Error())
 			return
 		}
-		break
 	case model.RESTRICT_EMOJI_CREATION_ADMIN:
 		role, err = a.GetRoleByName(model.TEAM_ADMIN_ROLE_ID)
 		if err != nil {
@@ -650,10 +649,8 @@ func (a *App) DoEmojisPermissionsMigration() {
 			mlog.Critical(err.Error())
 			return
 		}
-		break
 	case model.RESTRICT_EMOJI_CREATION_SYSTEM_ADMIN:
 		role = nil
-		break
 	default:
 		mlog.Critical("Failed to migrate emojis creation permissions from mattermost config.")
 		mlog.Critical("Invalid restrict emoji creation setting")
@@ -703,13 +700,13 @@ func (a *App) StartElasticsearch() {
 	})
 
 	a.AddConfigListener(func(oldConfig *model.Config, newConfig *model.Config) {
-		if *oldConfig.ElasticsearchSettings.EnableIndexing == false && *newConfig.ElasticsearchSettings.EnableIndexing == true {
+		if !*oldConfig.ElasticsearchSettings.EnableIndexing && *newConfig.ElasticsearchSettings.EnableIndexing {
 			a.Go(func() {
 				if err := a.Elasticsearch.Start(); err != nil {
 					mlog.Error(err.Error())
 				}
 			})
-		} else if *oldConfig.ElasticsearchSettings.EnableIndexing == true && *newConfig.ElasticsearchSettings.EnableIndexing == false {
+		} else if *oldConfig.ElasticsearchSettings.EnableIndexing && !*newConfig.ElasticsearchSettings.EnableIndexing {
 			a.Go(func() {
 				if err := a.Elasticsearch.Stop(); err != nil {
 					mlog.Error(err.Error())
@@ -717,7 +714,7 @@ func (a *App) StartElasticsearch() {
 			})
 		} else if *oldConfig.ElasticsearchSettings.Password != *newConfig.ElasticsearchSettings.Password || *oldConfig.ElasticsearchSettings.Username != *newConfig.ElasticsearchSettings.Username || *oldConfig.ElasticsearchSettings.ConnectionUrl != *newConfig.ElasticsearchSettings.ConnectionUrl || *oldConfig.ElasticsearchSettings.Sniff != *newConfig.ElasticsearchSettings.Sniff {
 			a.Go(func() {
-				if *oldConfig.ElasticsearchSettings.EnableIndexing == true {
+				if *oldConfig.ElasticsearchSettings.EnableIndexing {
 					if err := a.Elasticsearch.Stop(); err != nil {
 						mlog.Error(err.Error())
 					}
diff --git a/app/auto_responder_test.go b/app/auto_responder_test.go
index f78bbc669..4afa03348 100644
--- a/app/auto_responder_test.go
+++ b/app/auto_responder_test.go
@@ -94,7 +94,7 @@ func TestSendAutoResponseSuccess(t *testing.T) {
 	userUpdated1, err := th.App.PatchUser(user.Id, patch, true)
 	require.Nil(t, err)
 
-	firstPost, err := th.App.CreatePost(&model.Post{
+	firstPost, _ := th.App.CreatePost(&model.Post{
 		ChannelId: th.BasicChannel.Id,
 		Message:   "zz" + model.NewId() + "a",
 		UserId:    th.BasicUser.Id},
@@ -134,7 +134,7 @@ func TestSendAutoResponseFailure(t *testing.T) {
 	userUpdated1, err := th.App.PatchUser(user.Id, patch, true)
 	require.Nil(t, err)
 
-	firstPost, err := th.App.CreatePost(&model.Post{
+	firstPost, _ := th.App.CreatePost(&model.Post{
 		ChannelId: th.BasicChannel.Id,
 		Message:   "zz" + model.NewId() + "a",
 		UserId:    th.BasicUser.Id},
diff --git a/app/brand.go b/app/brand.go
index 4814264dd..f01393125 100644
--- a/app/brand.go
+++ b/app/brand.go
@@ -27,10 +27,10 @@ func (a *App) SaveBrandImage(imageData *multipart.FileHeader) *model.AppError {
 	}
 
 	file, err := imageData.Open()
-	defer file.Close()
 	if err != nil {
 		return model.NewAppError("SaveBrandImage", "brand.save_brand_image.open.app_error", nil, err.Error(), http.StatusBadRequest)
 	}
+	defer file.Close()
 
 	// Decode image config first to check dimensions before loading the whole thing into memory later on
 	config, _, err := image.DecodeConfig(file)
diff --git a/app/channel.go b/app/channel.go
index 535eff724..0ac2894fb 100644
--- a/app/channel.go
+++ b/app/channel.go
@@ -52,7 +52,7 @@ func (a *App) JoinDefaultChannels(teamId string, user *model.User, shouldBeAdmin
 	} else {
 		seenChannels := map[string]bool{}
 		for _, channelName := range a.Config().TeamSettings.ExperimentalDefaultChannels {
-			if seenChannels[channelName] != true {
+			if !seenChannels[channelName] {
 				defaultChannelList = append(defaultChannelList, channelName)
 				seenChannels[channelName] = true
 			}
@@ -1398,7 +1398,7 @@ func (a *App) removeUserFromChannel(userIdToRemove string, removerUserId string,
 
 		var actorUser *model.User
 		if removerUserId != "" {
-			actorUser, err = a.GetUser(removerUserId)
+			actorUser, _ = a.GetUser(removerUserId)
 		}
 
 		a.Go(func() {
diff --git a/app/config_test.go b/app/config_test.go
index eb3fa8a53..abaf00167 100644
--- a/app/config_test.go
+++ b/app/config_test.go
@@ -122,12 +122,10 @@ func TestEnsureInstallationDate(t *testing.T) {
 			sqlStore := th.App.Srv.Store.User().(*sqlstore.SqlUserStore)
 			sqlStore.GetMaster().Exec("DELETE FROM Users")
 
-			var users []*model.User
 			for _, createAt := range tc.UsersCreationDates {
 				user := th.CreateUser()
 				user.CreateAt = createAt
 				sqlStore.GetMaster().Exec("UPDATE Users SET CreateAt = :CreateAt WHERE Id = :UserId", map[string]interface{}{"CreateAt": createAt, "UserId": user.Id})
-				users = append(users, user)
 			}
 
 			if tc.PrevInstallationDate == nil {
diff --git a/app/diagnostics_test.go b/app/diagnostics_test.go
index 1dfcbecd1..8d4e57107 100644
--- a/app/diagnostics_test.go
+++ b/app/diagnostics_test.go
@@ -103,19 +103,13 @@ func TestDiagnostics(t *testing.T) {
 
 		info := ""
 		// Collect the info sent.
+		Loop:
 		for {
-			done := false
 			select {
 			case result := <-data:
 				info += result
 			case <-time.After(time.Second * 1):
-				// Done recieving
-				done = true
-				break
-			}
-
-			if done {
-				break
+				break Loop
 			}
 		}
 
diff --git a/app/file.go b/app/file.go
index d2a145c81..278990b49 100644
--- a/app/file.go
+++ b/app/file.go
@@ -613,19 +613,18 @@ func (a *App) GetFileInfo(fileId string) (*model.FileInfo, *model.AppError) {
 }
 
 func (a *App) CopyFileInfos(userId string, fileIds []string) ([]string, *model.AppError) {
-	newFileIds := []string{}
+	var newFileIds []string
 
 	now := model.GetMillis()
 
 	for _, fileId := range fileIds {
-		fileInfo := &model.FileInfo{}
+		result := <-a.Srv.Store.FileInfo().Get(fileId)
 
-		if result := <-a.Srv.Store.FileInfo().Get(fileId); result.Err != nil {
+		if result.Err != nil {
 			return nil, result.Err
-		} else {
-			fileInfo = result.Data.(*model.FileInfo)
 		}
 
+		fileInfo := result.Data.(*model.FileInfo)
 		fileInfo.Id = model.NewId()
 		fileInfo.CreatorId = userId
 		fileInfo.CreateAt = now
diff --git a/app/import_functions.go b/app/import_functions.go
index 1490dc6fa..dce84feb5 100644
--- a/app/import_functions.go
+++ b/app/import_functions.go
@@ -123,9 +123,9 @@ func (a *App) ImportRole(data *RoleImportData, dryRun bool, isSchemeRole bool) *
 	}
 
 	if len(role.Id) == 0 {
-		role, err = a.CreateRole(role)
+		_, err = a.CreateRole(role)
 	} else {
-		role, err = a.UpdateRole(role)
+		_, err = a.UpdateRole(role)
 	}
 
 	return err
diff --git a/app/notification_email.go b/app/notification_email.go
index cccd02eba..f5b55f9a8 100644
--- a/app/notification_email.go
+++ b/app/notification_email.go
@@ -306,7 +306,7 @@ func getFormattedPostTime(user *model.User, post *model.Post, useMilitaryTime bo
 		Year:     fmt.Sprintf("%d", localTime.Year()),
 		Month:    translateFunc(localTime.Month().String()),
 		Day:      fmt.Sprintf("%d", localTime.Day()),
-		Hour:     fmt.Sprintf("%s", hour),
+		Hour:     hour,
 		Minute:   fmt.Sprintf("%02d"+period, localTime.Minute()),
 		TimeZone: zone,
 	}
diff --git a/app/permissions.go b/app/permissions.go
index 8b4df7c56..ca5c8a165 100644
--- a/app/permissions.go
+++ b/app/permissions.go
@@ -138,11 +138,7 @@ func (a *App) ExportPermissions(w io.Writer) error {
 	schemeExport = append(schemeExport, []byte("\n")...)
 
 	_, err = w.Write(schemeExport)
-	if err != nil {
-		return err
-	}
-
-	return nil
+	return err
 }
 
 func (a *App) ImportPermissions(jsonl io.Reader) error {
diff --git a/app/plugin_hooks_test.go b/app/plugin_hooks_test.go
index f098374ad..0eabbddbc 100644
--- a/app/plugin_hooks_test.go
+++ b/app/plugin_hooks_test.go
@@ -89,7 +89,7 @@ func TestHookMessageWillBePosted(t *testing.T) {
 			Message:   "message_",
 			CreateAt:  model.GetMillis() - 10000,
 		}
-		post, err := th.App.CreatePost(post, th.BasicChannel, false)
+		_, err := th.App.CreatePost(post, th.BasicChannel, false)
 		if assert.NotNil(t, err) {
 			assert.Equal(t, "Post rejected by plugin. rejected", err.Message)
 		}
@@ -129,7 +129,7 @@ func TestHookMessageWillBePosted(t *testing.T) {
 			Message:   "message_",
 			CreateAt:  model.GetMillis() - 10000,
 		}
-		post, err := th.App.CreatePost(post, th.BasicChannel, false)
+		_, err := th.App.CreatePost(post, th.BasicChannel, false)
 		if assert.NotNil(t, err) {
 			assert.Equal(t, "Post rejected by plugin. rejected", err.Message)
 		}
@@ -327,7 +327,7 @@ func TestHookMessageHasBeenPosted(t *testing.T) {
 		Message:   "message",
 		CreateAt:  model.GetMillis() - 10000,
 	}
-	post, err := th.App.CreatePost(post, th.BasicChannel, false)
+	_, err := th.App.CreatePost(post, th.BasicChannel, false)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -424,7 +424,7 @@ func TestHookMessageHasBeenUpdated(t *testing.T) {
 	}
 	assert.Equal(t, "message_", post.Message)
 	post.Message = post.Message + "edited"
-	post, err = th.App.UpdatePost(post, true)
+	_, err = th.App.UpdatePost(post, true)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -785,7 +785,7 @@ func TestUserHasLoggedIn(t *testing.T) {
 
 	time.Sleep(2 * time.Second)
 
-	user, err = th.App.GetUser(th.BasicUser.Id)
+	user, _ = th.App.GetUser(th.BasicUser.Id)
 
 	if user.FirstName != "plugin-callback-success" {
 		t.Errorf("Expected firstname overwrite, got default")
diff --git a/app/session_test.go b/app/session_test.go
index bf8198a4e..8349a4cec 100644
--- a/app/session_test.go
+++ b/app/session_test.go
@@ -54,8 +54,6 @@ func TestGetSessionIdleTimeoutInMinutes(t *testing.T) {
 	require.Nil(t, err)
 	assert.Equal(t, rsession.Id, session.Id)
 
-	rsession, err = th.App.GetSession(session.Token)
-
 	// Test regular session, should timeout
 	time := session.LastActivityAt - (1000 * 60 * 6)
 	<-th.App.Srv.Store.Session().UpdateLastActivityAt(session.Id, time)
diff --git a/app/team.go b/app/team.go
index dd372a99a..e7b25dddf 100644
--- a/app/team.go
+++ b/app/team.go
@@ -468,7 +468,7 @@ func (a *App) JoinUserToTeam(team *model.Team, user *model.User, userRequestorId
 	if a.PluginsReady() {
 		var actor *model.User
 		if userRequestorId != "" {
-			actor, err = a.GetUser(userRequestorId)
+			actor, _ = a.GetUser(userRequestorId)
 		}
 
 		a.Go(func() {
@@ -798,7 +798,7 @@ func (a *App) LeaveTeam(team *model.Team, user *model.User, requestorId string)
 	if a.PluginsReady() {
 		var actor *model.User
 		if requestorId != "" {
-			actor, err = a.GetUser(requestorId)
+			actor, _ = a.GetUser(requestorId)
 		}
 
 		a.Go(func() {