Repository: mgechev/revive
Pull request #650 (2022-03-16 02:06:09)

Allow revive to be called with extra linters
This change allows revive to be called from other libraries and pass in a list of custom linters to be added to the built-in linters found in config.

After this change, users who want to extend Revive can do it without forking. It's a simple matter of creating a new folder with `main.go`:
```go
package main

import "github.com/mgechev/revive/cli"

func main() {
    cli.RunRevive(&CustomRule{}, &OtherRule{})
}

// Rules ommitted for clarity, but they could be in this file,
// or in another package. Could even be packs of rules
// added as dependency of the project
```

Then in their .toml file:

```toml
ignoreGeneratedHeader = false
severity = "warning"
confidence = 0.8
errorCode = 0
warningCode = 0

[rule.blank-imports]
[rule.context-as-argument]
[rule.context-keys-type]
[rule.dot-imports]
[rule.error-return]
[rule.error-strings]
[rule.error-naming]
[rule.exported]
[rule.if-return]
[rule.increment-decrement]
[rule.var-naming]
[rule.var-declaration]
[rule.package-comments]
[rule.range]
[rule.receiver-naming]
[rule.time-naming]
[rule.unexported-return]
[rule.indent-error-flow]
[rule.errorf]
[rule.custom-rule]
  arguments = [0]
```

