Update error checks that use the `os` package to use `errors.Is` (#1461)

## Changes

From the [documentation](https://pkg.go.dev/os#IsNotExist) on the
functions in the `os` package:
> This function predates errors.Is. It only supports errors returned by
the os package.
> New code should use errors.Is(err, fs.ErrNotExist).

This issue surfaced while working on using a different `vfs.Path`
implementation that uses errors from the `fs` package. Calls to
`os.IsNotExist` didn't return true for errors that wrap
`fs.ErrNotExist`.

## Tests

n/a
This commit is contained in:
Pieter Noordhuis 2024-06-03 14:39:36 +02:00 committed by GitHub
parent 30fd84893f
commit c9b4f11947
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
19 changed files with 64 additions and 38 deletions

View File

@ -2,6 +2,8 @@ package bundle
import ( import (
"context" "context"
"errors"
"io/fs"
"os" "os"
"path/filepath" "path/filepath"
"testing" "testing"
@ -14,7 +16,7 @@ import (
func TestLoadNotExists(t *testing.T) { func TestLoadNotExists(t *testing.T) {
b, err := Load(context.Background(), "/doesntexist") b, err := Load(context.Background(), "/doesntexist")
assert.True(t, os.IsNotExist(err)) assert.True(t, errors.Is(err, fs.ErrNotExist))
assert.Nil(t, b) assert.Nil(t, b)
} }

View File

@ -4,6 +4,7 @@ import (
"context" "context"
"errors" "errors"
"fmt" "fmt"
"io/fs"
"net/url" "net/url"
"os" "os"
"path" "path"
@ -109,7 +110,7 @@ func (m *translatePaths) rewritePath(
func translateNotebookPath(literal, localFullPath, localRelPath, remotePath string) (string, error) { func translateNotebookPath(literal, localFullPath, localRelPath, remotePath string) (string, error) {
nb, _, err := notebook.Detect(localFullPath) nb, _, err := notebook.Detect(localFullPath)
if os.IsNotExist(err) { if errors.Is(err, fs.ErrNotExist) {
return "", fmt.Errorf("notebook %s not found", literal) return "", fmt.Errorf("notebook %s not found", literal)
} }
if err != nil { if err != nil {
@ -125,7 +126,7 @@ func translateNotebookPath(literal, localFullPath, localRelPath, remotePath stri
func translateFilePath(literal, localFullPath, localRelPath, remotePath string) (string, error) { func translateFilePath(literal, localFullPath, localRelPath, remotePath string) (string, error) {
nb, _, err := notebook.Detect(localFullPath) nb, _, err := notebook.Detect(localFullPath)
if os.IsNotExist(err) { if errors.Is(err, fs.ErrNotExist) {
return "", fmt.Errorf("file %s not found", literal) return "", fmt.Errorf("file %s not found", literal)
} }
if err != nil { if err != nil {

View File

@ -2,7 +2,9 @@ package files
import ( import (
"context" "context"
"errors"
"fmt" "fmt"
"io/fs"
"os" "os"
"github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle"
@ -67,7 +69,7 @@ func deleteSnapshotFile(ctx context.Context, b *bundle.Bundle) error {
return err return err
} }
err = os.Remove(sp) err = os.Remove(sp)
if err != nil && !os.IsNotExist(err) { if err != nil && !errors.Is(err, fs.ErrNotExist) {
return fmt.Errorf("failed to destroy sync snapshot file: %s", err) return fmt.Errorf("failed to destroy sync snapshot file: %s", err)
} }
return nil return nil

View File

@ -4,7 +4,9 @@ import (
"bytes" "bytes"
"context" "context"
"encoding/json" "encoding/json"
"errors"
"io" "io"
"io/fs"
"os" "os"
"testing" "testing"
@ -270,7 +272,7 @@ func TestStatePullNoState(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
_, err = os.Stat(statePath) _, err = os.Stat(statePath)
require.True(t, os.IsNotExist(err)) require.True(t, errors.Is(err, fs.ErrNotExist))
} }
func TestStatePullOlderState(t *testing.T) { func TestStatePullOlderState(t *testing.T) {

View File

@ -4,7 +4,9 @@ import (
"bytes" "bytes"
"context" "context"
"encoding/json" "encoding/json"
"errors"
"io" "io"
"io/fs"
"os" "os"
"time" "time"
@ -95,7 +97,7 @@ func load(ctx context.Context, b *bundle.Bundle) (*DeploymentState, error) {
log.Infof(ctx, "Loading deployment state from %s", statePath) log.Infof(ctx, "Loading deployment state from %s", statePath)
f, err := os.Open(statePath) f, err := os.Open(statePath)
if err != nil { if err != nil {
if os.IsNotExist(err) { if errors.Is(err, fs.ErrNotExist) {
log.Infof(ctx, "No deployment state file found") log.Infof(ctx, "No deployment state file found")
return &DeploymentState{ return &DeploymentState{
Version: DeploymentStateVersion, Version: DeploymentStateVersion,

View File

@ -2,7 +2,9 @@ package terraform
import ( import (
"context" "context"
"errors"
"fmt" "fmt"
"io/fs"
"os" "os"
"os/exec" "os/exec"
"path/filepath" "path/filepath"
@ -59,7 +61,7 @@ func (m *initialize) findExecPath(ctx context.Context, b *bundle.Bundle, tf *con
// If the execPath already exists, return it. // If the execPath already exists, return it.
execPath := filepath.Join(binDir, product.Terraform.BinaryName()) execPath := filepath.Join(binDir, product.Terraform.BinaryName())
_, err = os.Stat(execPath) _, err = os.Stat(execPath)
if err != nil && !os.IsNotExist(err) { if err != nil && !errors.Is(err, fs.ErrNotExist) {
return "", err return "", err
} }
if err == nil { if err == nil {
@ -148,7 +150,7 @@ func getEnvVarWithMatchingVersion(ctx context.Context, envVarName string, versio
// If the path does not exist, we return early. // If the path does not exist, we return early.
_, err := os.Stat(envValue) _, err := os.Stat(envValue)
if err != nil { if err != nil {
if os.IsNotExist(err) { if errors.Is(err, fs.ErrNotExist) {
log.Debugf(ctx, "%s at %s does not exist", envVarName, envValue) log.Debugf(ctx, "%s at %s does not exist", envVarName, envValue)
return "", nil return "", nil
} else { } else {

View File

@ -2,6 +2,8 @@ package schema
import ( import (
"context" "context"
"errors"
"io/fs"
"os" "os"
"path/filepath" "path/filepath"
@ -41,7 +43,7 @@ func Load(ctx context.Context) (*tfjson.ProviderSchema, error) {
} }
// Generate schema file if it doesn't exist. // Generate schema file if it doesn't exist.
if _, err := os.Stat(s.ProviderSchemaFile); os.IsNotExist(err) { if _, err := os.Stat(s.ProviderSchemaFile); errors.Is(err, fs.ErrNotExist) {
err = s.Generate(ctx) err = s.Generate(ctx)
if err != nil { if err != nil {
return nil, err return nil, err

View File

@ -2,8 +2,9 @@ package auth
import ( import (
"context" "context"
"errors"
"fmt" "fmt"
"os" "io/fs"
"sync" "sync"
"time" "time"
@ -95,7 +96,7 @@ func newProfilesCommand() *cobra.Command {
cmd.RunE = func(cmd *cobra.Command, args []string) error { cmd.RunE = func(cmd *cobra.Command, args []string) error {
var profiles []*profileMetadata var profiles []*profileMetadata
iniFile, err := profile.DefaultProfiler.Get(cmd.Context()) iniFile, err := profile.DefaultProfiler.Get(cmd.Context())
if os.IsNotExist(err) { if errors.Is(err, fs.ErrNotExist) {
// return empty list for non-configured machines // return empty list for non-configured machines
iniFile = &config.File{ iniFile = &config.File{
File: &ini.File{}, File: &ini.File{},

View File

@ -4,7 +4,7 @@ import (
"context" "context"
"errors" "errors"
"fmt" "fmt"
"os" "io/fs"
"strings" "strings"
"github.com/databricks/cli/libs/log" "github.com/databricks/cli/libs/log"
@ -68,7 +68,7 @@ func (l profileFromHostLoader) Configure(cfg *config.Config) error {
ctx := context.Background() ctx := context.Background()
configFile, err := config.LoadFile(cfg.ConfigFile) configFile, err := config.LoadFile(cfg.ConfigFile)
if err != nil { if err != nil {
if os.IsNotExist(err) { if errors.Is(err, fs.ErrNotExist) {
return nil return nil
} }
return fmt.Errorf("cannot parse config file: %w", err) return fmt.Errorf("cannot parse config file: %w", err)

View File

@ -2,7 +2,9 @@ package databrickscfg
import ( import (
"context" "context"
"errors"
"fmt" "fmt"
"io/fs"
"os" "os"
"strings" "strings"
@ -29,7 +31,7 @@ func loadOrCreateConfigFile(filename string) (*config.File, error) {
filename = fmt.Sprintf("%s%s", homedir, filename[1:]) filename = fmt.Sprintf("%s%s", homedir, filename[1:])
} }
configFile, err := config.LoadFile(filename) configFile, err := config.LoadFile(filename)
if err != nil && os.IsNotExist(err) { if err != nil && errors.Is(err, fs.ErrNotExist) {
file, err := os.OpenFile(filename, os.O_RDWR|os.O_CREATE|os.O_TRUNC, fileMode) file, err := os.OpenFile(filename, os.O_RDWR|os.O_CREATE|os.O_TRUNC, fileMode)
if err != nil { if err != nil {
return nil, fmt.Errorf("create %s: %w", filename, err) return nil, fmt.Errorf("create %s: %w", filename, err)

View File

@ -2,6 +2,7 @@ package filer
import ( import (
"context" "context"
"errors"
"io" "io"
"io/fs" "io/fs"
"os" "os"
@ -35,7 +36,7 @@ func (w *LocalClient) Write(ctx context.Context, name string, reader io.Reader,
} }
f, err := os.OpenFile(absPath, flags, 0644) f, err := os.OpenFile(absPath, flags, 0644)
if os.IsNotExist(err) && slices.Contains(mode, CreateParentDirectories) { if errors.Is(err, fs.ErrNotExist) && slices.Contains(mode, CreateParentDirectories) {
// Create parent directories if they don't exist. // Create parent directories if they don't exist.
err = os.MkdirAll(filepath.Dir(absPath), 0755) err = os.MkdirAll(filepath.Dir(absPath), 0755)
if err != nil { if err != nil {
@ -47,9 +48,9 @@ func (w *LocalClient) Write(ctx context.Context, name string, reader io.Reader,
if err != nil { if err != nil {
switch { switch {
case os.IsNotExist(err): case errors.Is(err, fs.ErrNotExist):
return NoSuchDirectoryError{path: absPath} return NoSuchDirectoryError{path: absPath}
case os.IsExist(err): case errors.Is(err, fs.ErrExist):
return FileAlreadyExistsError{path: absPath} return FileAlreadyExistsError{path: absPath}
default: default:
return err return err
@ -77,7 +78,7 @@ func (w *LocalClient) Read(ctx context.Context, name string) (io.ReadCloser, err
// 2. Allows us to error out if the path is a directory // 2. Allows us to error out if the path is a directory
stat, err := os.Stat(absPath) stat, err := os.Stat(absPath)
if err != nil { if err != nil {
if os.IsNotExist(err) { if errors.Is(err, fs.ErrNotExist) {
return nil, FileDoesNotExistError{path: absPath} return nil, FileDoesNotExistError{path: absPath}
} }
return nil, err return nil, err
@ -108,11 +109,11 @@ func (w *LocalClient) Delete(ctx context.Context, name string, mode ...DeleteMod
return nil return nil
} }
if os.IsNotExist(err) { if errors.Is(err, fs.ErrNotExist) {
return FileDoesNotExistError{path: absPath} return FileDoesNotExistError{path: absPath}
} }
if os.IsExist(err) { if errors.Is(err, fs.ErrExist) {
if slices.Contains(mode, DeleteRecursively) { if slices.Contains(mode, DeleteRecursively) {
return os.RemoveAll(absPath) return os.RemoveAll(absPath)
} }
@ -130,7 +131,7 @@ func (w *LocalClient) ReadDir(ctx context.Context, name string) ([]fs.DirEntry,
stat, err := os.Stat(absPath) stat, err := os.Stat(absPath)
if err != nil { if err != nil {
if os.IsNotExist(err) { if errors.Is(err, fs.ErrNotExist) {
return nil, NoSuchDirectoryError{path: absPath} return nil, NoSuchDirectoryError{path: absPath}
} }
return nil, err return nil, err
@ -159,7 +160,7 @@ func (w *LocalClient) Stat(ctx context.Context, name string) (fs.FileInfo, error
} }
stat, err := os.Stat(absPath) stat, err := os.Stat(absPath)
if os.IsNotExist(err) { if errors.Is(err, fs.ErrNotExist) {
return nil, FileDoesNotExistError{path: absPath} return nil, FileDoesNotExistError{path: absPath}
} }
return stat, err return stat, err

View File

@ -1,8 +1,10 @@
package git package git
import ( import (
"errors"
"fmt" "fmt"
"io" "io"
"io/fs"
"os" "os"
"path/filepath" "path/filepath"
"regexp" "regexp"
@ -88,12 +90,12 @@ func (c config) load(r io.Reader) error {
return nil return nil
} }
func (c config) loadFile(fs vfs.Path, path string) error { func (c config) loadFile(root vfs.Path, path string) error {
f, err := fs.Open(path) f, err := root.Open(path)
if err != nil { if err != nil {
// If the file doesn't exist it is ignored. // If the file doesn't exist it is ignored.
// This is the case for both global and repository specific config files. // This is the case for both global and repository specific config files.
if os.IsNotExist(err) { if errors.Is(err, fs.ErrNotExist) {
return nil return nil
} }
return err return err
@ -130,7 +132,7 @@ func (c config) coreExcludesFile() (string, error) {
// If there are other problems accessing this file we would // If there are other problems accessing this file we would
// run into them at a later point anyway. // run into them at a later point anyway.
_, err := os.Stat(path) _, err := os.Stat(path)
if err != nil && !os.IsNotExist(err) { if err != nil && !errors.Is(err, fs.ErrNotExist) {
return "", err return "", err
} }

View File

@ -1,8 +1,8 @@
package git package git
import ( import (
"errors"
"io/fs" "io/fs"
"os"
"strings" "strings"
"time" "time"
@ -74,7 +74,7 @@ func (f *ignoreFile) load() error {
// If it doesn't exist, treat it as an empty file. // If it doesn't exist, treat it as an empty file.
stat, err := fs.Stat(f.root, f.path) stat, err := fs.Stat(f.root, f.path)
if err != nil { if err != nil {
if os.IsNotExist(err) { if errors.Is(err, fs.ErrNotExist) {
return nil return nil
} }
return err return err

View File

@ -1,9 +1,9 @@
package git package git
import ( import (
"errors"
"fmt" "fmt"
"io/fs" "io/fs"
"os"
"regexp" "regexp"
"strings" "strings"
@ -42,7 +42,7 @@ func isSHA1(s string) bool {
func LoadReferenceFile(root vfs.Path, path string) (*Reference, error) { func LoadReferenceFile(root vfs.Path, path string) (*Reference, error) {
// read reference file content // read reference file content
b, err := fs.ReadFile(root, path) b, err := fs.ReadFile(root, path)
if os.IsNotExist(err) { if errors.Is(err, fs.ErrNotExist) {
return nil, nil return nil, nil
} }
if err != nil { if err != nil {

View File

@ -1,8 +1,9 @@
package git package git
import ( import (
"errors"
"fmt" "fmt"
"os" "io/fs"
"path" "path"
"path/filepath" "path/filepath"
"strings" "strings"
@ -190,7 +191,7 @@ func NewRepository(path vfs.Path) (*Repository, error) {
real := true real := true
rootPath, err := vfs.FindLeafInTree(path, GitDirectoryName) rootPath, err := vfs.FindLeafInTree(path, GitDirectoryName)
if err != nil { if err != nil {
if !os.IsNotExist(err) { if !errors.Is(err, fs.ErrNotExist) {
return nil, err return nil, err
} }
// Cannot find `.git` directory. // Cannot find `.git` directory.

View File

@ -1,6 +1,8 @@
package notebook package notebook
import ( import (
"errors"
"io/fs"
"os" "os"
"path/filepath" "path/filepath"
"testing" "testing"
@ -50,7 +52,7 @@ func TestDetectCallsDetectJupyter(t *testing.T) {
func TestDetectUnknownExtension(t *testing.T) { func TestDetectUnknownExtension(t *testing.T) {
_, _, err := Detect("./testdata/doesntexist.foobar") _, _, err := Detect("./testdata/doesntexist.foobar")
assert.True(t, os.IsNotExist(err)) assert.True(t, errors.Is(err, fs.ErrNotExist))
nb, _, err := Detect("./testdata/unknown_extension.foobar") nb, _, err := Detect("./testdata/unknown_extension.foobar")
require.NoError(t, err) require.NoError(t, err)
@ -59,7 +61,7 @@ func TestDetectUnknownExtension(t *testing.T) {
func TestDetectNoExtension(t *testing.T) { func TestDetectNoExtension(t *testing.T) {
_, _, err := Detect("./testdata/doesntexist") _, _, err := Detect("./testdata/doesntexist")
assert.True(t, os.IsNotExist(err)) assert.True(t, errors.Is(err, fs.ErrNotExist))
nb, _, err := Detect("./testdata/no_extension") nb, _, err := Detect("./testdata/no_extension")
require.NoError(t, err) require.NoError(t, err)

View File

@ -3,7 +3,9 @@ package sync
import ( import (
"context" "context"
"encoding/json" "encoding/json"
"errors"
"fmt" "fmt"
"io/fs"
"os" "os"
"path/filepath" "path/filepath"
"time" "time"
@ -88,7 +90,7 @@ func GetFileName(host, remotePath string) string {
// precisely it's the first 16 characters of md5(concat(host, remotePath)) // precisely it's the first 16 characters of md5(concat(host, remotePath))
func SnapshotPath(opts *SyncOptions) (string, error) { func SnapshotPath(opts *SyncOptions) (string, error) {
snapshotDir := filepath.Join(opts.SnapshotBasePath, syncSnapshotDirName) snapshotDir := filepath.Join(opts.SnapshotBasePath, syncSnapshotDirName)
if _, err := os.Stat(snapshotDir); os.IsNotExist(err) { if _, err := os.Stat(snapshotDir); errors.Is(err, fs.ErrNotExist) {
err = os.MkdirAll(snapshotDir, 0755) err = os.MkdirAll(snapshotDir, 0755)
if err != nil { if err != nil {
return "", fmt.Errorf("failed to create config directory: %s", err) return "", fmt.Errorf("failed to create config directory: %s", err)
@ -145,7 +147,7 @@ func loadOrNewSnapshot(ctx context.Context, opts *SyncOptions) (*Snapshot, error
} }
// Snapshot file not found. We return the new copy. // Snapshot file not found. We return the new copy.
if _, err := os.Stat(snapshot.SnapshotPath); os.IsNotExist(err) { if _, err := os.Stat(snapshot.SnapshotPath); errors.Is(err, fs.ErrNotExist) {
return snapshot, nil return snapshot, nil
} }

View File

@ -3,6 +3,7 @@ package template
import ( import (
"context" "context"
"embed" "embed"
"errors"
"fmt" "fmt"
"io/fs" "io/fs"
"os" "os"
@ -44,7 +45,7 @@ func Materialize(ctx context.Context, configFilePath, templateRoot, outputDir st
schemaPath := filepath.Join(templateRoot, schemaFileName) schemaPath := filepath.Join(templateRoot, schemaFileName)
helpers := loadHelpers(ctx) helpers := loadHelpers(ctx)
if _, err := os.Stat(schemaPath); os.IsNotExist(err) { if _, err := os.Stat(schemaPath); errors.Is(err, fs.ErrNotExist) {
return fmt.Errorf("not a bundle template: expected to find a template schema file at %s", schemaPath) return fmt.Errorf("not a bundle template: expected to find a template schema file at %s", schemaPath)
} }

View File

@ -5,6 +5,7 @@ import (
"errors" "errors"
"fmt" "fmt"
"io" "io"
"io/fs"
"os" "os"
"path" "path"
"path/filepath" "path/filepath"
@ -313,7 +314,7 @@ func (r *renderer) persistToDisk() error {
if err == nil { if err == nil {
return fmt.Errorf("failed to initialize template, one or more files already exist: %s", path) return fmt.Errorf("failed to initialize template, one or more files already exist: %s", path)
} }
if err != nil && !os.IsNotExist(err) { if err != nil && !errors.Is(err, fs.ErrNotExist) {
return fmt.Errorf("error while verifying file %s does not already exist: %w", path, err) return fmt.Errorf("error while verifying file %s does not already exist: %w", path, err)
} }
} }