Navigation Menu

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

Jump to bottom

x/tools/go/analysis/passes/nilness: detect wrapping of nil errors #32808

Open
ainar-g opened this issue Jun 27, 2019 · 3 comments
Open

x/tools/go/analysis/passes/nilness: detect wrapping of nil errors #32808

ainar-g opened this issue Jun 27, 2019 · 3 comments
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@ainar-g
Copy link
Contributor

ainar-g commented Jun 27, 2019 •
edited

As of 4874f863, the nilness check doesn't seem to trigger on fmt.Errorf calls with an error that is proven nil. Example:

func f() error {
	exists, err := g()
	if err != nil {
		return errors.Wrap(err, "error1")
	}

	if !exists {
		return fmt.Errorf("nothing: %w", err)
	}

	return nil
}

Same with the %v verb. I feel like most people would not want to actually produce a nothing: <nil> or a nothing: %!w(<nil>) message here.

Inspired by dominikh/go-tools#529.
@andybons
Copy link
Member

@mpvl @jba @neild

@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 28, 2019
@andybons andybons added this to the Unreleased milestone Jun 28, 2019
@jba
Copy link
Contributor

jba commented Jul 5, 2019

Currently the nilness check only knows about the Go language, not the standard library. We could extend it to check static calls to known functions, maybe along the lines of https://golang.org/cl/184939. Then we could borrow format-parsing code from the printf check to perform the %w check.

I don't know if any of this is a good idea or not. @matloob, @ianthehat, @alandonovan, your thoughts?

@alandonovan
Copy link
Contributor

The nilness check could be made to return a result value to other analyses through which they could query whether a function call argument is nil. The Printf checker (and others) could then use it to refine their heuristics.

However, that would make the printf check depend on nilness, and the standard vet contains only the watered-down nilfunc check, not full nilness, because the latter depends on golang.org/x/tools/go/ssa, which is more expensive to compute. We could evaluate whether the benefit of better checks exceeds the cost of computing full SSA.

@stephens2424 stephens2424 mentioned this issue Feb 25, 2020
Closed
@seankhliao seankhliao changed the title x/tools/go/analysis/passes/nilness: detect wrapping of nil errors x/tools/go/analysis/passes/nilness: detect wrapping of nil errors Jun 18, 2021
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Jun 18, 2021
@adonovan adonovan added the Analysis Issues related to static analysis (vet, x/tools/go/analysis) label Apr 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
6 participants
@andybons @ainar-g @adonovan @alandonovan @gopherbot @jba