diff --git a/docs/changelog.md b/docs/changelog.md index fa526acd..86e8faa6 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -10,6 +10,8 @@ rules. - pint will now try to create fewer BitBucket comments by merging multiple problem reports into a single comment. +- Control comment handling code was refactored, there are some additional rules + that comment must follow. See `Control comments` section in [pint docs](./index.md). ## v0.49.2 diff --git a/docs/index.md b/docs/index.md index e6111b2d..7abd87ef 100644 --- a/docs/index.md +++ b/docs/index.md @@ -209,6 +209,31 @@ Here's an example alert you can use for problems detected by pint: {% endraw %} +## Control comments + +There is a number of comments you can add to your rule files in order to change +pint behaviour, some of them allow you to exclude selected files or line, see +[docs here](./ignoring.md) for details. + +There are a few requirement for any comment to be recognized by pint: + +- All comments must have a `pint` prefix. +- All comments must have at least one space between `#` symbol and `pint` prefix. + +Good comment examples: + +```yaml +# pint file/owner bob +# pint file/owner bob +``` + +Bad comment examples: + +```yaml +# file/owner bob +#pint file/owner bob +``` + ## Release Notes See [changelog](changelog.md) for history of changes. diff --git a/internal/comments/comments.go b/internal/comments/comments.go index 4cdf86ad..a7b9319c 100644 --- a/internal/comments/comments.go +++ b/internal/comments/comments.go @@ -8,7 +8,7 @@ import ( "unicode" ) -type Type int +type Type uint8 const ( UnknownType Type = iota @@ -270,7 +270,7 @@ func parseComment(s string) (c Comment, err error) { func Parse(text string) (comments []Comment) { sc := bufio.NewScanner(strings.NewReader(text)) for sc.Scan() { - elems := strings.SplitN(sc.Text(), "#", 2) + elems := strings.SplitN(sc.Text(), "# ", 2) if len(elems) != 2 { continue } diff --git a/internal/comments/comments_test.go b/internal/comments/comments_test.go index 58f1dd9d..f8d4648e 100644 --- a/internal/comments/comments_test.go +++ b/internal/comments/comments_test.go @@ -194,6 +194,15 @@ func TestParse(t *testing.T) { }, }, }, + { + input: `# pint disable promql/series(http_errors_total{label="this has spaces and a # symbol"})`, + output: []comments.Comment{ + { + Type: comments.DisableType, + Value: comments.Disable{Match: `promql/series(http_errors_total{label="this has spaces and a # symbol"})`}, + }, + }, + }, { input: "# pint file/snooze", output: []comments.Comment{ @@ -290,6 +299,18 @@ func TestParse(t *testing.T) { }, }, }, + { + input: `# pint snooze 2023-12-31 promql/series(http_errors_total{label="this has spaces"})`, + output: []comments.Comment{ + { + Type: comments.SnoozeType, + Value: comments.Snooze{ + Until: parseUntil("2023-12-31T00:00:00Z"), + Match: `promql/series(http_errors_total{label="this has spaces"})`, + }, + }, + }, + }, { input: "# pint rule/set", output: []comments.Comment{ @@ -339,6 +360,14 @@ func TestParse(t *testing.T) { }, }, }, + { + input: "{#- comment #} # pint ignore/line", + output: []comments.Comment{ + { + Type: comments.IgnoreLineType, + }, + }, + }, } for _, tc := range testCases { diff --git a/internal/discovery/discovery.go b/internal/discovery/discovery.go index 02f3a0a2..f0b0909f 100644 --- a/internal/discovery/discovery.go +++ b/internal/discovery/discovery.go @@ -96,7 +96,7 @@ type Entry struct { func readRules(reportedPath, sourcePath string, r io.Reader, isStrict bool) (entries []Entry, err error) { p := parser.NewParser() - content, err := parser.ReadContent(r) + content, fileComments, err := parser.ReadContent(r) if err != nil { return nil, err } @@ -106,11 +106,9 @@ func readRules(reportedPath, sourcePath string, r io.Reader, isStrict bool) (ent contentLines = append(contentLines, i) } - body := string(content.Body) - var fileOwner string var disabledChecks []string - for _, comment := range comments.Parse(body) { + for _, comment := range fileComments { // nolint:exhaustive switch comment.Type { case comments.FileOwnerType: diff --git a/internal/parser/read.go b/internal/parser/read.go index b73133f9..72878175 100644 --- a/internal/parser/read.go +++ b/internal/parser/read.go @@ -9,7 +9,7 @@ import ( "github.com/cloudflare/pint/internal/comments" ) -type skipMode int +type skipMode uint8 const ( skipNone skipMode = iota @@ -20,10 +20,6 @@ const ( skipFile ) -func removeRedundantSpaces(line string) string { - return strings.Join(strings.Fields(line), " ") -} - func emptyLine(line string) (emptied string) { preComment := strings.TrimSuffix(line, "\n") var comment string @@ -41,45 +37,12 @@ func emptyLine(line string) (emptied string) { return emptied } -func hasComment(line, comment string) bool { - sc := bufio.NewScanner(strings.NewReader(line)) - for sc.Scan() { - elems := strings.Split(sc.Text(), "#") - lastComment := elems[len(elems)-1] - parts := strings.SplitN(removeRedundantSpaces(lastComment), " ", 2) - if len(parts) < 2 { - continue - } - if parts[0] == comments.Prefix && parts[1] == comment { - return true - } - } - return false -} - -func parseSkipComment(line string) (skipMode, bool) { - switch { - case hasComment(line, comments.IgnoreFileComment): - return skipFile, true - case hasComment(line, comments.IgnoreLineComment): - return skipCurrentLine, true - case hasComment(line, comments.IgnoreNextLineComment): - return skipNextLine, true - case hasComment(line, comments.IgnoreBeginComment): - return skipBegin, true - case hasComment(line, comments.IgnoreEndComment): - return skipEnd, true - default: - return skipNone, false - } -} - type Content struct { Body []byte Ignored bool } -func ReadContent(r io.Reader) (out Content, err error) { +func ReadContent(r io.Reader) (out Content, fileComments []comments.Comment, err error) { reader := bufio.NewReader(r) var line string var found bool @@ -98,7 +61,45 @@ func ReadContent(r io.Reader) (out Content, err error) { if skipAll { out.Body = append(out.Body, []byte(emptyLine(line))...) } else { - skip, found = parseSkipComment(line) + skip = skipNone + found = false + for _, comment := range comments.Parse(line) { + switch comment.Type { + case comments.IgnoreFileType: + skip = skipFile + found = true + case comments.IgnoreLineType: + skip = skipCurrentLine + found = true + case comments.IgnoreBeginType: + skip = skipBegin + found = true + case comments.IgnoreEndType: + skip = skipEnd + found = true + case comments.IgnoreNextLineType: + skip = skipNextLine + found = true + case comments.FileOwnerType: + fileComments = append(fileComments, comment) + case comments.RuleOwnerType: + // pass + case comments.FileDisableType: + fileComments = append(fileComments, comment) + case comments.DisableType: + // pass + case comments.FileSnoozeType: + fileComments = append(fileComments, comment) + case comments.SnoozeType: + // pass + case comments.RuleSetType: + // pass + case comments.InvalidComment: + fileComments = append(fileComments, comment) + case comments.UnknownType: + // pass + } + } switch { case found: switch skip { @@ -147,8 +148,8 @@ func ReadContent(r io.Reader) (out Content, err error) { } if !errors.Is(err, io.EOF) { - return out, err + return out, fileComments, err } - return out, nil + return out, fileComments, nil } diff --git a/internal/parser/read_test.go b/internal/parser/read_test.go index 7e08a987..cb9bd8ba 100644 --- a/internal/parser/read_test.go +++ b/internal/parser/read_test.go @@ -6,8 +6,10 @@ import ( "strings" "testing" + "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/require" + "github.com/cloudflare/pint/internal/comments" "github.com/cloudflare/pint/internal/parser" ) @@ -15,6 +17,7 @@ func TestReadContent(t *testing.T) { type testCaseT struct { input []byte output []byte + comments []comments.Comment ignored bool shouldError bool } @@ -77,12 +80,12 @@ func TestReadContent(t *testing.T) { output: []byte("# pint ignore/next-line \n \n"), }, { - input: []byte("#pint ignore/next-line \nfoo\n"), - output: []byte("#pint ignore/next-line \n \n"), + input: []byte("# pint ignore/next-line \nfoo\n"), + output: []byte("# pint ignore/next-line \n \n"), }, { - input: []byte("#pintignore/next-line\nfoo\n"), - output: []byte("#pintignore/next-line\nfoo\n"), + input: []byte("# pintignore/next-line\nfoo\n"), + output: []byte("# pintignore/next-line\nfoo\n"), }, { input: []byte("# pint ignore/next-linex\nfoo\n"), @@ -136,10 +139,21 @@ func TestReadContent(t *testing.T) { }, } + cmpErrorText := cmp.Comparer(func(x, y interface{}) bool { + xe := x.(error) + ye := y.(error) + return xe.Error() == ye.Error() + }) + sameErrorText := cmp.FilterValues(func(x, y interface{}) bool { + _, xe := x.(error) + _, ye := y.(error) + return xe && ye + }, cmpErrorText) + for i, tc := range testCases { t.Run(strconv.Itoa(i+1), func(t *testing.T) { r := bytes.NewReader(tc.input) - output, err := parser.ReadContent(r) + output, comments, err := parser.ReadContent(r) hadError := err != nil if hadError != tc.shouldError { @@ -154,6 +168,11 @@ func TestReadContent(t *testing.T) { return } + if diff := cmp.Diff(tc.comments, comments, sameErrorText); diff != "" { + t.Errorf("ReadContent() returned wrong comments (-want +got):\n%s", diff) + return + } + require.Equal(t, string(tc.output), string(output.Body), "ReadContent() returned wrong output") require.Equal(t, tc.ignored, output.Ignored, "ReadContent() returned wrong Ignored value") })