ruleguard: add filter by tags for analyzer mode (#1205)

ruleguard `-enable` and `-disable` flags now support tags.

checkers/analyzer/run.go

@@ -7,9 +7,10 @@ import (
 	"strings"
 	"sync"
 
+	"golang.org/x/tools/go/analysis"
+
 	_ "github.com/go-critic/go-critic/checkers" // Register go-critic checkers
 	"github.com/go-critic/go-critic/framework/linter"
-	"golang.org/x/tools/go/analysis"
 )
 
 type gocritic struct {

checkers/ruleguard_checker.go

@@ -135,33 +135,68 @@ func newRuleguardChecker(info *linter.CheckerInfo, ctx *linter.CheckerContext) (
 
 	enabledGroups := make(map[string]bool)
 	disabledGroups := make(map[string]bool)
+	enabledTags := make(map[string]bool)
+	disabledTags := make(map[string]bool)
 
 	for _, g := range strings.Split(info.Params.String("disable"), ",") {
 		g = strings.TrimSpace(g)
+		if strings.HasPrefix(g, "#") {
+			disabledTags[strings.TrimPrefix(g, "#")] = true
+			continue
+		}
+
 		disabledGroups[g] = true
 	}
 	flagEnable := info.Params.String("enable")
 	if flagEnable != "<all>" {
 		for _, g := range strings.Split(flagEnable, ",") {
 			g = strings.TrimSpace(g)
+			if strings.HasPrefix(g, "#") {
+				enabledTags[strings.TrimPrefix(g, "#")] = true
+				continue
+			}
+
 			enabledGroups[g] = true
 		}
 	}
 	ruleguardDebug := os.Getenv("GOCRITIC_RULEGUARD_DEBUG") != ""
 
+	inEnabledTags := func(g *ruleguard.GoRuleGroup) bool {
+		for _, t := range g.DocTags {
+			if enabledTags[t] {
+				return true
+			}
+		}
+		return false
+	}
+	inDisabledTags := func(g *ruleguard.GoRuleGroup) string {
+		for _, t := range g.DocTags {
+			if disabledTags[t] {
+				return t
+			}
+		}
+		return ""
+	}
+
 	loadContext := &ruleguard.LoadContext{
 		Fset:         fset,
 		DebugImports: ruleguardDebug,
 		DebugPrint:   debugPrint,
 		GroupFilter: func(g *ruleguard.GoRuleGroup) bool {
+			enabled := flagEnable == "<all>" || enabledGroups[g.Name] || inEnabledTags(g)
 			whyDisabled := ""
-			enabled := flagEnable == "<all>" || enabledGroups[g.Name]
+
 			switch {
 			case !enabled:
-				whyDisabled = "not enabled by -enabled flag"
+				whyDisabled = "not enabled by name or tag (-enable flag)"
 			case disabledGroups[g.Name]:
-				whyDisabled = "disabled by -disable flag"
+				whyDisabled = "disabled by name (-disable flag)"
+			default:
+				if tag := inDisabledTags(g); tag != "" {
+					whyDisabled = fmt.Sprintf("disabled by %s tag (-disable flag)", tag)
+				}
 			}
+
 			if ruleguardDebug {
 				if whyDisabled != "" {
 					debugPrint(fmt.Sprintf("(-) %s is %s", g.Name, whyDisabled))

checkers/testdata/_integration/ruleguard/f1.go

@@ -8,3 +8,19 @@ func _() {
 	var s1, s2 string
 	var _ = fmt.Sprintf("%s%s", s1, s2)
 }
+
+func _() {
+	var s1, s2 string
+	if s1 == s2 {
+		if s1 != "" {
+			println(s1, s2)
+		}
+	}
+}
+
+func _() {
+	k := func() string {
+		return "test"
+	}
+	_ = fmt.Errorf(k())
+}

checkers/testdata/_integration/ruleguard/linttest.golden

@@ -1,3 +1,4 @@
 exit status 1
 ./f1.go:5:1: ruleguard: error as an underlying type is probably a mistake
-./f1.go:9:10: ruleguard: suggestion: s1+s2
+./f1.go:9:10: ruleguard: suggestion: s1+s2
+./f1.go:25:6: ruleguard: use errors.New(k()) or fmt.Errorf("%s", k()) instead

checkers/testdata/_integration/ruleguard/linttest.golden2

@@ -0,0 +1,2 @@
+exit status 1
+./f1.go:25:6: ruleguard: use errors.New(k()) or fmt.Errorf("%s", k()) instead

checkers/testdata/_integration/ruleguard/linttest.golden3

@@ -0,0 +1,3 @@
+exit status 1
+./f1.go:14:2: ruleguard: may be simplified to one if
+./f1.go:25:6: ruleguard: use errors.New(k()) or fmt.Errorf("%s", k()) instead

checkers/testdata/_integration/ruleguard/linttest.params

@@ -1 +1,3 @@
-check -@ruleguard.rules ./rules.go -enable ruleguard ./... | linttest.golden
+check -@ruleguard.rules ./rules.go -enable ruleguard -@ruleguard.disable=#test ./... | linttest.golden
+check -@ruleguard.rules ./rules.go -enable ruleguard -@ruleguard.enable=#style -@ruleguard.disable=#test ./... | linttest.golden2
+check -@ruleguard.rules ./rules.go -enable ruleguard -@ruleguard.enable=#style,#test ./... | linttest.golden3

checkers/testdata/_integration/ruleguard/rules.go

@@ -1,3 +1,4 @@
+//go:build ignore
 // +build ignore
 
 package gorules
@@ -15,3 +16,16 @@ func sprintfConcat(m dsl.Matcher) {
 		Where(m["a"].Type.Is(`string`) && m["b"].Type.Is(`string`)).
 		Suggest(`$a+$b`)
 }
+
+//doc:tags test
+func stackedIf(m dsl.Matcher) {
+	m.Match(`if $*_ { if $*_ { $*_ } }`).
+		Report(`may be simplified to one if`)
+}
+
+//doc:tags style
+func dynamicFmtString(m dsl.Matcher) {
+	m.Match(`fmt.Errorf($f($*args))`).
+		Suggest("errors.New($f($args))").
+		Report(`use errors.New($f($args)) or fmt.Errorf("%s", $f($args)) instead`)
+}

framework/lintmain/internal/check/check.go

@@ -19,10 +19,11 @@ import (
 	"strings"
 	"sync"
 
-	"github.com/go-critic/go-critic/framework/linter"
-	"github.com/go-critic/go-critic/framework/lintmain/internal/hotload"
 	"github.com/go-toolsmith/pkgload"
 	"golang.org/x/tools/go/packages"
+
+	"github.com/go-critic/go-critic/framework/linter"
+	"github.com/go-critic/go-critic/framework/lintmain/internal/hotload"
 )
 
 // Main implements sub-command entry point.