Skip to content

Commit

Permalink
per-rule file exclude filters (mgechev#850)
Browse files Browse the repository at this point in the history
  • Loading branch information
comdiv committed Aug 5, 2023
1 parent df39256 commit a91f203
Show file tree
Hide file tree
Showing 11 changed files with 378 additions and 0 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ vendor
*.swp
dist/
*.log
.vscode/settings.json
28 changes: 28 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,34 @@ warningCode = 0
[rule.redefines-builtin-id]
```

### Rule-level file excludes

You also can setup custom excludes for each rule.

It's alternative for global `-exclude` program arg.

```toml
ignoreGeneratedHeader = false
severity = "warning"
confidence = 0.8
errorCode = 0
warningCode = 0

[rule.blank-imports]
exclude=["**/*.pb.go"]
[rule.context-as-argument]
exclude=["src/somepkg/*.go", "TEST"]
```

You can use following exclude patterns

1. full paths to files `src/pkg/mypkg/some.go`
2. globs `src/**/*.pb.go`
3. regexes (should have prefix ~, will be ignored) `~\.(pb|auto|generated)\.go$`
4. well-known `TEST` (same as `**/*_test.go`)

> NOTE: do not mess with `exclude` that can be used at top level of TOML file, that mean "exclude package patterns", not "exclude file patterns"
## Available Rules

List of all available rules. The rules ported from `golint` are left unchanged and indicated in the `golint` column.
Expand Down
8 changes: 8 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,14 @@ func parseConfig(path string, config *lint.Config) error {
if err != nil {
return fmt.Errorf("cannot parse the config file: %v", err)
}
for k, r := range config.Rules {
err := r.Initialize()
if err != nil {
return fmt.Errorf("error in config of rule [%s] : [%v]", k, err)
}
config.Rules[k] = r
}

return nil
}

Expand Down
21 changes: 21 additions & 0 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,27 @@ func TestGetConfig(t *testing.T) {
}
})
}

t.Run("rule-level file filter excludes", func(t *testing.T) {
cfg, err := GetConfig("testdata/rule-level-exclude-850.toml")
if err != nil {
t.Fatal("should be valid config")
}
r1 := cfg.Rules["r1"]
if len(r1.Exclude) > 0 {
t.Fatal("r1 should have empty excludes")
}
r2 := cfg.Rules["r2"]
if len(r2.Exclude) != 1 {
t.Fatal("r2 should have execlude set")
}
if r2.Match(&lint.File{Name: "some/file.go"}) {
t.Fatal("r2 should be initialized and exclude some/file.go")
}
if !r2.Match(&lint.File{Name: "some/any-other.go"}) {
t.Fatal("r2 should not exclude some/any-other.go")
}
})
}

func TestGetLintingRules(t *testing.T) {
Expand Down
13 changes: 13 additions & 0 deletions config/testdata/rule-level-exclude-850.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
ignoreGeneratedHeader = false
severity = "warning"
confidence = 0.8
errorCode = 0
warningCode = 0

enableAllRules = false

[rule.r1]
# no excludes

[rule.r2]
exclude=["some/file.go"]
28 changes: 28 additions & 0 deletions lint/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,44 @@ package lint
// Arguments is type used for the arguments of a rule.
type Arguments = []interface{}

type FileFilters = []*FileFilter

// RuleConfig is type used for the rule configuration.
type RuleConfig struct {
Arguments Arguments
Severity Severity
Disabled bool
// Exclude - rule-level file excludes, TOML related (strings)
Exclude []string
// excludeFilters - regex-based file filters, initialized from Exclude
excludeFilters []*FileFilter
}

// Initialize - should be called after reading from TOML file
func (rc *RuleConfig) Initialize() error {
for _, f := range rc.Exclude {
ff, err := ParseFileFilter(f)
if err != nil {
return err
}
rc.excludeFilters = append(rc.excludeFilters, ff)
}
return nil
}

// RulesConfig defines the config for all rules.
type RulesConfig = map[string]RuleConfig

// Match - checks if given [File] `f` should be covered with configured rule (not excluded)
func (rcfg *RuleConfig) Match(f *File) bool {
for _, exclude := range rcfg.excludeFilters {
if exclude.Match(f) {
return false
}
}
return true
}

// DirectiveConfig is type used for the linter directive configuration.
type DirectiveConfig struct {
Severity Severity
Expand Down
3 changes: 3 additions & 0 deletions lint/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ func (f *File) lint(rules []Rule, config Config, failures chan Failure) {
disabledIntervals := f.disabledIntervals(rules, mustSpecifyDisableReason, failures)
for _, currentRule := range rules {
ruleConfig := rulesConfig[currentRule.Name()]
if !ruleConfig.Match(f) {
continue
}
currentFailures := currentRule.Apply(f, ruleConfig.Arguments)
for idx, failure := range currentFailures {
if failure.RuleName == "" {
Expand Down
126 changes: 126 additions & 0 deletions lint/filefilter.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
package lint

import (
"fmt"
"regexp"
"strings"
)

// FileFilter - file filter to exclude some files for rule
// supports whole
// 1. file/dir names : pkg/mypkg/my.go,
// 2. globs: **/*.pb.go,
// 3. regexes (~ prefix) ~-tmp\.\d+\.go
// 4. special test marker `TEST` - treats as `~_test\.go`
type FileFilter struct {
// raw definition of filter inside config
raw string
// don't care what was at start, will use regexes inside
rx *regexp.Regexp
// marks that it was empty rule that matches everything
matchesAll bool
}

// ParseFileFilter - creates [FileFilter] for given raw filter
// if empty string, or `*`, or `~` is used it means "always true"
// while regexp could be invalid, it could return it's compilation error
func ParseFileFilter(cfgFilter string) (*FileFilter, error) {
cfgFilter = strings.TrimSpace(cfgFilter)
result := new(FileFilter)
result.raw = cfgFilter
result.matchesAll = len(result.raw) == 0 || result.raw == "*" || result.raw == "~"
if !result.matchesAll {
if err := result.prepareRegexp(); err != nil {
return nil, err
}
}
return result, nil
}

func (ff *FileFilter) String() string { return ff.raw }

// MatchFileName - checks if file name matches filter
func (ff *FileFilter) MatchFileName(name string) bool {
if ff.matchesAll {
return true
}
name = strings.ReplaceAll(name, "\\", "/")
return ff.rx.MatchString(name)
}

// Match - checks if given [File] matches filter
func (ff *FileFilter) Match(f *File) bool {
return ff.MatchFileName(f.Name)
}

var fileFilterInvalidGlobRegexp = regexp.MustCompile(`[^/]\*\*[^/]`)
var escapeRegexSymbols = ".+{}()[]^$"

func (ff *FileFilter) prepareRegexp() error {
var err error
var src = ff.raw
if src == "TEST" {
src = "~_test\\.go"
}
if strings.HasPrefix(src, "~") {
ff.rx, err = regexp.Compile(src[1:])
if err != nil {
return fmt.Errorf("invalid file filter [%s], regexp compile error: [%v]", ff.raw, err)
}
return nil
}
/* globs */
if strings.Contains(src, "*") {
if fileFilterInvalidGlobRegexp.MatchString(src) {
return fmt.Errorf("invalid file filter [%s], invalid glob pattern", ff.raw)
}
var rxBuild strings.Builder
rxBuild.WriteByte('^')
wasStar := false
justDirGlob := false
for _, c := range src {
if c == '*' {
if wasStar {
rxBuild.WriteString(`[\s\S]*`)
wasStar = false
justDirGlob = true
continue
}
wasStar = true
continue
}
if wasStar {
rxBuild.WriteString("[^/]*")
wasStar = false
}
if strings.ContainsRune(escapeRegexSymbols, c) {
rxBuild.WriteByte('\\')
}
rxBuild.WriteRune(c)
if c == '/' && justDirGlob {
rxBuild.WriteRune('?')
}
justDirGlob = false
}
if wasStar {
rxBuild.WriteString("[^/]*")
}
rxBuild.WriteByte('$')
ff.rx, err = regexp.Compile(rxBuild.String())
if err != nil {
return fmt.Errorf("invalid file filter [%s], regexp compile error after glob expand: [%v]", ff.raw, err)
}
return nil
}

// it's whole file mask, just escape dots and normilze separators
fillRx := src
fillRx = strings.ReplaceAll(fillRx, "\\", "/")
fillRx = strings.ReplaceAll(fillRx, ".", `\.`)
fillRx = "^" + fillRx + "$"
ff.rx, err = regexp.Compile(fillRx)
if err != nil {
return fmt.Errorf("invalid file filter [%s], regexp compile full path: [%v]", ff.raw, err)
}
return nil
}
100 changes: 100 additions & 0 deletions lint/filefilter_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
package lint_test

import (
"testing"

"github.com/mgechev/revive/lint"
)

func TestFileFilter(t *testing.T) {
t.Run("whole file name", func(t *testing.T) {
ff, err := lint.ParseFileFilter("a/b/c.go")
if err != nil {
t.Fatal(err)
}
if !ff.MatchFileName("a/b/c.go") {
t.Fatal("should match a/b/c.go")
}
if ff.MatchFileName("a/b/d.go") {
t.Fatal("should not match")
}
})

t.Run("regex", func(t *testing.T) {
ff, err := lint.ParseFileFilter("~b/[cd].go$")
if err != nil {
t.Fatal(err)
}
if !ff.MatchFileName("a/b/c.go") {
t.Fatal("should match a/b/c.go")
}
if !ff.MatchFileName("b/d.go") {
t.Fatal("should match b/d.go")
}
if ff.MatchFileName("b/x.go") {
t.Fatal("should not match b/x.go")
}
})

t.Run("TEST well-known", func(t *testing.T) {
ff, err := lint.ParseFileFilter("TEST")
if err != nil {
t.Fatal(err)
}
if !ff.MatchFileName("a/b/c_test.go") {
t.Fatal("should match a/b/c_test.go")
}
if ff.MatchFileName("a/b/c_test_no.go") {
t.Fatal("should not match a/b/c_test_no.go")
}
})

t.Run("glob *", func(t *testing.T) {
ff, err := lint.ParseFileFilter("a/b/*.pb.go")
if err != nil {
t.Fatal(err)
}
if !ff.MatchFileName("a/b/xxx.pb.go") {
t.Fatal("should match a/b/xxx.pb.go")
}
if !ff.MatchFileName("a/b/yyy.pb.go") {
t.Fatal("should match a/b/yyy.pb.go")
}
if ff.MatchFileName("a/b/xxx.nopb.go") {
t.Fatal("should not match a/b/xxx.nopb.go")
}
})

t.Run("glob **", func(t *testing.T) {
ff, err := lint.ParseFileFilter("a/**/*.pb.go")
if err != nil {
t.Fatal(err)
}
if !ff.MatchFileName("a/x/xxx.pb.go") {
t.Fatal("should match a/x/xxx.pb.go")
}
if !ff.MatchFileName("a/xxx.pb.go") {
t.Fatal("should match a/xxx.pb.go")
}
if !ff.MatchFileName("a/x/y/z/yyy.pb.go") {
t.Fatal("should match a/x/y/z/yyy.pb.go")
}
if ff.MatchFileName("a/b/xxx.nopb.go") {
t.Fatal("should not match a/b/xxx.nopb.go")
}
})

t.Run("just to cover Match", func(t *testing.T) {
ff, err := lint.ParseFileFilter("a/b/c.go")
if err != nil {
t.Fatal(err)
}
if !ff.Match(&lint.File{Name: "a/b/c.go"}) {
t.Fatal("should match")
}
if ff.Match(&lint.File{Name: "a/b/d.go"}) {
t.Fatal("should not match")
}
})

}
Loading

0 comments on commit a91f203

Please sign in to comment.