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

go/analysis: add -category=... filter flag to driver commands #565

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions go/analysis/checker/checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,9 @@ func (act *Action) execOnce() {
if err := analysisinternal.ValidateFixes(act.Package.Fset, act.Analyzer, d.SuggestedFixes); err != nil {
panic(err)
}
if !analysisinternal.EnabledCategory(analysisflags.Category, d.Category) {
return
}
act.Diagnostics = append(act.Diagnostics, d)
},
ImportObjectFact: act.ObjectFact,
Expand Down
6 changes: 4 additions & 2 deletions go/analysis/internal/analysisflags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ import (

// flags common to all {single,multi,unit}checkers.
var (
JSON = false // -json
Context = -1 // -c=N: if N>0, display offending line plus N lines of context
JSON = false // -json
Context = -1 // -c=N: if N>0, display offending line plus N lines of context
Category = "" // -category=... comma-separated list of categories (may be negated with -)
)

// Parse creates a flag for each of the analyzer's flags,
Expand Down Expand Up @@ -74,6 +75,7 @@ func Parse(analyzers []*analysis.Analyzer, multi bool) []*analysis.Analyzer {
// flags common to all checkers
flag.BoolVar(&JSON, "json", JSON, "emit JSON output")
flag.IntVar(&Context, "c", Context, `display offending line with this many lines of context`)
flag.StringVar(&Category, "category", Category, "report only diagnostics of the specified categories (a comma-separated list); with a leading '-', discard diagnostics of the specified categories.")

// Add shims for legacy vet flags to enable existing
// scripts that run vet to continue to work.
Expand Down
3 changes: 3 additions & 0 deletions go/analysis/unitchecker/unitchecker.go
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,9 @@ func run(fset *token.FileSet, cfg *Config, analyzers []*analysis.Analyzer) ([]re
log.Println(err)
d.SuggestedFixes = nil
}
if !analysisinternal.EnabledCategory(analysisflags.Category, d.Category) {
return
}
act.diagnostics = append(act.diagnostics, d)
},
ImportObjectFact: facts.ImportObjectFact,
Expand Down
7 changes: 7 additions & 0 deletions gopls/doc/analyzers.md
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,13 @@ following command:

$ go run golang.org/x/tools/gopls/internal/analysis/modernize/cmd/modernize@latest -test ./...

To apply a subset of modernization fixes, you can use the -category filter:

$ go run golang.org/x/tools/gopls/internal/analysis/modernize/cmd/modernize@latest -category=efaceany,rangeint -test ./...

A negated category list (e.g. "-category=-a,b") applies all fix categories
except a and b.

If the tool warns of conflicting fixes, you may need to run it more
than once until it has applied all fixes cleanly. This command is
not an officially supported interface and may change in the future.
Expand Down
7 changes: 7 additions & 0 deletions gopls/internal/analysis/modernize/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@
//
// $ go run golang.org/x/tools/gopls/internal/analysis/modernize/cmd/modernize@latest -test ./...
//
// To apply a subset of modernization fixes, you can use the -category filter:
//
// $ go run golang.org/x/tools/gopls/internal/analysis/modernize/cmd/modernize@latest -category=efaceany,rangeint -test ./...
//
// A negated category list (e.g. "-category=-a,b") applies all fix categories
// except a and b.
//
// If the tool warns of conflicting fixes, you may need to run it more
// than once until it has applied all fixes cleanly. This command is
// not an officially supported interface and may change in the future.
Expand Down
4 changes: 2 additions & 2 deletions gopls/internal/doc/api.json
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@
},
{
"Name": "\"modernize\"",
"Doc": "simplify code by using modern constructs\n\nThis analyzer reports opportunities for simplifying and clarifying\nexisting code by using more modern features of Go, such as:\n\n - replacing an if/else conditional assignment by a call to the\n built-in min or max functions added in go1.21;\n - replacing sort.Slice(x, func(i, j int) bool) { return s[i] \u003c s[j] }\n by a call to slices.Sort(s), added in go1.21;\n - replacing interface{} by the 'any' type added in go1.18;\n - replacing append([]T(nil), s...) by slices.Clone(s) or\n slices.Concat(s), added in go1.21;\n - replacing a loop around an m[k]=v map update by a call\n to one of the Collect, Copy, Clone, or Insert functions\n from the maps package, added in go1.21;\n - replacing []byte(fmt.Sprintf...) by fmt.Appendf(nil, ...),\n added in go1.19;\n - replacing uses of context.WithCancel in tests with t.Context, added in\n go1.24;\n - replacing omitempty by omitzero on structs, added in go1.24;\n - replacing append(s[:i], s[i+1]...) by slices.Delete(s, i, i+1),\n added in go1.21\n - replacing a 3-clause for i := 0; i \u003c n; i++ {} loop by\n for i := range n {}, added in go1.22;\n - replacing Split in \"for range strings.Split(...)\" by go1.24's\n more efficient SplitSeq, or Fields with FieldSeq;\n\nTo apply all modernization fixes en masse, you can use the\nfollowing command:\n\n\t$ go run golang.org/x/tools/gopls/internal/analysis/modernize/cmd/modernize@latest -test ./...\n\nIf the tool warns of conflicting fixes, you may need to run it more\nthan once until it has applied all fixes cleanly. This command is\nnot an officially supported interface and may change in the future.",
"Doc": "simplify code by using modern constructs\n\nThis analyzer reports opportunities for simplifying and clarifying\nexisting code by using more modern features of Go, such as:\n\n - replacing an if/else conditional assignment by a call to the\n built-in min or max functions added in go1.21;\n - replacing sort.Slice(x, func(i, j int) bool) { return s[i] \u003c s[j] }\n by a call to slices.Sort(s), added in go1.21;\n - replacing interface{} by the 'any' type added in go1.18;\n - replacing append([]T(nil), s...) by slices.Clone(s) or\n slices.Concat(s), added in go1.21;\n - replacing a loop around an m[k]=v map update by a call\n to one of the Collect, Copy, Clone, or Insert functions\n from the maps package, added in go1.21;\n - replacing []byte(fmt.Sprintf...) by fmt.Appendf(nil, ...),\n added in go1.19;\n - replacing uses of context.WithCancel in tests with t.Context, added in\n go1.24;\n - replacing omitempty by omitzero on structs, added in go1.24;\n - replacing append(s[:i], s[i+1]...) by slices.Delete(s, i, i+1),\n added in go1.21\n - replacing a 3-clause for i := 0; i \u003c n; i++ {} loop by\n for i := range n {}, added in go1.22;\n - replacing Split in \"for range strings.Split(...)\" by go1.24's\n more efficient SplitSeq, or Fields with FieldSeq;\n\nTo apply all modernization fixes en masse, you can use the\nfollowing command:\n\n\t$ go run golang.org/x/tools/gopls/internal/analysis/modernize/cmd/modernize@latest -test ./...\n\nTo apply a subset of modernization fixes, you can use the -category filter:\n\n\t$ go run golang.org/x/tools/gopls/internal/analysis/modernize/cmd/modernize@latest -category=efaceany,rangeint -test ./...\n\nA negated category list (e.g. \"-category=-a,b\") applies all fix categories\nexcept a and b.\n\nIf the tool warns of conflicting fixes, you may need to run it more\nthan once until it has applied all fixes cleanly. This command is\nnot an officially supported interface and may change in the future.",
"Default": "true",
"Status": ""
},
Expand Down Expand Up @@ -1338,7 +1338,7 @@
},
{
"Name": "modernize",
"Doc": "simplify code by using modern constructs\n\nThis analyzer reports opportunities for simplifying and clarifying\nexisting code by using more modern features of Go, such as:\n\n - replacing an if/else conditional assignment by a call to the\n built-in min or max functions added in go1.21;\n - replacing sort.Slice(x, func(i, j int) bool) { return s[i] \u003c s[j] }\n by a call to slices.Sort(s), added in go1.21;\n - replacing interface{} by the 'any' type added in go1.18;\n - replacing append([]T(nil), s...) by slices.Clone(s) or\n slices.Concat(s), added in go1.21;\n - replacing a loop around an m[k]=v map update by a call\n to one of the Collect, Copy, Clone, or Insert functions\n from the maps package, added in go1.21;\n - replacing []byte(fmt.Sprintf...) by fmt.Appendf(nil, ...),\n added in go1.19;\n - replacing uses of context.WithCancel in tests with t.Context, added in\n go1.24;\n - replacing omitempty by omitzero on structs, added in go1.24;\n - replacing append(s[:i], s[i+1]...) by slices.Delete(s, i, i+1),\n added in go1.21\n - replacing a 3-clause for i := 0; i \u003c n; i++ {} loop by\n for i := range n {}, added in go1.22;\n - replacing Split in \"for range strings.Split(...)\" by go1.24's\n more efficient SplitSeq, or Fields with FieldSeq;\n\nTo apply all modernization fixes en masse, you can use the\nfollowing command:\n\n\t$ go run golang.org/x/tools/gopls/internal/analysis/modernize/cmd/modernize@latest -test ./...\n\nIf the tool warns of conflicting fixes, you may need to run it more\nthan once until it has applied all fixes cleanly. This command is\nnot an officially supported interface and may change in the future.",
"Doc": "simplify code by using modern constructs\n\nThis analyzer reports opportunities for simplifying and clarifying\nexisting code by using more modern features of Go, such as:\n\n - replacing an if/else conditional assignment by a call to the\n built-in min or max functions added in go1.21;\n - replacing sort.Slice(x, func(i, j int) bool) { return s[i] \u003c s[j] }\n by a call to slices.Sort(s), added in go1.21;\n - replacing interface{} by the 'any' type added in go1.18;\n - replacing append([]T(nil), s...) by slices.Clone(s) or\n slices.Concat(s), added in go1.21;\n - replacing a loop around an m[k]=v map update by a call\n to one of the Collect, Copy, Clone, or Insert functions\n from the maps package, added in go1.21;\n - replacing []byte(fmt.Sprintf...) by fmt.Appendf(nil, ...),\n added in go1.19;\n - replacing uses of context.WithCancel in tests with t.Context, added in\n go1.24;\n - replacing omitempty by omitzero on structs, added in go1.24;\n - replacing append(s[:i], s[i+1]...) by slices.Delete(s, i, i+1),\n added in go1.21\n - replacing a 3-clause for i := 0; i \u003c n; i++ {} loop by\n for i := range n {}, added in go1.22;\n - replacing Split in \"for range strings.Split(...)\" by go1.24's\n more efficient SplitSeq, or Fields with FieldSeq;\n\nTo apply all modernization fixes en masse, you can use the\nfollowing command:\n\n\t$ go run golang.org/x/tools/gopls/internal/analysis/modernize/cmd/modernize@latest -test ./...\n\nTo apply a subset of modernization fixes, you can use the -category filter:\n\n\t$ go run golang.org/x/tools/gopls/internal/analysis/modernize/cmd/modernize@latest -category=efaceany,rangeint -test ./...\n\nA negated category list (e.g. \"-category=-a,b\") applies all fix categories\nexcept a and b.\n\nIf the tool warns of conflicting fixes, you may need to run it more\nthan once until it has applied all fixes cleanly. This command is\nnot an officially supported interface and may change in the future.",
"URL": "https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/modernize",
"Default": true
},
Expand Down
17 changes: 17 additions & 0 deletions internal/analysisinternal/analysis.go
Original file line number Diff line number Diff line change
Expand Up @@ -487,3 +487,20 @@ func CanImport(from, to string) bool {
}
return true
}

