Skip to content

Commit

Permalink
feat: Add include & exclude pattern support for the linting command. …
Browse files Browse the repository at this point in the history
…Move CI config include & exclude validation logic into common code"
  • Loading branch information
Joseph Burnitz committed Apr 7, 2024
1 parent 9a668ae commit ea3b6a6
Show file tree
Hide file tree
Showing 9 changed files with 184 additions and 22 deletions.
40 changes: 30 additions & 10 deletions cmd/pint/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,18 +56,38 @@ func actionLint(c *cli.Context) error {
return err
}

paths := c.Args().Slice()
if len(paths) == 0 {
return fmt.Errorf("at least one file or directory required")
// If there's includes and excludes in the config file make sure to get those
includeRe := []*regexp.Regexp{}
for _, pattern := range meta.cfg.Lint.Include {
includeRe = append(includeRe, regexp.MustCompile("^"+pattern+"$"))
}

slog.Info("Finding all rules to check", slog.Any("paths", paths))
finder := discovery.NewGlobFinder(paths, git.NewPathFilter(nil, nil, meta.cfg.Parser.CompileRelaxed()))
entries, err := finder.Find()
if err != nil {
return err
excludeRe := []*regexp.Regexp{}
for _, pattern := range meta.cfg.Lint.Exclude {
excludeRe = append(excludeRe, regexp.MustCompile("^"+pattern+"$"))
}

// Get any paths form the command line
cliPaths := c.Args().Slice()

// check to see we have at least something specified
if len(cliPaths) == 0 && len(meta.cfg.Lint.Include) == 0 {
return fmt.Errorf("at least one file or directory must be specified as a cli argument or in the configuration file")
}

// Find all matches from the include/exclude from the config file
var cfgEntries []discovery.Entry
filter := git.NewPathFilter(includeRe, excludeRe, meta.cfg.Parser.CompileRelaxed())

cfgEntries, cfgErr := discovery.NewGlobFinder([]string{"*"}, filter).Find()

cliEntries, cliErr := discovery.NewGlobFinder(cliPaths, filter).Find()
if cliErr != nil && cfgErr != nil {
return fmt.Errorf("error finding rules: %w %w", cliErr, cfgErr)
}

cfgEntries = append(cfgEntries, cliEntries...)

ctx := context.WithValue(context.Background(), config.CommandKey, config.LintCommand)

gen := config.NewPrometheusGenerator(meta.cfg, metricsRegistry)
Expand All @@ -77,13 +97,13 @@ func actionLint(c *cli.Context) error {
return err
}

summary, err := checkRules(ctx, meta.workers, meta.isOffline, gen, meta.cfg, entries)
summary, err := checkRules(ctx, meta.workers, meta.isOffline, gen, meta.cfg, cfgEntries)
if err != nil {
return err
}

if c.Bool(requireOwnerFlag) {
summary.Report(verifyOwners(entries, meta.cfg.Owners.CompileAllowed())...)
summary.Report(verifyOwners(cfgEntries, meta.cfg.Owners.CompileAllowed())...)
}

minSeverity, err := checks.ParseSeverity(c.String(minSeverityFlag))
Expand Down
19 changes: 19 additions & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,25 @@ owners {
If there's no `owners:allowed` configuration block, or if it's empty, then any
owner name is accepted.

## Lint

Check rules defined as cli arguments or matching a pattern specified in the hcl config file

Syntax:

```js
lint {
include = [ "(.*)", ... ]
exclude = [ "(.*)", ... ]
}
```

- `include` - list of file patterns to check when running checks. Only files
matching those regexp rules will be checked.
- `exclude` - list of file patterns to ignore when running linting.
This option takes precedence over `include`, so if a file path matches both
`include` & `exclude` patterns, it will be excluded.

## CI

Configure continuous integration environments.
Expand Down
9 changes: 9 additions & 0 deletions docs/examples/lint.hcl
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# This is an example config to be used when running pint as a linter

lint {
# Check all files inside rules/alerting and rules/recording dirs.
include = ["rules/(alerting|recording)/.+"]

# Ignore all *.md and *.txt files.
exclude = [".+.md", ".+.txt"]
}
17 changes: 5 additions & 12 deletions internal/config/ci.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package config

import (
"errors"
"regexp"
)

type CI struct {
Expand All @@ -17,18 +16,12 @@ func (ci CI) validate() error {
return errors.New("maxCommits cannot be <= 0")
}

for _, pattern := range ci.Include {
_, err := regexp.Compile(pattern)
if err != nil {
return err
}
if err := ValidatePaths(ci.Include); err != nil {
return err
}

for _, pattern := range ci.Exclude {
_, err := regexp.Compile(pattern)
if err != nil {
return err
}
if err := ValidatePaths(ci.Exclude); err != nil {
return err
}

return nil
}
2 changes: 2 additions & 0 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

type Config struct {
CI *CI `hcl:"ci,block" json:"ci,omitempty"`
Lint *Lint `hcl:"lint,block" json:"lint,omitempty"`
Parser *Parser `hcl:"parser,block" json:"parser,omitempty"`
Repository *Repository `hcl:"repository,block" json:"repository,omitempty"`
Discovery *Discovery `hcl:"discovery,block" json:"discovery,omitempty"`
Expand Down Expand Up @@ -220,6 +221,7 @@ func Load(path string, failOnMissing bool) (cfg Config, err error) {
MaxCommits: 20,
BaseBranch: "master",
},
Lint: &Lint{},
Parser: &Parser{},
Checks: &Checks{
Enabled: checks.CheckNames,
Expand Down
17 changes: 17 additions & 0 deletions internal/config/lint.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package config

type Lint struct {
Include []string `hcl:"include,optional" json:"include,omitempty"`
Exclude []string `hcl:"exclude,optional" json:"exclude,omitempty"`
}

func (lint Lint) validate() error {
if err := ValidatePaths(lint.Include); err != nil {
return err
}
if err := ValidatePaths(lint.Exclude); err != nil {
return err
}

return nil
}
50 changes: 50 additions & 0 deletions internal/config/lint_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package config

import (
"errors"
"fmt"
"testing"

"github.com/stretchr/testify/require"
)

func TestLintSettings(t *testing.T) {
type testCaseT struct {
err error
conf Lint
}

testCases := []testCaseT{
{
conf: Lint{
Include: []string{"foo/.+"},
Exclude: []string{"foo/.+"},
},
},
{
conf: Lint{
Include: []string{".+++"},
Exclude: []string{"foo/.+"},
},
err: errors.New("error parsing regexp: invalid nested repetition operator: `++`"),
},
{
conf: Lint{
Include: []string{"foo/.+"},
Exclude: []string{".+++"},
},
err: errors.New("error parsing regexp: invalid nested repetition operator: `++`"),
},
}

for _, tc := range testCases {
t.Run(fmt.Sprintf("%v", tc.conf), func(t *testing.T) {
err := tc.conf.validate()
if err == nil || tc.err == nil {
require.Equal(t, err, tc.err)
} else {
require.EqualError(t, err, tc.err.Error())
}
})
}
}
15 changes: 15 additions & 0 deletions internal/config/paths.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package config

import (
"regexp"
)

func ValidatePaths(paths []string) error {
for _, pattern := range paths {
_, err := regexp.Compile(pattern)
if err != nil {
return err
}
}
return nil
}
37 changes: 37 additions & 0 deletions internal/config/paths_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package config

import (
"errors"
"fmt"
"testing"

"github.com/stretchr/testify/require"
)

func TestValidatePaths(t *testing.T) {
type testCaseT struct {
err error
paths []string
}

testCases := []testCaseT{
{
paths: []string{"foo/.+"},
},
{
paths: []string{".+++"},
err: errors.New("error parsing regexp: invalid nested repetition operator: `++`"),
},
}

for _, tc := range testCases {
t.Run(fmt.Sprintf("%v", tc.paths), func(t *testing.T) {
err := ValidatePaths(tc.paths)
if err == nil || tc.err == nil {
require.Equal(t, err, tc.err)
} else {
require.EqualError(t, err, tc.err.Error())
}
})
}
}

0 comments on commit ea3b6a6

Please sign in to comment.