Proposal: Clothe naked returns

[flimzy]

This may not be a good fit for this tool, and even if it is, it may be controversial. It may even be technically infeasible for reasons I haven't been able to imagine yet. So this is as much an RFC as a proposal.

Proposal

But to the meat of the matter: I'm proposing that gofumpt rewrite naked returns to no longer be naked. I think this can be a simple matter of copying the names of return values from the function signature to any naked return statements. Example:

func sum(a, b int) (result int) {
    result = a+b
    return
}

would be rewritten to:

func sum(a, b int) (result int) {
    result = a+b
    return result
}

Rationale

IMHO, naked returns are one of the worst features of Go. I know I'm not alone in this, although I haven't done any sort of formal survey. For a language that values readability, this feature seemingly always goes against that ideal. Granted, for trivially short functions (like my example above), the naked return doesn't really hurt readability that much. The problems arise for much longer functions. But even in these short cases, adding the return result doesn't seem to hurt readability, either.

The only argument I can think of in favor of naked returns is the popular "less typing". But by having a tool like gofumpt do this, those who like less typing can still have their way, without hurting readability.

Possible Problems

I've spent some time trying to find places where the simple rule of "copy the named return values to the naked return statement" might break things. I can't think of any. There are a few places where a naked return might be ambiguous, with regard to shadowed variables, but these are already prohibited by the compiler. So I think we can assume that any valid Go code can apply this rule safely.

[mvdan]

"clothing" returns is such a neat little name for this rewrite :)

I'm fully behind your rationale. I avoid naked returns practically everywhere.

I'm not sure this belongs in a formatter, though, even though it's meant to be more aggressive and strict than gofmt. You could squint a bit and say that such a rewrite would increase consistency, but the rewrite itself adds new identifiers to the syntax tree, which is not something that gofumpt has ever done before.

Another way to look at this is that such a rewrite is only safe to do with type information. gofumpt works off of the syntax tree alone, so it would have to start keeping track of what function body it's currently in, whether it's got any named result parameters, and what their names are. Not terribly expensive to do this, but it would be the first instance where we'd need to make gofumpt aware of types, further indicating it's not a formatting concept.

All that said, if this were added in another tool, I'd use it. Which tool that would be is a good question, and I'm not sure. If that's how you ended up suggesting it here, I completely understand.

[flimzy]

Thanks @mvdan. I appreciate your arguments against including this in this tool. They're in line with some of my skepticism about including it here as well. Let me offer a small response, and some additional thoughts:

• From a useability standpoint, I think most would consider naked-vs-clothed returns to be a formatting issue (or a "style" issue, which is often considered synonymous, IME). I've never seen a code review discussion discussing this in terms of the AST or type information etc. While these are important implementation details, I'm not sure if they enter into the minds of the users of such tools.
• Indeed, what other tool would this belong in? It wouldn't be hard to create a new, stand-alone tool. But it would be hard to get meaningful adoption of such another tool.
• Selfish motivation: Now that Go 2 is "off the table", my dreams of a Go 2 without naked returns have been dashed. The next best thing I can imagine is a world in which gofmt itself employs the proposed rule. I figured the best way to explore that possibility would be by starting here. 😉

Probably the next best option for another tool would be a golangci-lint linter that not only flags, but rewrites naked returns. This would be better than another stand-alone tool, for adoption, but does pose a number of additional challenges. In particular, it's often problematic to enable rewriting rules in golangci-lint for various reasons. Most important, it's currently not possible to enable rewriting on a per-linter basis, and some rewrite rules aren't always safe, so it's dangerous to enable rewriting across all linters (this is why I've virtually never used golangci-lint's rewriting capabilities).

[mvdan]

I think you're right that most people would see this as a style issue, and the line between formatting and styling is a fuzzy one.

I also agree that golangci-lint wouldn't be a good home for such a rewriter. Personally I'm not a fan of that tool, so I'd avoid using the rewriter even if it did exist there.

I'm open to accepting a contribution for this. Would you be able to do a brief study on how prevalent naked returns are across popular Go codebases? My personal impression is that they are very rare, so adding this to gofumpt would not be controversial. But I have no idea if other codebases use it often or perhaps even consistently. It would be particularly interesting to see if any Go repos use gofumpt and use naked returns with some frequency.

[mvdan]

One option we didn't talk about is an analyzer (https://pkg.go.dev/golang.org/x/tools/go/analysis#hdr-Analyzer) which could then be added to gopls or other "meta linters" easily. However, the bar for adding these to gopls is somewhat high, since they need to be bundled at compile time. So far I think they've only added staticcheck and gofumpt, and gofumpt isn't even an analyzer (yet).

[flimzy]

golangci-lint also uses the analyzer interface, so creating a linter for that project would kill that second bird with the same stone.

[flimzy]

Here are some very high-level, preliminary results from my investigation. I've counted total number of clothed and naked returns, as well as the number of functions that mix naked and non-naked returns. The discrepancy between total and clothed+naked represent returns in functions that have no return values. I've also excluded generated code where it's easy to identify, and the testdata and vendor directories.

I've compared three code corpora:

1. The Go standard library (1.21.0)
2. The repos from the top 100 modules listed at grank.io
3. The subset of 2 that mention gofumpt within the repo, as determined by a simple naïve grep

Corpus	Total returns	Clothed	Naked	Mixed funcs
stdlib	60337	54561	1664 (2.76%)	283
grank Top 100	287574	247692	32635 (11.35%)	10382
gofumpt'ed repos	39581	37706	337 (0.85%)	56

I used this very unsophisticated, thrown-together tool to do the analysis.

---

Update: The majority of the naked returns in the grank list come from a single repo: https://github.com/tencentcloud/tencentcloud-sdk-go

If I pull that one out, the results look like:

Corpus	Total returns	Clothed	Naked	Mixed funcs
stdlib	60337	54561	1664 (2.76%)	283
grank Top 100	175542	166514	1889 (1.08%)	10382
gofumpt'ed repos	39581	37706	337 (0.85%)	56
tencentcloud-sdk-go	112032	81178	30746 (27.44%)	10061

[mvdan]

I used this very unsophisticated, thrown-together tool to do the analysis.

Did you mean to make that open source, or is the link perhaps incorrect?

Ignoring that SDK module seems fine; the examples in the README don't even follow gofmt :) Though note that grank appears to heavily skew towards libraries. The top ten are all libraries, and out of them six are primarily for testing, for example. I would personally think that large non-library projects are significantly more relevant than testing libraries. For example, here's a rather simple ranking based on stars and forks on github: https://github.com/mvdan/corpus/blob/master/top-100.tsv

~1% across popular repos and ~0.8% across those which are aware of gofumpt sounds like what I would expect, and that doesn't worry me in terms of giving this a try. ~2.7% in std is definitely more worrying, though. I think we'd have to implement a first version of this, run it across some good codebases like std, and see whether the result looks better or not. If we're not sure, then we shouldn't release it.

[flimzy]

Did you mean to make that open source, or is the link perhaps incorrect?

I did. It's public now.

here's a rather simple ranking based on stars and forks on github

I'll follow up with an analysis of some of these.

[philoserf]

related items:

• https://github.com/alexkohler/nakedret
• https://github.com/gavv/returnstyles

[flimzy]

Took me a while to get back to this, but I've now run the naked return analysis on the collection of repos at https://github.com/mvdan/corpus/blob/master/top-100.tsv:

Corpus	Total returns	Clothed	Naked	Mixed funcs
top-100	1264051	1184829	33330 (2.64%)	3501