// EnabledCategory reports whether a given category is enabled by the specified
// filter. filter is a comma-separated list of categories, optionally prefixed
// with `-` to disable all provided categories. All categories are enabled with
// an empty filter.
func EnabledCategory(filter, category string) bool {
if filter == "" {
return true
}
// negation must be specified at the start
filter, exclude := strings.CutPrefix(filter, "-")
filters := strings.Split(filter, ",")
if slices.Contains(filters, category) {
return !exclude
}
return exclude
}
27 changes: 27 additions & 0 deletions internal/analysisinternal/analysis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,30 @@ func TestCanImport(t *testing.T) {
}
}
}

func TestEnabledCategory(t *testing.T) {
for _, tt := range []struct {
filter string
category string
want bool
}{
{"", "a", true},
{"a", "a", true},
{"-a", "a", false},
{"-b", "a", true},
{"b", "a", false},
{"a,b", "a", true},
{"a,b", "b", true},
{"-b,a", "a", false},
{"-b,a", "b", false},
{"a,-a", "a", true}, // negation must be at beginning
{"-b,c", "a", true},
{"b", "", false},
{"", "", true},
} {
got := EnabledCategory(tt.filter, tt.category)
if got != tt.want {
t.Errorf("EnabledCategory(%q,%q) = %v, want %v", tt.filter, tt.category, got, tt.want)
}
}
}