[MM-57743] Enable errcheck linter (#26723)

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"

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 != "" {

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

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

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

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,59 +91,59 @@ 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")
 
 	// validate AvailableLocales
 	*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")

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

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 {

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