Figure out a better way to accept both Path and String arguments

[yorickpeterse]

Description

If a method wants to accept both a Path and String, then turn those into a Path, there are a few ways of doing this:

1. Take an owned Path and leave the conversion up to the user, and take over ownership
2. Take a ref Path and clone it
3. Take a IntoPath and move it into a Path
4. Take a ref ToPath and create a new Path from it

Option 1 isn't great because it means the user has to convert the types every time, which is tedious.

Option 2 means that if I already have a Path and I'm OK with giving up ownership, I now run into a redundant clone. In addition, if I have a String I have to first convert it.

Option 3 allows for both Path and String arguments, but if a Path is given we'll take over ownership. This is annoying if the caller wants to reuse the Path, as they now have to explicitly clone it first. In addition, I really want to get red of Into* traits, as we currently only have few legitimate use cases for them, and it's just confusing for users to determine if they should use ToX or IntoX.

Option 4 means always creating a new Path, which may be redundant if the caller doesn't make further use of its input Path.

In its current setup, cloning a Path is cheap enough, as it's backed by a String. This means using ref ToPath would probably be sufficient, and allow us to get rid of IntoPath. However, if we change the internal implementation (see #314), then cloning a Path may end up becoming more expensive, so I'm unsure as to how future-proof this approach is.

Option 1 incurs the least amount of "type soup", as you just say "I need a Path" and leave the conversion up to the user. This might not be that bad given you can just do some_string.to_path, but it can get repetitive.

Related work

No response

[yorickpeterse]

One option is to do away with ToPath and IntoPath entirely, and expect Path/ref Path arguments directly. This makes the decision easier, and comes with the added benefit of making the conversion explicit. This in turn means you can't just read a file (for example) and pass its contents to something that expect a path, at least not without an explicit conversion.

[yorickpeterse]

4047c8b removes IntoPath as it's mostly useless, but ToPath remains.

The decision to keep ToPath comes down to this: is there a security risk of using ToPath as an argument instead of an explicit Path? If so, we should remove ToPath and make the conversion explicit.

Whether this poses a risk is difficult to answer. Many file paths will be string literals, in which case there's no benefit to using "foo/bar".to_path instead of just passing "foo/bar". But if the initial String is user input, then making the conversion might just make it more explicit and force the developer to think "Hey wait a minute, should this string actually be used as a path?". This however is just speculation, and it's quite possible developers just end up cargo-cult copy-pasting x.to_path every time they have a String but need a Path.

What is clear is that this:

fn foo[T: ToPath](path: ref T) {
  do_something(path.to_path)
}

foo('...')

is more verbose compared to this:

fn foo(path: Path) {
  do_something(path)
}

foo('...'.to_path)

[yorickpeterse]

We actually have a similar problem with methods such as Write.print: this method takes a String instead of IntoString or ToString, requiring an explicit conversion. This is in fact a little annoying when printing non-string values (e.g. an Int).

[yorickpeterse]

Another potential challenge: if you accept a ToPath and already have a Path, the result is an extra clone that is redundant (assuming you don't use the Path elsewhere). A hypothetical future optimization pass could eliminate that, but I prefer to not design an API around the idea that the compiler will just make it not be wasteful.

[yorickpeterse]

This problem/pattern also surfaces with the socket module: some methods take a T: ToString so you can specify an IP address as both a String or an IpAddress, or to support both String and Path for Unix sockets.

I'm leaning more and more towards just expecting the final concrete types (e.g. ref IpAddress) and place the burden of conversion on the caller.