catenacyber / perfsprint

Golang linter to use strconv
MIT License
19 stars 2 forks source link

fmt.Sprint(err) -> err.Error() is not always safe #11

Closed rski closed 9 months ago

rski commented 9 months ago
    var err error = nil
    _ = fmt.Sprint(err)
    _ = err.Error()

this will panic on .Error. The warning printed could explain that this is only recommended for non-nil errors

catenacyber commented 9 months ago

Thanks, where did you spot this ? (which open source project ?)

rski commented 9 months ago

it's not from an open source codebase :-)

the above is basically a case I found in multiple places that do something like

err := foo()
errStr := fmt.Sprint(err)
if !strings.Contains(errStr, "some expected thingy") { t.Fatal() }

which would panic if it were replaced with this and the error were nil

err := foo()
if !strings.Contains(err.Error(), "some expected thingy") { t.Fatal() }
catenacyber commented 9 months ago

Solved by making this an optional off by default feature

I suggest that you change your code into

err := foo()
if err != nil {
    if !strings.Contains(fmt.Sprint(err), "some expected thingy") { t.Fatal() }
}

Then, the optimization will work