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

os: document behavior of IsNotExist, IsExist, etc. for nil errors #31065

Closed
dpinela opened this issue Mar 27, 2019 · 6 comments
Closed

os: document behavior of IsNotExist, IsExist, etc. for nil errors #31065

dpinela opened this issue Mar 27, 2019 · 6 comments
Labels
Documentation help wanted NeedsFix The path to resolution is known, but the work has not been done. Suggested Issues that may be good for new contributors looking for work to do.
Milestone

Comments

@dpinela
Copy link
Contributor

dpinela commented Mar 27, 2019

The IsNotExist, IsExist, IsPermission and IsTimeout functions in package os all return false when passed a nil error, but the documentation isn't too clear about that. I presume the behavior is intentional, since there are tests for it (except for IsTimeout?). Being able to rely on this property would be nice, as it allows one to write code like the following, to treat such errors specially:

if os.IsNotExist(err) {
   // ...
} else if err != nil {
  // ...
}

I think the root of the issue is that the term "the error" in these functions' docs is ambiguous - it could mean an error value (which can be nil) or an error condition (which is represented by a non-nil error value).

@agnivade
Copy link
Contributor

@ianlancetaylor

@ianlancetaylor
Copy link
Contributor

The behavior is definitely intentional. I'm fine with a small documentation clarification. Maybe we could just s/this error/err/.

@ianlancetaylor ianlancetaylor added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Mar 28, 2019
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Mar 28, 2019
@agnivade agnivade added the Suggested Issues that may be good for new contributors looking for work to do. label Mar 28, 2019
@seebs
Copy link
Contributor

seebs commented Mar 29, 2019

I was just about to write exactly this bug report, because I just found some code which was convoluted enough to trip up a linter because it was trying to avoid calling IsNotExist on a nil. Intuitively, I would prefer to get the false answer rather than a panic, because it is not the case that a nil is an error which represents a file not existing.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/170077 mentions this issue: os: clarify that IsNotExist, IsExist, IsPermission and IsTimeout work with nil errors

@SpaceKittyCow
Copy link

I talked to @jba and @ianlancetaylor about working on this today. I'll pick it up after gophercon to workon. Thanks again!

@jba
Copy link
Contributor

jba commented Jul 9, 2024

@SpaceKittyCow, it looks like https://golang.org/cl/170077 is already in progress to address this. I pinged the author to see if he's still interested in working on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation help wanted NeedsFix The path to resolution is known, but the work has not been done. Suggested Issues that may be good for new contributors looking for work to do.
Projects
None yet
Development

No branches or pull requests

7 participants