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

Implement our own linter driver for NilAway as a standalone checker #175

Closed
yuxincs opened this issue Jan 21, 2024 · 1 comment · Fixed by #234
Closed

Implement our own linter driver for NilAway as a standalone checker #175

yuxincs opened this issue Jan 21, 2024 · 1 comment · Fixed by #234
Labels
driver enhancement New feature or request

Comments

@yuxincs
Copy link
Contributor

yuxincs commented Jan 21, 2024

There are 3 ways (although one of them is broken) to run NilAway:

(1) standalone checker: we use the standard linter driver from x/tools for running NilAway, such that NilAway can be run as a standalone executable.
(2) bazel/nogo: NilAway is configured as nogo analyzer and executed by nogo.
(3) golangci-lint: NilAway is configured as a linter in golangci-lint and executed by it.

(1) runs NilAway completely in memory, where NilAway has to re-analyze unmodified packages in consecutive runs, plus consuming a huge amount of memory sinc all facts are always stored instead of being fetched on demand (see #127, #161 for resource consumption when running NilAway). (2) and (3) offer fact caching, making them much better options for running NilAway given the sophistication of the analyses that NilAway does.

However, (2) requires users to use bazel to build their Go projects, and (3) is broken and likely won't work for some time (until NilAway is mature enough). Both of these "better" options seem to be out of reach for the majority of users. Therefore, most are simply using (1) and living with the performance penalties.

Given the demand, we might try to implement our own minimum caching linter driver that handles scheduling of NilAway sub-analyzers, as well as storing fact files to the filesystem (as well as loading them on-demand). We may need to re-invent some wheels, but in the long run could offer a lot of flexibilities for us (e.g., unlock us from being unable to fix driver-related issues, see driver ).

@yuxincs yuxincs added driver enhancement New feature or request labels Jan 21, 2024
@yuxincs
Copy link
Contributor Author

yuxincs commented Jan 21, 2024

Meanwhile, since golang/go#61324 proposal is accepted, let's wait some time for that to happen before moving this further. Hacking on top that importable linter driver seems a lot easier than to re-invent everything ourselves.

yuxincs added a commit that referenced this issue Apr 29, 2024
Since golangci-lint v1.57.0, it offers a new [Module Plugin
System](https://golangci-lint.run/plugins/module-plugins/), which is a
lot more robust and easier to use than the plain Go plugin system. This
unlocks the possibility of running NilAway in golangci-lint as a private
linter.

This PR implements the require interface for the module plugin system
for golangci-lint and updates the README to list the instructions on how
to run NilAway with golangci-lint.

With this, our official recommendation to run NilAway would be to either
run it via golangci-lint or bazel/nogo, the standalone checker is only
provided for evaluation purposes due to its limitations (lack of fact
caching, lack of support for conditional build etc.).

This also closes #175 since it is no longer required.

We should switch our own linting task and CI to run NilAway via
golangci-lint as well, but that will happen in follow-up PRs.

---------

Co-authored-by: Sonal Mahajan <101232472+sonalmahajan15@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
driver enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant