]> Cypherpunks repositories - gostls13.git/commitdiff
os: make errors.Is work with ErrPermission et al.
authorDamien Neil <dneil@google.com>
Tue, 19 Feb 2019 21:03:55 +0000 (13:03 -0800)
committerDamien Neil <dneil@google.com>
Wed, 20 Mar 2019 16:02:01 +0000 (16:02 +0000)
As proposed in Issue #29934, update errors produced by the os package to
work with errors.Is sentinel tests. For example,
errors.Is(err, os.ErrPermission) is equivalent to os.IsPermission(err)
with added unwrapping support.

Move the definition for os.ErrPermission and others into the syscall
package. Add an Is method to syscall.Errno and others. Add an Unwrap
method to os.PathError and others.

Updates #30322
Updates #29934

Change-Id: I95727d26c18a5354c720de316dff0bffc04dd926
Reviewed-on: https://go-review.googlesource.com/c/go/+/163058
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
14 files changed:
src/fmt/errors_test.go
src/go/build/deps_test.go
src/internal/oserror/errors.go [new file with mode: 0644]
src/os/error.go
src/os/error_plan9.go [deleted file]
src/os/error_test.go
src/os/error_unix.go [deleted file]
src/os/error_windows.go [deleted file]
src/os/file.go
src/syscall/syscall_js.go
src/syscall/syscall_nacl.go
src/syscall/syscall_plan9.go
src/syscall/syscall_unix.go
src/syscall/syscall_windows.go

index 0183ba77e5b0f16143afcaf1f3cbcc14243f19bc..39f247e06d1ca19175dad79cdd7f5a99d80f6e42 100644 (file)
@@ -157,8 +157,8 @@ func TestErrorFormatter(t *testing.T) {
                want: "fallback:" +
                        "\n    somefile.go:123" +
                        "\n  - file does not exist:" +
-                       "\n    os.init" +
-                       "\n        .+/os/error.go:\\d\\d",
+                       "\n    .*" +
+                       "\n        .+.go:\\d+",
                regexp: true,
        }, {
                err: &wrapped{"outer",
index 31a5d2741d77f1851957e2acd04f3b2ad28317eb..df1d8dd3b373200b652640d0284e72b0fa3c24a8 100644 (file)
@@ -146,8 +146,9 @@ var pkgDeps = map[string][]string{
        // End of linear dependency definitions.
 
        // Operating system access.
-       "syscall":                           {"L0", "internal/race", "internal/syscall/windows/sysdll", "syscall/js", "unicode/utf16"},
+       "syscall":                           {"L0", "internal/oserror", "internal/race", "internal/syscall/windows/sysdll", "syscall/js", "unicode/utf16"},
        "syscall/js":                        {"L0"},
+       "internal/oserror":                  {"L0"},
        "internal/syscall/unix":             {"L0", "syscall"},
        "internal/syscall/windows":          {"L0", "syscall", "internal/syscall/windows/sysdll"},
        "internal/syscall/windows/registry": {"L0", "syscall", "internal/syscall/windows/sysdll", "unicode/utf16"},
@@ -167,7 +168,7 @@ var pkgDeps = map[string][]string{
 
        "internal/poll":    {"L0", "internal/race", "syscall", "time", "unicode/utf16", "unicode/utf8", "internal/syscall/windows"},
        "internal/testlog": {"L0"},
-       "os":               {"L1", "os", "syscall", "time", "internal/poll", "internal/syscall/windows", "internal/syscall/unix", "internal/testlog"},
+       "os":               {"L1", "os", "syscall", "time", "internal/oserror", "internal/poll", "internal/syscall/windows", "internal/syscall/unix", "internal/testlog"},
        "path/filepath":    {"L2", "os", "syscall", "internal/syscall/windows"},
        "io/ioutil":        {"L2", "os", "path/filepath", "time"},
        "os/exec":          {"L2", "os", "context", "path/filepath", "syscall"},
diff --git a/src/internal/oserror/errors.go b/src/internal/oserror/errors.go
new file mode 100644 (file)
index 0000000..66ed7fa
--- /dev/null
@@ -0,0 +1,20 @@
+// Copyright 2019 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Package oserror defines errors values used in the os package.
+//
+// These types are defined here to permit the syscall package to reference them.
+package oserror
+
+import "errors"
+
+var (
+       ErrInvalid    = errors.New("invalid argument")
+       ErrPermission = errors.New("permission denied")
+       ErrExist      = errors.New("file already exists")
+       ErrNotExist   = errors.New("file does not exist")
+       ErrClosed     = errors.New("file already closed")
+       ErrTemporary  = errors.New("temporary error")
+       ErrTimeout    = errors.New("deadline exceeded")
+)
index 16e5cb5786b59315f4b07aa16a9f1b81d2075350..4cf35f2b773f237daa4b4e64564f0b41ef9c7ebc 100644 (file)
@@ -5,20 +5,35 @@
 package os
 
 import (
-       "errors"
+       "internal/oserror"
        "internal/poll"
 )
 
 // Portable analogs of some common system call errors.
+//
+// Errors returned from this package may be tested against these errors
+// with errors.Is.
 var (
-       ErrInvalid    = errors.New("invalid argument") // methods on File will return this error when the receiver is nil
-       ErrPermission = errors.New("permission denied")
-       ErrExist      = errors.New("file already exists")
-       ErrNotExist   = errors.New("file does not exist")
-       ErrClosed     = errors.New("file already closed")
-       ErrNoDeadline = poll.ErrNoDeadline
+       // ErrInvalid indicates an invalid argument.
+       // Methods on File will return this error when the receiver is nil.
+       ErrInvalid = errInvalid() // "invalid argument"
+
+       ErrPermission = errPermission() // "permission denied"
+       ErrExist      = errExist()      // "file already exists"
+       ErrNotExist   = errNotExist()   // "file does not exist"
+       ErrClosed     = errClosed()     // "file already closed"
+       ErrTimeout    = errTimeout()    // "deadline exceeded"
+       ErrNoDeadline = errNoDeadline() // "file type does not support deadline"
 )
 
+func errInvalid() error    { return oserror.ErrInvalid }
+func errPermission() error { return oserror.ErrPermission }
+func errExist() error      { return oserror.ErrExist }
+func errNotExist() error   { return oserror.ErrNotExist }
+func errClosed() error     { return oserror.ErrClosed }
+func errTimeout() error    { return oserror.ErrTimeout }
+func errNoDeadline() error { return poll.ErrNoDeadline }
+
 type timeout interface {
        Timeout() bool
 }
@@ -48,6 +63,8 @@ type SyscallError struct {
 
 func (e *SyscallError) Error() string { return e.Syscall + ": " + e.Err.Error() }
 
+func (e *SyscallError) Unwrap() error { return e.Err }
+
 // Timeout reports whether this error represents a timeout.
 func (e *SyscallError) Timeout() bool {
        t, ok := e.Err.(timeout)
@@ -68,21 +85,21 @@ func NewSyscallError(syscall string, err error) error {
 // that a file or directory already exists. It is satisfied by ErrExist as
 // well as some syscall errors.
 func IsExist(err error) bool {
-       return isExist(err)
+       return underlyingErrorIs(err, ErrExist)
 }
 
 // IsNotExist returns a boolean indicating whether the error is known to
 // report that a file or directory does not exist. It is satisfied by
 // ErrNotExist as well as some syscall errors.
 func IsNotExist(err error) bool {
-       return isNotExist(err)
+       return underlyingErrorIs(err, ErrNotExist)
 }
 
 // IsPermission returns a boolean indicating whether the error is known to
 // report that permission is denied. It is satisfied by ErrPermission as well
 // as some syscall errors.
 func IsPermission(err error) bool {
-       return isPermission(err)
+       return underlyingErrorIs(err, ErrPermission)
 }
 
 // IsTimeout returns a boolean indicating whether the error is known
@@ -92,6 +109,18 @@ func IsTimeout(err error) bool {
        return ok && terr.Timeout()
 }
 
+func underlyingErrorIs(err, target error) bool {
+       // Note that this function is not errors.Is:
+       // underlyingError only unwraps the specific error-wrapping types
+       // that it historically did, not all errors.Wrapper implementations.
+       err = underlyingError(err)
+       if err == target {
+               return true
+       }
+       e, ok := err.(interface{ Is(error) bool })
+       return ok && e.Is(target)
+}
+
 // underlyingError returns the underlying error for known os error types.
 func underlyingError(err error) error {
        switch err := err.(type) {
diff --git a/src/os/error_plan9.go b/src/os/error_plan9.go
deleted file mode 100644 (file)
index b82bf0d..0000000
+++ /dev/null
@@ -1,44 +0,0 @@
-// Copyright 2011 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package os
-
-func isExist(err error) bool {
-       return checkErrMessageContent(err, "exists", "is a directory")
-}
-
-func isNotExist(err error) bool {
-       return checkErrMessageContent(err, "does not exist", "not found",
-               "has been removed", "no parent")
-}
-
-func isPermission(err error) bool {
-       return checkErrMessageContent(err, "permission denied")
-}
-
-// checkErrMessageContent checks if err message contains one of msgs.
-func checkErrMessageContent(err error, msgs ...string) bool {
-       if err == nil {
-               return false
-       }
-       err = underlyingError(err)
-       for _, msg := range msgs {
-               if contains(err.Error(), msg) {
-                       return true
-               }
-       }
-       return false
-}
-
-// contains is a local version of strings.Contains. It knows len(sep) > 1.
-func contains(s, sep string) bool {
-       n := len(sep)
-       c := sep[0]
-       for i := 0; i+n <= len(s); i++ {
-               if s[i] == c && s[i:i+n] == sep {
-                       return true
-               }
-       }
-       return false
-}
index 0e3570996ec9dae311cdb18fa2aa4a998b845c8b..a03bd28b9ae908277bd88e50deb6e890fc6a8e6b 100644 (file)
@@ -27,7 +27,7 @@ func TestErrIsExist(t *testing.T) {
                t.Fatal("Open should have failed")
                return
        }
-       if s := checkErrorPredicate("os.IsExist", os.IsExist, err); s != "" {
+       if s := checkErrorPredicate("os.IsExist", os.IsExist, err, os.ErrExist); s != "" {
                t.Fatal(s)
                return
        }
@@ -39,7 +39,7 @@ func testErrNotExist(name string) string {
                f.Close()
                return "Open should have failed"
        }
-       if s := checkErrorPredicate("os.IsNotExist", os.IsNotExist, err); s != "" {
+       if s := checkErrorPredicate("os.IsNotExist", os.IsNotExist, err, os.ErrNotExist); s != "" {
                return s
        }
 
@@ -47,7 +47,7 @@ func testErrNotExist(name string) string {
        if err == nil {
                return "Chdir should have failed"
        }
-       if s := checkErrorPredicate("os.IsNotExist", os.IsNotExist, err); s != "" {
+       if s := checkErrorPredicate("os.IsNotExist", os.IsNotExist, err, os.ErrNotExist); s != "" {
                return s
        }
        return ""
@@ -74,10 +74,13 @@ func TestErrIsNotExist(t *testing.T) {
        }
 }
 
-func checkErrorPredicate(predName string, pred func(error) bool, err error) string {
+func checkErrorPredicate(predName string, pred func(error) bool, err, target error) string {
        if !pred(err) {
                return fmt.Sprintf("%s does not work as expected for %#v", predName, err)
        }
+       if !errors.Is(err, target) {
+               return fmt.Sprintf("errors.Is(%#v, %#v) = false, want true", err, target)
+       }
        return ""
 }
 
@@ -108,9 +111,15 @@ func TestIsExist(t *testing.T) {
                if is := os.IsExist(tt.err); is != tt.is {
                        t.Errorf("os.IsExist(%T %v) = %v, want %v", tt.err, tt.err, is, tt.is)
                }
+               if is := errors.Is(tt.err, os.ErrExist); is != tt.is {
+                       t.Errorf("errors.Is(%T %v, os.ErrExist) = %v, want %v", tt.err, tt.err, is, tt.is)
+               }
                if isnot := os.IsNotExist(tt.err); isnot != tt.isnot {
                        t.Errorf("os.IsNotExist(%T %v) = %v, want %v", tt.err, tt.err, isnot, tt.isnot)
                }
+               if isnot := errors.Is(tt.err, os.ErrNotExist); isnot != tt.isnot {
+                       t.Errorf("errors.Is(%T %v, os.ErrNotExist) = %v, want %v", tt.err, tt.err, isnot, tt.isnot)
+               }
        }
 }
 
@@ -130,6 +139,9 @@ func TestIsPermission(t *testing.T) {
                if got := os.IsPermission(tt.err); got != tt.want {
                        t.Errorf("os.IsPermission(%#v) = %v; want %v", tt.err, got, tt.want)
                }
+               if got := errors.Is(tt.err, os.ErrPermission); got != tt.want {
+                       t.Errorf("errors.Is(%#v, os.ErrPermission) = %v; want %v", tt.err, got, tt.want)
+               }
        }
 }
 
diff --git a/src/os/error_unix.go b/src/os/error_unix.go
deleted file mode 100644 (file)
index bb6bbcc..0000000
+++ /dev/null
@@ -1,24 +0,0 @@
-// Copyright 2009 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// +build aix darwin dragonfly freebsd js,wasm linux nacl netbsd openbsd solaris
-
-package os
-
-import "syscall"
-
-func isExist(err error) bool {
-       err = underlyingError(err)
-       return err == syscall.EEXIST || err == syscall.ENOTEMPTY || err == ErrExist
-}
-
-func isNotExist(err error) bool {
-       err = underlyingError(err)
-       return err == syscall.ENOENT || err == ErrNotExist
-}
-
-func isPermission(err error) bool {
-       err = underlyingError(err)
-       return err == syscall.EACCES || err == syscall.EPERM || err == ErrPermission
-}
diff --git a/src/os/error_windows.go b/src/os/error_windows.go
deleted file mode 100644 (file)
index 02593b5..0000000
+++ /dev/null
@@ -1,28 +0,0 @@
-// Copyright 2012 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package os
-
-import "syscall"
-
-func isExist(err error) bool {
-       err = underlyingError(err)
-       return err == syscall.ERROR_ALREADY_EXISTS ||
-               err == syscall.ERROR_DIR_NOT_EMPTY ||
-               err == syscall.ERROR_FILE_EXISTS || err == ErrExist
-}
-
-const _ERROR_BAD_NETPATH = syscall.Errno(53)
-
-func isNotExist(err error) bool {
-       err = underlyingError(err)
-       return err == syscall.ERROR_FILE_NOT_FOUND ||
-               err == _ERROR_BAD_NETPATH ||
-               err == syscall.ERROR_PATH_NOT_FOUND || err == ErrNotExist
-}
-
-func isPermission(err error) bool {
-       err = underlyingError(err)
-       return err == syscall.ERROR_ACCESS_DENIED || err == ErrPermission
-}
index 5f715f4275293a9e2d29443b4d7d0df2584f8791..a44263ee8acbce29711fab5dc755158d79311c5a 100644 (file)
@@ -98,6 +98,10 @@ func (e *LinkError) Error() string {
        return e.Op + " " + e.Old + " " + e.New + ": " + e.Err.Error()
 }
 
+func (e *LinkError) Unwrap() error {
+       return e.Err
+}
+
 // Read reads up to len(b) bytes from the File.
 // It returns the number of bytes read and any error encountered.
 // At end of file, Read returns 0, io.EOF.
index 2e1a9ec9f1fef01cdd7019844986a1f153577a66..4dfcc6ed6415cdc85bc2193966be8221b6c99a88 100644 (file)
@@ -7,6 +7,7 @@
 package syscall
 
 import (
+       "internal/oserror"
        "sync"
        "unsafe"
 )
@@ -55,6 +56,22 @@ func (e Errno) Error() string {
        return "errno " + itoa(int(e))
 }
 
+func (e Errno) Is(target error) bool {
+       switch target {
+       case oserror.ErrTemporary:
+               return e.Temporary()
+       case oserror.ErrTimeout:
+               return e.Timeout()
+       case oserror.ErrPermission:
+               return e == EACCES || e == EPERM
+       case oserror.ErrExist:
+               return e == EEXIST || e == ENOTEMPTY
+       case oserror.ErrNotExist:
+               return e == ENOENT
+       }
+       return false
+}
+
 func (e Errno) Temporary() bool {
        return e == EINTR || e == EMFILE || e.Timeout()
 }
index 1102cd66e3866e688bbc7d22a9c92eafbfe994ac..3fc504fd9f2c4d3946c0d639f2eb5d09fe565bb7 100644 (file)
@@ -5,6 +5,7 @@
 package syscall
 
 import (
+       "internal/oserror"
        "sync"
        "unsafe"
 )
@@ -62,6 +63,22 @@ func (e Errno) Error() string {
        return "errno " + itoa(int(e))
 }
 
+func (e Errno) Is(target error) bool {
+       switch target {
+       case oserror.ErrTemporary:
+               return e.Temporary()
+       case oserror.ErrTimeout:
+               return e.Timeout()
+       case oserror.ErrPermission:
+               return e == EACCES || e == EPERM
+       case oserror.ErrExist:
+               return e == EEXIST || e == ENOTEMPTY
+       case oserror.ErrNotExist:
+               return e == ENOENT
+       }
+       return false
+}
+
 func (e Errno) Temporary() bool {
        return e == EINTR || e == EMFILE || e.Timeout()
 }
index 48513c73c94b96edae1361186ad580d4297efacc..9b5a2940b07aeb1a855f0a8ddd02dbc275e6e0ab 100644 (file)
 
 package syscall
 
-import "unsafe"
+import (
+       "internal/oserror"
+       "unsafe"
+)
 
 const ImplementsGetwd = true
 const bitSize16 = 2
@@ -24,6 +27,45 @@ func (e ErrorString) Error() string { return string(e) }
 // NewError converts s to an ErrorString, which satisfies the Error interface.
 func NewError(s string) error { return ErrorString(s) }
 
+func (e ErrorString) Is(target error) bool {
+       switch target {
+       case oserror.ErrTemporary:
+               return e.Temporary()
+       case oserror.ErrTimeout:
+               return e.Timeout()
+       case oserror.ErrPermission:
+               return checkErrMessageContent(e, "permission denied")
+       case oserror.ErrExist:
+               return checkErrMessageContent(e, "exists", "is a directory")
+       case oserror.ErrNotExist:
+               return checkErrMessageContent(e, "does not exist", "not found",
+                       "has been removed", "no parent")
+       }
+       return false
+}
+
+// checkErrMessageContent checks if err message contains one of msgs.
+func checkErrMessageContent(e ErrorString, msgs ...string) bool {
+       for _, msg := range msgs {
+               if contains(string(e), msg) {
+                       return true
+               }
+       }
+       return false
+}
+
+// contains is a local version of strings.Contains. It knows len(sep) > 1.
+func contains(s, sep string) bool {
+       n := len(sep)
+       c := sep[0]
+       for i := 0; i+n <= len(s); i++ {
+               if s[i] == c && s[i:i+n] == sep {
+                       return true
+               }
+       }
+       return false
+}
+
 func (e ErrorString) Temporary() bool {
        return e == EINTR || e == EMFILE || e.Timeout()
 }
index 4336851554801b43b23f1dcce5767abf664a0393..fd54dc0dc769e7875e62087554eb84c81440d215 100644 (file)
@@ -7,6 +7,7 @@
 package syscall
 
 import (
+       "internal/oserror"
        "internal/race"
        "runtime"
        "sync"
@@ -120,6 +121,22 @@ func (e Errno) Error() string {
        return "errno " + itoa(int(e))
 }
 
+func (e Errno) Is(target error) bool {
+       switch target {
+       case oserror.ErrTemporary:
+               return e.Temporary()
+       case oserror.ErrTimeout:
+               return e.Timeout()
+       case oserror.ErrPermission:
+               return e == EACCES || e == EPERM
+       case oserror.ErrExist:
+               return e == EEXIST || e == ENOTEMPTY
+       case oserror.ErrNotExist:
+               return e == ENOENT
+       }
+       return false
+}
+
 func (e Errno) Temporary() bool {
        return e == EINTR || e == EMFILE || e.Timeout()
 }
index de05840386280405616fc470fa95039fdc739386..22c9e50a4417816aae150a4b115cb3eb084feeec 100644 (file)
@@ -8,6 +8,7 @@ package syscall
 
 import (
        errorspkg "errors"
+       "internal/oserror"
        "internal/race"
        "runtime"
        "sync"
@@ -110,6 +111,28 @@ func (e Errno) Error() string {
        return string(utf16.Decode(b[:n]))
 }
 
+const _ERROR_BAD_NETPATH = Errno(53)
+
+func (e Errno) Is(target error) bool {
+       switch target {
+       case oserror.ErrTemporary:
+               return e.Temporary()
+       case oserror.ErrTimeout:
+               return e.Timeout()
+       case oserror.ErrPermission:
+               return e == ERROR_ACCESS_DENIED
+       case oserror.ErrExist:
+               return e == ERROR_ALREADY_EXISTS ||
+                       e == ERROR_DIR_NOT_EMPTY ||
+                       e == ERROR_FILE_EXISTS
+       case oserror.ErrNotExist:
+               return e == ERROR_FILE_NOT_FOUND ||
+                       e == _ERROR_BAD_NETPATH ||
+                       e == ERROR_PATH_NOT_FOUND
+       }
+       return false
+}
+
 func (e Errno) Temporary() bool {
        return e == EINTR || e == EMFILE || e.Timeout()
 }