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.

Sign up for GitHub

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

Jump to bottom

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 Issues asking for a new feature that does not need a proposal. 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
gopls/v0.8.0

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 Issues asking for a new feature that does not need a proposal. 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
Member

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

@findleyr
Copy link
Member

findleyr commented Jan 4, 2022

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

@findleyr findleyr reopened this Jan 4, 2022
@findleyr findleyr modified the milestones: gopls/on-deck, gopls/v0.8.0 Jan 4, 2022
@hyangah hyangah mentioned this issue Feb 8, 2022
Closed
@gopherbot
Copy link
Contributor

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

@findleyr
Copy link
Member

findleyr commented Feb 11, 2022 •
edited
Loading

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
@ansaba
go/analysis/passes/tests: Check malformed fuzz target.
2ff4db7 
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 <[email protected]>
Trust: Peter Weinberger <[email protected]>
gopherbot pushed a commit to golang/tools that referenced this issue Feb 16, 2022
go/analysis/passes/tests: fix a missed rename from CL 374495
9095d10 
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 <[email protected]>
Run-TryBot: Bryan Mills <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
gopls-CI: kokoro <[email protected]>
Reviewed-by: Tim King <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
@gopherbot
Copy link
Contributor

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

@findleyr findleyr mentioned this issue Feb 28, 2022
Closed
@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
Member

Marking this as done for gopls/v0.8.0. Have filed #51398 for remaining functionality, which we can perhaps do for [email protected].

@rsc rsc mentioned this issue Mar 2, 2022
Open
4 tasks
@suzmue suzmue mentioned this issue Mar 21, 2022
Open
@rsc rsc unassigned ansaba Jun 22, 2022
@gopherbot
Copy link
Contributor

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
@ansaba @gopherbot
go/analysis/passes/tests: enable fuzz checks in 'tests' analysis pass…
f4e613e 
… 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 <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
Auto-Submit: Nooras Saba‎ <[email protected]>
@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.
Assignees
No one assigned
Labels
FeatureRequest Issues asking for a new feature that does not need a proposal. 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
Milestone
gopls/v0.8.0
Development

No branches or pull requests
5 participants
@ansaba @timothy-king @hyangah @gopherbot @findleyr