From 62fa7b93503495e9e8b109587a46b8ccf6fe752e Mon Sep 17 00:00:00 2001 From: Mahmudul Haque Date: Tue, 16 Mar 2021 13:18:20 +0600 Subject: [PATCH] unparam lint (#16927) * fix "always receives ..." lint err * add unparam lint check * fix failed test * rm details param * ignore unparam lint * magic string replaced with model.NewRandomString * rm unused enableComplianceFeatures param * generate random message inside createPost Co-authored-by: Mattermod --- .golangci.yml | 1 + app/file.go | 16 ++++++---------- app/helper_test.go | 13 +++++-------- app/server_test.go | 18 +++++++++--------- app/svg_test.go | 3 ++- .../searchengine/bleveengine/bleve_test.go | 16 ++++++++-------- services/searchengine/bleveengine/testlib.go | 4 ++-- shared/mail/mail.go | 4 ++-- shared/mail/mail_test.go | 2 +- store/searchtest/channel_layer.go | 2 +- store/searchtest/file_info_layer.go | 4 ++-- web/oauth_test.go | 18 +++++++----------- 12 files changed, 46 insertions(+), 55 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 0d3025dcf1..e7b9bed24d 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -28,6 +28,7 @@ linters: - varcheck - misspell - goimports + - unparam # TODO: enable this later # - errcheck diff --git a/app/file.go b/app/file.go index 7075ba3300..bf983c1d02 100644 --- a/app/file.go +++ b/app/file.go @@ -710,12 +710,10 @@ func (a *App) UploadFileX(channelID, name string, input io.Reader, } if *a.Config().FileSettings.DriverName == "" { - return nil, t.newAppError("api.file.upload_file.storage.app_error", - "", http.StatusNotImplemented) + return nil, t.newAppError("api.file.upload_file.storage.app_error", http.StatusNotImplemented) } if t.ContentLength > t.maxFileSize { - return nil, t.newAppError("api.file.upload_file.too_large_detailed.app_error", - "", http.StatusRequestEntityTooLarge, "Length", t.ContentLength, "Limit", t.maxFileSize) + return nil, t.newAppError("api.file.upload_file.too_large_detailed.app_error", http.StatusRequestEntityTooLarge, "Length", t.ContentLength, "Limit", t.maxFileSize) } t.init(a) @@ -737,8 +735,7 @@ func (a *App) UploadFileX(channelID, name string, input io.Reader, if fileErr := a.RemoveFile(t.fileinfo.Path); fileErr != nil { mlog.Error("Failed to remove file", mlog.Err(fileErr)) } - return nil, t.newAppError("api.file.upload_file.too_large_detailed.app_error", - "", http.StatusRequestEntityTooLarge, "Length", t.ContentLength, "Limit", t.maxFileSize) + return nil, t.newAppError("api.file.upload_file.too_large_detailed.app_error", http.StatusRequestEntityTooLarge, "Length", t.ContentLength, "Limit", t.maxFileSize) } t.fileinfo.Size = written @@ -827,8 +824,7 @@ func (t *UploadFileTask) preprocessImage() *model.AppError { // in 64 bits systems because images can't have more than 32 bits height or // width) if int64(t.fileinfo.Width)*int64(t.fileinfo.Height) > MaxImageSize { - return t.newAppError("api.file.upload_file.large_image_detailed.app_error", - "", http.StatusBadRequest) + return t.newAppError("api.file.upload_file.large_image_detailed.app_error", http.StatusBadRequest) } t.fileinfo.HasPreviewImage = true nameWithoutExtension := t.Name[:strings.LastIndex(t.Name, ".")] @@ -942,7 +938,7 @@ func (t UploadFileTask) pathPrefix() string { "/" + t.fileinfo.Id + "/" } -func (t UploadFileTask) newAppError(id string, details interface{}, httpStatus int, extra ...interface{}) *model.AppError { +func (t UploadFileTask) newAppError(id string, httpStatus int, extra ...interface{}) *model.AppError { params := map[string]interface{}{ "Name": t.Name, "Filename": t.Name, @@ -960,7 +956,7 @@ func (t UploadFileTask) newAppError(id string, details interface{}, httpStatus i params[fmt.Sprintf("%v", extra[i])] = extra[i+1] } - return model.NewAppError("uploadFileTask", id, params, fmt.Sprintf("%v", details), httpStatus) + return model.NewAppError("uploadFileTask", id, params, "", httpStatus) } func (a *App) DoUploadFileExpectModification(now time.Time, rawTeamId string, rawChannelId string, rawUserId string, rawFilename string, data []byte) (*model.FileInfo, []byte, *model.AppError) { diff --git a/app/helper_test.go b/app/helper_test.go index f9a6e10c19..9802cc9d9e 100644 --- a/app/helper_test.go +++ b/app/helper_test.go @@ -42,7 +42,7 @@ type TestHelper struct { tempWorkspace string } -func setupTestHelper(dbStore store.Store, enterprise bool, includeCacheLayer bool, tb testing.TB, configSet func(*model.Config)) *TestHelper { +func setupTestHelper(dbStore store.Store, enterprise bool, includeCacheLayer bool, tb testing.TB) *TestHelper { tempWorkspace, err := ioutil.TempDir("", "apptest") if err != nil { panic(err) @@ -51,9 +51,6 @@ func setupTestHelper(dbStore store.Store, enterprise bool, includeCacheLayer boo configStore := config.NewTestMemoryStore() config := configStore.Get() - if configSet != nil { - configSet(config) - } *config.PluginSettings.Directory = filepath.Join(tempWorkspace, "plugins") *config.PluginSettings.ClientDirectory = filepath.Join(tempWorkspace, "webapp") *config.PluginSettings.AutomaticPrepackagedPlugins = false @@ -142,7 +139,7 @@ func Setup(tb testing.TB) *TestHelper { dbStore.MarkSystemRanUnitTests() mainHelper.PreloadMigrations() - return setupTestHelper(dbStore, false, true, tb, nil) + return setupTestHelper(dbStore, false, true, tb) } func SetupWithoutPreloadMigrations(tb testing.TB) *TestHelper { @@ -153,12 +150,12 @@ func SetupWithoutPreloadMigrations(tb testing.TB) *TestHelper { dbStore.DropAllTables() dbStore.MarkSystemRanUnitTests() - return setupTestHelper(dbStore, false, true, tb, nil) + return setupTestHelper(dbStore, false, true, tb) } func SetupWithStoreMock(tb testing.TB) *TestHelper { mockStore := testlib.GetMockStoreForSetupFunctions() - th := setupTestHelper(mockStore, false, false, tb, nil) + th := setupTestHelper(mockStore, false, false, tb) emptyMockStore := mocks.Store{} emptyMockStore.On("Close").Return(nil) th.App.Srv().Store = &emptyMockStore @@ -167,7 +164,7 @@ func SetupWithStoreMock(tb testing.TB) *TestHelper { func SetupEnterpriseWithStoreMock(tb testing.TB) *TestHelper { mockStore := testlib.GetMockStoreForSetupFunctions() - th := setupTestHelper(mockStore, true, false, tb, nil) + th := setupTestHelper(mockStore, true, false, tb) emptyMockStore := mocks.Store{} emptyMockStore.On("Close").Return(nil) th.App.Srv().Store = &emptyMockStore diff --git a/app/server_test.go b/app/server_test.go index 096b1e489b..6b14749e61 100644 --- a/app/server_test.go +++ b/app/server_test.go @@ -39,7 +39,7 @@ func TestStartServerSuccess(t *testing.T) { serverErr := s.Start() client := &http.Client{} - checkEndpoint(t, client, "http://localhost:"+strconv.Itoa(s.ListenAddr.Port)+"/", http.StatusNotFound) + checkEndpoint(t, client, "http://localhost:"+strconv.Itoa(s.ListenAddr.Port)+"/") s.Shutdown() require.NoError(t, serverErr) @@ -156,7 +156,7 @@ func TestStartServerTLSSuccess(t *testing.T) { } client := &http.Client{Transport: tr} - checkEndpoint(t, client, "https://localhost:"+strconv.Itoa(s.ListenAddr.Port)+"/", http.StatusNotFound) + checkEndpoint(t, client, "https://localhost:"+strconv.Itoa(s.ListenAddr.Port)+"/") s.Shutdown() require.NoError(t, serverErr) @@ -362,7 +362,7 @@ func TestStartServerTLSVersion(t *testing.T) { } client := &http.Client{Transport: tr} - err = checkEndpoint(t, client, "https://localhost:"+strconv.Itoa(s.ListenAddr.Port)+"/", http.StatusNotFound) + err = checkEndpoint(t, client, "https://localhost:"+strconv.Itoa(s.ListenAddr.Port)+"/") if !strings.Contains(err.Error(), "remote error: tls: protocol version not supported") { t.Errorf("Expected protocol version error, got %s", err) @@ -374,7 +374,7 @@ func TestStartServerTLSVersion(t *testing.T) { }, } - err = checkEndpoint(t, client, "https://localhost:"+strconv.Itoa(s.ListenAddr.Port)+"/", http.StatusNotFound) + err = checkEndpoint(t, client, "https://localhost:"+strconv.Itoa(s.ListenAddr.Port)+"/") if err != nil { t.Errorf("Expected nil, got %s", err) @@ -412,7 +412,7 @@ func TestStartServerTLSOverwriteCipher(t *testing.T) { } client := &http.Client{Transport: tr} - err = checkEndpoint(t, client, "https://localhost:"+strconv.Itoa(s.ListenAddr.Port)+"/", http.StatusNotFound) + err = checkEndpoint(t, client, "https://localhost:"+strconv.Itoa(s.ListenAddr.Port)+"/") require.Error(t, err, "Expected error due to Cipher mismatch") if !strings.Contains(err.Error(), "remote error: tls: handshake failure") { t.Errorf("Expected protocol version error, got %s", err) @@ -429,7 +429,7 @@ func TestStartServerTLSOverwriteCipher(t *testing.T) { }, } - err = checkEndpoint(t, client, "https://localhost:"+strconv.Itoa(s.ListenAddr.Port)+"/", http.StatusNotFound) + err = checkEndpoint(t, client, "https://localhost:"+strconv.Itoa(s.ListenAddr.Port)+"/") if err != nil { t.Errorf("Expected nil, got %s", err) @@ -439,7 +439,7 @@ func TestStartServerTLSOverwriteCipher(t *testing.T) { require.NoError(t, serverErr) } -func checkEndpoint(t *testing.T, client *http.Client, url string, expectedStatus int) error { +func checkEndpoint(t *testing.T, client *http.Client, url string) error { res, err := client.Get(url) if err != nil { @@ -448,8 +448,8 @@ func checkEndpoint(t *testing.T, client *http.Client, url string, expectedStatus defer res.Body.Close() - if res.StatusCode != expectedStatus { - t.Errorf("Response code was %d; want %d", res.StatusCode, expectedStatus) + if res.StatusCode != http.StatusNotFound { + t.Errorf("Response code was %d; want %d", res.StatusCode, http.StatusNotFound) } return nil diff --git a/app/svg_test.go b/app/svg_test.go index 923c8b25f9..93380910bf 100644 --- a/app/svg_test.go +++ b/app/svg_test.go @@ -10,6 +10,7 @@ import ( "testing" ) +//nolint:unparam func generateSVGData(width int, height int, useViewBox bool, useDimensions bool, useAlternateFormat bool) io.Reader { var ( viewBoxAttribute = "" @@ -64,7 +65,7 @@ func TestParseInvalidSVGData(t *testing.T) { invalidSVGs := []io.Reader{ generateSVGData(width, height, false, false, false), // missing viewBox, width & height generateSVGData(width, 0, false, true, false), // missing viewBox, malformed width & height - generateSVGData(300, 0, false, true, false), // missing viewBox, malformed height, properly formed width + generateSVGData(width, 0, false, true, false), // missing viewBox, malformed height, properly formed width } for index, svg := range invalidSVGs { _, err := parseSVG(svg) diff --git a/services/searchengine/bleveengine/bleve_test.go b/services/searchengine/bleveengine/bleve_test.go index e86ba2e40d..e8a7aee314 100644 --- a/services/searchengine/bleveengine/bleve_test.go +++ b/services/searchengine/bleveengine/bleve_test.go @@ -113,12 +113,12 @@ func (s *BleveEngineTestSuite) TestDeleteChannelPosts() { channelToAvoidID := model.NewId() posts := make([]*model.Post, 0) for i := 0; i < 10; i++ { - post := createPost(userID, channelID, "test one two three") + post := createPost(userID, channelID) appErr := s.SearchEngine.BleveEngine.IndexPost(post, teamID) require.Nil(s.T(), appErr) posts = append(posts, post) } - postToAvoid := createPost(userID, channelToAvoidID, "test one two three") + postToAvoid := createPost(userID, channelToAvoidID) appErr := s.SearchEngine.BleveEngine.IndexPost(postToAvoid, teamID) require.Nil(s.T(), appErr) @@ -138,7 +138,7 @@ func (s *BleveEngineTestSuite) TestDeleteChannelPosts() { userID := model.NewId() channelID := model.NewId() channelToDeleteID := model.NewId() - post := createPost(userID, channelID, "test one two three") + post := createPost(userID, channelID) appErr := s.SearchEngine.BleveEngine.IndexPost(post, teamID) require.Nil(s.T(), appErr) @@ -161,12 +161,12 @@ func (s *BleveEngineTestSuite) TestDeleteUserPosts() { channelID := model.NewId() posts := make([]*model.Post, 0) for i := 0; i < 10; i++ { - post := createPost(userID, channelID, "test one two three") + post := createPost(userID, channelID) appErr := s.SearchEngine.BleveEngine.IndexPost(post, teamID) require.Nil(s.T(), appErr) posts = append(posts, post) } - postToAvoid := createPost(userToAvoidID, channelID, "test one two three") + postToAvoid := createPost(userToAvoidID, channelID) appErr := s.SearchEngine.BleveEngine.IndexPost(postToAvoid, teamID) require.Nil(s.T(), appErr) @@ -186,7 +186,7 @@ func (s *BleveEngineTestSuite) TestDeleteUserPosts() { userID := model.NewId() userToDeleteID := model.NewId() channelID := model.NewId() - post := createPost(userID, channelID, "test one two three") + post := createPost(userID, channelID) appErr := s.SearchEngine.BleveEngine.IndexPost(post, teamID) require.Nil(s.T(), appErr) @@ -208,12 +208,12 @@ func (s *BleveEngineTestSuite) TestDeletePosts() { channelID := model.NewId() posts := make([]*model.Post, 0) for i := 0; i < 10; i++ { - post := createPost(userID, channelID, "test one two three") + post := createPost(userID, channelID) appErr := s.SearchEngine.BleveEngine.IndexPost(post, teamID) require.Nil(s.T(), appErr) posts = append(posts, post) } - postToAvoid := createPost(userToAvoidID, channelID, "test one two three") + postToAvoid := createPost(userToAvoidID, channelID) appErr := s.SearchEngine.BleveEngine.IndexPost(postToAvoid, teamID) require.Nil(s.T(), appErr) diff --git a/services/searchengine/bleveengine/testlib.go b/services/searchengine/bleveengine/testlib.go index c1cf7bef9a..f4fb96c75c 100644 --- a/services/searchengine/bleveengine/testlib.go +++ b/services/searchengine/bleveengine/testlib.go @@ -9,9 +9,9 @@ import ( "github.com/mattermost/mattermost-server/v5/model" ) -func createPost(userId string, channelId string, message string) *model.Post { +func createPost(userId string, channelId string) *model.Post { post := &model.Post{ - Message: message, + Message: model.NewRandomString(15), ChannelId: channelId, PendingPostId: model.NewId() + ":" + fmt.Sprint(model.GetMillis()), UserId: userId, diff --git a/shared/mail/mail.go b/shared/mail/mail.go index f5f24ee093..e915623f43 100644 --- a/shared/mail/mail.go +++ b/shared/mail/mail.go @@ -249,7 +249,7 @@ func SendMailWithEmbeddedFilesUsingConfig(to, subject, htmlBody string, embedded embeddedFiles: embeddedFiles, } - return sendMailUsingConfigAdvanced(mail, config, enableComplianceFeatures) + return sendMailUsingConfigAdvanced(mail, config) } func SendMailUsingConfig(to, subject, htmlBody string, config *SMTPConfig, enableComplianceFeatures bool, ccMail string) error { @@ -257,7 +257,7 @@ func SendMailUsingConfig(to, subject, htmlBody string, config *SMTPConfig, enabl } // allows for sending an email with differing MIME/SMTP recipients -func sendMailUsingConfigAdvanced(mail mailData, config *SMTPConfig, enableComplianceFeatures bool) error { +func sendMailUsingConfigAdvanced(mail mailData, config *SMTPConfig) error { if config.Server == "" { return nil } diff --git a/shared/mail/mail_test.go b/shared/mail/mail_test.go index 53732ed348..e1e24a6cca 100644 --- a/shared/mail/mail_test.go +++ b/shared/mail/mail_test.go @@ -225,7 +225,7 @@ func TestSendMailUsingConfigAdvanced(t *testing.T) { mimeHeaders: headers, } - err = sendMailUsingConfigAdvanced(mail, cfg, true) + err = sendMailUsingConfigAdvanced(mail, cfg) require.NoError(t, err, "Should connect to the STMP Server: %v", err) //Check if the email was send to the right email address diff --git a/store/searchtest/channel_layer.go b/store/searchtest/channel_layer.go index c28fcd8632..08a1110e21 100644 --- a/store/searchtest/channel_layer.go +++ b/store/searchtest/channel_layer.go @@ -76,7 +76,7 @@ func TestSearchChannelStore(t *testing.T, s store.Store, testEngine *SearchTestE } func testAutocompleteChannelByName(t *testing.T, th *SearchTestHelper) { - alternate, err := th.createChannel(th.Team.Id, "channel-alternate", "Channel Alternate", "", model.CHANNEL_OPEN, false) + alternate, err := th.createChannel(th.Team.Id, "channel-alternate", "Channel Alternate", "Channel Alternate", model.CHANNEL_OPEN, false) require.NoError(t, err) defer th.deleteChannel(alternate) res, err := th.Store.Channel().AutocompleteInTeam(th.Team.Id, "channel-a", false) diff --git a/store/searchtest/file_info_layer.go b/store/searchtest/file_info_layer.go index 45661683b8..0ac41a33f2 100644 --- a/store/searchtest/file_info_layer.go +++ b/store/searchtest/file_info_layer.go @@ -196,7 +196,7 @@ func TestSearchFileInfoStore(t *testing.T, s store.Store, testEngine *SearchTest } func testFileInfoSearchFileInfosIncludingDMs(t *testing.T, th *SearchTestHelper) { - direct, err := th.createDirectChannel(th.Team.Id, "direct", "direct", []*model.User{th.User, th.User2}) + direct, err := th.createDirectChannel(th.Team.Id, "direct-"+th.Team.Id, "direct-"+th.Team.Id, []*model.User{th.User, th.User2}) require.NoError(t, err) defer th.deleteChannel(direct) @@ -207,7 +207,7 @@ func testFileInfoSearchFileInfosIncludingDMs(t *testing.T, th *SearchTestHelper) post2, err := th.createPost(th.User.Id, th.ChannelBasic.Id, "dm test", "", model.POST_DEFAULT, 0, false) require.NoError(t, err) - p1, err := th.createFileInfo(th.User.Id, post.Id, "dm test filename", "dm contenttest filename", "jpg", "image/jpeg", 0, 0) + p1, err := th.createFileInfo(th.User.Id, post.Id, "dm test filename", "dm contenttest filename", "jpg", "image/jpeg", 0, 1) require.NoError(t, err) _, err = th.createFileInfo(th.User.Id, post.Id, "dm other filename", "dm other filename", "jpg", "image/jpeg", 0, 0) require.NoError(t, err) diff --git a/web/oauth_test.go b/web/oauth_test.go index 325c6a0396..b66974a42b 100644 --- a/web/oauth_test.go +++ b/web/oauth_test.go @@ -619,38 +619,34 @@ func GenerateTestAppName() string { return "fakeoauthapp" + model.NewRandomString(10) } -func checkHTTPStatus(t *testing.T, resp *model.Response, expectedStatus int, expectError bool) { +func checkHTTPStatus(t *testing.T, resp *model.Response, expectedStatus int) { t.Helper() - require.NotNil(t, resp, "Unexpected nil response, expected http:%v, expectError:%v)", expectedStatus, expectError) + require.NotNil(t, resp, "Unexpected nil response, expected http:%v, expectError:%v)", expectedStatus, true) - if expectError { - require.NotNil(t, resp.Error, "Expected a non-nil error and http status:%v, got nil, %v", expectedStatus, resp.StatusCode) - } else { - require.Nil(t, resp.Error, "Expected no error and http status:%v, got %q, http:%v", expectedStatus, resp.Error, resp.StatusCode) - } + require.NotNil(t, resp.Error, "Expected a non-nil error and http status:%v, got nil, %v", expectedStatus, resp.StatusCode) require.Equal(t, resp.StatusCode, expectedStatus, "Expected http status:%v, got %v (err: %q)", expectedStatus, resp.StatusCode, resp.Error) } func CheckForbiddenStatus(t *testing.T, resp *model.Response) { t.Helper() - checkHTTPStatus(t, resp, http.StatusForbidden, true) + checkHTTPStatus(t, resp, http.StatusForbidden) } func CheckUnauthorizedStatus(t *testing.T, resp *model.Response) { t.Helper() - checkHTTPStatus(t, resp, http.StatusUnauthorized, true) + checkHTTPStatus(t, resp, http.StatusUnauthorized) } func CheckNotFoundStatus(t *testing.T, resp *model.Response) { t.Helper() - checkHTTPStatus(t, resp, http.StatusNotFound, true) + checkHTTPStatus(t, resp, http.StatusNotFound) } func CheckBadRequestStatus(t *testing.T, resp *model.Response) { t.Helper() - checkHTTPStatus(t, resp, http.StatusBadRequest, true) + checkHTTPStatus(t, resp, http.StatusBadRequest) } func (th *TestHelper) Login(client *model.Client4, user *model.User) {