x/lint: freeze and deprecate
ianlancetaylor opened this issue · 38 comments
The golang.org/x/lint aka github.com/golang/lint program is essentially unmaintained. There have been no substantive changes since 2018. The issue tracker (https://github.com/golang/lint/issues) has 85 open issues as I write this, the majority of them with no comments.
Because programmers have been trained over the years to use linters, and because this is a program in the official Go repo named "lint", it looks like this is the lint program recommended by the Go project. But it isn't. As it says in the README file (https://go.googlesource.com/lint/+/refs/heads/master/README.md) this is a style guide checker for Effective Go (https://golang.org/doc/effective_go.html) and the list of code review comments on the Go wiki (https://golang.org/wiki/CodeReviewComments). While those are good aspects of Go style, they are not the only possible Go style. People can reasonably prefer different styles. They can reasonably want to adopt some aspects of the style or not. They can reasonably want additional checks. They can reasonably want various knobs and options for a linter. The golang.org/x/lint package does none of those things. In particular, as its README file says: "We will not be adding pragmas or other knobs to suppress specific warnings."
There is a heated discussion on the (closed) issue golang/lint#263 about what exactly this lint program is for.
In short, I think that golang.org/x/lint has become an attractive nuisance: an unmaintained program that people want to find to be useful for their purposes, but often isn't. It does not bring people joy. We should throw it out.
I propose that we explicitly freeze the repository, and create a plan for removing it entirely.
This will free up the mental space for other people to write lint programs, perhaps based on this one, that will be maintained and useful. It will get the Go project out of the business of appearing to recommend a program that we don't actually maintain.
(Alternatively, we could decide to actively maintain it, but I don't know who would actually do that.)
This will free up the mental space for other people to write lint programs, perhaps based on this one, that will be maintained and useful. It will get the Go project out of the business of appearing that recommend a program that we don't actually maintain.
As a (not so random) data point: Staticcheck implements a number of style checks, some of which overlap with golint. Not all of them are enabled by default (such as requiring documentation on all exported identifiers), but users can opt into them. That is to say, golint disappearing wouldn't leave behind a style-checking void, and there is the customizability that people have been demanding in golang/lint#263. Staticcheck is also implemented on top of the go/analysis framework and an optional feature of gopls, another outstanding issue of golint.
First off, I agree with Ian's proposal and the rationale for it. If it appears to be something that is blessed by the core Go team, it will engender certain expectations from the community and ultimately frustration if it fails to live up the the community's needs or, as in this case, becomes more trouble than its worth. And that makes the Go team look bad.
With that said, and having been writing code in a number of languages and various contexts for a very long time, I've found that there are three domains of authority: errors, warnings, and you suck at writing code.
Errors are clearly the domain of the compiler and warnings a combination of the compiler and potentially other tools (static analyzers). It's the third thing that gets thorny and problematic.
All languages to some extent need to be (and should be) opinionated. But there is a very important line to draw between the language being opinionated and the person typing (or designing the language) being opinionated. The former really needs to be very carefully considered, not just what the opinion should be, but whether that opinion should be part of the identity of the core language at all.
The slippery slope of linters is a long travelled one and, while they can provide very positive feedback for inexperienced and experienced programmers alike, you either end up with a draconian monolithic voice on how you should code, or hundreds of switches to turn on or off certain opinions about wether the code you've written is acceptable or not.
In my opinion, the former is untenable and will fail to serve the needs of any person or group who wants to maintain a consistent coding style. The only way to maintain a tool that encourages a certain coding style is to make it highly customizable, so that if that particular style doesn't fix in a specific context or the opinions of a group of coders it can be turned off.
When you're working in teams I think consistency is far more important than adherence to third-party opinions, regardless of how wise or informed those opinions may be. We all know the cognitive friction experienced when reading Poorly Written Code ™. But any good programmer (or his/her peer) will eventually discover how it could be better. If a tool can do this and skip the cycle of discovery, discussion and fixing, that's great. But if you can't decide for yourself (or yourselves as a team) what Poorly Written Code ™ exactly means, then any tool that tries to do it by fiat is doomed to fail.
The existing lint command line tool is to all intents and purposes dead. The new analysis framework allows for more powerful linters with less false positives (package wide, with type information, and allowing suggested/automatic fixes). Porting checks to that framework has effectively already been done by Dominik in staticcheck, which as he says is also included in gopls, and thus exposed efficiently to the editors as well as being available on the command line.
There is no point spending effort maintaining a second inferior set of those rules, which is part of the reason that golint is unmaintained.
If there are checks that are missing or could be improved, I am sure Dominik would welcome suggestions or contributions, and if they don't belong in the staticcheck suite it should be easy to write small targeted tools using that framework to run alongside.
Formally deprecating this tool seems like a reasonable recognition of that reality to me.
If there turns out to be a large class of analyses that we really thing the core go team should maintain but don't belong in vet, we could talk about have a library of them available such that people can pick and choose which analyses get included into their customized runners. So far though I don't see a compelling list of checks that are not already handled well enough in existing non-core tooling.
I'm not going to say that golint is entirely useless, or that it fails at its job, or that it would be impossible to bring it up to speed with the other Go tools like @ianthehat says.
However, I fully agree that it's extremely confusing to new Go developers. I've seen the following train of thought multiple times:
- Go is working out well for us! Let's add linting to catch more issues.
- golint is the first result for "golang lint", so let's add that. Huh, it gives a lot of output, and most of it isn't that useful.
- The second "golang lint" result is golangci-lint, so let's try that. Huh, it's pretty slow, and I need to read about twenty different tools and how to configure it all.
I don't think the Go project needs to "bless" one way to lint Go code. Some will prefer a single and consistent tool with sane defaults like staticcheck, while others will prefer a fancy meta-tool that runs many third-party linters like golangci-lint. Either way, golint in its position seems to do more harm than good, so I agree that we should freeze+remove golint.
I propose that we explicitly freeze the repository, and create a plan for removing it entirely.
I think we can achieve the desired outcome without removing lint entirely. Quite a few people use the tool, and it’s unclear whether the cost of removing it would outweigh the benefits. Dep is no longer maintained nor is it recommended for use anymore, but there is no plan for it to be removed entirely from the golang organization.
I’m supportive of freezing the repository and having very clear messaging that we no longer support/maintain it, but I worry that removing it creates even more problems.
Has an attempt been made to find maintainers for the golint project? It seems that would be a prudent course of action before simply discarding the work of others and disrupting the current users.
It's worth noting that the issue that gave rise to this proposal was simply a request to add some level of configuration. Even the most basic knobs, such as inline comments to ignore certain warnings were rejected -- as far as I can tell, for no fair reason (just an arbitrary statement in the readme).
Given the interest in the issue, I find it exceedingly unlikely that someone from the community would not volunteer to introduce this capability, but nobody has (or will) because the maintainers have indicated it would not be welcome. In fact, a poster on the issue in question indicated they have withheld their PR for specifically this reason.
I would sum up the history of this proposal like so:
- The community requests a reasonable change to the tool
- Maintainers argue against it
- The community is not satisfied with the argument
- It is declared an un-maintained tool that people receive no joy from and should be removed
@adamrbennett Certainly anybody is welcome to copy the project and maintain the source code in pursuit of different goals.
I think that what you are saying is something like "let's keep maintaining golang.org/x/lint with different maintainers and with different goals." Yes, that is an option. It's not an option that I would choose myself, but this is a good place to present the arguments for taking that option. Thanks.
Retitled to make clear that we won't remove it. At most we will archive the repo and mark it read-only.
I appreciate @dominikh's comment that staticcheck is in a better position to carry the lint banner forward, and I am happy that Google is also funding staticcheck now.
The overall conversation here seems to be trending toward accepting the proposal. Will add to the next minutes.
@dominikh first off, I think I can safely speak for the majority of the Golang community at how much we all appreciate all of your work, staticcheck is truly one of those gold-standard tools that others should mimic.
Do you think a "quasi-compatibility" guide might be added to staticcheck.io that has entries for i.e. golint, errcheck, ineffassign, where each document says "here's how staticcheck overlaps with X tools, here's how to configure staticcheck to get all of/most of the functionality of X tool"? That understanding and explicit configuration advice would help users all get just onto staticcheck, and stop using golint for example.
The discussion so far seems overwhelmingly in favor of accepting this proposal.
I see one comment suggesting that we have the community take over the project inside the golang org, but that hasn't worked too well in the past - we still end up responsible for it in the long run. Freezing it opens space for any number of possible community projects instead of us having to bless one.
Based on the discussion above, this seems like a likely accept.
It seems unlikely that I’ll be able to change opinions here, but I feel like saying I personally find this decision slightly disappointing. I don’t really have any opinions on the technical aspects, but it feels to me that through this decision the Go team and larger community is taking one step back from promoting a consistent style across Go code.
In Go at Google: Language Design in the Service of Software Engineering, Rob Pike says about gofmt that
Gofmt also affects scalability: since all code looks the same, teams find it easier to work together or with others' code.
Perhaps I misunderstood the intention, but I always thought that Effective Go, the Code Review Comments and associated x/lint tooling were part of this goal of making Go code "look the same", again in the service of software engineering. That is, I agree with @dsnet in his comment golang/lint#263 (comment)
The purpose of lint is to encourage a consistent style of Go across the community. An individual developer may not agree with its style rules (I don't agree with some of them), but there is a strong benefit to consistency in the Go community.
As a result, I am surprised now to see the following quote from this issue:
People can reasonably prefer different styles. They can reasonably want to adopt some aspects of the style or not.
To be clear, I do not have a problem with configurable linters, I use them heavily myself and don't expect others to agree with all my preferences. However, I do believe that there remains a place for a common core set of lint rules recommended by the Go community. For me it feels like by adopting this proposal, the Go community is giving up on this goal of recommending a consistent style, leaving it up to whatever configurable linters people decide to write.
The argument has frequently been made in this discussion that these lint rules are availble in staticcheck. I am very thankful to Dominik Honnef for all his work, I support him with a small donation each month and I use the staticcheck tool regularly. Nonetheless, staticcheck isn't an official Go project, it's still under Dominik's Github account and he has no obligation to take feedback from anyone. I would be very happy if this proposal was instead: x/lint will be deprecated and staticcheck will replace it as an official Go project under github.com/golang/staticcheck or golang.org/x/tools/cmd/staticcheck. Moreover, as far as I understand it, staticcheck does not implement the same defaults as golint so it doesn't necessarily enforce the same "consistent style of Go across the community" that golint did. (For me the loss of the documentation lint rule is particularly disappointing.)
So ultimately I don't have strong opinions about what happens to the x/lint repository, or even the golint tool itself. I'm more concerned about what this means for the Go community's intention to promote a consistent style across Go projects.
I appreciate the concern that you raise, @mmcloughlin. I’m not sure that the adoption of this proposal is at odds with promoting a consistent style, though. x/lint will still exist for people to use, as will the style guidelines that it enforces. By freezing the repo we are formally setting expectations with x/lint’s users, while creating space for the community to fill the void for those users who want features that are outside of our documented scope.
I also appreciate that Go has very opinionated tooling to enforce consistency across codebases. From experience, though, it seems that user expectations for linters differ from code formatters or correctness tools. Perhaps it’s because we state that these are suggestions and not a gold standard, opening us up to scrutiny on what the tool should do. In any case, despite very clear documentation around the purpose, scope, and expected behavior of the tool, heated discussions asking for things we explicitly say we’re not going to do still occur.
I’m not sure how to solve these problems except by adopting this proposal for the same reasons others have stated above.
Staticcheck implements a number of style checks, some of which overlap with golint.
Are all the golang/lint rules available in staticcheck ?
Are all the golang/lint rules available in staticcheck ?
No. Staticcheck's style checks and golint overlap, but neither is a subset of the other. In most cases that's because I disagree with the checks as they exist in golint (see https://github.com/dominikh/go-tools/issues?q=is%3Aissue+is%3Aopen+decide+fate+stylecheck for examples of problematic checks), There is also some functionality, such as checking for missing docstrings, that is on my todo list.
You can consult https://staticcheck.io/docs/checks for a full list of currently existing checks in staticcheck. The ones numbered ST* are the ones pertaining to stylistic issues.
No change in consensus here, so accepted.
Sorry, typo. This proposal - to freeze and deprecate x/lint - is accepted.
(Lint itself is kind of now declined.)
The next steps for this seem to be:
- Update the README.md at the repo root with appropriate language
- Add
archivedOnGitHuboption to x/build/repos (similar to CL 289699)- Redeploy
x/build/cmd/gitmirror
- Redeploy
- Archive the GitHub repository
- Close all CLs (https://go-review.googlesource.com/q/project:lint+status:open)
- Make an announcement on golang-nuts@
Anything I’m missing?
Can you clarify the "Freeze the repo" step please: do you mean using the "frozen" convention, using the GitHub repository archive feature?
Closing all CL https://go-review.googlesource.com/q/project:lint maybe?
Sorry, typo. This proposal - to freeze and deprecate x/lint - is accepted.
(Lint itself is kind of now declined.)
@rsc Hi Russ, sorry, I'm unsure how to interpret your update, in particular "Lint itself is kind of now declined".
@ianlancetaylor, @andybons, @dominikh it's not clear to me from the conversation whether the Go team will officially promote staticcheck (or golangci-lint, gofumpt, or other similar tools popular in the community). Considering that the VSCode plugin seems to be moving to staticcheck (https://golang.org/cl/279212), would it be fair to say that official replacement for golint will be staticcheck? Or is it more likely that gopls will supersede every other tool, including staticcheck?
I was also wondering how this affects gofmt: gofmt's rules are not individually configurable in the same way the rules in the other tools mentioned above are. This would seem a contradiction. For example, gofmt does not allow if statements on a single line, although they seem to be syntactically valid and accepted by the compiler. Are there any plans or discussions around whether that is considered an issue by the core team? Or will any of the official tools change to match the spirit of the others (for example gofmt becoming configurable, or staticcheck becoming non-configurable)?
While I am on this subject, I see that gopls allows to apply gofumpt (docs), does it mean that gofumpt might one day replace gofmt?
In conclusion I would appreciate inestimably if you could shed some light on what is the officially recommended lint tooling today.
I'm desperate for clarity on the official stance, as I'm working on a large codebase and it would be nice if we could reaffirm the original idea that "Gofmt's style is no one's favorite, yet gofmt is everyone's favorite.".
Years ago the lack of configurability of golint and gofmt was for me one of the main attraction points, I didn't mind disagreeing with some choices. Instead it allowed to definitively do away with styling disputes, thus saving energy, sparing morale-sapping disagreements, and making teams happier and more productive.
I'm sorry, there is no officially recommended lint tool. You should choose a lint tool that will work well for your project. staticcheck is a fine choice.
To answer a few of your gopls questions:
Or is it more likely that
goplswill supersede every other tool, includingstaticcheck?
gopls itself allows you to view staticcheck diagnostics in the editor, but it is not a lint tool. While gopls may include some additional checks, it is mostly go vet + staticcheck (if its enabled). gopls also includes some opt-in checks that users may want to use occasionally but not in every project or not all of the time. See Analyzers for the full list.
While I am on this subject, I see that
goplsallows to applygofumpt(docs), does it mean thatgofumptmight one day replacegofmt?
We added support for gofumpt in gopls because it is a popular formatter that many users wanted to add to their editors. The VS Code Go extension supported it as a custom formatter, and we didn't want users to lose out on that behavior with gopls. We are looking into adding support for gofumpt as an analyzer (mvdan/gofumpt#78) instead of a formatter. But to fully answer your question, gofumpt will not replace gofmt.
Is anyone opposed to me going through with the steps outlined in #38968 (comment)? I'm not an owner of x/lint, but I guess there hasn't really been one for years now. I have enough permissions to abandon CLs, and I can do the README change as a CL itself.
Ah, I would need to be made an owner of the repository to be able to archive it. Or someone with the access can do that after I'm done with the other steps, I don't mind. Who can help with either of those?
Thanks, I think that would be great. This is always about number 10 on my list and never makes it any higher.
Thank you @mvdan, I agree it would be very helpful.
I added one more step to Andy's comment, to add archivedOnGitHub option to x/build/repos (similar to CL 289699), which I know needs to be done from experience archiving the gddo repository.
I can assist you on the GitHub repository archival which needs admin access and redeploying x/build/cmd/gitmirror after the configuration change.
@dmitshur I just realised I can't close PRs that never made it to Gerrit (e.g. because they lacked CLA), and neither can I close issues. Could you give me write permissions on the repo? It would be unfortunate to only close the PRs with corresponding Gerrit CLs - gopherbot is closing those for me.
As written by @ianlancetaylor, golint is a style guide checker for Effective Go. With the addition of generics I suppose that Effective Go will be updated; will there be a style guide checker for the update version?
Change https://golang.org/cl/318189 mentions this issue: README: make it clear that golint is deprecated and frozen
Change https://golang.org/cl/318190 mentions this issue: repos: x/lint is being deprecated and frozen
It's getting late, so I'll leave it here for now. All CLs/PRs/issues closed, and I've sent the changes to the README and the build repository. Assuming those are merged in the next day or two, we can go ahead with archiving the repository and I'll send a short announcement email to go-nuts.
@perillo There are no current plans for writing a new style checker.
Thanks @dmitshur and @ianlancetaylor for the speedy reviews. I've archived the repository. The only two items remaining are redeploying gitmirror and sending the announcement email. I'll do the latter tomorrow.
Thanks very much for taking care of this.
gitmirror is redeployed with CL 318190, and its errors are now fixed (they could be seen at https://farmer.golang.org/#health previously).
The announcement to golang-nuts is done: https://groups.google.com/g/golang-nuts/c/rCP70Aq_tBc/m/8QHp6_cqBgAJ
Thanks all for your help and patience. Closing this as done.