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

x/lint: freeze and deprecate #38968

Closed
ianlancetaylor opened this issue May 8, 2020 · 38 comments
Closed

x/lint: freeze and deprecate #38968

ianlancetaylor opened this issue May 8, 2020 · 38 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. Proposal Proposal-Accepted
Milestone

Comments

@ianlancetaylor
Copy link
Contributor

ianlancetaylor commented May 8, 2020

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.)

CC @dsymonds @andybons @dominikh

@gopherbot gopherbot added this to the Proposal milestone May 8, 2020
@dominikh
Copy link
Member

dominikh commented May 8, 2020

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.

@dgoodine
Copy link

dgoodine commented May 8, 2020

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.

@ianthehat
Copy link

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.

@mvdan
Copy link
Member

mvdan commented May 9, 2020

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:

  1. Go is working out well for us! Let's add linting to catch more issues.
  2. 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.
  3. 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.

@andybons
Copy link
Member

andybons commented May 9, 2020

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.

@adamrbennett
Copy link

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:

  1. The community requests a reasonable change to the tool
  2. Maintainers argue against it
  3. The community is not satisfied with the argument
  4. It is declared an un-maintained tool that people receive no joy from and should be removed

@ianlancetaylor
Copy link
Contributor Author

@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.

@rsc rsc changed the title proposal: freeze and perhaps remove golang.org/x/lint proposal: freeze and deprecate golang.org/x/lint May 20, 2020
@rsc rsc changed the title proposal: freeze and deprecate golang.org/x/lint proposal: x/lint: freeze and deprecate May 20, 2020
@rsc
Copy link
Contributor

rsc commented May 20, 2020

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.

@bufdev
Copy link

bufdev commented May 24, 2020

@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.

@rsc
Copy link
Contributor

rsc commented May 27, 2020

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.

@mmcloughlin
Copy link
Contributor

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.

@andybons
Copy link
Member

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.

@pierrre
Copy link

pierrre commented May 28, 2020

Staticcheck implements a number of style checks, some of which overlap with golint.

Are all the golang/lint rules available in staticcheck ?

@dominikh
Copy link
Member

dominikh commented Jun 1, 2020

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.

@rsc
Copy link
Contributor

rsc commented Jun 3, 2020

No change in consensus here, so accepted.

@rsc rsc closed this as completed Jun 3, 2020
@rsc rsc reopened this Jun 3, 2020
@rsc
Copy link
Contributor

rsc commented Jun 3, 2020

Sorry, typo. This proposal - to freeze and deprecate x/lint - is accepted.
(Lint itself is kind of now declined.)

minamijoyo added a commit to minamijoyo/tfmigrate that referenced this issue Dec 27, 2021
We have been using golint as a linter, but golint was officially deprecated.
golang/go#38968

The revive is a drop-in replacement of golint.
https://github.com/mgechev/revive

It's possible to use revive as it is, but I'd like to use revive via
golangci-lint so that we can add more linters easily.
https://github.com/golangci/golangci-lint

The golangci-lint is a meta linter.
I'll take this opportunity to introduce some additional checks.
minamijoyo added a commit to minamijoyo/tfmigrate that referenced this issue Dec 27, 2021
We have been using golint as a linter, but golint was officially deprecated.
golang/go#38968

The revive is a drop-in replacement of golint.
https://github.com/mgechev/revive

It's possible to use revive as it is, but I'd like to use revive via
golangci-lint so that we can add more linters easily.
https://github.com/golangci/golangci-lint

The golangci-lint is a meta linter.
I'll take this opportunity to introduce some additional checks.
minamijoyo added a commit to minamijoyo/hcledit that referenced this issue Jan 6, 2022
We have been using golint as a linter, but golint was officially deprecated.
golang/go#38968

The revive is a drop-in replacement of golint.
https://github.com/mgechev/revive

It's possible to use revive as it is, but I'd like to use revive via
golangci-lint so that we can add more linters easily.
https://github.com/golangci/golangci-lint

The golangci-lint is a meta linter.
I'll take this opportunity to introduce some additional checks.
minamijoyo added a commit to minamijoyo/tfupdate that referenced this issue Jan 13, 2022
We have been using golint as a linter, but golint was officially deprecated.
golang/go#38968

The revive is a drop-in replacement of golint.
https://github.com/mgechev/revive

It's possible to use revive as it is, but I'd like to use revive via
golangci-lint so that we can add more linters easily.
https://github.com/golangci/golangci-lint

The golangci-lint is a meta linter.
I'll take this opportunity to introduce some additional checks.
minamijoyo added a commit to minamijoyo/tfschema that referenced this issue Jan 20, 2022
We have been using golint as a linter, but golint was officially deprecated.
golang/go#38968

The revive is a drop-in replacement of golint.
https://github.com/mgechev/revive

It's possible to use revive as it is, but I'd like to use revive via
golangci-lint so that we can add more linters easily.
https://github.com/golangci/golangci-lint

The golangci-lint is a meta linter.
I'll take this opportunity to introduce some additional checks.
minamijoyo added a commit to minamijoyo/tfschema that referenced this issue Jan 20, 2022
We have been using golint as a linter, but golint was officially deprecated.
golang/go#38968

The revive is a drop-in replacement of golint.
https://github.com/mgechev/revive

It's possible to use revive as it is, but I'd like to use revive via
golangci-lint so that we can add more linters easily.
https://github.com/golangci/golangci-lint

The golangci-lint is a meta linter.
I'll take this opportunity to introduce some additional checks.
minamijoyo added a commit to minamijoyo/myaws that referenced this issue Jan 27, 2022
We have been using golint as a linter, but golint was officially deprecated.
golang/go#38968

The revive is a drop-in replacement of golint.
https://github.com/mgechev/revive

It's possible to use revive as it is, but I'd like to use revive via
golangci-lint so that we can add more linters easily.
https://github.com/golangci/golangci-lint

The golangci-lint is a meta linter.
I'll take this opportunity to introduce some additional checks.
fundthmcalculus added a commit to trinsic-id/okapi that referenced this issue Feb 22, 2022
@golang golang locked and limited conversation to collaborators May 14, 2022
@rsc rsc unassigned mvdan Jun 23, 2022
@rsc rsc moved this to Accepted in Proposals Aug 10, 2022
@rsc rsc added this to Proposals Aug 10, 2022
@rsc rsc removed this from Proposals Oct 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. Proposal Proposal-Accepted
Projects
None yet
Development

No branches or pull requests