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

ITP: linter to enforce named / unnamed result parameters [intent-to-provide] #1201

Closed
leonklingele opened this issue Jan 27, 2022 · 4 comments

Comments

@leonklingele
Copy link
Contributor

I maintain a linter which enforces the use of named / unnamed result parameters: https://github.com/leonklingele/funcresult
Unfortunately, it did not find its way to golangci-lint — was rejected: golangci/golangci-lint#2526 (comment)

Would you be willing to accept a linter which provides such functionality?

@quasilyte
Copy link
Member

quasilyte commented Jan 28, 2022

I'm not sure either of these options can be followed consistently across a real codebase.

Suppose that you want to avoid the named results at all costs. Then you'll have a problem when you want to override the return value from the deferred call. It's useful in combination with recover(). You can catch a panic and assign some error to the output parameter.

Using the named results all the time looks just too weird. In my experience, people tend to use them only in a few situations. The one is already mentioned above and another one is when your return values are not self-explanatory.

There is also an unnamedResult checker that does a somewhat similar check.

While it may have some merit when you want to catch accidental named params, I don't think that unconditional lint is a way to go. A new checker that reports unnecessary named parameters could be useful.
For example, it can use the opposite heuristics of unnamedResult checker. When returned types are already named or "obvious" (like error), we may omit dull names like (result Result, err error) (but only if the function body doesn't use a defer+assignment, see the case above).

@leonklingele
Copy link
Contributor Author

I'm not sure either of these options can be followed consistently across a real codebase.

For me personally, the linter is useful to prevent accidentally-named-result-params. When named result params are needed from time to time, there's always the option to whitelist them with a nolint comment, which can easily be grep-ed for.
So it's more about setting a general coding standard than strictly enforcing some coding style.

@quasilyte
Copy link
Member

Don't get me wrong, I'm not saying that this idea shouldn't exist.
But it doesn't seem to fit go-critic well enough.
It does fit some very strict custom rules for a specific team though.

We have an extension point via https://github.com/quasilyte/go-ruleguard to allow such very opinionated, team-specific checks. It's possible to express funcresult linter in most cases, but it'll be a little bit awkward.

@leonklingele
Copy link
Contributor Author

Part of golangci/golangci-lint#2701.

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