Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
ainar-g opened this issue Dec 10, 2021 · 3 comments
Open

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

ainar-g opened this issue Dec 10, 2021 · 3 comments

Comments

@ainar-g
Copy link
Contributor

ainar-g commented Dec 10, 2021

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
Copy link
Owner

dgryski commented Dec 13, 2021

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

@dgryski
Copy link
Owner

dgryski commented Mar 2, 2023

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

@danp
Copy link

danp commented Jul 24, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants