Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ruleguard: add filter by tags for analyzer mode #1205

Merged
merged 12 commits into from Mar 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion checkers/analyzer/run.go
Expand Up @@ -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 {
Expand Down
41 changes: 38 additions & 3 deletions checkers/ruleguard_checker.go
Expand Up @@ -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))
Expand Down
16 changes: 16 additions & 0 deletions checkers/testdata/_integration/ruleguard/f1.go
Expand Up @@ -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())
}
3 changes: 2 additions & 1 deletion 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
2 changes: 2 additions & 0 deletions 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
3 changes: 3 additions & 0 deletions 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
4 changes: 3 additions & 1 deletion 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
14 changes: 14 additions & 0 deletions checkers/testdata/_integration/ruleguard/rules.go
@@ -1,3 +1,4 @@
//go:build ignore
// +build ignore

package gorules
Expand All @@ -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`)
}
5 changes: 3 additions & 2 deletions framework/lintmain/internal/check/check.go
Expand Up @@ -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.
Expand Down