Document comment handling

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
 

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.

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
 		}

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 {

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:

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
 }

internal/parser/read_test.go

@@ -6,15 +6,18 @@ 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"
 )
 
 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")
 		})