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

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())
 					}

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},

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)

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() {

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 {

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
 			}
 		}
 

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

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

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,
 	}

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 {

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")

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)

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() {