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

vet: point out unused function arguments #7892

Closed
gopherbot opened this issue Apr 29, 2014 · 3 comments
Closed

vet: point out unused function arguments #7892

gopherbot opened this issue Apr 29, 2014 · 3 comments

Comments

@gopherbot
Copy link
Contributor

by tobias.schwab@dynport.de:

When applying the "extract method pattern" I quite often forget to actually
use extracted arguments and keep the hardcoded values in the the extracted function
which leads to errors.

I think that for not exported functions at least vet should complain about unused
function arguments. My personal opinion is that I would treat unused arguments of not
exported functions the same as unused variables (and cause a compiler error).

For exported functions things are a bit different because of API stability reasons. A
similar workaround to handling unused variables ("_ = unusedArgument" at the
beginning of the function) would be possible though and also enable documenting (via
comment) why a specific arguments is not used.
@cznic
Copy link
Contributor

cznic commented Apr 29, 2014

Comment 1:

""""
My personal opinion is that I would treat unused arguments of not exported functions the
same as unused variables (and cause a compiler error).
""""
Methods implementing an interface it's completely ok that some implementations of a
particular interface are legitimately never using some of the arguments. Consider
type Fooer interface {
        Foo()
        CloseTempFile(AndRemoveIt bool)
}
An all memory implementation of Fooer has no use of the AndRemoveIt argument.
Now consider that also plain functions can be used in the same, or almost the same way
as interface implementing types. Either passed as a callback or converted to an
interface type (like http.HandlerFunc) and then again, unused arguments might be
perfectly legitimate.
IOW, making unused args, even when only for exported function, a compile error is IMO
not an option.

@griesemer
Copy link
Contributor

Comment 2:

We are not making language changes; especially not changes that would invalidate
existing code.
Treating this as a request for potential future vet functionality; changed title
accordingly. Handing over to r.

Labels changed: added repo-tools.

Owner changed to @robpike.

@robpike
Copy link
Contributor

robpike commented May 16, 2014

Comment 3:

The issue here is eliminating false positives. The request is fine but if vet complains
too often incorrectly, the check is not valuable.
If it is a method, whether internal or exported, it might satisfy some interface and
there's no practical way to know which one because interfaces are not bound to the
package.
func (t or T) Read(p buf) (int, error) { return 0, io.EOF }
satisfies io.Reader, which means the parameter cannot be removed. Plus it might make
perfect sense to document that p is unused. Thus complaining about unused parameters to
methods will generate too many false positives.
The check cannot be made to work reliably for methods. What about functions?
If it is an exported function, there could be many reasons that the parameter exists but
is unused; for instance, it might be historical but now irrelevant. Again, too many
false positives.
If it is an internal function, we could provide this test usefully, I believe, but there
are very few such things, on average, plus the question explicitly asked about methods,
not functions.
We therefore conclude that vet cannot provide a low-enough noise signal for enough cases
to make this worthwhile.
Marking this WontFix.

Status changed to WontFix.

@golang golang locked and limited conversation to collaborators Jun 25, 2016
@rsc rsc unassigned robpike Jun 23, 2022
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants