Go plugin system #841
Go plugin system #841
Conversation
Still needs unit tests & custom linters don't show up when running `golangci-lint linters` unless they're enabled on the command line
|
|
|
build seems to be failing for reasons unrelated to this PR |
|
Hi, thank you for your PR and sorry for long response! Will link this PR to other maintainers and hopefully we will get some feedback. |
pkg/lint/lintersdb/manager.go
Outdated
| } | ||
|
|
||
| type AnalyzerPlugin interface { | ||
| GetLinterName() string |
There was a problem hiding this comment.
name is already in the config, why duplicate it?
also, maybe we can move description to config to simplify things for linters authors?
There was a problem hiding this comment.
Ok, yeah, I can get onboard with that. Sounds good!
.golangci.example.yml
Outdated
| # Intended to point to the repo location of the linter. Optional, just for documentation purposes. | ||
| original-url: github.com/dbraley/example-linter | ||
| # Indicates if this linter should be considered slow. Optional, false by default. | ||
| slow: false |
There was a problem hiding this comment.
please, remove it for simplification. Linters with go/analysis interface can't be slow by current rules. The only slow linter is unused.
There was a problem hiding this comment.
I can remove it from the documentation with ease. Actually getting the code to ignore it could be slightly more problematic. But I think for what you're asking just removing the documentation of it should suffice, yes? edit: NM, was misremembering how this worked. That's easy to implement, will do.
.golangci.example.yml
Outdated
| path: /path/to/example.so | ||
| # If the linter should be enabled by default. Options in the `linters` | ||
| # section override this, including `disable-all`. Optional, false by default. | ||
| enabled: true |
There was a problem hiding this comment.
Why duplicate it with linters section? It looks difficult to understand for users.
There was a problem hiding this comment.
Hm. There are a couple of ways to reduce that duplication. The easiest and I think most consistent is to set the linter to enabled by default and remove this option. That does leave the 'problem' that if the user has linters:disable-all: true in the linters section, that will override enable by default even for the custom linters. Thus, the user will be forced to define the linter again in linters:enable. But that also preserves the ability to turn on and off custom linters on the command line.
README.md
Outdated
| In order to use plugins, you'll need a golangci-lint executable that can run them. The normal version of this project | ||
| is built with the vendors option, which breaks plugins that have overlapping dependencies. | ||
|
|
||
| 1. Download the `i473` branch of https://github.com/dbraley/golangci-lint/tree/i473 |
There was a problem hiding this comment.
We're going to merge it. Let's fix description.
There was a problem hiding this comment.
Yup, sorry, wanted the documentation to be accurate during the review. Will fix now.
README.md
Outdated
|
|
||
| ### Configure Your Project for Linting | ||
| If you already have a linter plugin available, you can follow these steps to define it's usage in a projects | ||
| `.golangci.yml` file. An example linter can be found at https://github.com/dbraley/example-linter. If you're looking for |
There was a problem hiding this comment.
please point to github.com/golangci/example-plugin-linter. I will fork your repo into it.
|
@jirfag Just checking in, haven't seen any motion on this for a bit. Is there anything else that needs to be done to keep this moving forward? |
|
Sorry for the delay, merged it |
This PR attempts to address #473 & #839 by introducing the ability to use custom linter plugins.
I do not believe this adds significant rigidity to the project, as it leverages the existing
LinterConfigstruct to import custom linters from the config into theManager, and then only when the.WithCustomLintersfunction is called. Because failure to load a custom linter doesn't stop the process, it was desirable to give the manager access to the logger as well.The readme file currently references github.com/dbraley/example-linter for an example of how to create your own custom linter. Feel free to fork that or create another example if you'd rather not have the documentation point to an outside authors github. I completely understand if that's the case, and will gladly change the documentation to point to a different repo at your request.