From 32d93fd4695aac4906b5f5bcc9e747736acf29a6 Mon Sep 17 00:00:00 2001 From: Ben Schumacher Date: Mon, 29 Apr 2024 11:23:01 +0200 Subject: [PATCH] [MM-57743] Enable errcheck linter (#26723) --- server/.golangci.yml | 9 ++++--- server/channels/utils/i18n.go | 4 ++-- server/cmd/mmctl/commands/root_test.go | 3 ++- server/config/file_test.go | 12 ++++++---- server/config/utils.go | 9 +++---- server/config/utils_test.go | 24 ++++++++++--------- server/i18n/en.json | 12 ++++++---- .../public/plugin/interface_generator/main.go | 10 ++++++-- server/public/utils/fileutils_test.go | 8 +++++-- 9 files changed, 56 insertions(+), 35 deletions(-) diff --git a/server/.golangci.yml b/server/.golangci.yml index a281c92cafc..d83128e5f04 100644 --- a/server/.golangci.yml +++ b/server/.golangci.yml @@ -26,11 +26,10 @@ linters: - goimports - makezero - whitespace - # TODO: enable this later - # - errcheck + - errcheck issues: - skip-dirs: + exclude-dirs: - channels/store/storetest/mocks exclude-rules: - linters: @@ -57,3 +56,7 @@ issues: - linters: - staticcheck text: SA1019 + + - linters: + - errcheck + path: "channels|platform|cmd/mattermost|public/model|public/plugin|public/shared/mlog|enterprise/data_retention|enterprise/saml|enterprise/cloud|enterprise/ldap" diff --git a/server/channels/utils/i18n.go b/server/channels/utils/i18n.go index b40eb8babbc..2162ae4ae4f 100644 --- a/server/channels/utils/i18n.go +++ b/server/channels/utils/i18n.go @@ -12,8 +12,8 @@ import ( "github.com/mattermost/mattermost/server/v8/channels/utils/fileutils" ) -// this functions loads translations from filesystem if they are not -// loaded already and assigns english while loading server config +// TranslationsPreInit loads translations from filesystem if they are not +// loaded already and assigns english while loading server config. func TranslationsPreInit() error { translationsDir := "i18n" if mattermostPath := os.Getenv("MM_SERVER_PATH"); mattermostPath != "" { diff --git a/server/cmd/mmctl/commands/root_test.go b/server/cmd/mmctl/commands/root_test.go index b3bf87423d5..e4040df95c8 100644 --- a/server/cmd/mmctl/commands/root_test.go +++ b/server/cmd/mmctl/commands/root_test.go @@ -37,7 +37,8 @@ func (s *MmctlUnitTestSuite) TestArgumentsHaveWhitespaceTrimmed() { mockCommand := &cobra.Command{Use: "test", Run: commandFunction} commandString := strings.Join([]string{"test", " ", arguments[0], lineEnding, " ", arguments[1], lineEnding}, "") RootCmd.AddCommand(mockCommand) - executeRawCommand(RootCmd, commandString) + _, _, err := executeRawCommand(RootCmd, commandString) + s.Require().NoError(err) s.Require().True(commandCalled, "Expected mock command to be called") }) } diff --git a/server/config/file_test.go b/server/config/file_test.go index 4f6d55c8809..00b7b1225bb 100644 --- a/server/config/file_test.go +++ b/server/config/file_test.go @@ -37,7 +37,8 @@ func setupConfigFile(t *testing.T, cfg *model.Config) (string, func()) { cfgData, err := marshalConfig(cfg) require.NoError(t, err) - os.WriteFile(f.Name(), cfgData, 0644) + err = os.WriteFile(f.Name(), cfgData, 0644) + require.NoError(t, err) name = f.Name() } @@ -94,7 +95,8 @@ func assertFileNotEqualsConfig(t *testing.T, expectedCfg *model.Config, path str } func TestFileStoreNew(t *testing.T) { - utils.TranslationsPreInit() + err := utils.TranslationsPreInit() + require.NoError(t, err) t.Run("absolute path, initialization required", func(t *testing.T) { path, tearDown := setupConfigFile(t, testConfig) @@ -221,7 +223,8 @@ func TestFileStoreNew(t *testing.T) { cfgData, err := marshalConfig(testConfig) require.NoError(t, err) - os.WriteFile(path, cfgData, 0644) + err = os.WriteFile(path, cfgData, 0644) + require.NoError(t, err) fs, err := NewFileStore(path, false) require.NoError(t, err) @@ -811,7 +814,8 @@ func TestFileStoreLoad(t *testing.T) { cfgData, err := marshalConfig(invalidConfig) require.NoError(t, err) - os.WriteFile(path, cfgData, 0644) + err = os.WriteFile(path, cfgData, 0644) + require.NoError(t, err) err = fs.Load() if assert.Error(t, err) { diff --git a/server/config/utils.go b/server/config/utils.go index 7e4e61e88fd..930681ed09d 100644 --- a/server/config/utils.go +++ b/server/config/utils.go @@ -104,14 +104,11 @@ func fixConfig(cfg *model.Config) { } } - FixInvalidLocales(cfg) + fixInvalidLocales(cfg) } -// FixInvalidLocales checks and corrects the given config for invalid locale-related settings. -// -// Ideally, this function would be completely internal, but it's currently exposed to allow the cli -// to test the config change before allowing the save. -func FixInvalidLocales(cfg *model.Config) bool { +// fixInvalidLocales checks and corrects the given config for invalid locale-related settings. +func fixInvalidLocales(cfg *model.Config) bool { var changed bool locales := i18n.GetSupportedLocales() diff --git a/server/config/utils_test.go b/server/config/utils_test.go index 65e979df443..18dce8561f5 100644 --- a/server/config/utils_test.go +++ b/server/config/utils_test.go @@ -80,7 +80,9 @@ func TestDesanitize(t *testing.T) { } func TestFixInvalidLocales(t *testing.T) { - utils.TranslationsPreInit() + // utils.TranslationsPreInit errors when TestFixInvalidLocales is run as part of testing the package, + // but doesn't error when the test is run individually. + _ = utils.TranslationsPreInit() cfg := &model.Config{} cfg.SetDefaults() @@ -89,39 +91,39 @@ func TestFixInvalidLocales(t *testing.T) { *cfg.LocalizationSettings.DefaultClientLocale = "en" *cfg.LocalizationSettings.AvailableLocales = "" - changed := FixInvalidLocales(cfg) + changed := fixInvalidLocales(cfg) assert.False(t, changed) *cfg.LocalizationSettings.DefaultServerLocale = "junk" - changed = FixInvalidLocales(cfg) + changed = fixInvalidLocales(cfg) assert.True(t, changed) assert.Equal(t, "en", *cfg.LocalizationSettings.DefaultServerLocale) *cfg.LocalizationSettings.DefaultServerLocale = "" - changed = FixInvalidLocales(cfg) + changed = fixInvalidLocales(cfg) assert.True(t, changed) assert.Equal(t, "en", *cfg.LocalizationSettings.DefaultServerLocale) *cfg.LocalizationSettings.AvailableLocales = "en" *cfg.LocalizationSettings.DefaultServerLocale = "de" - changed = FixInvalidLocales(cfg) + changed = fixInvalidLocales(cfg) assert.False(t, changed) assert.NotContains(t, *cfg.LocalizationSettings.AvailableLocales, *cfg.LocalizationSettings.DefaultServerLocale, "DefaultServerLocale should not be added to AvailableLocales") *cfg.LocalizationSettings.AvailableLocales = "" *cfg.LocalizationSettings.DefaultClientLocale = "junk" - changed = FixInvalidLocales(cfg) + changed = fixInvalidLocales(cfg) assert.True(t, changed) assert.Equal(t, "en", *cfg.LocalizationSettings.DefaultClientLocale) *cfg.LocalizationSettings.DefaultClientLocale = "" - changed = FixInvalidLocales(cfg) + changed = fixInvalidLocales(cfg) assert.True(t, changed) assert.Equal(t, "en", *cfg.LocalizationSettings.DefaultClientLocale) *cfg.LocalizationSettings.AvailableLocales = "en" *cfg.LocalizationSettings.DefaultClientLocale = "de" - changed = FixInvalidLocales(cfg) + changed = fixInvalidLocales(cfg) assert.True(t, changed) assert.Contains(t, *cfg.LocalizationSettings.AvailableLocales, *cfg.LocalizationSettings.DefaultServerLocale, "DefaultClientLocale should have been added to AvailableLocales") @@ -129,19 +131,19 @@ func TestFixInvalidLocales(t *testing.T) { *cfg.LocalizationSettings.DefaultServerLocale = "en" *cfg.LocalizationSettings.DefaultClientLocale = "en" *cfg.LocalizationSettings.AvailableLocales = "junk" - changed = FixInvalidLocales(cfg) + changed = fixInvalidLocales(cfg) assert.True(t, changed) assert.Equal(t, "", *cfg.LocalizationSettings.AvailableLocales) *cfg.LocalizationSettings.AvailableLocales = "en,de,junk" - changed = FixInvalidLocales(cfg) + changed = fixInvalidLocales(cfg) assert.True(t, changed) assert.Equal(t, "", *cfg.LocalizationSettings.AvailableLocales) *cfg.LocalizationSettings.DefaultServerLocale = "fr" *cfg.LocalizationSettings.DefaultClientLocale = "de" *cfg.LocalizationSettings.AvailableLocales = "en" - changed = FixInvalidLocales(cfg) + changed = fixInvalidLocales(cfg) assert.True(t, changed) assert.NotContains(t, *cfg.LocalizationSettings.AvailableLocales, *cfg.LocalizationSettings.DefaultServerLocale, "DefaultServerLocale should not be added to AvailableLocales") assert.Contains(t, *cfg.LocalizationSettings.AvailableLocales, *cfg.LocalizationSettings.DefaultClientLocale, "DefaultClientLocale should have been added to AvailableLocales") diff --git a/server/i18n/en.json b/server/i18n/en.json index 7567c47c248..95ca3acf638 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -7322,10 +7322,6 @@ "id": "ent.api.post.send_notifications_and_forget.push_image_only", "translation": " attached a file." }, - { - "id": "ent.cluster.404.app_error", - "translation": "Cluster API endpoint not found." - }, { "id": "ent.cluster.config_changed.info", "translation": "Cluster configuration has changed for id={{ .id }}. The cluster may become unstable and a restart is required. To ensure the cluster is configured correctly you should perform a rolling restart immediately." @@ -7362,6 +7358,10 @@ "id": "ent.compliance.csv.header.export.appError", "translation": "Unable to add header to the CSV export." }, + { + "id": "ent.compliance.csv.metadata.close.appError", + "translation": "Unable to close the zip file" + }, { "id": "ent.compliance.csv.metadata.export.appError", "translation": "Unable to add metadata file to the zip file." @@ -7378,6 +7378,10 @@ "id": "ent.compliance.csv.post.export.appError", "translation": "Unable to export a post." }, + { + "id": "ent.compliance.csv.seek.appError", + "translation": "Unable to seek to start of export file." + }, { "id": "ent.compliance.csv.warning.appError", "translation": "Unable to create the warning file." diff --git a/server/public/plugin/interface_generator/main.go b/server/public/plugin/interface_generator/main.go index 26050948421..2a05d7879b5 100644 --- a/server/public/plugin/interface_generator/main.go +++ b/server/public/plugin/interface_generator/main.go @@ -511,7 +511,10 @@ func generateHooksGlue(info *PluginInterfaceInfo) { }) } templateResult := &bytes.Buffer{} - hooksTemplate.Execute(templateResult, &templateParams) + err = hooksTemplate.Execute(templateResult, &templateParams) + if err != nil { + panic(err) + } formatted, err := imports.Process("", templateResult.Bytes(), nil) if err != nil { @@ -565,7 +568,10 @@ func generatePluginTimerLayer(info *PluginInterfaceInfo) { } templateResult := &bytes.Buffer{} - parsedTemplate.Execute(templateResult, &templateParams) + err = parsedTemplate.Execute(templateResult, &templateParams) + if err != nil { + panic(err) + } formatted, err := imports.Process("", templateResult.Bytes(), nil) if err != nil { diff --git a/server/public/utils/fileutils_test.go b/server/public/utils/fileutils_test.go index d4781effc45..688d5ad9ae6 100644 --- a/server/public/utils/fileutils_test.go +++ b/server/public/utils/fileutils_test.go @@ -112,8 +112,12 @@ func TestFindFile(t *testing.T) { if testCase.Cwd != nil { prevDir, err := os.Getwd() require.NoError(t, err) - defer os.Chdir(prevDir) - os.Chdir(*testCase.Cwd) + t.Cleanup(func() { + err = os.Chdir(prevDir) + assert.NoError(t, err) + }) + err = os.Chdir(*testCase.Cwd) + assert.NoError(t, err) } assert.Equal(t, testCase.Expected, FindFile(testCase.FileName))