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

make it possible to run go-consistent as a library #32

Open
quasilyte opened this issue Mar 9, 2019 · 7 comments
Open

make it possible to run go-consistent as a library #32

quasilyte opened this issue Mar 9, 2019 · 7 comments

Comments

@quasilyte
Copy link
Owner

This is handy when integrating linters into an already existing system.

@jlwt90
Copy link

jlwt90 commented Apr 12, 2020

@quasilyte Do you mind if I try on this issue? It seems that some refactoring has to be done though

@quasilyte
Copy link
Owner Author

@jlwt90 go ahead, it's all yours. :)

@jlwt90
Copy link

jlwt90 commented Apr 20, 2020

@quasilyte I finally got some time to go into the source code 🙇

In order to let go-consistent scan all source code and detect the majority of the variants, we probably need to run with all the target packages, instead of a single run per package. What do you think about the entry point of the library?

I am about to write an entry point with function function Run(...) []Issue but kinda struggling with the input of this entry point. Taking golangci-lint as an example, we probably need to pass a *loader.Program into the Run() function(?) with goanalysis.LoadModeWholeProgram so that that tool can pass everything to go-consistent in one shot?

Here is the example that I referred to:
https://github.com/golangci/golangci-lint/blob/master/pkg/golinters/deadcode.go#L24-L25
https://github.com/golangci/go-misc/blob/master/deadcode/deadcode.go#L27

Sorry for the question... I am still a golang / analysis tool newbie ><
Thanks!

@quasilyte
Copy link
Owner Author

quasilyte commented Apr 20, 2020

The problem with the traditional "load the whole program" approach is... it requires a lot of memory. Some projects can eat so much that analysis can't be run on local machines anymore.

go-consistent does do a whole-program analysis, but it does not keep all AST and types in memory the entire time. Once a package is processed, we update the facts set (counters) and continue with another package. It allows us to process almost arbitrary sized project with constant memory usage (it's less CPU efficient, but "a little slower" is faster than "never" in cases of the big projects).

go/analysis permits to consume one package at the time I believe. If that's the case, we can the same trick there (Although it might load all packages into the memory behind the scenes). We want to avoid reporting issues while "indexing" pass, we'll only collect "facts" (in analysis terminology). Then users can use that pass to do whatever they want. One example is to write another pass that uses these facts to report more/less used variants.

Maybe we can move all the state in a separate package, use that inside the current main and add third package that provides go/analysis compatible analyzer that can be used as a library (it will use that second package to do its work).

@quasilyte
Copy link
Owner Author

quasilyte commented Apr 20, 2020

It's also stated in the project traits (readme):

Can handle projects of any size. This means that there should be no significant memory consumption growth with an increased number of packages being checked. There can be "fast, but memory-hungry" option that can work best for small-average projects, but it should be always possible to check huge projects on the developer machine.

@quasilyte
Copy link
Owner Author

quasilyte commented Apr 21, 2020

I feel a little guilty for the time you could invest in this.

The initial idea of this project was "zero-configuration". It sounds nice, but in reality, there are at least two drawbacks:

  1. You need to analyze the entire scope (package/module/project) to infer appropriate suggestions.
  2. Sometimes inferred suggestions are not what you want.

Maybe it would be better to split this into two almost independent parts. One would run a consistency check over a project using the given config that describes what should be used and what shouldn't. The second part would allow you to run analysis over the code base and generate the config that the user may edit later.

With this new architecture, checking consistency does not require a full scan/indexing as we know all the right answers from the beginning.

Sorry for the late response.

@l0nax
Copy link

l0nax commented Sep 25, 2020

Any updates on this?

It would be really great if this is implemented because then we can start to integrate this awesome longer into golangci-lint

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants