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

x/tools/go/analysis: provide driver for running analysis programmatically #31897

Open
muirdm opened this issue May 7, 2019 · 9 comments
Open
Assignees
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@muirdm
Copy link

muirdm commented May 7, 2019

Currently there is no driver suitable for running analyzers from a go test (or from code, in general). The multichecker and singlechecker drivers are meant for cli tools and call os.Exit(), so they cannot be used.

We want to be able to run analysis from a go test rather than a separate binary. It is easier to integrate into our CI system if is a normal test.

My first thought was something that looks like this:

func Run(args []string, analyzers []*analysis.Analyzer) ([]analysis.Diagnostic, error)

Returning Diagnostic is not quite right though since the user doesn't have the FileSet to translate the diagnostics' token.Pos. I also assume something would have to be done for passing analyzer flags programatically.

/cc @ianthehat since you responded briefly in Slack regarding this feature request

@gopherbot gopherbot added this to the Unreleased milestone May 7, 2019
@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 8, 2019
@matloob
Copy link
Contributor

matloob commented Jun 6, 2019

Hi, could you provide more information about your use case?

Are you trying to test analyses or are you trying to create a test that runs analyses on other packages in your project?

@muirdm
Copy link
Author

muirdm commented Jun 6, 2019

The latter: trying to run analyzers against packages in our project via a go test. I tweaked the title to deemphasize the test part since I know it is confusing.

@muirdm muirdm changed the title x/tools/go/analysis: provide driver for running analysis in test x/tools/go/analysis: provide driver for running analysis programmatically Jun 6, 2019
@mikesep
Copy link

mikesep commented Jun 6, 2019

I'm interested in a similar use case. I'm working on a tool to run my Analyzer against packages to gather information, after which I'll want to do some final overall analysis on the collected data and output a report.

@fatih
Copy link
Member

fatih commented Jun 7, 2019

I just recently stumbled into the same problem. I wanted to run a long running service and not use any of the drivers (i.e: singlechecker) as those abstract more than I want.

@matloob
Copy link
Contributor

matloob commented Jun 19, 2019

I think the best way to move forward on this is to add such a library in an external package (outside of tools) and let it bake a little. I don't think there's anything (other than the obvious visibility and accessibility to potential users) that requires this to be in tools.

Once it's used more and we're happy with it we can move it back to tools.

On a related note, it would be nice if we had some sort of playground with fewer stability requirements than tools to do experiments in. As it is now once we add anything it's pretty much there forever and we can't make any changes.....

@gopherbot
Copy link

Change https://go.dev/cl/411907 mentions this issue: go/analysis/checker: a go/packages-based driver library

SilverRainZ pushed a commit to SilverRainZ/tools that referenced this issue Aug 24, 2022
The {single,multi}checker packages provide the main function
for a complete application, as a black box. Many users want
the ability to customize the analyzer behavior with additional
logic, as described in the attached issues.

This change creates a new package, go/analysis/checker, that
exposes an Analyze pure function---one that avoids global flags,
os.Exit, logging, profiling, and other side effects---that
runs a set of analyzers on a set of packages loaded (by the
client) using go/packages, and presents the graph of results
in a form that allows postprocessing.

This is just a sketch. API feedback welcome.

DO NOT SUBMIT

Updates golang/go#30231
Updates golang/go#30219
Updates golang/go#31007
Updates golang/go#31897
Updates golang/go#50265
Updates golang/go#53215
Updates golang/go#53336

Change-Id: I745d319a587dca506564a4624b52a7f1eb5f4751
SilverRainZ pushed a commit to SilverRainZ/tools that referenced this issue Apr 10, 2023
The {single,multi}checker packages provide the main function
for a complete application, as a black box. Many users want
the ability to customize the analyzer behavior with additional
logic, as described in the attached issues.

This change creates a new package, go/analysis/checker, that
exposes an Analyze pure function---one that avoids global flags,
os.Exit, logging, profiling, and other side effects---that
runs a set of analyzers on a set of packages loaded (by the
client) using go/packages, and presents the graph of results
in a form that allows postprocessing.

This is just a sketch. API feedback welcome.

DO NOT SUBMIT

Updates golang/go#30231
Updates golang/go#30219
Updates golang/go#31007
Updates golang/go#31897
Updates golang/go#50265
Updates golang/go#53215
Updates golang/go#53336

Change-Id: I745d319a587dca506564a4624b52a7f1eb5f4751
SilverRainZ pushed a commit to SilverRainZ/tools that referenced this issue Apr 11, 2023
The {single,multi}checker packages provide the main function
for a complete application, as a black box. Many users want
the ability to customize the analyzer behavior with additional
logic, as described in the attached issues.

This change creates a new package, go/analysis/checker, that
exposes an Analyze pure function---one that avoids global flags,
os.Exit, logging, profiling, and other side effects---that
runs a set of analyzers on a set of packages loaded (by the
client) using go/packages, and presents the graph of results
in a form that allows postprocessing.

This is just a sketch. API feedback welcome.

DO NOT SUBMIT

Updates golang/go#30231
Updates golang/go#30219
Updates golang/go#31007
Updates golang/go#31897
Updates golang/go#50265
Updates golang/go#53215
Updates golang/go#53336

Change-Id: I745d319a587dca506564a4624b52a7f1eb5f4751
@adonovan adonovan added the Analysis Issues related to static analysis (vet, x/tools/go/analysis) label Apr 23, 2023
@IceflowRE
Copy link

There is golang.org/x/tools/go/analysis/analysistest you can use this in tests, but it does not cover everything this issue requests.

@adonovan
Copy link
Member

adonovan commented Mar 4, 2024

Update: this issue (and some related ones) resulted in proposal #61324, which was accepted. It defines a new public API for tools that use packages.Load to load and type-check a set of packages from source and then run analyzers over them. I have a code change to implement it, but it's stale and needs a little work to bring it back up to date.

My main concern is: is it still needed? I've heard very little clamor for it since we went through the proposal process. The main downside of publishing the new API is that it may cause people to reach for it unnecessarily, and it may encourage people to write analyzers that only work in "all packages in same address space" mode, because they use stateful variables or out-of-band communication with the rest of the application. By contrast the unitchecker driver that is part of 'go vet' ensures that analyzers use the proper hygiene, and delivers incremental separate analysis, with a file-based cache, which is more scalable for large code bases.

So: do people still want this?

@cce
Copy link

cce commented Mar 11, 2024

I think it would be great — there seem to continue to be recent comments and backlinks referencing #61324 and the related issues. IMO supporting more flexible uses of analyzers without the main/exit would encourage people to write and deploy more analyzers, which seems overall like a good outcome! (Even if they are not run from go vet)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

9 participants