Report invalid comments

cmd/pint/scan.go

@@ -14,6 +14,7 @@ import (
 	"go.uber.org/atomic"
 
 	"github.com/cloudflare/pint/internal/checks"
+	"github.com/cloudflare/pint/internal/comments"
 	"github.com/cloudflare/pint/internal/config"
 	"github.com/cloudflare/pint/internal/discovery"
 	"github.com/cloudflare/pint/internal/output"
@@ -29,8 +30,9 @@ var (
 )
 
 const (
-	yamlParseReporter  = "yaml/parse"
-	ignoreFileReporter = "ignore/file"
+	yamlParseReporter   = "yaml/parse"
+	ignoreFileReporter  = "ignore/file"
+	pintCommentReporter = "pint/comment"
 )
 
 func tryDecodingYamlError(err error) (l int, s string) {
@@ -185,20 +187,35 @@ func scanWorker(ctx context.Context, jobs <-chan scanJob, results chan<- reporte
 		case <-ctx.Done():
 			return
 		default:
+			var commentErr comments.CommentError
+			var ignoreErr discovery.FileIgnoreError
 			switch {
-			case errors.Is(job.entry.PathError, discovery.ErrFileIsIgnored):
+			case errors.As(job.entry.PathError, &ignoreErr):
 				results <- reporter.Report{
 					ReportedPath:  job.entry.ReportedPath,
 					SourcePath:    job.entry.SourcePath,
 					ModifiedLines: job.entry.ModifiedLines,
 					Problem: checks.Problem{
-						Lines:    job.entry.ModifiedLines,
+						Lines:    []int{ignoreErr.Line},
 						Reporter: ignoreFileReporter,
-						Text:     "This file was excluded from pint checks.",
+						Text:     ignoreErr.Error(),
 						Severity: checks.Information,
 					},
 					Owner: job.entry.Owner,
 				}
+			case errors.As(job.entry.PathError, &commentErr):
+				results <- reporter.Report{
+					ReportedPath:  job.entry.ReportedPath,
+					SourcePath:    job.entry.SourcePath,
+					ModifiedLines: job.entry.ModifiedLines,
+					Problem: checks.Problem{
+						Lines:    []int{commentErr.Line},
+						Reporter: pintCommentReporter,
+						Text:     fmt.Sprintf("This comment is not a valid pint control comment: %s", commentErr.Error()),
+						Severity: checks.Warning,
+					},
+					Owner: job.entry.Owner,
+				}
 			case job.entry.PathError != nil:
 				line, e := tryDecodingYamlError(job.entry.PathError)
 				results <- reporter.Report{

cmd/pint/tests/0093_ci_bitbucket_ignore_file.txt

@@ -24,8 +24,8 @@ pint.ok --no-color -l debug ci
 ! stdout .
 stderr 'level=INFO msg="Problems found" Information=1'
 stderr 'result":"PASS"'
-stderr ',"line":7,'
-stderr '"message":"ignore/file:'
+stderr ',"line":3,'
+stderr '"message":"Problem reported on unmodified line 1, annotation moved here: ignore/file: This file was excluded from pint checks."'
 stderr 'msg="Sending a request to BitBucket" method=PUT'
 stderr 'msg="BitBucket request completed" status=200'
 stderr 'msg="Sending a request to BitBucket" method=DELETE'

cmd/pint/tests/0165_pint_comment_error.txt

@@ -0,0 +1,27 @@
+pint.ok --no-color lint --min-severity=info rules
+! stdout .
+cmp stderr stderr.txt
+
+-- stderr.txt --
+level=INFO msg="Finding all rules to check" paths=["rules"]
+rules/1.yml:4 Warning: This comment is not a valid pint control comment: unexpected comment suffix: "this line" (pint/comment)
+ 4 |   # pint ignore/line this line
+
+rules/2.yml:4 Information: This file was excluded from pint checks. (ignore/file)
+ 4 |   # pint ignore/file
+
+level=INFO msg="Problems found" Warning=1 Information=1
+-- rules/1.yml --
+groups:
+- name: g1
+  rules:
+  # pint ignore/line this line
+  - record: up:count
+    expr: count(up == 1)
+-- rules/2.yml --
+groups:
+- name: g1
+  rules:
+  # pint ignore/file
+  - record: up:count
+    expr: count(up == 1)

docs/changelog.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## v0.53.0
+
+### Added
+
+- pint will now report any comment that looks like [pint control comment](./ignoring.md)
+  but cannot be parsed correctly.
+
 ## v0.52.0
 
 ### Added

docs/checks/pint/comment.md

@@ -0,0 +1,48 @@
+---
+layout: default
+parent: Checks
+grand_parent: Documentation
+---
+
+# pint/comment
+
+You will only ever see this check reporting problems if you have a file
+with a comment that looks like one of pint control comments
+(see [page](../../ignoring.md)) but pint cannot parse it.
+
+This might be, for example, when you're trying to disable some checks
+for a specific rule using `# pint disable ...`, but the comment is not
+"touching" any rule and so pint cannot apply it.
+
+Valid rule specific comments:
+
+```yaml
+# pint disable promql/series(my_metric)
+- record: foo
+  expr: sum(my_metric) without(instance)
+
+- record: foo
+  # pint disable promql/series(my_metric)
+  expr: sum(my_metric) without(instance)
+```
+
+Invalid comment that's not attached to any rule:
+
+```yaml
+# pint disable promql/series(my_metric)
+
+- record: foo
+  expr: sum(my_metric) without(instance)
+```
+
+## Configuration
+
+This check doesn't have any configuration options.
+
+## How to enable it
+
+This check is enabled by default.
+
+## How to disable it
+
+You cannot disable this check.

internal/comments/comments.go

@@ -84,8 +84,17 @@ func parseType(s string) Type {
 	}
 }
 
+type CommentError struct {
+	Line int
+	Err  error
+}
+
+func (ce CommentError) Error() string {
+	return ce.Err.Error()
+}
+
 type Invalid struct {
-	Err error
+	Err CommentError
 }
 
 func (i Invalid) String() string {
@@ -287,19 +296,21 @@ func parseComment(s string) (parsed []Comment, err error) {
 	return parsed, err
 }
 
-func Parse(text string) (comments []Comment) {
+func Parse(lineno int, text string) (comments []Comment) {
 	sc := bufio.NewScanner(strings.NewReader(text))
+	var index int
 	for sc.Scan() {
 		line := sc.Text()
 		parsed, err := parseComment(line)
 		if err != nil {
 			comments = append(comments, Comment{
 				Type:  InvalidComment,
-				Value: Invalid{Err: err},
+				Value: Invalid{Err: CommentError{Line: lineno + index, Err: err}},
 			})
 			continue
 		}
 		comments = append(comments, parsed...)
+		index++
 	}
 	return comments
 }