ruleguard: implement MatchComment function from the DSL

Most `Where()` filters work for comments as one would expect.

Named captures `?P<name>` can be accessed via `m["name"]`.

`Report()` and `Suggest()` interpolation can work with named captures.
The `$$` variable is bound to the entire regexp match (like `$0`).

_docs/dsl.md

@@ -31,7 +31,7 @@ Every **matcher function** accepts exactly 1 argument, a [`dsl.Matcher`](https:/
 Every **rule** definition starts with a [`Match()`](https://godoc.org/github.com/quasilyte/go-ruleguard/dsl#Matcher.Match) or [`MatchComment()`](https://godoc.org/github.com/quasilyte/go-ruleguard/dsl#Matcher.MatchComment) method call.
 
 * For `Match()`, you specify one or more [AST patterns](https://github.com/mvdan/gogrep) that should represent what kind of Go code a rule is supposed to match.
-* For `MatchComment()`, you provide one or more regular expressions that sould match a comment of interest.
+* For `MatchComment()`, you provide one or more regular expressions that should match a comment of interest.
 
 Another mandatory part is [`Report()`](https://godoc.org/github.com/quasilyte/go-ruleguard/dsl#Matcher.Report) or [`Suggest()`](https://godoc.org/github.com/quasilyte/go-ruleguard/dsl#Matcher.Suggest) that describe a rule match action. `Report()` will print a warning message while `Suggest()` can be used to provide a quickfix action (a syntax rewrite pattern).
 
@@ -240,17 +240,7 @@ m.Match(`!!$x`).Suggest(`$x`)
 m.Match(`!!$x`).Suggest(`$x`).Report(`suggested: $x`)
 ```
 
-Be careful when using `Suggest()` with `MatchComment()`. An entire comment node will be rewritten by the rule, so in this case you can't rely on the partial matching. Match an entire comment and use named groups to reconstruct the fixed form of the comment:
-
-```go
-// Bad! If the comment has some other text, it will be removed.
-// "// nolint foo bar" => "//nolint"
-m.MatchComment(`// nolint`).Suggest(`//nolint`)
-
-// Good. No information loss in this case.
-// "// nolint foo bar" => "//nolint foo bar"
-m.MatchComment(`// nolint(?P<rest>)`).Suggest(`//nolint$rest`)
-```
+Be careful when using `Suggest()` with `MatchComment()`. As regexp may match a subset of the comment, you'll replace that exact comment portion with `Suggest()` pattern. If you want to replace an entire comment, be sure that your pattern contains `^` and `$` anchors.
 
 ## Ruleguard bundles
 

analyzer/analyzer_test.go

@@ -23,6 +23,7 @@ func TestAnalyzer(t *testing.T) {
 		"quasigo",
 		"matching",
 		"dgryski",
+		"comments",
 	}
 
 	analyzer.ForceNewEngine = true

analyzer/testdata/src/comments/rules.go

@@ -0,0 +1,66 @@
+// +build ignore
+
+package gorules
+
+import "github.com/quasilyte/go-ruleguard/dsl"
+
+func nolintFormat(m dsl.Matcher) {
+	m.MatchComment(`// nolint(?:$|\s)`).Report(`remove a space between // and "nolint" directive`)
+
+	// Using ?s flag to match multiline text.
+	m.MatchComment(`(?s)/\*.*nolint.*`).
+		Report(`don't put "nolint" inside a multi-line comment`)
+}
+
+// Testing a pattern without capture groups.
+func nolintGocritic(m dsl.Matcher) {
+	m.MatchComment(`//nolint:gocritic`).
+		Report(`hey, this is kinda upsetting`)
+}
+
+// Testing Suggest.
+func nolint2quickfix(m dsl.Matcher) {
+	m.MatchComment(`// nolint2(?P<rest>.*)`).Suggest(`//nolint2$rest`)
+}
+
+// Testing named groups.
+func directiveFormat(m dsl.Matcher) {
+	m.MatchComment(`/\*(?P<directive>[\w-]+):`).
+		Report(`directive should be written as //$directive`)
+}
+
+// Testing Where clause.
+func forbiddenGoDirective(m dsl.Matcher) {
+	m.MatchComment(`//go:(?P<x>\w+)`).
+		Where(m["x"].Text != "generate" && !m["x"].Text.Matches(`embed|noinline`)).
+		Report("don't use $x go directive")
+}
+
+// Test that file-related things work for MatchComment too.
+func fooDirectives(m dsl.Matcher) {
+	m.MatchComment(`//go:embed`).
+		Where(m.File().Name.Matches(`^.*_foo.go$`)).
+		Report("don't use go:embed in _foo files")
+}
+
+// Test multi-pattern matching.
+// Also test $$ interpolation.
+func commentTypo(m dsl.Matcher) {
+	m.MatchComment(`begining`, `bizzare`).Report(`"$$" may contain a typo`)
+}
+
+// Test $$ with a more complex regexp that captures something.
+func commentTypo2(m dsl.Matcher) {
+	m.MatchComment(`(?P<word>buisness) advice`).Report(`"$$" may contain a typo`)
+}
+
+// Test mixture of the named and unnamed captures.
+func commentTypo3(m dsl.Matcher) {
+	m.MatchComment(`(?P<first>calender)|(error)`).Report(`first=$first`)
+	m.MatchComment(`(error)|(?P<second>cemetary)`).Report(`second=$second`)
+}
+
+// Test a case where named group is empty.
+func commentTypo4(m dsl.Matcher) {
+	m.MatchComment(`(?P<x>collegue)|(commitee)`).Report(`x="$x"`)
+}

analyzer/testdata/src/comments/target.go

@@ -0,0 +1,33 @@
+package comments
+
+//go:embed
+
+/*foo-bar: baz*/ // want `\Qdirective should be written as //foo-bar`
+
+/*go:noinline*/ // want `\Qdirective should be written as //go`
+func f() {
+	_ = 12 // nolint // want `\Qremove a space between // and "nolint" directive`
+
+	_ = 30 // nolint2 foo bar // want `\Qsuggestion: //nolint2 foo bar`
+
+	/*
+		nolint // want `\Qdon't put "nolint" inside a multi-line comment`
+	*/
+
+	//go:baddirective // want `\Qdon't use baddirective go directive`
+	//go:noinline
+	//go:generate foo bar
+
+	//nolint:gocritic // want `hey, this is kinda upsetting`
+
+	// This is a begining // want `\Q"begining" may contain a typo`
+	// Of a bizzare text with typos. // want `\Q"bizzare" may contain a typo`
+
+	// I can't give you a buisness advice. // want `\Q"buisness advice" may contain a typo`
+
+	// calender // want `\Qfirst=calender`
+	// cemetary // want `\Qsecond=cemetary`
+
+	// collegue // want `\Qx="collegue"`
+	// commitee // want `\Qx=""`
+}

analyzer/testdata/src/comments/target_foo.go

@@ -0,0 +1,3 @@
+package comments
+
+//go:embed data.txt // want `don't use go:embed in _foo files`

go.mod

@@ -5,7 +5,7 @@ go 1.15
 require (
 	github.com/go-toolsmith/astequal v1.0.0
 	github.com/google/go-cmp v0.5.2
-	github.com/quasilyte/go-ruleguard/dsl v0.3.1
+	github.com/quasilyte/go-ruleguard/dsl v0.3.2
 	github.com/quasilyte/go-ruleguard/rules v0.0.0-20210203162857-b223e0831f88
 	golang.org/x/tools v0.0.0-20201230224404-63754364767c
 )

go.sum

@@ -13,6 +13,8 @@ github.com/quasilyte/go-ruleguard/dsl v0.3.0 h1:+KRgVKXGdkhPCJdHD3rszyiuFHmMy/av
 github.com/quasilyte/go-ruleguard/dsl v0.3.0/go.mod h1:KeCP03KrjuSO0H1kTuZQCWlQPulDV6YMIXmpQss17rU=
 github.com/quasilyte/go-ruleguard/dsl v0.3.1 h1:CHGOKP2LDz35P49TjW4Bx4BCfFI6ZZU/8zcneECD0q4=
 github.com/quasilyte/go-ruleguard/dsl v0.3.1/go.mod h1:KeCP03KrjuSO0H1kTuZQCWlQPulDV6YMIXmpQss17rU=
+github.com/quasilyte/go-ruleguard/dsl v0.3.2 h1:ULi3SLXvDUgb0u2IM5xU6er9KeWBSaUh1NlDjCgLHU8=
+github.com/quasilyte/go-ruleguard/dsl v0.3.2/go.mod h1:KeCP03KrjuSO0H1kTuZQCWlQPulDV6YMIXmpQss17rU=
 github.com/quasilyte/go-ruleguard/rules v0.0.0-20201231183845-9e62ed36efe1 h1:PX/E0GYUnSV8vwVfpOUEIBKnPG3KmYunmNOBlL+zDko=
 github.com/quasilyte/go-ruleguard/rules v0.0.0-20201231183845-9e62ed36efe1/go.mod h1:7JTjp89EGyU1d6XfBiXihJNG37wB2VRkd125Q1u7Plc=
 github.com/quasilyte/go-ruleguard/rules v0.0.0-20210203162857-b223e0831f88 h1:PeTrJiH/dSeruL/Z9Db39NRMwI/yoA3oHCdCkg+Wh8A=

ruleguard/debug_test.go

@@ -15,6 +15,15 @@ import (
 
 func TestDebug(t *testing.T) {
 	allTests := map[string]map[string][]string{
+		`m.MatchComment("// (?P<x>\\w+)").Where(m["x"].Text.Matches("^Test"))`: {
+			`// TestFoo`: nil,
+
+			`// Foo`: {
+				`input.go:4: [rules.go:5] rejected by m["x"].Text.Matches("^Test")`,
+				`  $x: Foo`,
+			},
+		},
+
 		`m.Match("f($x)").Where(m["x"].Type.Is("string"))`: {
 			`f("abc")`: nil,
 
@@ -200,7 +209,7 @@ func newDebugTestRunner(input string) (*debugTestRunner, error) {
 		var sink interface{}`, input)
 
 	fset := token.NewFileSet()
-	f, err := parser.ParseFile(fset, "input.go", []byte(fullInput), 0)
+	f, err := parser.ParseFile(fset, "input.go", []byte(fullInput), parser.ParseComments)
 	if err != nil {
 		return nil, err
 	}

ruleguard/filters.go

@@ -212,7 +212,7 @@ func makeValueIntFilter(src, varname string, op token.Token, rhsVarname string)
 
 func makeTextConstFilter(src, varname string, op token.Token, rhsValue constant.Value) filterFunc {
 	return func(params *filterParams) matchFilterResult {
-		s := params.nodeText(params.subExpr(varname))
+		s := params.nodeText(params.subNode(varname))
 		lhsValue := constant.MakeString(string(s))
 		if constant.Compare(lhsValue, op, rhsValue) {
 			return filterSuccess
@@ -223,7 +223,7 @@ func makeTextConstFilter(src, varname string, op token.Token, rhsValue constant.
 
 func makeTextFilter(src, varname string, op token.Token, rhsVarname string) filterFunc {
 	return func(params *filterParams) matchFilterResult {
-		s1 := params.nodeText(params.subExpr(varname))
+		s1 := params.nodeText(params.subNode(varname))
 		lhsValue := constant.MakeString(string(s1))
 		n, _ := params.match.CapturedByName(rhsVarname)
 		s2 := params.nodeText(n)
@@ -237,14 +237,15 @@ func makeTextFilter(src, varname string, op token.Token, rhsVarname string) filt
 
 func makeTextMatchesFilter(src, varname string, re *regexp.Regexp) filterFunc {
 	return func(params *filterParams) matchFilterResult {
-		if re.Match(params.nodeText(params.subExpr(varname))) {
+		if re.Match(params.nodeText(params.subNode(varname))) {
 			return filterSuccess
 		}
 		return filterFailure(src)
 	}
 }
 
 func makeNodeIsFilter(src, varname string, tag nodetag.Value) filterFunc {
+	// TODO: add comment nodes support?
 	return func(params *filterParams) matchFilterResult {
 		n := params.subExpr(varname)
 		var matched bool

ruleguard/gorule.go

@@ -5,6 +5,7 @@ import (
 	"go/ast"
 	"go/token"
 	"go/types"
+	"regexp"
 
 	"github.com/quasilyte/go-ruleguard/internal/gogrep"
 	"github.com/quasilyte/go-ruleguard/nodetag"
@@ -20,10 +21,13 @@ type goRuleSet struct {
 type scopedGoRuleSet struct {
 	categorizedNum int
 	rulesByTag     [nodetag.NumBuckets][]goRule
+	commentRules   []goCommentRule
 }
 
-func (rset *scopedGoRuleSet) Empty() bool {
-	return rset.categorizedNum != 0
+type goCommentRule struct {
+	base          goRule
+	pat           *regexp.Regexp
+	captureGroups bool
 }
 
 type goRule struct {
@@ -58,14 +62,19 @@ type filterParams struct {
 
 	importer *goImporter
 
-	match gogrep.MatchData
+	match matchData
 
 	nodeText func(n ast.Node) []byte
 
 	// varname is set only for custom filters before bytecode function is called.
 	varname string
 }
 
+func (params *filterParams) subNode(name string) ast.Node {
+	n, _ := params.match.CapturedByName(name)
+	return n
+}
+
 func (params *filterParams) subExpr(name string) ast.Expr {
 	n, _ := params.match.CapturedByName(name)
 	switch n := n.(type) {
@@ -122,6 +131,7 @@ func appendScopedRuleSet(dst, src *scopedGoRuleSet) *scopedGoRuleSet {
 		dst.rulesByTag[tag] = append(dst.rulesByTag[tag], cloneRuleSlice(rules)...)
 		dst.categorizedNum += len(rules)
 	}
+	dst.commentRules = append(dst.commentRules, src.commentRules...)
 	return dst
 }
 

ruleguard/match_data.go

@@ -0,0 +1,46 @@
+package ruleguard
+
+import (
+	"go/ast"
+
+	"github.com/quasilyte/go-ruleguard/internal/gogrep"
+)
+
+// matchData is used to handle both regexp and AST match sets in the same way.
+type matchData interface {
+	// TODO: don't use gogrep.CapturedNode type here.
+
+	Node() ast.Node
+	CaptureList() []gogrep.CapturedNode
+	CapturedByName(name string) (ast.Node, bool)
+}
+
+type commentMatchData struct {
+	node    ast.Node
+	capture []gogrep.CapturedNode
+}
+
+func (m commentMatchData) Node() ast.Node { return m.node }
+
+func (m commentMatchData) CaptureList() []gogrep.CapturedNode { return m.capture }
+
+func (m commentMatchData) CapturedByName(name string) (ast.Node, bool) {
+	for _, c := range m.capture {
+		if c.Name == name {
+			return c.Node, true
+		}
+	}
+	return nil, false
+}
+
+type astMatchData struct {
+	match gogrep.MatchData
+}
+
+func (m astMatchData) Node() ast.Node { return m.match.Node }
+
+func (m astMatchData) CaptureList() []gogrep.CapturedNode { return m.match.Capture }
+
+func (m astMatchData) CapturedByName(name string) (ast.Node, bool) {
+	return m.match.CapturedByName(name)
+}