search

dgryski / semgrep-go

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

A comment exception for err-nil-check? #43

Open ainar-g opened 2 years ago

ainar-g commented 2 years ago

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 2 years ago

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

dgryski commented 1 year ago

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

danp commented 1 year ago

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