[tools] go/analysis: add Config, Run in single/multichecker
101 views
Skip to first unread message
Hyang-Ah Hana Kim (Gerrit)
Jun 2, 2022, 8:57:27 PM6/2/22
to goph...@pubsubhelper.golang.org, Hyang-Ah Hana Kim, golang-co...@googlegroups.com
Hyang-Ah Hana Kim has uploaded this change for review.
go/analysis: add Config, Run in single/multichecker
The existing APIs (singlechecker.Main and multichecker.Main)
took package patterns (e.g. ./...), loaded them with a hard-coded
go/packages.Config, and applied analyzers on the loaded packages.
The APIs are simple, but not flexible enough to accomodate
analyzers that need special setup or complex go/packages.Config.
For example,
* An analyzer may want to initialize its state based on the
module information loadable with go/packages.NeedModules load
mode. Current APIs don't use NeedModules nor expose the loaded
packages.
* A vulnerability check analyzer may want to fetch the list of
OSV entries relevant for the analyzed packages&modules, from
a remote vuln database.
This change adds a new Config type and a Run function that takes
a Config object. For now, the Config object has only a Load
function. If set, it is used to load packages instead of the
default load function used by Main.
This hook allows analyzer authors the followings:
* Use a different load mode, configuration, or even filter/fake
the packages.
* Inject an extra initialization logic that requires loaded
packages before running analysis.
Change-Id: I1cb4c46abfcf56d8e0aa1cc065019502024084e8
---
M go/analysis/internal/checker/checker.go
M go/analysis/internal/checker/checker_test.go
M go/analysis/multichecker/multichecker.go
M go/analysis/singlechecker/singlechecker.go
4 files changed, 68 insertions(+), 11 deletions(-)
diff --git a/go/analysis/internal/checker/checker.go b/go/analysis/internal/checker/checker.go
index 090e96c..7d191ce 100644
--- a/go/analysis/internal/checker/checker.go
+++ b/go/analysis/internal/checker/checker.go
@@ -69,13 +69,18 @@
flag.BoolVar(&Fix, "fix", false, "apply all suggested fixes")
}
+type Config struct {
+ // Load is called once at the start of Run to load packages specified by patterns.
+ Load func(mode packages.LoadMode, patterns []string) ([]*packages.Package, error)
+}
+
// Run loads the packages specified by args using go/packages,
// then applies the specified analyzers to them.
// Analysis flags must already have been set.
// It provides most of the logic for the main functions of both the
// singlechecker and the multi-analysis commands.
// It returns the appropriate exit code.
-func Run(args []string, analyzers []*analysis.Analyzer) (exitcode int) {
+func Run(cfg Config, args []string, analyzers []*analysis.Analyzer) (exitcode int) {
if CPUProfile != "" {
f, err := os.Create(CPUProfile)
if err != nil {
@@ -128,7 +133,7 @@
// Optimization: if the selected analyzers don't produce/consume
// facts, we need source only for the initial packages.
allSyntax := needFacts(analyzers)
- initial, err := load(args, allSyntax)
+ initial, err := cfg.load(args, allSyntax)
if err != nil {
if _, ok := err.(typeParseError); !ok {
// Fail when some of the errors are not
@@ -156,16 +161,17 @@
// load loads the initial packages. If all loading issues are related to
// typing and parsing, the returned error is of type typeParseError.
-func load(patterns []string, allSyntax bool) ([]*packages.Package, error) {
+func (cfg *Config) load(patterns []string, allSyntax bool) (initial []*packages.Package, err error) {
mode := packages.LoadSyntax
if allSyntax {
mode = packages.LoadAllSyntax
}
- conf := packages.Config{
- Mode: mode,
- Tests: true,
+
+ if cfg.Load != nil {
+ initial, err = cfg.Load(mode, patterns)
+ } else {
+ initial, err = packages.Load(&packages.Config{Mode: mode, Tests: true}, patterns...)
}
- initial, err := packages.Load(&conf, patterns...)
if err == nil {
if len(initial) == 0 {
err = fmt.Errorf("%s matched no packages", strings.Join(patterns, " "))
diff --git a/go/analysis/internal/checker/checker_test.go b/go/analysis/internal/checker/checker_test.go
index eee211c..a996c9a 100644
--- a/go/analysis/internal/checker/checker_test.go
+++ b/go/analysis/internal/checker/checker_test.go
@@ -53,7 +53,7 @@
}
path := filepath.Join(testdata, "src/rename/test.go")
checker.Fix = true
- checker.Run([]string{"file=" + path}, []*analysis.Analyzer{analyzer})
+ checker.Run(checker.Config{}, []string{"file=" + path}, []*analysis.Analyzer{analyzer})
contents, err := ioutil.ReadFile(path)
if err != nil {
@@ -151,7 +151,7 @@
// no errors
{name: "no-errors", pattern: []string{"sort"}, analyzers: []*analysis.Analyzer{analyzer, noop}, code: 0},
} {
- if got := checker.Run(test.pattern, test.analyzers); got != test.code {
+ if got := checker.Run(checker.Config{}, test.pattern, test.analyzers); got != test.code {
t.Errorf("got incorrect exit code %d for test %s; want %d", got, test.name, test.code)
}
}
diff --git a/go/analysis/multichecker/multichecker.go b/go/analysis/multichecker/multichecker.go
index 3c62be5..7fb286f 100644
--- a/go/analysis/multichecker/multichecker.go
+++ b/go/analysis/multichecker/multichecker.go
@@ -22,6 +22,12 @@
)
func Main(analyzers ...*analysis.Analyzer) {
+ Run(Config{}, analyzers...)
+}
+
+type Config = checker.Config
+
+func Run(cfg Config, analyzers ...*analysis.Analyzer) {
progname := filepath.Base(os.Args[0])
log.SetFlags(0)
log.SetPrefix(progname + ": ") // e.g. "vet: "
@@ -56,5 +62,5 @@
panic("unreachable")
}
- os.Exit(checker.Run(args, analyzers))
+ os.Exit(checker.Run(cfg, args, analyzers))
}
diff --git a/go/analysis/singlechecker/singlechecker.go b/go/analysis/singlechecker/singlechecker.go
index 91044ca..a5d6b57 100644
--- a/go/analysis/singlechecker/singlechecker.go
+++ b/go/analysis/singlechecker/singlechecker.go
@@ -37,6 +37,14 @@
// Main is the main function for a checker command for a single analysis.
func Main(a *analysis.Analyzer) {
+ Run(Config{}, a)
+}
+
+// Config is the configuration for a checker command.
+type Config = checker.Config
+
+// Run is the main function for a checker command for a single analysis.
+func Run(cfg Config, a *analysis.Analyzer) {
log.SetFlags(0)
log.SetPrefix(a.Name + ": ")
@@ -72,5 +80,5 @@
panic("unreachable")
}
- os.Exit(checker.Run(args, analyzers))
+ os.Exit(checker.Run(cfg, args, analyzers))
}
To view, visit change 410254. To unsubscribe, or for help writing mail filters, visit settings.
Tim King (Gerrit)
Jun 2, 2022, 11:56:27 PM6/2/22
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, Michael Matloob, Zvonimir Pavlinovic, Gopher Robot, golang-co...@googlegroups.com
Attention is currently required from: Hyang-Ah Hana Kim, Michael Matloob.
1 comment:
Patchset:
- An analyzer may want to initialize its state based on the
- module information loadable with go/packages.NeedModules load
- mode. Current APIs don't use NeedModules nor expose the loaded
- packages.
- A vulnerability check analyzer may want to fetch the list of
- OSV entries relevant for the analyzed packages&modules, from
- a remote vuln database.
I am not yet sure how this change is enabling you to do both of these things. It would help to see the Analyzer you are trying to build (or to chat offline next week).
To view, visit change 410254. To unsubscribe, or for help writing mail filters, visit settings.
Hyang-Ah Hana Kim (Gerrit)
Jun 3, 2022, 8:48:36 AM6/3/22
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Hyang-Ah Hana Kim, Michael Matloob.
Hyang-Ah Hana Kim uploaded patch set #2 to this change.
go/analysis: add Config, Run in single/multichecker
The existing APIs (singlechecker.Main and multichecker.Main)
took package patterns (e.g. ./...), loaded them with a hard-coded
go/packages.Config, and applied analyzers on the loaded packages.
The APIs are simple, but not flexible enough to accommodate
analyzers that need special setup or complex go/packages.Config.
For example,
* An analyzer may want to initialize its state based on the
module information loadable with go/packages.NeedModules load
mode. Current APIs don't use NeedModules nor expose the loaded
packages.
* A vulnerability check analyzer may want to fetch the list of
OSV entries relevant for the analyzed packages&modules, from
a remote vuln database.
This change adds a new Config type and a Run function that takes
a Config object. For now, the Config object has only a Load
function. If set, it is used to load packages instead of the
default load function used by Main.
This hook allows analyzer authors the followings:
* Use a different load mode, configuration, or even filter/fake
the packages.
* Inject an extra initialization logic that requires loaded
packages before running analysis.
For golang/go#53215
Change-Id: I1cb4c46abfcf56d8e0aa1cc065019502024084e8
---
M go/analysis/internal/checker/checker.go
M go/analysis/internal/checker/checker_test.go
M go/analysis/multichecker/multichecker.go
M go/analysis/singlechecker/singlechecker.go
4 files changed, 70 insertions(+), 11 deletions(-)
To view, visit change 410254. To unsubscribe, or for help writing mail filters, visit settings.
Hyang-Ah Hana Kim (Gerrit)
Jun 3, 2022, 9:02:06 AM6/3/22
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, Michael Matloob, Zvonimir Pavlinovic, Tim King, Gopher Robot, golang-co...@googlegroups.com
Attention is currently required from: Michael Matloob, Tim King.
1 comment:
Patchset:
> * An analyzer may want to initialize its state based on the […]
I hope https://go-review.googlesource.com/c/tools/+/410125 answers your questions.
For simplicity I didn't include vulndb query logic in the example, but you can imagine that can be in the ModuleMapper.Init logic.
To view, visit change 410254. To unsubscribe, or for help writing mail filters, visit settings.
Hyang-Ah Hana Kim (Gerrit)
Jan 30, 2023, 1:04:20 PM1/30/23
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, Michael Matloob, Zvonimir Pavlinovic, Tim King, Gopher Robot, golang-co...@googlegroups.com
Hyang-Ah Hana Kim abandoned this change.
Abandoned
To view, visit change 410254. To unsubscribe, or for help writing mail filters, visit settings.
Reply all
Reply to author
Forward
0 new messages