dgryski / semgrep-go

Go rules for semgrep and go-ruleguard
MIT License
457 stars 37 forks source link

err-todo triggered on code that does not handle an error #64

Open ainar-g opened 1 year ago

ainar-g commented 1 year ago

Synopsis:

package main

import (
    "fmt"
)

func weird() (n int, err error) {
    return 0, nil
}

func main() {
    n, err := weird()
    if err != nil {
        panic(err)
    } else if n == 0 {
        // TODO(ainar-g): weird API returns zero sometimes.  Use
        // the default value in such cases.
        n = 42
    }

    fmt.Println(n)
}
( cd "${HOME}/dev/semgrep-go/" && git show --oneline )
2a74033 (HEAD -> master, origin/master, origin/HEAD) Merge pull request #61 from yyoshiki41/improve/readeof
semgrep --vim -f "${HOME}/dev/semgrep-go/" -q ./main.go
main.go:16:3:E:home.ainar.dev.semgrep-go.err-todo:TODO in error handling code

It seems like the rules currently consider the entire if-else block “error handling code” if even only one branch actually works with err. I feel like it's more correct to think of that block as “result handling” as opposed to “error handling”.

It's a minor annoyance, so I don't mind it as much, but I still decided to report, just in case.