dgryski/semgrep-go

A comment exception for err-nil-check?

ainar-g opened this issue · 3 comments

ainar-g commented

I generally like the check, as it shows a lot of places where errors aren't enriched with context, but in some of the projects I'm working on we have a rule: whenever you return an error, you must either add context to it or write a comment on why you didn't do it. For example:

func f() (err error) {
        // …

        err = g()
        if err != nil {
                // Don't wrap the error, because g provides all the necessary
                // context already.
                return err
        }

        return nil
}

I was wondering if branches with comments could be excluded from the check? Just writing return g() may be a bit too unnoticeable, I think.

dgryski commented

Yeah, that seems reasonable. You could also add a nolint directive or equivalent if semgrep supports those (which I think it does... ?)

dgryski commented

To be fair, I almost never take any action on this warning. I wonder it's worth having at all...

danp commented

Came across this issue as part of hunting for good rules (thanks, @dgryski and friends!).

If it helps, semgrep does support ignoring rules with a comment:

https://semgrep.dev/docs/ignoring-files-folders-code/#ignoring-code-through-nosemgrep

  • 👍1