As soon as they remove [rule.custom-rule] from their config, it is not run (even though it is passed as an extra rule.

The motivation for this PR is that I intend to base the custom linters for my company on revive and do not wish to maintain a fork of it. It feels much safer to just use a function that does everything and receives the extra rules as arguments.

The change in revive's codebase is so small that it felt like a good way moving forward to enable even easier extensibility for this amazing project.

If you feel like this is a valuable contribution I'm more than happy to add tests and update the docs on how to create and use custom rules without forking.

heynemann: Can confirm that using my fork I already set up revive with additional linters in our repo. It is working great :D That way I can build and distribute internal versions of our linter to every eng in the company (using brew probably). 

Thanks once again for such a great tool!

Patch:
diff --git a/cli/main.go b/cli/main.go
new file mode 100644
index 0000000..c1d8cba
--- /dev/null
+++ b/cli/main.go
@@ -0,0 +1,281 @@
+package cli
+
+import (
+	"flag"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"runtime/debug"
+	"strings"
+
+	"github.com/fatih/color"
+	"github.com/mgechev/dots"
+	"github.com/mgechev/revive/config"
+	"github.com/mgechev/revive/lint"
+	"github.com/mgechev/revive/logging"
+	"github.com/mitchellh/go-homedir"
+)
+
+var (
+	version = "dev"
+	commit  = "none"
+	date    = "unknown"
+	builtBy = "unknown"
+)
+
+func fail(err string) {
+	fmt.Fprintln(os.Stderr, err)
+	os.Exit(1)
+}
+
+// ExtraRule configures a new rule to be used with revive.
+type ExtraRule struct {
+	Rule          lint.Rule
+	DefaultConfig lint.RuleConfig
+}
+
+// NewExtraRule returns a configured extra rule
+func NewExtraRule(rule lint.Rule, defaultConfig lint.RuleConfig) ExtraRule {
+	return ExtraRule{
+		Rule:          rule,
+		DefaultConfig: defaultConfig,
+	}
+}
+
+// RunRevive runs the CLI for revive.
+func RunRevive(extraRules ...ExtraRule) {
+	log, err := logging.GetLogger()
+	if err != nil {
+		fail(err.Error())
+	}
+
+	formatter, err := config.GetFormatter(formatterName)
+	if err != nil {
+		fail(err.Error())
+	}
+
+	conf, err := config.GetConfig(configPath)
+	if err != nil {
+		fail(err.Error())
+	}
+
+	if setExitStatus {
+		conf.ErrorCode = 1
+		conf.WarningCode = 1
+	}
+
+	extraRuleInstances := make([]lint.Rule, len(extraRules))
+	for i, extraRule := range extraRules {
+		extraRuleInstances[i] = extraRule.Rule
+
+		ruleName := extraRule.Rule.Name()
+		_, isRuleAlreadyConfigured := conf.Rules[ruleName]
+		if !isRuleAlreadyConfigured {
+			conf.Rules[ruleName] = extraRule.DefaultConfig
+		}
+	}
+
+	lintingRules, err := config.GetLintingRules(conf, extraRuleInstances)
+	if err != nil {
+		fail(err.Error())
+	}
+
+	log.Println("Config loaded")
+
+	if len(excludePaths) == 0 { // if no excludes were set in the command line
+		excludePaths = conf.Exclude // use those from the configuration
+	}
+
+	packages, err := getPackages(excludePaths)
+	if err != nil {
+		fail(err.Error())
+	}
+	revive := lint.New(func(file string) ([]byte, error) {
+		return ioutil.ReadFile(file)
+	}, maxOpenFiles)
+
+	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
+	excludePaths  arrayFlags
+	formatterName string
+	help          bool
+	versionFlag   bool
+	setExitStatus bool
+	maxOpenFiles  int
+)
+
+var originalUsage = flag.Usage
+
+func getLogo() string {
+	return color.YellowString(` _ __ _____   _(_)__  _____
+| '__/ _ \ \ / / \ \ / / _ \
+| | |  __/\ V /| |\ V /  __/
+|_|  \___| \_/ |_| \_/ \___|`)
+}
+
+func getCall() string {
+	return color.MagentaString("revive -config c.toml -formatter friendly -exclude a.go -exclude b.go ./...")
+}
+
+func getBanner() string {
+	return fmt.Sprintf(`
+%s
+
+Example:
+  %s
+`, getLogo(), getCall())
+}
+
+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
+}
+
+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)"
+		versionUsage      = "get revive version"
+		exitStatusUsage   = "set exit status to 1 if any issues are found, overwrites errorCode and warningCode in config"
+		maxOpenFilesUsage = "maximum number of open files at the same time"
+	)
+
+	defaultConfigPath := buildDefaultConfigPath()
+
+	flag.StringVar(&configPath, "config", defaultConfigPath, configUsage)
+	flag.Var(&excludePaths, "exclude", excludeUsage)
+	flag.StringVar(&formatterName, "formatter", "", formatterUsage)
+	flag.BoolVar(&versionFlag, "version", false, versionUsage)
+	flag.BoolVar(&setExitStatus, "set_exit_status", false, exitStatusUsage)
+	flag.IntVar(&maxOpenFiles, "max_open_files", 0, maxOpenFilesUsage)
+	flag.Parse()
+
+	// Output build info (version, commit, date and builtBy)
+	if versionFlag {
+		var buildInfo string
+		if date != "unknown" && builtBy != "unknown" {
+			buildInfo = fmt.Sprintf("Built\t\t%s by %s\n", date, builtBy)
+		}
+
+		if commit != "none" {
+			buildInfo = fmt.Sprintf("Commit:\t\t%s\n%s", commit, buildInfo)
+		}
+
+		if version == "dev" {
+			bi, ok := debug.ReadBuildInfo()
+			if ok {
+				version = bi.Main.Version
+				if strings.HasPrefix(version, "v") {
+					version = bi.Main.Version[1:]
+				}
+				if len(buildInfo) == 0 {
+					fmt.Printf("version %s\n", version)
+					os.Exit(0)
+				}
+			}
+		}
+
+		fmt.Printf("Version:\t%s\n%s", version, buildInfo)
+		os.Exit(0)
+	}
+}
diff --git a/config/config.go b/config/config.go
index 4620329..7286547 100644
--- a/config/config.go
+++ b/config/config.go
@@ -107,11 +107,17 @@ func getFormatters() map[string]lint.Formatter {
 }
 
 // GetLintingRules yields the linting rules that must be applied by the linter
