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

cmd/vet: assigned-but-not-used errors could be more aggressive #23142

Open
rmanansa opened this Issue Dec 14, 2017 · 5 comments

Comments

Projects
None yet
3 participants
@rmanansa

rmanansa commented Dec 14, 2017

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go1.9 darwin/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"

on MacOS X Sierra

What did you do?

I wrote this code below and then run go vet. err is never used or checked.

	defer resp.Body.Close()
	body, err := ioutil.ReadAll(resp.Body)

	return body, resp.StatusCode, nil
}

What did you expect to see?

I expect that when I run $> go vet then I should see some error
notification that this err variable is not used or something.

What did you see instead?

None.

@bradfitz bradfitz changed the title from go vet does not catch when err is not used before a return to cmd/vet: does not catch when err is not used before a return Dec 14, 2017

@bradfitz bradfitz added this to the Go1.11 milestone Dec 14, 2017

@bradfitz

This comment has been minimized.

Member

bradfitz commented Dec 14, 2017

Please show complete code.

Is that a new err variable or an existing err? Maybe only body is new in that snippet. If so, that's a different feature request.

@rmanansa

This comment has been minimized.

rmanansa commented Dec 15, 2017

func (tc *RTClient) requestDo(req *http.Request, auth bool) ([]byte, int, error) {
	if auth {
		tokenBasic, err := tc.getToken()
		if err != nil {
			return nil, ExitCode, err
		}
		req.Header.Set(Authorization, Basic+" "+tokenBasic)
	}

	resp, err := tc.httpClient.Do(req)
	if err != nil {
		return nil, ExitCode, err
	}

	defer resp.Body.Close()
	body, err := ioutil.ReadAll(resp.Body)

	return body, resp.StatusCode, nil
}
@rmanansa

This comment has been minimized.

rmanansa commented Dec 15, 2017

Hello @bradfitz thanks for the quick reply. The second to the last line of the function has err redeclared. Was expecting that running go vet will flag it when there are no usage of err var after it got redeclared.

@bradfitz

This comment has been minimized.

Member

bradfitz commented Dec 15, 2017

The err variable isn't redeclared there. Only body is declared. The err variable is just reassigned.

That is, your bug report or feature request is equivalent to:

package vettest

func foo() {
        a := 1
        if a == 2 {
                // ...
        }
        a = 3 // no vet warning here                                                                                                                                              
}               

@bradfitz bradfitz changed the title from cmd/vet: does not catch when err is not used before a return to cmd/vet: assigned-but-not-used errors could be more aggressive Dec 15, 2017

@bradfitz bradfitz modified the milestones: Go1.11, Unplanned Dec 15, 2017

@dominikh

This comment has been minimized.

Member

dominikh commented Dec 15, 2017

FTR, staticcheck as well as ineffassign catch this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment