proposal: io/fs: return ErrInvalid if the pattern argument of Glob is not a valid path

[gazerro]

If pattern is a well-formed pattern but it does not satisfy fs.ValidPath(pattern), fs.Glob(fsys, pattern) does not return matchings and does not return errors.

For example, if fs.Glob(fsys, "a") returns []string{"a"} and nil, these other calls

fs.Glob(fsys, "/a")
fs.Glob(fsys, "a/")
fs.Glob(fsys, "./a")

return nil and nil:

I propose, before the 1.16 release, that the fs.Glob function returns the fs.ErrInvalid error if pattern is a well-formed pattern but does not satisfy fs.ValidPath instead of returning a nil matched and a nil error.

Note that the filepath.Glob function, as fs.Glob, can return only the path.ErrBadPattern error but filepath.Glob cleans the pattern before matching the files, fs.Glob doesn't.

Note also that //go:embed has the same pattern syntax of fs.Glob (except ".") but

//go:embed pattern
var files embed.FS

does not compile if pattern does not satisfy fs.ValidPath, instead of assigning to files an empty file system.

[bcmills]

I think it would also be reasonable to instead return ErrBadPattern for these inputs: ErrBadPattern already indicates a pattern that cannot possibly match any files.

[rsc]

I am not at all sure we should make any change here.

fs.Glob(fsys, "/a")

The directory "/" does not exist in an FS; that's why "/*" doesn't have any matches.
It seems wrong to report that as path.ErrBadPattern, especially since path.Match accepts it.
It's also not invalid. It's a valid pattern, it just doesn't describe any files.
Instead we currently treat it like other non-existent paths, like "does-not-exist/*".

As another analogy, if we are running on a file system without support for Unicode,
we don't make filepath.Glob("☺*") return ErrBadPattern or ErrInvalid.
It just returns no match.

fs.Glob(fsys, "a/")

filepath.Glob returns no matches and no error for this too.
It seems especially wrong to make fs.Glob return an error that filepath.Glob does not.

fs.Glob(fsys, "./a")

I am more sympathetic to this example than /a, but not enough to make an exception.
Open("./a") fails too, of course, just like Open("/a").
These are valid patterns. They just don't describe openable files.

In the first two examples the behavior seems clearly correct (given the non-existence of / and the current behavior of filepath.Glob). That leaves only the third, but a special case would add more complexity about interpreting error results and deviate from the other two.

[gazerro]

@rsc Thanks, I get your point and I'm fine to close this issue. Perhaps a note in the documentation could help to better understand this behavior, which after your exposition I believe correct.

[rsc]

No change in consensus, so declined.
— rsc for the proposal review group