-func GetLintingRules(config *lint.Config) ([]lint.Rule, error) {
+func GetLintingRules(config *lint.Config, extraRules []lint.Rule) ([]lint.Rule, error) {
 	rulesMap := map[string]lint.Rule{}
 	for _, r := range allRules {
 		rulesMap[r.Name()] = r
 	}
+	for _, r := range extraRules {
+		if _, ok := rulesMap[r.Name()]; ok {
+			continue
+		}
+		rulesMap[r.Name()] = r
+	}
 
 	var lintingRules []lint.Rule
 	for name, ruleConfig := range config.Rules {
diff --git a/go.mod b/go.mod
index 495d18e..6878a04 100644
--- a/go.mod
+++ b/go.mod
@@ -4,7 +4,7 @@ go 1.12
 
 require (
 	github.com/BurntSushi/toml v1.0.0
-	github.com/chavacava/garif v0.0.0-20210405164556-e8a0a408d6af
+	github.com/chavacava/garif v0.0.0-20220316182200-5cad0b5181d4
 	github.com/fatih/color v1.13.0
 	github.com/fatih/structtag v1.2.0
 	github.com/mgechev/dots v0.0.0-20210922191527-e955255bf517
diff --git a/go.sum b/go.sum
index 717f9f8..ebcacba 100644
--- a/go.sum
+++ b/go.sum
@@ -2,6 +2,8 @@ github.com/BurntSushi/toml v1.0.0 h1:dtDWrepsVPfW9H/4y7dDgFc2MBUSeJhlaDtK13CxFlU
 github.com/BurntSushi/toml v1.0.0/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ=
 github.com/chavacava/garif v0.0.0-20210405164556-e8a0a408d6af h1:spmv8nSH9h5oCQf40jt/ufBCt9j0/58u4G+rkeMqXGI=
 github.com/chavacava/garif v0.0.0-20210405164556-e8a0a408d6af/go.mod h1:Qjyv4H3//PWVzTeCezG2b9IRn6myJxJSr4TD/xo6ojU=
+github.com/chavacava/garif v0.0.0-20220316182200-5cad0b5181d4 h1:tFXjAxje9thrTF4h57Ckik+scJjTWdwAtZqZPtOT48M=
+github.com/chavacava/garif v0.0.0-20220316182200-5cad0b5181d4/go.mod h1:W8EnPSQ8Nv4fUjc/v1/8tHFqhuOJXnRub0dTfuAQktU=
 github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/fatih/color v1.13.0 h1:8LOYc1KYPPmyKMuN8QV2DNRWNbLo6LZ0iLs8+mlH53w=
@@ -28,6 +30,7 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY=
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/yuin/goldmark v1.4.1/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
diff --git a/main.go b/main.go
index 3e27b8c..50694cb 100644
--- a/main.go
+++ b/main.go
@@ -1,254 +1,7 @@
 package main
 
-import (
-	"flag"
-	"fmt"
-	"io/ioutil"
-	"os"
-	"path/filepath"
-	"runtime/debug"
-	"strings"
-
-	"github.com/fatih/color"
-	"github.com/mgechev/dots"
-	"github.com/mgechev/revive/config"
-	"github.com/mgechev/revive/lint"
-	"github.com/mgechev/revive/logging"
-	"github.com/mitchellh/go-homedir"
-)
-
-var (
-	version = "dev"
-	commit  = "none"
-	date    = "unknown"
-	builtBy = "unknown"
-)
-
-func fail(err string) {
-	fmt.Fprintln(os.Stderr, err)
-	os.Exit(1)
-}
+import "github.com/mgechev/revive/cli"
 
 func main() {
-	log, err := logging.GetLogger()
-	if err != nil {
-		fail(err.Error())
-	}
-
-	conf, err := config.GetConfig(configPath)
-	if err != nil {
-		fail(err.Error())
-	}
-	formatter, err := config.GetFormatter(formatterName)
-	if err != nil {
-		fail(err.Error())
-	}
-	if setExitStatus {
-		conf.ErrorCode = 1
-		conf.WarningCode = 1
-	}
-
-	if len(excludePaths) == 0 { // if no excludes were set in the command line
-		excludePaths = conf.Exclude // use those from the configuration
-	}
-
-	packages, err := getPackages(excludePaths)
-	if err != nil {
-		fail(err.Error())
-	}
-
-	revive := lint.New(func(file string) ([]byte, error) {
-		return ioutil.ReadFile(file)
-	}, maxOpenFiles)
-
-	lintingRules, err := config.GetLintingRules(conf)
-	if err != nil {
-		fail(err.Error())
-	}
-
-	log.Println("Config loaded")
-
-	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
-	excludePaths  arrayFlags
-	formatterName string
-	help          bool
-	versionFlag   bool
-	setExitStatus bool
-	maxOpenFiles  int
-)
-
-var originalUsage = flag.Usage
-
-func getLogo() string {
-	return color.YellowString(` _ __ _____   _(_)__  _____
-| '__/ _ \ \ / / \ \ / / _ \
-| | |  __/\ V /| |\ V /  __/
-|_|  \___| \_/ |_| \_/ \___|`)
-}
-
-func getCall() string {
-	return color.MagentaString("revive -config c.toml -formatter friendly -exclude a.go -exclude b.go ./...")
-}
-
-func getBanner() string {
-	return fmt.Sprintf(`
-%s
-
-Example:
-  %s
-`, getLogo(), getCall())
-}
-
-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
-}
-
-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)"
-		versionUsage      = "get revive version"
-		exitStatusUsage   = "set exit status to 1 if any issues are found, overwrites errorCode and warningCode in config"
-		maxOpenFilesUsage = "maximum number of open files at the same time"
-	)
-
-	defaultConfigPath := buildDefaultConfigPath()
-
-	flag.StringVar(&configPath, "config", defaultConfigPath, configUsage)
-	flag.Var(&excludePaths, "exclude", excludeUsage)
-	flag.StringVar(&formatterName, "formatter", "", formatterUsage)
-	flag.BoolVar(&versionFlag, "version", false, versionUsage)
-	flag.BoolVar(&setExitStatus, "set_exit_status", false, exitStatusUsage)
-	flag.IntVar(&maxOpenFiles, "max_open_files", 0, maxOpenFilesUsage)
-	flag.Parse()
-
-	// Output build info (version, commit, date and builtBy)
-	if versionFlag {
-		var buildInfo string
-		if date != "unknown" && builtBy != "unknown" {
-			buildInfo = fmt.Sprintf("Built\t\t%s by %s\n", date, builtBy)
-		}
-
-		if commit != "none" {
-			buildInfo = fmt.Sprintf("Commit:\t\t%s\n%s", commit, buildInfo)
-		}
-
-		if version == "dev" {
-			bi, ok := debug.ReadBuildInfo()
-			if ok {
-				version = bi.Main.Version
-				if strings.HasPrefix(version, "v") {
-					version = bi.Main.Version[1:]
-				}
-				if len(buildInfo) == 0 {
-					fmt.Printf("version %s\n", version)
-					os.Exit(0)
-				}
-			}
-		}
-
-		fmt.Printf("Version:\t%s\n%s", version, buildInfo)
-		os.Exit(0)
-	}
+	cli.RunRevive()
 }
