fix issue 470 (#477)

2025-09-16 09:06:22 +02:00 · 2020-10-29 14:11:52 +01:00
parent ff1ebcd54b
commit 50f01f660d
8 changed files with 246 additions and 184 deletions
--- a/2
+++ b/2
@@ -9,5 +9,5 @@ build:
 	@go build

 test:
-	@go test -v ./test/...
+	@go test -v ./...

--- a/config/config.go
+++ b/config/config.go
@@ -1,29 +1,17 @@
-package main
+package config

 import (
-	"flag"
+	"errors"
 	"fmt"
 	"io/ioutil"
-	"os"
-	"path/filepath"
-	"strings"
-
-	"github.com/fatih/color"
-	"github.com/mgechev/dots"
-	"github.com/mitchellh/go-homedir"

 	"github.com/mgechev/revive/formatter"

-	toml "github.com/pelletier/go-toml"
+	"github.com/BurntSushi/toml"
 	"github.com/mgechev/revive/lint"
 	"github.com/mgechev/revive/rule"
 )

-func fail(err string) {
-	fmt.Fprintln(os.Stderr, err)
-	os.Exit(1)
-}
-
 var defaultRules = []lint.Rule{
 	&rule.VarDeclarationsRule{},
 	&rule.PackageCommentsRule{},
@@ -110,7 +98,8 @@ func getFormatters() map[string]lint.Formatter {
 	return result
 }

-func getLintingRules(config *lint.Config) []lint.Rule {
+// GetLintingRules yields the linting rules activated in the configuration
+func GetLintingRules(config *lint.Config) ([]lint.Rule, error) {
 	rulesMap := map[string]lint.Rule{}
 	for _, r := range allRules {
 		rulesMap[r.Name()] = r
@@ -120,25 +109,25 @@ func getLintingRules(config *lint.Config) []lint.Rule {
 	for name := range config.Rules {
 		rule, ok := rulesMap[name]
 		if !ok {
-			fail("cannot find rule: " + name)
+			return nil, fmt.Errorf("cannot find rule: %s", name)
 		}
 		lintingRules = append(lintingRules, rule)
 	}

-	return lintingRules
+	return lintingRules, nil
 }

-func parseConfig(path string) *lint.Config {
+func parseConfig(path string) (*lint.Config, error) {
 	config := &lint.Config{}
 	file, err := ioutil.ReadFile(path)
 	if err != nil {
-		fail("cannot read the config file")
+		return nil, errors.New("cannot read the config file")
 	}
-	err = toml.Unmarshal(file, config)
+	_, err = toml.Decode(string(file), config)
 	if err != nil {
-		fail("cannot parse the config file: " + err.Error())
+		return nil, fmt.Errorf("cannot parse the config file: %v", err)
 	}
-	return config
+	return config, nil
 }

 func normalizeConfig(config *lint.Config) {
@@ -162,38 +151,32 @@ func normalizeConfig(config *lint.Config) {
 	}
 }

-func getConfig() *lint.Config {
+// GetConfig yields the configuration
+func GetConfig(configPath string) (*lint.Config, error) {
 	config := defaultConfig()
 	if configPath != "" {
-		config = parseConfig(configPath)
+		var err error
+		config, err = parseConfig(configPath)
+		if err != nil {
+			return nil, err
+		}
 	}
 	normalizeConfig(config)
-	return config
+	return config, nil
 }

-func getFormatter() lint.Formatter {
+// GetFormatter yields the formatter for lint failures
+func GetFormatter(formatterName string) (lint.Formatter, error) {
 	formatters := getFormatters()
 	formatter := formatters["default"]
 	if formatterName != "" {
 		f, ok := formatters[formatterName]
 		if !ok {
-			fail("unknown formatter " + formatterName)
+			return nil, fmt.Errorf("unknown formatter %v", formatterName)
 		}
 		formatter = f
 	}
-	return formatter
-}
-
-func buildDefaultConfigPath() string {
-	var result string
-	if homeDir, err := homedir.Dir(); err == nil {
-		result = filepath.Join(homeDir, "revive.toml")
-		if _, err := os.Stat(result); err != nil {
-			result = ""
-		}
-	}
-
-	return result
+	return formatter, nil
 }

 func defaultConfig() *lint.Config {
@@ -207,71 +190,3 @@ func defaultConfig() *lint.Config {
 	}
 	return &defaultConfig
 }
-
-func normalizeSplit(strs []string) []string {
-	res := []string{}
-	for _, s := range strs {
-		t := strings.Trim(s, " \t")
-		if len(t) > 0 {
-			res = append(res, t)
-		}
-	}
-	return res
-}
-
-func getPackages() [][]string {
-	globs := normalizeSplit(flag.Args())
-	if len(globs) == 0 {
-		globs = append(globs, ".")
-	}
-
-	packages, err := dots.ResolvePackages(globs, normalizeSplit(excludePaths))
-	if err != nil {
-		fail(err.Error())
-	}
-
-	return packages
-}
-
-type arrayFlags []string
-
-func (i *arrayFlags) String() string {
-	return strings.Join([]string(*i), " ")
-}
-
-func (i *arrayFlags) Set(value string) error {
-	*i = append(*i, value)
-	return nil
-}
-
-var configPath string
-var excludePaths arrayFlags
-var formatterName string
-var help bool
-
-var originalUsage = flag.Usage
-
-func init() {
-	// Force colorizing for no TTY environments
-	if os.Getenv("REVIVE_FORCE_COLOR") == "1" {
-		color.NoColor = false
-	}
-
-	flag.Usage = func() {
-		fmt.Println(getBanner())
-		originalUsage()
-	}
-	// command line help strings
-	const (
-		configUsage    = "path to the configuration TOML file, defaults to $HOME/revive.toml, if present (i.e. -config myconf.toml)"
-		excludeUsage   = "list of globs which specify files to be excluded (i.e. -exclude foo/...)"
-		formatterUsage = "formatter to be used for the output (i.e. -formatter stylish)"
-	)
-
-	defaultConfigPath := buildDefaultConfigPath()
-
-	flag.StringVar(&configPath, "config", defaultConfigPath, configUsage)
-	flag.Var(&excludePaths, "exclude", excludeUsage)
-	flag.StringVar(&formatterName, "formatter", "", formatterUsage)
-	flag.Parse()
-}
--- a/config/config_test.go
+++ b/config/config_test.go
@@ -0,0 +1,48 @@
+package config
+
+import (
+	"reflect"
+	"strings"
+	"testing"
+
+	"github.com/mgechev/revive/lint"
+)
+
+func TestGetConfig(t *testing.T) {
+	tt := map[string]struct {
+		confPath   string
+		wantConfig *lint.Config
+		wantError  string
+	}{
+		"non-reg issue #470": {
+			confPath:  "testdata/issue-470.toml",
+			wantError: "",
+		},
+		"unknown file": {
+			confPath:  "unknown",
+			wantError: "cannot read the config file",
+		},
+		"malformed file": {
+			confPath:  "testdata/malformed.toml",
+			wantError: "cannot parse the config file",
+		},
+		"default config": {
+			wantConfig: defaultConfig(),
+		},
+	}
+
+	for name, tc := range tt {
+		t.Run(name, func(t *testing.T) {
+			cfg, err := GetConfig(tc.confPath)
+			switch {
+			case err != nil && tc.wantError == "":
+				t.Fatalf("Unexpected error\n\t%v", err)
+			case err != nil && !strings.Contains(err.Error(), tc.wantError):
+				t.Fatalf("Expected error\n\t%q\ngot:\n\t%v", tc.wantError, err)
+			case tc.wantConfig != nil && reflect.DeepEqual(cfg, tc.wantConfig):
+				t.Fatalf("Expected config\n\t%+v\ngot:\n\t%+v", tc.wantConfig, cfg)
+			}
+
+		})
+	}
+}
--- a/config/testdata/issue-470.toml
+++ b/config/testdata/issue-470.toml
@@ -0,0 +1,8 @@
+ignoreGeneratedHeader = false
+severity = "warning"
+confidence = 0.8
+errorCode = 0
+warningCode = 0
+
+[rule.add-constant]
+  arguments = [{maxLitCount = "3",allowStrs ="\"\"",allowInts="0,1,2",allowFloats="0.0,0.,1.0,1.,2.0,2."}]
--- a/config/testdata/malformed.toml
+++ b/config/testdata/malformed.toml
@@ -0,0 +1,8 @@
+ignoreGeneratedHeader = false
+severity = "warning"
+confidence = 0.8
+errorCode = 0
+warningCode = 0
+
+[rule.add-constant]
+  arguments = [maxLitCount = "3",allowStrs ="\"\"",allowInts="0,1,2",allowFloats="0.0,0.,1.0,1.,2.0,2."}]
--- a/go.mod
+++ b/go.mod
@@ -3,12 +3,12 @@ module github.com/mgechev/revive
 go 1.12

 require (
+	github.com/BurntSushi/toml v0.3.1
 	github.com/fatih/color v1.9.0
 	github.com/fatih/structtag v1.2.0
 	github.com/mgechev/dots v0.0.0-20190921121421-c36f7dcfbb81
 	github.com/mitchellh/go-homedir v1.1.0
 	github.com/olekukonko/tablewriter v0.0.4
-	github.com/pelletier/go-toml v1.8.1
 	github.com/pkg/errors v0.9.1
 	golang.org/x/tools v0.0.0-20201019175715-b894a3290fff
 )
--- a/go.sum
+++ b/go.sum
@@ -1,5 +1,5 @@
-github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
-github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
+github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
 github.com/fatih/color v1.9.0 h1:8xPHl4/q1VyqGIPif1F+1V3Y3lSmrq01EabUW3CoW5s=
 github.com/fatih/color v1.9.0/go.mod h1:eQcE1qtQxscV5RaZvpXrrb8Drkc3/DdQ+uUYCNjL+zU=
 github.com/fatih/structtag v1.2.0 h1:/OdNE99OxoI/PqaW/SuSK9uxxT3f/tcSZgon/ssNSx4=
@@ -17,8 +17,6 @@ github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG
 github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
 github.com/olekukonko/tablewriter v0.0.4 h1:vHD/YYe1Wolo78koG299f7V/VAS08c6IpCLn+Ejf/w8=
 github.com/olekukonko/tablewriter v0.0.4/go.mod h1:zq6QwlOf5SlnkVbMSr5EoBv3636FWnp+qbPhuoO21uA=
-github.com/pelletier/go-toml v1.8.1 h1:1Nf83orprkJyknT6h7zbuEGUEjcyVlCxSUGTENmNCRM=
-github.com/pelletier/go-toml v1.8.1/go.mod h1:T2/BmBdy8dvIRq1a/8aqjN41wvWlN4lrapLU/GW4pbc=
 github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
 github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
@@ -41,24 +39,6 @@ golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd h1:xhmwyvizuTgC2qz7ZlMluP20u
 golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
-golang.org/x/tools v0.0.0-20200917192154-75ebdcb73b49 h1:cSlTPh0jd/6I6bv6XnLSqQCCHUP6CIprgjNz7KlrK6c=
-golang.org/x/tools v0.0.0-20200917192154-75ebdcb73b49/go.mod h1:z6u4i615ZeAfBE4XtMziQW1fSVJXACjjbWkB/mvPzlU=
-golang.org/x/tools v0.0.0-20200921210052-fa0125251cc4 h1:v8Jgq9X6Es9K9otVr9jxENEJigepKMZgA9OmrIZDtFA=
-golang.org/x/tools v0.0.0-20200921210052-fa0125251cc4/go.mod h1:z6u4i615ZeAfBE4XtMziQW1fSVJXACjjbWkB/mvPzlU=
-golang.org/x/tools v0.0.0-20200922173257-82fe25c37531 h1:FS7ZiladzQ5yC5TWXke5sO9bHgSg37DItOho2WWf43U=
-golang.org/x/tools v0.0.0-20200922173257-82fe25c37531/go.mod h1:z6u4i615ZeAfBE4XtMziQW1fSVJXACjjbWkB/mvPzlU=
-golang.org/x/tools v0.0.0-20200925150135-34b80a0a46ad h1:2MIdXOXYj26hsEudIYgvVlkEqkyD1S13W7CRzN3Bui4=
-golang.org/x/tools v0.0.0-20200925150135-34b80a0a46ad/go.mod h1:z6u4i615ZeAfBE4XtMziQW1fSVJXACjjbWkB/mvPzlU=
-golang.org/x/tools v0.0.0-20201002184944-ecd9fd270d5d h1:vWQvJ/Z0Lu+9/8oQ/pAYXNzbc7CMnBl+tULGVHOy3oE=
-golang.org/x/tools v0.0.0-20201002184944-ecd9fd270d5d/go.mod h1:z6u4i615ZeAfBE4XtMziQW1fSVJXACjjbWkB/mvPzlU=
-golang.org/x/tools v0.0.0-20201005185003-576e169c3de7 h1:YTAUHYgZh/ZOA35/OrjTDmFFKb6ddkBL1Zgtl9r8Di8=
-golang.org/x/tools v0.0.0-20201005185003-576e169c3de7/go.mod h1:z6u4i615ZeAfBE4XtMziQW1fSVJXACjjbWkB/mvPzlU=
-golang.org/x/tools v0.0.0-20201007032633-0806396f153e h1:FJA2W4BQfMGZ+CD/tiAc39HXecuRsJl3EuczaSUu/Yk=
-golang.org/x/tools v0.0.0-20201007032633-0806396f153e/go.mod h1:z6u4i615ZeAfBE4XtMziQW1fSVJXACjjbWkB/mvPzlU=
-golang.org/x/tools v0.0.0-20201013201025-64a9e34f3752 h1:2ntEwh02rqo2jSsrYmp4yKHHjh0CbXP3ZtSUetSB+q8=
-golang.org/x/tools v0.0.0-20201013201025-64a9e34f3752/go.mod h1:z6u4i615ZeAfBE4XtMziQW1fSVJXACjjbWkB/mvPzlU=
-golang.org/x/tools v0.0.0-20201019160706-0a3dcccdcf7a h1:6sJXojG2x1LNOzpQ62sFFIoVKVHnmVkaRiU2zeoEm0g=
-golang.org/x/tools v0.0.0-20201019160706-0a3dcccdcf7a/go.mod h1:z6u4i615ZeAfBE4XtMziQW1fSVJXACjjbWkB/mvPzlU=
 golang.org/x/tools v0.0.0-20201019175715-b894a3290fff h1:HiwHyqQ9ttqCHuTa++R4wNxOg6MY1hduSDT8j2aXoMM=
 golang.org/x/tools v0.0.0-20201019175715-b894a3290fff/go.mod h1:z6u4i615ZeAfBE4XtMziQW1fSVJXACjjbWkB/mvPzlU=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
--- a/main.go
+++ b/main.go
@@ -1,14 +1,135 @@
 package main

 import (
+	"flag"
 	"fmt"
 	"io/ioutil"
 	"os"
+	"path/filepath"
+	"strings"

 	"github.com/fatih/color"
+	"github.com/mgechev/dots"
+	"github.com/mgechev/revive/config"
 	"github.com/mgechev/revive/lint"
+	"github.com/mitchellh/go-homedir"
 )

+func fail(err string) {
+	fmt.Fprintln(os.Stderr, err)
+	os.Exit(1)
+}
+
+func main() {
+	conf, err := config.GetConfig(configPath)
+	if err != nil {
+		fail(err.Error())
+	}
+	formatter, err := config.GetFormatter(formatterName)
+	if err != nil {
+		fail(err.Error())
+	}
+	packages, err := getPackages(excludePaths)
+	if err != nil {
+		fail(err.Error())
+	}
+
+	revive := lint.New(func(file string) ([]byte, error) {
+		return ioutil.ReadFile(file)
+	})
+
+	lintingRules, err := config.GetLintingRules(conf)
+	if err != nil {
+		fail(err.Error())
+	}
+
+	failures, err := revive.Lint(packages, lintingRules, *conf)
+	if err != nil {
+		fail(err.Error())
+	}
+
+	formatChan := make(chan lint.Failure)
+	exitChan := make(chan bool)
+
+	var output string
+	go (func() {
+		output, err = formatter.Format(formatChan, *conf)
+		if err != nil {
+			fail(err.Error())
+		}
+		exitChan <- true
+	})()
+
+	exitCode := 0
+	for f := range failures {
+		if f.Confidence < conf.Confidence {
+			continue
+		}
+		if exitCode == 0 {
+			exitCode = conf.WarningCode
+		}
+		if c, ok := conf.Rules[f.RuleName]; ok && c.Severity == lint.SeverityError {
+			exitCode = conf.ErrorCode
+		}
+		if c, ok := conf.Directives[f.RuleName]; ok && c.Severity == lint.SeverityError {
+			exitCode = conf.ErrorCode
+		}
+
+		formatChan <- f
+	}
+
+	close(formatChan)
+	<-exitChan
+	if output != "" {
+		fmt.Println(output)
+	}
+
+	os.Exit(exitCode)
+}
+
+func normalizeSplit(strs []string) []string {
+	res := []string{}
+	for _, s := range strs {
+		t := strings.Trim(s, " \t")
+		if len(t) > 0 {
+			res = append(res, t)
+		}
+	}
+	return res
+}
+
+func getPackages(excludePaths arrayFlags) ([][]string, error) {
+	globs := normalizeSplit(flag.Args())
+	if len(globs) == 0 {
+		globs = append(globs, ".")
+	}
+
+	packages, err := dots.ResolvePackages(globs, normalizeSplit(excludePaths))
+	if err != nil {
+		return nil, err
+	}
+
+	return packages, nil
+}
+
+type arrayFlags []string
+
+func (i *arrayFlags) String() string {
+	return strings.Join([]string(*i), " ")
+}
+
+func (i *arrayFlags) Set(value string) error {
+	*i = append(*i, value)
+	return nil
+}
+
+var configPath string
+var excludePaths arrayFlags
+var formatterName string
+var help bool
+
+var originalUsage = flag.Usage
+
 func getLogo() string {
 	return color.YellowString(` _ __ _____   _(_)__  _____
 | '__/ _ \ \ / / \ \ / / _ \
@@ -29,57 +150,39 @@ Example:
 `, getLogo(), getCall())
 }

-func main() {
-	config := getConfig()
-	formatter := getFormatter()
-	packages := getPackages()
-
-	revive := lint.New(func(file string) ([]byte, error) {
-		return ioutil.ReadFile(file)
-	})
-
-	lintingRules := getLintingRules(config)
-
-	failures, err := revive.Lint(packages, lintingRules, *config)
-	if err != nil {
-		fail(err.Error())
+func buildDefaultConfigPath() string {
+	var result string
+	if homeDir, err := homedir.Dir(); err == nil {
+		result = filepath.Join(homeDir, "revive.toml")
+		if _, err := os.Stat(result); err != nil {
+			result = ""
+		}
 	}

-	formatChan := make(chan lint.Failure)
-	exitChan := make(chan bool)
-
-	var output string
-	go (func() {
-		output, err = formatter.Format(formatChan, *config)
-		if err != nil {
-			fail(err.Error())
-		}
-		exitChan <- true
-	})()
-
-	exitCode := 0
-	for f := range failures {
-		if f.Confidence < config.Confidence {
-			continue
-		}
-		if exitCode == 0 {
-			exitCode = config.WarningCode
-		}
-		if c, ok := config.Rules[f.RuleName]; ok && c.Severity == lint.SeverityError {
-			exitCode = config.ErrorCode
-		}
-		if c, ok := config.Directives[f.RuleName]; ok && c.Severity == lint.SeverityError {
-			exitCode = config.ErrorCode
-		}
-
-		formatChan <- f
-	}
-
-	close(formatChan)
-	<-exitChan
-	if output != "" {
-		fmt.Println(output)
-	}
-
-	os.Exit(exitCode)
+	return result
+}
+
+func init() {
+	// Force colorizing for no TTY environments
+	if os.Getenv("REVIVE_FORCE_COLOR") == "1" {
+		color.NoColor = false
+	}
+
+	flag.Usage = func() {
+		fmt.Println(getBanner())
+		originalUsage()
+	}
+	// command line help strings
+	const (
+		configUsage    = "path to the configuration TOML file, defaults to $HOME/revive.toml, if present (i.e. -config myconf.toml)"
+		excludeUsage   = "list of globs which specify files to be excluded (i.e. -exclude foo/...)"
+		formatterUsage = "formatter to be used for the output (i.e. -formatter stylish)"
+	)
+
+	defaultConfigPath := buildDefaultConfigPath()
+
+	flag.StringVar(&configPath, "config", defaultConfigPath, configUsage)
+	flag.Var(&excludePaths, "exclude", excludeUsage)
+	flag.StringVar(&formatterName, "formatter", "", formatterUsage)
+	flag.Parse()
 }