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

Switch to using analysis framework natively with fact export #5

Open
navijation opened this issue Jan 12, 2024 · 4 comments
Open

Switch to using analysis framework natively with fact export #5

navijation opened this issue Jan 12, 2024 · 4 comments

Comments

@navijation
Copy link

As noted in golangci/golangci-lint#4158, this linter is incompatible with the analysis framework because it strongly relies on state across packages, and this state is not exported using the facts API.

The analyzer shim present in golangci-lint seems insufficient in dealing with this limitation. This entire module would likely need to be re-written as an analyzer.

@alecthomas
Copy link
Owner

Ah makes sense, I'll take a look.

@navijation
Copy link
Author

Thanks for taking a look. As far as in-memory caching goes, my recommendation is the following strategy:

  1. store a cache which maps packages to their own caches; access to this cache must be synchronized since drivers may execute independent packages in parallel
  2. in each package, only perform writes to the cache of the same package and reads from other packages; these caches do not need to be synchronized because no well-behaved driver will perform concurrent passes over dependent packages or multiple passes against the same package
  3. at the beginning of an analysis pass, delete and re-initialize the cache for the package in question to avoid any risk of staleness

Another thing to worry about with using the facts API: once you add analyzer facts, drivers will start running the analyzer against the dependency closure rather than just the packages in question. Sadly, this will drop performance of the singlechecker considerably. There's not really a workaround aside from adding some config option to exclude reporting and fact gathering on certain packages.

@alecthomas
Copy link
Owner

What caching are you referring to? I've used the facts API before and looking at how it would be used here, it seems sufficient for this use case.

@navijation
Copy link
Author

It would be acceptable to use no in memory caching and purely rely on the facts API.

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

2 participants