Deprecate std::path::Path's aliases of std::fs functions?

libs
#1

std::path::Path contains 5 functions which are trivial aliases of functions in std::fs:

and 3 which are convenience wrappers around std::fs::metadata:

By my rough survey, these 8 are the only functions in std outside of std::fs, std::os::*::{fs, net}, and std::process that can initiate filesystem I/O. And they're surprising, because everything else in the std::path module is just a pure data structures and algorithms that don't do any I/O.

These 8 functions are also unfortunate for the cap-std project, which uses std::path::Path in its sandboxed filesystem API, since it means that cap-std's transitive API includes functions which unceremoniously do unsandboxed filesystem accesses. We could fix this by defining a custom Path etc. that wrap std::path::Path etc. and expose everything except these 8 functions. Alternatively, we could define a clippy lint our users could enable to warn about these 8 functions. However before we do either of these, I'm curious if there's any appetite for fixing the underlying cause.

Would would people think about a PR to deprecate these 8 functions? The 5 trivial aliases don't add value other than the ergonomics of eg. p.metadata() vs. fs::metadata(p). For the 3 convenience wrappers, new functions fs::is_file, fs::is_dir, and fs::exists could be added (though this is debatable, since these operations are somewhat prone to TOCTTOU bugs).

4 Likes
#2

I get the appeal of "inert data structure" flavor of paths, however, given that these APIs are fairly popular, and that the change is cosmetic, I feel that it is way outside of the acceptable churn rate at this point.

On a less meta level, I am surprised with the set of first five functions exist. Like, how come we have convenience wrapper for read_dir, but not for open / create? I think the only commonly used fn in this set is canonicalize, and that is de-facto semi-broken on windows.

However, the last three I personally use fairly often, and it would be a bummer to have to write fs::exists(path) rather than path.exists(). TOCTOU is a thing in some cases, but often a non-TOCTOU solution doesn't exist.

For cap-std, I imagine you'll need a lint against std::fs anyway, so I don't think there's a fundamental win here? Doesn't cap-std need a strict separation between absolute and relative path as well?

Also, rust-analyzer has 4 different kinds of paths (in addition to std::path::Path, not counting Buf variants), so I am probably the wrong person to talk about this :smiley:

5 Likes
#3

I second this. The Java File API initially also had operations that did IO (including some heavy ones like to move/rename files). In nio2, they corrected this mistake by adding the Path API which does only path manipulation and nothing else. You always need a FileSystem instance to actually do things (theoretically, you even need one to create paths).

It's a bit sad that the Rust stdlib re-does this mistake, as I'm used to it doing things right™ most of the time. I personally don't mind calling fs::exists(path) instead of path.exists(), even tough I agree that it is less ergonomic and I prefer postfix operations in general.

When deprecating the three non-alias methods, I'd suggest adding equivalents to std::fs. In general, we can take a good look at all the utility methods in java.nio.file.Files and more broadly, the java.nio.file package as a whole.

#4

I'm not really comfortable with the implication of accepting this deprecation, which would be that we guarantee that we will limit filesystem IO to the certain submodules. We've had negative experiences with the ways we are already limited by a module-based capability approach in terms of the core/std distinction; sometimes the best API is to extend a type with a method once you have a new capability turned on.

In general, what I'd like to see is a single std/core with certain functionality controlled by the set of features the user uses with it; cap-std could then lint against having the filesystem feature turned on.

12 Likes
#5

Would it make sense to move them into an extension trait included in prelude (for now) instead of deprecating them altogether?

#6

Thanks for the feedback! I'll look into the lint and/or custom Path approaches. I also agree that std/core having feature sets which could be individually disabled sounds like a good direction to head in.

@tanriol I appreciate the idea, however I expect it'd be a breaking change for code using #![no_implicit_prelude].