diff --git a/rule/exported.go b/rule/exported.go
index 679e164..621fae5 100644
--- a/rule/exported.go
+++ b/rule/exported.go
@@ -23,7 +23,7 @@ type ExportedRule struct {
 func (r *ExportedRule) Apply(file *lint.File, args lint.Arguments) []lint.Failure {
 	var failures []lint.Failure
 
-	if isTest(file) {
+	if file.IsTest() {
 		return failures
 	}
 
diff --git a/rule/package-comments.go b/rule/package-comments.go
index 00fc5bb..035e113 100644
--- a/rule/package-comments.go
+++ b/rule/package-comments.go
@@ -20,7 +20,7 @@ type PackageCommentsRule struct{}
 func (r *PackageCommentsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
 	var failures []lint.Failure
 
-	if isTest(file) {
+	if file.IsTest() {
 		return failures
 	}
 
diff --git a/rule/utils.go b/rule/utils.go
index 8b0f556..2de9712 100644
--- a/rule/utils.go
+++ b/rule/utils.go
@@ -19,10 +19,6 @@ const styleGuideBase = "https://golang.org/wiki/CodeReviewComments"
 // If id == nil, the answer is false.
 func isBlank(id *ast.Ident) bool { return id != nil && id.Name == "_" }
 
-func isTest(f *lint.File) bool {
-	return strings.HasSuffix(f.Name, "_test.go")
-}
-
 var commonMethods = map[string]bool{
 	"Error":     true,
 	"Read":      true,


Test patch:
diff --git a/config/config_test.go b/config/config_test.go
index 53bff5f..57b7a97 100644
--- a/config/config_test.go
+++ b/config/config_test.go
@@ -92,7 +92,7 @@ func TestGetLintingRules(t *testing.T) {
 			if err != nil {
 				t.Fatalf("Unexpected error while loading conf: %v", err)
 			}
-			rules, err := GetLintingRules(cfg)
+			rules, err := GetLintingRules(cfg, []lint.Rule{})
 			switch {
 			case err != nil:
 				t.Fatalf("Unexpected error\n\t%v", err)
@@ -130,7 +130,7 @@ func TestGetGlobalSeverity(t *testing.T) {
 			if err != nil {
 				t.Fatalf("Unexpected error while loading conf: %v", err)
 			}
-			rules, err := GetLintingRules(cfg)
+			rules, err := GetLintingRules(cfg, []lint.Rule{})
 			if err != nil {
 				t.Fatalf("Unexpected error while loading conf: %v", err)
 			}