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

exclude maphash methods that "never fail" #252

Merged
merged 2 commits into from
Oct 16, 2024

Conversation

djdv
Copy link
Contributor

@djdv djdv commented Aug 23, 2024

These methods are documented to "never fail", so it should be okay to ignore the return values.

hash.Hash is already excluded, but unlike the other subpkgs of hash, hash/maphash doesn't follow the convention of using a New() hash.Hash constructor; instead it exposes a struct *maphash.Hash, who's methods are called directly.

Since this is part of the standard library and likely to be called in this way, it seemed appropriate to add them to the default exclusion list.
You think so?

@kisielk
Copy link
Owner

kisielk commented Aug 23, 2024

Seems right to me.

@djdv
Copy link
Contributor Author

djdv commented Aug 24, 2024

Cool 😎
I tested this commit manually against the original code I was linting, as well as this:

package main
import "hash/maphash"
func main() {
	var hasher maphash.Hash
	hasher.Write(nil)
	hasher.WriteByte(0)
	hasher.WriteString("")
}

and it seems to detect (and exclude) the signatures correctly.

@dtcaciuc
Copy link
Collaborator

dtcaciuc commented Oct 9, 2024

@djdv Do you mind adding a test case? You can either add those calls to an existing function under testdata or make a new one just for hashmaps (as long as its in main package)

@djdv
Copy link
Contributor Author

djdv commented Oct 16, 2024

I wasn't so sure how to add this test case, is something like this right?
If so, maybe it also makes sense to include c277fb1 but if not it could be dropped.

Edit:
I was hesitant about this because running go test was yielding this error before and after these patches:

internal error: package "crypto/sha256" without types was imported from "github.com/kisielk/errcheck/testdata"

Turns out I was running the tip build of the go command (oops).
Running the tests on Go stable seems to work as expected. The tests fail before the patch, but not after.

@dtcaciuc
Copy link
Collaborator

Looks good, thank you!

@dtcaciuc dtcaciuc merged commit df44f75 into kisielk:master Oct 16, 2024
5 checks passed
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

Successfully merging this pull request may close these issues.

4 participants