go-tools icon indicating copy to clipboard operation
go-tools copied to clipboard

SA4020 could be slightly more aggressive

Open seebs opened this issue 2 years ago • 1 comments

type badRequest struct { error }
type notFound error

func checkError(e error) int {
    switch e.(type) {
    case badRequest: return http.StatusBadRequest
    case notFound: return http.StatusNotFound
    default: return http.InternalServerError
    }
    return http.StatusOK
}

this is simplified from an actual case where we got bitten by this. this is equivalent to:

case notFound: return http.StatusNotFound
case error: return http.InternalServerError

which I think SA4020 would catch.

seebs avatar Dec 06 '23 18:12 seebs

@seebs why is the origin code equivalent to

case notFound: return http.StatusNotFound
case error: return http.InternalServerError

the below test cases all passed

func TestCheckRequestError(t *testing.T) {
	badRequestErr := badRequest{errors.New("xxx")}
	assert.Equal(t, 400, checkRequestError(badRequestErr))

	var notFoundErr notFound = errors.New("not found")
	assert.Equal(t, 404, checkRequestError(notFoundErr))

	err := errors.New("error")
	assert.Equal(t, 404, checkRequestError(err))
}

meteorgan avatar Apr 19 '24 04:04 meteorgan