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:
- 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
}
- 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
}
- 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)
}
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?
The
LstatIfPossibledoc string says it will callStatifLstatis not available.However,
SymlinkIfPossibleandReadlinkIfPossiblereturnErrNoSymlinkandErrNoReadlinkif the underlying functionality is not available.I think in the
SymlinkandReadlinkcase the suffix is redundant and we can remove it entirely. And in theLstatcase we can replace it with a more informativeLstatOrStat. Additionally, with some package-level functions we can save users from some type assertions and saveafero.Fsimplementations from needing anLstatOrStatmethod.I suggest the following changes for the next major version:
Lstater,Linker,Readlinker,Symlinkerinterfaces with:stat, ok, err := afero.LstatOrStat(fs, name)instead of:err := afero.Symlink(fs, oldname, newname)instead of:I think this has the benefit of not polluting
afero.Fsimplementations with non-standard*IfPossiblemethods, and a more ergonomic way of working with optional interfaces.What's the community's and maintainers' feeling about this?