x/tools/go/analysis/internal: Add an option to emit diagnostics for external packages

[Matts966]

What version of golang/tools are you using?

~/tools $ git rev-parse HEAD
a30bf2db82d4f645ded9030efbbf6d4fbe5e70eb

Proposal

Currently the analysis driver in golang/tools prints diagnostics only for root packages. We can't change this behavior without changing the golang/tools repository or developing a driver from scratch because this behavior is written in the internal package.

Some analyses could be more beneficial if they could emit diagnostics for dependencies because problems in dependencies can also be problems in the root packages.

So I would like to implement an option to emit diagnostics for external packages by adding an argument to golang.org/x/tools/go/analysis/internal/checker.Run, and adding a command-line argument to golang.org/x/tools/go/analysis/singlechecker.Run and golang.org/x/tools/go/analysis/multichecker.Run.

Is it OK, or are there any reasons why the option is not in the packages?

Thank you.

[andybons]

@matloob @alandonovan

[alandonovan]

(By "external packages", I assume you mean dependencies.)

The driver runs the requested analyzers (plus any indirectly required due to Requires) on dependencies only if facts are needed from analyzing lower-level packages; most analyzers don't actually need to look at packages other than the root.

Doesn't some variant of this command do what you need?

$ go vet $(go list -deps myproj/...)

[Matts966]

@matloob, @alandonovan I tested the command & understand the workaround can what I need (linting all dependencies recursively). Thank you for the suggestion 🙇

However, I found some differences. I compared that workaround with my copied and modified driver like below.

# with modified driver
~/knil$ time knil golang.org/x/tools/... 2>&1 | wc -l
   15800

real    0m18.837s
user    0m47.938s
sys     0m4.163s

# with `go list -deps`
~/knil$ time knil $(go list -deps golang.org/x/tools/...) 2>&1 | wc -l
   15159

real    0m45.245s
user    1m41.810s
sys     0m14.848s

I don't know the cause of the difference of line count returned by wc (maybe the version differences of dependencies?), however, the performance difference is probably because the current driver double-checks the dependencies both as roots and dependencies.

This performance difference probably will not be resolved by the workaround, and we should prepare the option to optimize the analysis?

[alandonovan]

Thanks for the report. I'm not surprised it's slower. The differences are that:

1) the default command runs all analyses on the root packages, and modular analyses (the few that use Facts) on all dependencies too. It then reports diagnostics just for the root packages. 2) your fullchecker does the same, but reports diagnostics for the dependencies. These will necessarily include only the diagnostics produced by modular analyses. 3) the 'go list' workaround runs all analyses on all packages and prints all diagnostics.

1 and #3 are easy to explain. #2 is not. That's why I don't want to add a flag.

If you're feeling motivated, it would be good to profile #3 and see if there are easy performs gains to be made.