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

proposal: Go 2: error on unused function parameters #39118

Open
Denton-L opened this issue May 17, 2020 · 4 comments
Open

proposal: Go 2: error on unused function parameters #39118

Denton-L opened this issue May 17, 2020 · 4 comments
Labels
Milestone

Comments

@Denton-L
Copy link

@Denton-L Denton-L commented May 17, 2020

Currently, if there are any unused local variables in a function, it will fail to compile with something like x declared but not used. However, the same is not true for any unused function parameters. The following is perfectly valid Go code:

package main

func f(f int) {}

func main() {
	f(0)
}

Proposal

Disallow unused parameters in functions. In case an unused variable is desired, it will have to be written as _, just like other unused variables. f() in the above would be rewritten as

func f(_ int) {}

Discussion

Although failing to use a parameter is not always a bug, in the case where it's unintentionally done, it is just as bad as failing to use a local variable.

There is one case where an unused parameter is not a bug. This is when a function is written to satisfy an interface and one of the parameters is actually not needed. It would be better to force programmers to explicitly mark the parameter as unused via _ so that they are consciously choosing not to use it.

One argument against this is that the parameter's name, even if unused, serves as implicit documentation as to the purpose of the parameter. However, I don't think that this is necessarily true. If a developer were using the function as part of an interface, the interface would've documented the purpose of the parameter via its name, otherwise it wasn't important anyway.

If a developer were using the function directly on a concrete type, then they would not need to know the purpose of the unused variable at all since it's, well, unused. Explicitly marking this with _ would allow the user to clearly see this from the function signature. Thus, it would alleviate any (albeit small) cognitive overhead about deciding what arg to pass and just specify a zero-value.

@gopherbot gopherbot added this to the Proposal milestone May 17, 2020
@gopherbot gopherbot added the Proposal label May 17, 2020
@seankhliao
Copy link
Contributor

@seankhliao seankhliao commented May 17, 2020

see also #7892

@mvdan
Copy link
Member

@mvdan mvdan commented May 18, 2020

I agree with previous comments that this would break far too much existing code.

Having said that, I agree that there is value in finding truly unused parameters. You can take a look at https://github.com/mvdan/unparam, which attempts the static analysis route.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented May 18, 2020

Yeah, maybe this would have been a good idea early on, but at present it seems impossible. Even if we provide conversion tools, we can't realistically require thousands of Go programs to be changed. Or, to put it another way, if we are going to require thousands of Go programs to be changed, I think we need a much bigger benefit than this will give us.

@robpike
Copy link
Contributor

@robpike robpike commented May 19, 2020

When there is only one argument, you don't need to write

f(_ int)

You could just write

f(int)

I agree that the benefit is too small for the work involved now. One way to narrow the focus might be to add a vet test that reports, only when we know all the call sites, that a function parameter is unused. That might catch a few things at the cost of a dance being required when the function is being developed, analogous to assigning variables to _ when code is under development.

Even with that restriction, it's close to harmless. It's my feeling that a value that returns from a function matters much more than a variable being passed to one, as ignoring the former has been shown to adumbrate many bugs, but there is no evidence I've seen for an unused function parameter mattering much at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.