Clothe returns #287
Clothe returns #287
Conversation
|
This is failing to properly adjust the location of comments after inserting the return values. I'll continue working on that. |
|
See flimzy@64e6495 for what this new transform does on the gofumpt repo itself. I'll do further testing on a larger collection of repos when time permits. |
|
I've run this change against the stdlib, as well as the repos in this list. I found two panics, which already existed in gofumpt prior to this change. Otherwise, the output appears as expected and, in my subjective opinion, much easier to read. I could share some diffs, but they're bulky and boring to read, honestly. If you'd like me to do any additional investigation, please let me know. |
flimzy
force-pushed
the
clotheReturns
branch
2 times, most recently
from
71eb4a2 to
61a1113
Compare
|
Anything else before merging this one, @mvdan ? |
|
Apologies for the slowness and thanks for bearing with me. I'm not sure why I didn't get to this sooner, I must admit. I hope it's okay that I've pushed to your branch. First, to rebase your changes. Second, to simplify the logic to track parent func types; see my commit message for details. Does my change look OK to you? If so, please squash all the commits and write a commit message summarizing the change. I'm happy to do this for you if you prefer, too. The change LGTM at this point, but I'd want to merge a single commit. |
|
@mvdan Looks great to me. Glad you had a chance to look at it and simplify things. Feel free to squash if that's quick for you. Otherwise I'll get to it in the coming days. |
|
ack, I'll do that then :) |
When a function has named results, one can use a "naked return"
to implicitly return those variables. That is, the code
func Foo() (err error) {
return
}
is functionally equivalent to
func Foo() (err error) {
return err
}
Naked returns exist primarily for the sake of avoiding repetition,
but even the Go tour recommends that their use should be limited:
Naked return statements should be used only in short functions,
as with the example shown here. They can harm readability in longer functions.
Most Go functions out in the wild aren't short, and in practice,
naked returns tend to be avoided in larger or collaborative codebases
as they confuse the reader about whether any values are being returned,
or which values are being returned if any.
Given that gofumpt is already opinionated, and its use tends to be on
Go modules on the larger side or with multiple contributors,
it seems best to rewrite naked returns to "clothe" them.
Fixes mvdan#285.
|
Squashed, and commit message written. I also tweaked the README and comments a bit. Will merge on green. |
Fixes #285
At present, this transformation ignores naked returns in functions with blank-named return values (i.e.
func() (_ int, err error)) because rewriting them requires a literal zero value, which seemed unnecessary to evaluate the usefulness of this rule. It can be updated if we decide to move forward with this.There's probably also room to improve the algorithm used to determine the nearest function parent to a given return statement. I didn't see any prior art within this project to copy, so I chose an easy option to implement, which I expect is enough to evaluate the usefulness. And we can optimize if deemed necessary.