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

x/tools/gopls: analyze Fuzz targets in the go/analysis/tests analyzer #50198

Closed
hyangah opened this issue Dec 15, 2021 · 9 comments
Closed

x/tools/gopls: analyze Fuzz targets in the go/analysis/tests analyzer #50198

hyangah opened this issue Dec 15, 2021 · 9 comments
Labels
FeatureRequest FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@hyangah
Copy link
Contributor

hyangah commented Dec 15, 2021

Idea came from #46896 (comment)

f.Fuzz takes any (aka interface{}) type but the parameter must be a fuction with no return value whose first argument is *T and whose remaining arguments match the matching testin.F.Add call.

Add an analyzer to detect misuse of Fuzz API.

@hyangah hyangah added help wanted FeatureRequest gopls Issues related to the Go language server, gopls. labels Dec 15, 2021
@hyangah hyangah added this to the gopls/on-deck milestone Dec 15, 2021
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Dec 15, 2021
@hyangah
Copy link
Contributor Author

hyangah commented Dec 16, 2021

We think this is useful outside gopls and we can consider to add this to vet in the future.

@timothy-king
Copy link
Contributor

Roughly a dup of #46218. The suggestion for putting this into go/analysis/passes/tests/tests.go is a good one. The only real reason I can see to delay going into cmd/vet is that cmd/vet is under code freeze ATM.

@findleyr
Copy link
Contributor

Let's close this as a dupe. We can start #46218 in gopls, but I think it eventually belongs in vet.

@findleyr
Copy link
Contributor

findleyr commented Jan 4, 2022

Err, as I'm putting together gopls@v0.8.0 planning, I think it would be nice to have a tracking issue for just the gopls part of this. Reopening.

@gopherbot
Copy link

Change https://go.dev/cl/374495 mentions this issue: go/analysis/passes/tests: Check malformed fuzz target.

@findleyr
Copy link
Contributor

findleyr commented Feb 11, 2022

Minor correct: fuzz targets can have return values.
EDIT: the testing package is fixed to disallow return values.

gopherbot pushed a commit to golang/tools that referenced this issue Feb 16, 2022
This will validate that first letter after FuzzFoo() should be uppercase
Also, following validation will be performed for f.Fuzz() calls :
	1. f.Fuzz() should call a function and it should be of type (*testing.F).Fuzz().
	2. The called function in f.Fuzz(func(){}) should not return result.
	3. First argument of func() should be of type *testing.T
	4. Second argument onwards should be of type []byte, string, bool, byte,
	   rune, float32, float64, int, int8, int16, int32, int64, uint, uint8, uint16,
	   uint32, uint64

For golang/go#50198

Change-Id: I540daf635f0fe03d954b010b9b5f8616fd5df47a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/374495
Reviewed-by: Robert Findley <rfindley@google.com>
Trust: Peter Weinberger <pjw@google.com>
gopherbot pushed a commit to golang/tools that referenced this issue Feb 16, 2022
The function printAcceptedFuzzType was renamed to
formatAcceptedFuzzType after PS6 of CL 374495, but the rename was
missed in one spot and didn't get caught by TryBots.

Updates golang/go#50198

Change-Id: Ib51d26b8c159dbdc3beeac9c7c98f19690e82228
Reviewed-on: https://go-review.googlesource.com/c/tools/+/386314
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Tim King <taking@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@gopherbot
Copy link

Change https://go.dev/cl/386314 mentions this issue: go/analysis/passes/tests: fix a missed rename from CL 374495

@findleyr findleyr changed the title x/tools/gopls: analyzer for Fuzz x/tools/gopls: analyze Fuzz targets in the go/analysis/tests analyzer Feb 28, 2022
@findleyr
Copy link
Contributor

Marking this as done for gopls/v0.8.0. Have filed #51398 for remaining functionality, which we can perhaps do for gopls@v0.8.1.

@gopherbot
Copy link

Change https://go.dev/cl/471295 mentions this issue: go/analysis/passes/tests: enable fuzz checks in 'tests' analysis pass for cmd/vet

gopherbot pushed a commit to golang/tools that referenced this issue Mar 30, 2023
… for cmd/vet

This will remove the flag analysisinternal.DiagnoseFuzzTests created during golang/go#50198.Malformed fuzz target check will be enabled for cmd/vet.

For golang/go#46218

Change-Id: I5cc8d685a57060f8dd84c1957f0d296a6205ddb6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/471295
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Nooras Saba‎ <saba@golang.org>
@golang golang locked and limited conversation to collaborators Feb 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FeatureRequest FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

5 participants