Skip to content

*IfPossible methods are confusing #545

@gaboose

Description

@gaboose

The LstatIfPossible doc string says it will call Stat if Lstat is not available.

However, SymlinkIfPossible and ReadlinkIfPossible return ErrNoSymlink and ErrNoReadlink if the underlying functionality is not available.

I think in the Symlink and Readlink case the suffix is redundant and we can remove it entirely. And in the Lstat case we can replace it with a more informative LstatOrStat. Additionally, with some package-level functions we can save users from some type assertions and save afero.Fs implementations from needing an LstatOrStat method.

I suggest the following changes for the next major version:

  1. Replace Lstater, Linker, Readlinker, Symlinker interfaces with:
type Lstater interface {
	Lstat(name string) (os.FileInfo, error)
}

type Readlinker interface {
	Readlink(name string) (string, error)
}

type Symlinker interface {
	Symlink(oldname, newname string) error
}

// optional: but at least leave this name available for the os.Link equivalent
type Linker interface {
	Link(oldname, newname string) error
}
  1. Add afero package-level functions:
func LstatOrStat(fs Fs, name string) (os.FileInfo, bool, error) {
    if lstater, ok := fs.(Lstater); ok {
        if lstat, err := lstater.Lstat(name); !errors.Is(err, ErrNoLstat) {
            return lstat, true, err
        }
    }
    stat, err := fs.Stat(name)
    return stat, false, err
}

func Symlink(fs Fs, oldname, newname string) error {
    if symlinker, ok := fs.(Symlinker); ok {
        return symlinker.Symlink(name)
    }
    return ErrNoSymlink
}

func Readlink(fs Fs, name string) (string, error) {
    if realinker, ok := fs.(Readlinker); ok {
        return realinker.Readlink(name)
    }
    return "", ErrNoReadlink
}
  1. Direct users to use package-level functions instead of asserting interfaces. I.e.:
  • Write stat, ok, err := afero.LstatOrStat(fs, name) instead of:
if lstater, ok := fs.(Lstater); ok {
    stat, ok, err := lstater.LstatIfPossible(name)
}
  • Write err := afero.Symlink(fs, oldname, newname) instead of:
if symlinker, ok := fs.(Linker); ok {
    err := symlinker.SymlinkIfPossible(oldname, newname)
}
  • etc.

I think this has the benefit of not polluting afero.Fs implementations with non-standard *IfPossible methods, and a more ergonomic way of working with optional interfaces.

What's the community's and maintainers' feeling about this?

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions