From 2c5300aa7591d22ae9350e4a0e0644456bd038c8 Mon Sep 17 00:00:00 2001 From: "Andrew S. Brown" Date: Sat, 20 Feb 2021 13:04:44 -0800 Subject: [PATCH 1/4] Add "enable-new" command that enables any linters that are not explicitly disabled This command updates the identified configuration file and adds new linters to it, trying to preserve the original file as closely as possible This offers an alternative to the deprecated "enable-all" setting. It allows projects to keep up-to-date with the latest linters without introducing new linters when the golangci-lint version is upgraded. --- go.mod | 1 + go.sum | 2 + pkg/commands/config.go | 14 ++- pkg/commands/enable_new.go | 70 +++++++++++++++ pkg/config/update_config_with_new_linters.go | 81 +++++++++++++++++ .../update_config_with_new_linters_test.go | 86 +++++++++++++++++++ 6 files changed, 251 insertions(+), 3 deletions(-) create mode 100644 pkg/commands/enable_new.go create mode 100644 pkg/config/update_config_with_new_linters.go create mode 100644 pkg/config/update_config_with_new_linters_test.go diff --git a/go.mod b/go.mod index 14518a6a674a..ac70020e7a0d 100644 --- a/go.mod +++ b/go.mod @@ -77,6 +77,7 @@ require ( golang.org/x/text v0.3.4 // indirect golang.org/x/tools v0.1.0 gopkg.in/yaml.v2 v2.4.0 + gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b honnef.co/go/tools v0.0.1-2020.1.6 mvdan.cc/gofumpt v0.1.0 mvdan.cc/interfacer v0.0.0-20180901003855-c20040233aed diff --git a/go.sum b/go.sum index 70ad38f793f6..0fa06f89b88d 100644 --- a/go.sum +++ b/go.sum @@ -653,6 +653,8 @@ gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b h1:h8qDotaEPuJATrMmW04NCwg7v22aHH28wwpauUhK9Oo= +gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= diff --git a/pkg/commands/config.go b/pkg/commands/config.go index 4b63e2e5239f..e4047e6e650b 100644 --- a/pkg/commands/config.go +++ b/pkg/commands/config.go @@ -33,6 +33,14 @@ func (e *Executor) initConfig() { } e.initRunConfiguration(pathCmd) // allow --config cmd.AddCommand(pathCmd) + + enableNewCmd := &cobra.Command{ + Use: "enable-new", + Short: "Enable all linters not explicitly disabled in config file", + Run: e.executeEnableNewCmd, + } + e.initRunConfiguration(enableNewCmd) // allow --config + cmd.AddCommand(enableNewCmd) } func (e *Executor) getUsedConfig() string { @@ -55,12 +63,12 @@ func (e *Executor) executePathCmd(_ *cobra.Command, args []string) { e.log.Fatalf("Usage: golangci-lint config path") } - usedConfigFile := e.getUsedConfig() - if usedConfigFile == "" { + usedConfigFilePath := e.getUsedConfig() + if usedConfigFilePath == "" { e.log.Warnf("No config file detected") os.Exit(exitcodes.NoConfigFileDetected) } - fmt.Println(usedConfigFile) + fmt.Println(usedConfigFilePath) os.Exit(0) } diff --git a/pkg/commands/enable_new.go b/pkg/commands/enable_new.go new file mode 100644 index 000000000000..a0b790eff350 --- /dev/null +++ b/pkg/commands/enable_new.go @@ -0,0 +1,70 @@ +package commands + +import ( + "os" + + "github.com/fatih/color" + "github.com/pkg/errors" + "github.com/spf13/cobra" + + "github.com/golangci/golangci-lint/pkg/config" + "github.com/golangci/golangci-lint/pkg/lint/linter" +) + +func (e *Executor) executeEnableNewCmd(_ *cobra.Command, args []string) { + if len(args) != 0 { + e.log.Fatalf("Usage: golangci-lint config enable-new") + } + + if e.cfg.Linters.EnableAll { + e.log.Fatalf("enable-new is not compatible with the enable-all linters setting") + } + + unmentionedLinters, err := e.getUnmentionedLinters() + if err != nil { + e.log.Fatalf("Failed to determine unmentioned linters: %s", err) + } + + var newLinterNames []string + for _, l := range unmentionedLinters { + newLinterNames = append(newLinterNames, l.Name()) + } + + configFilePath := e.getUsedConfig() + if configFilePath == "" { + e.log.Fatalf("No config file detected") + } + + color.Yellow("\nEnabling the following new linters in %q:\n", configFilePath) + printLinterConfigs(unmentionedLinters) + + config.UpdateConfigFileWithNewLinters(configFilePath, newLinterNames) + + os.Exit(0) +} + +func (e *Executor) getUnmentionedLinters() ([]*linter.Config, error) { + enabledLinters, err := e.EnabledLintersSet.GetEnabledLintersMap() + if err != nil { + return nil, errors.Wrap(err, "could not determine enabled linters") + } + + var newLinters []*linter.Config + +NextLinterConfig: + for _, lc := range e.DBManager.GetAllSupportedLinterConfigs() { + for _, name := range lc.AllNames() { + if enabledLinters[name] != nil { + continue NextLinterConfig + } + for _, e := range e.cfg.Linters.Disable { + if e == name { + continue NextLinterConfig + } + } + } + newLinters = append(newLinters, lc) + } + + return newLinters, nil +} diff --git a/pkg/config/update_config_with_new_linters.go b/pkg/config/update_config_with_new_linters.go new file mode 100644 index 000000000000..953322634543 --- /dev/null +++ b/pkg/config/update_config_with_new_linters.go @@ -0,0 +1,81 @@ +package config + +import ( + "io/ioutil" + "os" + + "github.com/pkg/errors" + "gopkg.in/yaml.v3" +) + +// UpdateConfigFileWithNewLinters adds new linters to the "linters" config in the file at the provided path +func UpdateConfigFileWithNewLinters(configFilePath string, newLinters []string) error { + configData, err := ioutil.ReadFile(configFilePath) + if err != nil { + return errors.Wrap(err, "could not read config file") + } + + var docNode yaml.Node + if err := yaml.Unmarshal(configData, &docNode); err != nil { + return errors.Wrapf(err, "failed to unmarshal config file %q", configFilePath) + } + + var configNode *yaml.Node + if len(docNode.Content) > 0 { + configNode = docNode.Content[0] + } else { + configNode = &yaml.Node{Kind: yaml.MappingNode} + docNode.Content = append(docNode.Content, configNode) + } + + // guess the indent level by looking at the column of second level nodes + indentSpaces := 2 +GuessSpaces: + for _, n := range configNode.Content { + for _, nn := range n.Content { + indentSpaces = nn.Column - 1 + break GuessSpaces + } + } + + lintersNode := findOrInsertKeyedValue(configNode, "linters", yaml.Node{Kind: yaml.MappingNode}) + + // find the "linters" -> "enable" node (or create it) + enableNode := findOrInsertKeyedValue(lintersNode, "enable", yaml.Node{Kind: yaml.SequenceNode}) + + for _, l := range newLinters { + node := &yaml.Node{} + node.SetString(l) + enableNode.Content = append(enableNode.Content, node) + } + + configFile, err := os.OpenFile(configFilePath, os.O_WRONLY|os.O_TRUNC, 0) + if err != nil { + return errors.Wrapf(err, "failed to open file %q for writing", configFilePath) + } + + encoder := yaml.NewEncoder(configFile) + encoder.SetIndent(indentSpaces) + err = encoder.Encode(docNode.Content[0]) + if err == nil { + err = encoder.Close() + } + if err != nil { + err = configFile.Close() + } + return errors.Wrapf(err, "failed to update config file %q", configFilePath) +} + +func findOrInsertKeyedValue(node *yaml.Node, key string, value yaml.Node) *yaml.Node { + for i, n := range node.Content { + var childKey string + err := n.Decode(&childKey) + if err == nil && key == childKey { + return node.Content[i+1] + } + } + keyNode := &yaml.Node{} + keyNode.SetString(key) + node.Content = append(node.Content, keyNode, &value) + return &value +} diff --git a/pkg/config/update_config_with_new_linters_test.go b/pkg/config/update_config_with_new_linters_test.go new file mode 100644 index 000000000000..84a026c2cefe --- /dev/null +++ b/pkg/config/update_config_with_new_linters_test.go @@ -0,0 +1,86 @@ +package config + +import ( + "io" + "io/ioutil" + "os" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/golangci/golangci-lint/pkg/logutils" +) + +func TestUpdateConfigWithNewLinters(t *testing.T) { + writeConfig := func(t *testing.T, cfg string) string { + tmpFile, err := ioutil.TempFile("", "golangci-lint-test-config-*.yml") + require.NoError(t, err) + + _, err = io.WriteString(tmpFile, cfg) + require.NoError(t, err) + + err = tmpFile.Close() + require.NoError(t, err) + + t.Cleanup(func() { + os.Remove(tmpFile.Name()) + }) + return tmpFile.Name() + } + + readConfig := func(t *testing.T, filePath string) Config { + var cfg Config + cmdLineCfg := Config{Run: Run{Config: filePath}} + r := NewFileReader(&cfg, &cmdLineCfg, logutils.NewStderrLog("testing")) + err := r.Read() + require.NoError(t, err) + return cfg + } + + t.Run(`when the "linters" -> "enable" node exists, we add to it, matching the indent size`, func(t *testing.T) { + cfgFilePath := writeConfig(t, ` +linters: + enable: + - other-linter +`) + err := UpdateConfigFileWithNewLinters(cfgFilePath, []string{"new-linter"}) + require.NoError(t, err) + cfg := readConfig(t, cfgFilePath) + require.Contains(t, cfg.Linters.Enable, "new-linter") + + data, err := ioutil.ReadFile(cfgFilePath) + require.NoError(t, err) + assert.Contains(t, string(data), "\n"+strings.Repeat(" ", 6)+"- new-linter", + "indent size does not match") + }) + + t.Run(`when there is no "enable" node, we create one`, func(t *testing.T) { + cfgFilePath := writeConfig(t, ` +linters: {} +`) + err := UpdateConfigFileWithNewLinters(cfgFilePath, []string{"new-linter"}) + require.NoError(t, err) + cfg := readConfig(t, cfgFilePath) + assert.Contains(t, cfg.Linters.Enable, "new-linter") + }) + + t.Run(`when the file is empty, we create values from scratch`, func(t *testing.T) { + cfgFilePath := writeConfig(t, ` +{} +`) + err := UpdateConfigFileWithNewLinters(cfgFilePath, []string{"new-linter"}) + require.NoError(t, err) + cfg := readConfig(t, cfgFilePath) + assert.Contains(t, cfg.Linters.Enable, "new-linter") + }) + + t.Run(`when there is no "linters" node, we create one`, func(t *testing.T) { + cfgFilePath := writeConfig(t, "") + err := UpdateConfigFileWithNewLinters(cfgFilePath, []string{"new-linter"}) + require.NoError(t, err) + cfg := readConfig(t, cfgFilePath) + assert.Contains(t, cfg.Linters.Enable, "new-linter") + }) +} From 69800e86f6117a6da360e2f5ee50742df0d5a47b Mon Sep 17 00:00:00 2001 From: "Andrew S. Brown" Date: Sat, 20 Feb 2021 13:09:15 -0800 Subject: [PATCH 2/4] Better description --- pkg/commands/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/commands/config.go b/pkg/commands/config.go index e4047e6e650b..230281160f8b 100644 --- a/pkg/commands/config.go +++ b/pkg/commands/config.go @@ -36,7 +36,7 @@ func (e *Executor) initConfig() { enableNewCmd := &cobra.Command{ Use: "enable-new", - Short: "Enable all linters not explicitly disabled in config file", + Short: "Enable all linters not explicitly disabled in the active config file", Run: e.executeEnableNewCmd, } e.initRunConfiguration(enableNewCmd) // allow --config From 66f3f08cb1821d3f26cb75eb6317f5f192ad12a5 Mon Sep 17 00:00:00 2001 From: "Andrew S. Brown" Date: Sat, 20 Feb 2021 13:15:02 -0800 Subject: [PATCH 3/4] Fix lint errors --- pkg/commands/enable_new.go | 4 +++- pkg/config/update_config_with_new_linters.go | 10 +++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/pkg/commands/enable_new.go b/pkg/commands/enable_new.go index a0b790eff350..d0a4ed9e0ea8 100644 --- a/pkg/commands/enable_new.go +++ b/pkg/commands/enable_new.go @@ -38,7 +38,9 @@ func (e *Executor) executeEnableNewCmd(_ *cobra.Command, args []string) { color.Yellow("\nEnabling the following new linters in %q:\n", configFilePath) printLinterConfigs(unmentionedLinters) - config.UpdateConfigFileWithNewLinters(configFilePath, newLinterNames) + if err = config.UpdateConfigFileWithNewLinters(configFilePath, newLinterNames); err != nil { + e.log.Fatalf("failed to update config file: %s", err) + } os.Exit(0) } diff --git a/pkg/config/update_config_with_new_linters.go b/pkg/config/update_config_with_new_linters.go index 953322634543..640ef78d4c33 100644 --- a/pkg/config/update_config_with_new_linters.go +++ b/pkg/config/update_config_with_new_linters.go @@ -38,10 +38,10 @@ GuessSpaces: } } - lintersNode := findOrInsertKeyedValue(configNode, "linters", yaml.Node{Kind: yaml.MappingNode}) + lintersNode := findOrInsertKeyedValue(configNode, "linters", &yaml.Node{Kind: yaml.MappingNode}) // find the "linters" -> "enable" node (or create it) - enableNode := findOrInsertKeyedValue(lintersNode, "enable", yaml.Node{Kind: yaml.SequenceNode}) + enableNode := findOrInsertKeyedValue(lintersNode, "enable", &yaml.Node{Kind: yaml.SequenceNode}) for _, l := range newLinters { node := &yaml.Node{} @@ -66,7 +66,7 @@ GuessSpaces: return errors.Wrapf(err, "failed to update config file %q", configFilePath) } -func findOrInsertKeyedValue(node *yaml.Node, key string, value yaml.Node) *yaml.Node { +func findOrInsertKeyedValue(node *yaml.Node, key string, value *yaml.Node) *yaml.Node { for i, n := range node.Content { var childKey string err := n.Decode(&childKey) @@ -76,6 +76,6 @@ func findOrInsertKeyedValue(node *yaml.Node, key string, value yaml.Node) *yaml. } keyNode := &yaml.Node{} keyNode.SetString(key) - node.Content = append(node.Content, keyNode, &value) - return &value + node.Content = append(node.Content, keyNode, value) + return value } From ec125f76dcc315b4f603112003513002c5640981 Mon Sep 17 00:00:00 2001 From: "Andrew S. Brown" Date: Sat, 20 Feb 2021 13:18:30 -0800 Subject: [PATCH 4/4] Fix lint error --- pkg/config/update_config_with_new_linters.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/config/update_config_with_new_linters.go b/pkg/config/update_config_with_new_linters.go index 640ef78d4c33..bda6b7b2e870 100644 --- a/pkg/config/update_config_with_new_linters.go +++ b/pkg/config/update_config_with_new_linters.go @@ -16,7 +16,7 @@ func UpdateConfigFileWithNewLinters(configFilePath string, newLinters []string) } var docNode yaml.Node - if err := yaml.Unmarshal(configData, &docNode); err != nil { + if err = yaml.Unmarshal(configData, &docNode); err != nil { return errors.Wrapf(err, "failed to unmarshal config file %q", configFilePath) }