From 531897b1f0d8176c1f983f921f1d1de618db0131 Mon Sep 17 00:00:00 2001 From: Daniel Schalla Date: Mon, 3 Sep 2018 14:08:40 +0200 Subject: 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 | 9 +++------ app/auto_responder_test.go | 4 ++-- app/brand.go | 2 +- app/channel.go | 4 ++-- app/config_test.go | 2 -- app/diagnostics_test.go | 10 ++-------- app/file.go | 9 ++++----- app/import_functions.go | 4 ++-- app/notification_email.go | 2 +- app/permissions.go | 6 +----- app/plugin_hooks_test.go | 10 +++++----- app/session_test.go | 2 -- app/team.go | 4 ++-- 13 files changed, 25 insertions(+), 43 deletions(-) (limited to 'app') 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() { -- cgit v1.2.3-1-g7c22