Enable errcheck for golangci-lint (#19976)

* Enable `errcheck` for golangci-lint
* Fix linting errors
* pkg/plugins: Fix tests
* pkg/plugins: Add test

pkg/plugins/datasource_plugin.go

@@ -10,6 +10,7 @@ import (
 	"time"
 
 	"github.com/grafana/grafana/pkg/setting"
+	"github.com/grafana/grafana/pkg/util/errutil"
 
 	datasourceV1 "github.com/grafana/grafana-plugin-model/go/datasource"
 	sdk "github.com/grafana/grafana-plugin-sdk-go"
@@ -46,16 +47,16 @@ type DataSourcePlugin struct {
 }
 
 func (p *DataSourcePlugin) Load(decoder *json.Decoder, pluginDir string) error {
-	if err := decoder.Decode(&p); err != nil {
-		return err
+	if err := decoder.Decode(p); err != nil {
+		return errutil.Wrapf(err, "Failed to decode datasource plugin")
 	}
 
 	if !p.isVersionOne() && !setting.IsExpressionsEnabled() {
-		return errors.New("A plugin version 2 was found but expressions feature toggle is not enabled")
+		return errors.New("A plugin version 2 was found, but expressions feature toggle is not enabled")
 	}
 
 	if err := p.registerPlugin(pluginDir); err != nil {
-		return err
+		return errutil.Wrapf(err, "Failed to register plugin")
 	}
 
 	DataSources[p.Id] = p

pkg/plugins/datasource_plugin_test.go

@@ -5,30 +5,68 @@ import (
 	"encoding/json"
 	"testing"
 
+	"github.com/grafana/grafana/pkg/setting"
+
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestLoadDatasourceVersion(t *testing.T) {
 	t.Run("If plugin version is not set, it should be treated as plugin version one", func(t *testing.T) {
-		pluginJSON, _ := json.Marshal(DataSourcePlugin{})
+		refPlug := DataSourcePlugin{}
+		pluginJSON, err := json.Marshal(refPlug)
+		require.NoError(t, err)
+
 		datasourcePlugin := DataSourcePlugin{}
-		(&datasourcePlugin).Load(json.NewDecoder(bytes.NewReader(pluginJSON)), "/tmp")
+		err = datasourcePlugin.Load(json.NewDecoder(bytes.NewReader(pluginJSON)), "/tmp")
+		require.NoError(t, err)
+		delete(Plugins, refPlug.Id)
+		delete(DataSources, refPlug.Id)
+
 		assert.True(t, datasourcePlugin.isVersionOne())
 	})
 
 	t.Run("If plugin version is set to one, it should be treated as plugin version one", func(t *testing.T) {
-		pluginJSON, _ := json.Marshal(DataSourcePlugin{SDK: false})
+		refPlug := DataSourcePlugin{SDK: false}
+		pluginJSON, err := json.Marshal(refPlug)
+		require.NoError(t, err)
+
 		datasourcePlugin := DataSourcePlugin{}
-		(&datasourcePlugin).Load(json.NewDecoder(bytes.NewReader(pluginJSON)), "/tmp")
+		err = datasourcePlugin.Load(json.NewDecoder(bytes.NewReader(pluginJSON)), "/tmp")
+		require.NoError(t, err)
+		delete(Plugins, refPlug.Id)
+		delete(DataSources, refPlug.Id)
+
 		assert.True(t, datasourcePlugin.isVersionOne())
 		assert.False(t, datasourcePlugin.SDK)
 	})
 
 	t.Run("If plugin version is set to two, it should not be treated as plugin version one", func(t *testing.T) {
-		pluginJSON, _ := json.Marshal(DataSourcePlugin{SDK: true})
+		refPlug := DataSourcePlugin{SDK: true}
+		pluginJSON, err := json.Marshal(refPlug)
+		require.NoError(t, err)
+
+		origToggles := setting.FeatureToggles
+		setting.FeatureToggles = map[string]bool{"expressions": true}
 		datasourcePlugin := DataSourcePlugin{}
-		(&datasourcePlugin).Load(json.NewDecoder(bytes.NewReader(pluginJSON)), "/tmp")
+		err = datasourcePlugin.Load(json.NewDecoder(bytes.NewReader(pluginJSON)), "/tmp")
+		setting.FeatureToggles = origToggles
+		require.NoError(t, err)
+		delete(Plugins, refPlug.Id)
+		delete(DataSources, refPlug.Id)
+
 		assert.False(t, datasourcePlugin.isVersionOne())
 		assert.True(t, datasourcePlugin.SDK)
 	})
+
+	t.Run("Plugin version two requires expressions feature to be toggled", func(t *testing.T) {
+		refPlug := DataSourcePlugin{SDK: true}
+		pluginJSON, err := json.Marshal(refPlug)
+		require.NoError(t, err)
+
+		require.Nil(t, setting.FeatureToggles, "setting.FeatureToggles shouldn't be set")
+		datasourcePlugin := DataSourcePlugin{}
+		err = datasourcePlugin.Load(json.NewDecoder(bytes.NewReader(pluginJSON)), "/tmp")
+		require.EqualError(t, err, "A plugin version 2 was found, but expressions feature toggle is not enabled")
+	})
 }

pkg/plugins/models.go

@@ -2,7 +2,6 @@ package plugins
 
 import (
 	"encoding/json"
-	"errors"
 	"fmt"
 	"strings"
 
@@ -61,7 +60,7 @@ type PluginBase struct {
 
 func (pb *PluginBase) registerPlugin(pluginDir string) error {
 	if _, exists := Plugins[pb.Id]; exists {
-		return errors.New("Plugin with same id already exists")
+		return fmt.Errorf("Plugin with ID %q already exists", pb.Id)
 	}
 
 	if !strings.HasPrefix(pluginDir, setting.StaticRootPath) {

scripts/go/configs/.golangci.yml

@@ -18,8 +18,8 @@ linters:
     - varcheck
     - goconst
     - staticcheck
+    - errcheck
 
 linter-settings:
   goconst:
     ignore-tests: true
-