Separating lib from cli (#655)

* Separating lib from cli * Renamed NewRevive to New * Added GetLintFailures helper function * Moved formatter to call to format since that's when it's needed * makes fields of Revive struct non-public * minor modifs in tests: remove unnamed constats * Added lint package management to lint command * README message for using revive as a library * README formatting * Removed unused method * Slightly improved wording in README * Handling format errors * Renaming file to better reflect intent * Refactoring pattern usage * README heads * renames excludePaths into excludePatterns Co-authored-by: Bernardo Heynemann <bernardo.heynemann@coinbase.com> Co-authored-by: chavacava <salvadorcavadini+github@gmail.com>
2024-11-21 17:16:40 +02:00 · 2022-03-29 12:31:52 -03:00 · 2022-03-29 12:31:52 -03:00 · 318db94210
commit 318db94210
parent fa939adbf0
7 changed files with 511 additions and 131 deletions
--- a/README.md
+++ b/README.md
@ -588,7 +588,57 @@ import (
 )

 func main() {
-	cli.RunRevive(cli.NewExtraRule(&myRule{}, lint.RuleConfig{}))
+	cli.RunRevive(revivelib.NewExtraRule(&myRule{}, lint.RuleConfig{}))
+}
+
+type myRule struct{}
+
+func (f myRule) Name() string {
+	return "myRule"
+}
+
+func (f myRule) Apply(*lint.File, lint.Arguments) []lint.Failure { ... }
+```
+
+You can still go further and use `revive` without its cli, as part of your library, or your own cli:
+
+```go
+package mylib
+
+import (
+	"github.com/mgechev/revive/cli"
+	"github.com/mgechev/revive/revivelib"
+	"github.com/mgechev/revive/lint"
+)
+
+// Error checking removed for clarity
+func LintMyFile(file string) {
+	conf, _:= config.GetConfig("../defaults.toml")
+
+	revive, _ := revivelib.New(
+		conf,  // Configuration file
+		true,  // Set exit status
+		2048,  // Max open files
+
+        // Then add as many extra rules as you need
+		revivelib.NewExtraRule(&myRule{}, lint.RuleConfig{}),
+	)
+
+	failuresChan, err := revive.Lint(
+ 		revivelib.Include(file),
+ 		revivelib.Exclude("./fixtures"),
+ 		// You can use as many revivelib.Include or revivelib.Exclude as required
+ 	)
+  	if err != nil {
+  	 	panic("Shouldn't have failed: " + err.Error)
+  	}
+
+  	// Now let's return the formatted errors
+	failures, exitCode, _ := revive.Format("stylish", failuresChan)
+
+  	// failures is the string with all formatted lint error messages
+  	// exit code is 0 if no errors, 1 if errors (unless config options change it)
+  	// ... do something with them
 }

 type myRule struct{}
@ -691,6 +741,12 @@ REVIVE_FORCE_COLOR=1 revive -formatter friendly ./... | tee revive.log
 :---: |:---: |:---: |:---: |:---: |:---: |
 [ridvansumset](https://github.com/ridvansumset) |[Jarema](https://github.com/Jarema) |[vkrol](https://github.com/vkrol) |[haya14busa](https://github.com/haya14busa) |[sina-devel](https://github.com/sina-devel) |[techknowlogick](https://github.com/techknowlogick) |

+
+[<img alt="heynemann" src="https://avatars.githubusercontent.com/u/60965?v=4&s=117" width="117">](https://github.com/heynemann) | | | | | |
+|:---: |:---: |:---: |:---: |:---: |:---: |
+[heynemann](https://github.com/heynemann) | | | | | |
+
+
 ## License

 MIT
--- a/cli/main.go
+++ b/cli/main.go
@ -3,17 +3,14 @@ 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/mgechev/revive/revivelib"
 	"github.com/mitchellh/go-homedir"
 )

@ -29,109 +26,44 @@ func fail(err string) {
 	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())
-	}
-
+func RunRevive(extraRules ...revivelib.ExtraRule) {
 	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)
+	revive, err := revivelib.New(
+		conf,
+		setExitStatus,
+		maxOpenFiles,
+		extraRules...,
+	)
 	if err != nil {
 		fail(err.Error())
 	}

-	log.Println("Config loaded")
+	files := flag.Args()
+	packages := []*revivelib.LintPattern{}

-	if len(excludePaths) == 0 { // if no excludes were set in the command line
-		excludePaths = conf.Exclude // use those from the configuration
+	for _, file := range files {
+		packages = append(packages, revivelib.Include(file))
 	}

-	packages, err := getPackages(excludePaths)
-	if err != nil {
-		fail(err.Error())
+	for _, file := range excludePatterns {
+		packages = append(packages, revivelib.Exclude(file))
 	}
-	revive := lint.New(func(file string) ([]byte, error) {
-		return ioutil.ReadFile(file)
-	}, maxOpenFiles)

-	failures, err := revive.Lint(packages, lintingRules, *conf)
+	failures, err := revive.Lint(packages...)
 	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
+	output, exitCode, err := revive.Format(formatterName, failures)
+	if err != nil {
+		fail(err.Error())
 	}

-	close(formatChan)
-	<-exitChan
 	if output != "" {
 		fmt.Println(output)
 	}
@ -139,50 +71,13 @@ func RunRevive(extraRules ...ExtraRule) {
 	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
+	configPath      string
+	excludePatterns revivelib.ArrayFlags
+	formatterName   string
+	versionFlag     bool
+	setExitStatus   bool
+	maxOpenFiles    int
 )

 var originalUsage = flag.Usage
@ -243,7 +138,7 @@ func init() {
 	defaultConfigPath := buildDefaultConfigPath()

 	flag.StringVar(&configPath, "config", defaultConfigPath, configUsage)
-	flag.Var(&excludePaths, "exclude", excludeUsage)
+	flag.Var(&excludePatterns, "exclude", excludeUsage)
 	flag.StringVar(&formatterName, "formatter", "", formatterUsage)
 	flag.BoolVar(&versionFlag, "version", false, versionUsage)
 	flag.BoolVar(&setExitStatus, "set_exit_status", false, exitStatusUsage)
--- a/revivelib/core.go
+++ b/revivelib/core.go
@ -0,0 +1,204 @@
+package revivelib
+
+import (
+	"io/ioutil"
+	"log"
+	"strings"
+
+	"github.com/mgechev/dots"
+	"github.com/mgechev/revive/config"
+	"github.com/mgechev/revive/lint"
+	"github.com/mgechev/revive/logging"
+	"github.com/pkg/errors"
+)
+
+// Revive is responsible for running linters and formatters
+// and returning a set of results.
+type Revive struct {
+	config       *lint.Config
+	lintingRules []lint.Rule
+	logger       *log.Logger
+	maxOpenFiles int
+}
+
+// New creates a new instance of Revive lint runner.
+func New(
+	conf *lint.Config,
+	setExitStatus bool,
+	maxOpenFiles int,
+	extraRules ...ExtraRule,
+) (*Revive, error) {
+	log, err := logging.GetLogger()
+	if err != nil {
+		return nil, errors.Wrap(err, "initializing revive - getting logger")
+	}
+
+	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 {
+		return nil, errors.Wrap(err, "initializing revive - gettint lint rules")
+	}
+
+	log.Println("Config loaded")
+
+	return &Revive{
+		logger:       log,
+		config:       conf,
+		lintingRules: lintingRules,
+		maxOpenFiles: maxOpenFiles,
+	}, nil
+}
+
+// Lint the included patterns, skipping excluded ones
+func (r *Revive) Lint(patterns ...*LintPattern) (<-chan lint.Failure, error) {
+	includePatterns := []string{}
+	excludePatterns := []string{}
+
+	for _, lintpkg := range patterns {
+		if lintpkg.IsExclude() {
+			excludePatterns = append(excludePatterns, lintpkg.GetPattern())
+		} else {
+			includePatterns = append(includePatterns, lintpkg.GetPattern())
+		}
+	}
+
+	if len(excludePatterns) == 0 { // if no excludes were set
+		excludePatterns = r.config.Exclude // use those from the configuration
+	}
+
+	packages, err := getPackages(includePatterns, excludePatterns)
+	if err != nil {
+		return nil, errors.Wrap(err, "linting - getting packages")
+	}
+
+	revive := lint.New(func(file string) ([]byte, error) {
+		contents, err := ioutil.ReadFile(file)
+
+		if err != nil {
+			return nil, errors.Wrap(err, "reading file "+file)
+		}
+
+		return contents, nil
+	}, r.maxOpenFiles)
+
+	failures, err := revive.Lint(packages, r.lintingRules, *r.config)
+	if err != nil {
+		return nil, errors.Wrap(err, "linting - retrieving failures channel")
+	}
+
+	return failures, nil
+}
+
+// Format gets the output for a given failures channel from Lint.
+func (r *Revive) Format(
+	formatterName string,
+	failuresChan <-chan lint.Failure,
+) (string, int, error) {
+	conf := r.config
+	formatChan := make(chan lint.Failure)
+	exitChan := make(chan bool)
+
+	formatter, err := config.GetFormatter(formatterName)
+	if err != nil {
+		return "", 0, errors.Wrap(err, "formatting - getting formatter")
+	}
+
+	var (
+		output    string
+		formatErr error
+	)
+
+	go func() {
+		output, formatErr = formatter.Format(formatChan, *conf)
+
+		exitChan <- true
+	}()
+
+	exitCode := 0
+
+	for failure := range failuresChan {
+		if failure.Confidence < conf.Confidence {
+			continue
+		}
+
+		if exitCode == 0 {
+			exitCode = conf.WarningCode
+		}
+
+		if c, ok := conf.Rules[failure.RuleName]; ok && c.Severity == lint.SeverityError {
+			exitCode = conf.ErrorCode
+		}
+
+		if c, ok := conf.Directives[failure.RuleName]; ok && c.Severity == lint.SeverityError {
+			exitCode = conf.ErrorCode
+		}
+
+		formatChan <- failure
+	}
+
+	close(formatChan)
+	<-exitChan
+
+	if formatErr != nil {
+		return "", exitCode, errors.Wrap(err, "formatting")
+	}
+
+	return output, exitCode, nil
+}
+
+func getPackages(includePatterns []string, excludePatterns ArrayFlags) ([][]string, error) {
+	globs := normalizeSplit(includePatterns)
+	if len(globs) == 0 {
+		globs = append(globs, ".")
+	}
+
+	packages, err := dots.ResolvePackages(globs, normalizeSplit(excludePatterns))
+	if err != nil {
+		return nil, errors.Wrap(err, "getting packages - resolving packages in dots")
+	}
+
+	return packages, nil
+}
+
+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
+}
+
+// ArrayFlags type for string list.
+type ArrayFlags []string
+
+func (i *ArrayFlags) String() string {
+	return strings.Join([]string(*i), " ")
+}
+
+// Set value for array flags.
+func (i *ArrayFlags) Set(value string) error {
+	*i = append(*i, value)
+
+	return nil
+}
--- a/revivelib/core_internal_test.go
+++ b/revivelib/core_internal_test.go
@ -0,0 +1,69 @@
+package revivelib
+
+import (
+	"testing"
+
+	"github.com/mgechev/revive/config"
+	"github.com/mgechev/revive/lint"
+)
+
+func TestReviveCreateInstance(t *testing.T) {
+	revive := getMockRevive(t)
+
+	if revive.config == nil {
+		t.Fatal("Could not load config.")
+	}
+
+	if revive.maxOpenFiles != 2048 {
+		t.Fatal("Expected MaxOpenFiles to be 2048")
+	}
+
+	if revive.lintingRules == nil || len(revive.lintingRules) == 0 {
+		t.Fatal("Linting rules not loaded.")
+	}
+
+	rules := map[string]lint.Rule{}
+	for _, rule := range revive.lintingRules {
+		rules[rule.Name()] = rule
+	}
+
+	if _, ok := rules["mock-rule"]; !ok {
+		t.Fatal("Didn't load mock rule.")
+	}
+
+	if revive.config.ErrorCode != 1 && revive.config.WarningCode != 1 {
+		t.Fatal("Didn't set the codes in the config instance.")
+	}
+}
+
+type mockRule struct {
+}
+
+func (r *mockRule) Name() string {
+	return "mock-rule"
+}
+
+func (r *mockRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure {
+	return nil
+}
+
+func getMockRevive(t *testing.T) *Revive {
+	t.Helper()
+
+	conf, err := config.GetConfig("../defaults.toml")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	revive, err := New(
+		conf,
+		true,
+		2048,
+		NewExtraRule(&mockRule{}, lint.RuleConfig{}),
+	)
+	if err != nil {
+		t.Fatal(err.Error())
+	}
+
+	return revive
+}
--- a/revivelib/core_test.go
+++ b/revivelib/core_test.go
@ -0,0 +1,106 @@
+package revivelib_test
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/mgechev/revive/config"
+	"github.com/mgechev/revive/lint"
+	"github.com/mgechev/revive/revivelib"
+)
+
+func TestReviveLint(t *testing.T) {
+	// ARRANGE
+	revive := getMockRevive(t)
+
+	// ACT
+	failures, err := revive.Lint(revivelib.Include("../testdata/if-return.go"))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// ASSERT
+	failureList := []lint.Failure{}
+
+	for failure := range failures {
+		failureList = append(failureList, failure)
+	}
+
+	const expected = 3
+
+	got := len(failureList)
+	if got != expected {
+		t.Fatalf("Expected failures to have %d failures, but it has %d.", expected, got)
+	}
+
+	errmsg := "redundant if ...; err != nil check, just return error instead."
+	if failureList[0].Failure != errmsg {
+		t.Fatalf("Expected failure[0] to be '%s', but it was '%s'", errmsg, failureList[0].Failure)
+	}
+}
+
+func TestReviveFormat(t *testing.T) {
+	// ARRANGE
+	revive := getMockRevive(t)
+
+	failuresChan, err := revive.Lint(revivelib.Include("../testdata/if-return.go"))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// ACT
+	failures, exitCode, err := revive.Format("stylish", failuresChan)
+
+	// ASSERT
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	errorMsgs := []string{
+		"(15, 2)  https://revive.run/r#if-return  redundant if ...; err != nil check, just return error instead.",
+		"(88, 3)  https://revive.run/r#if-return  redundant if ...; err != nil check, just return error instead.",
+		"(95, 3)  https://revive.run/r#if-return  redundant if ...; err != nil check, just return error instead.",
+	}
+	for _, errorMsg := range errorMsgs {
+		if !strings.Contains(failures, errorMsg) {
+			t.Fatalf("Expected formatted failures '%s' to contain '%s', but it didn't.", failures, errorMsg)
+		}
+	}
+
+	const expected = 1
+	if exitCode != expected {
+		t.Fatalf("Expected exit code to be %d, but it was %d.", expected, exitCode)
+	}
+}
+
+type mockRule struct {
+}
+
+func (r *mockRule) Name() string {
+	return "mock-rule"
+}
+
+func (r *mockRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure {
+	return nil
+}
+
+func getMockRevive(t *testing.T) *revivelib.Revive {
+	t.Helper()
+
+	conf, err := config.GetConfig("../defaults.toml")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	revive, err := revivelib.New(
+		conf,
+		true,
+		2048,
+		revivelib.NewExtraRule(&mockRule{}, lint.RuleConfig{}),
+	)
+	if err != nil {
+		t.Fatal(err.Error())
+	}
+
+	return revive
+}
--- a/revivelib/extra_rule.go
+++ b/revivelib/extra_rule.go
@ -0,0 +1,17 @@
+package revivelib
+
+import "github.com/mgechev/revive/lint"
+
+// 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,
+	}
+}
--- a/revivelib/pattern.go
+++ b/revivelib/pattern.go
@ -0,0 +1,33 @@
+package revivelib
+
+// LintPattern indicates a pattern to be included/excluded when linting
+type LintPattern struct {
+	isExclude bool
+	pattern   string
+}
+
+// IsExclude - should this pattern be included or excluded when linting
+func (p *LintPattern) IsExclude() bool {
+	return p.isExclude
+}
+
+// GetPattern - returns the actual pattern
+func (p *LintPattern) GetPattern() string {
+	return p.pattern
+}
+
+// Include this pattern when linting
+func Include(pattern string) *LintPattern {
+	return &LintPattern{
+		isExclude: false,
+		pattern:   pattern,
+	}
+}
+
+// Exclude this pattern when linting
+func Exclude(pattern string) *LintPattern {
+	return &LintPattern{
+		isExclude: true,
+		pattern:   pattern,
+	}
+}