Method to append any `Display` type to a String

libs
1

When you want to append something that implements Display to a String, you currently have two options:

  1. Use the write!() macro. However, this is cumbersome:

    • It requires importing the std::fmt::Write trait, which can not be done automatically by IDEs
    • For a newcomer, it's not obvious how to use it
    • When you forget the trait import, the compiler error suggests importing std::fmt::Write or std::io::Write, so you first have to figure out which one you need
    • It requires ignoring or unwrapping the returned Result, even though formatting with Display and writing to a String typically don't error. format!() and ToString don't require this.

  2. Convert it to a String first, which is inefficient because it allocates, but many prefer it because it's so much easier

A solution would be to add a String method for this:

impl String {
    pub fn append<T: Display>(&mut self, value: T) {
        use alloc::fmt::Write;
        write!(self, "{}", value).unwrap();
    }
}

So instead of this:

use std::fmt::Write;

let mut s = String::new();
let _ = write!(&mut s, "{}", foo());
let _ = write!(&mut s, "[{}]", bar());

You could write

let mut s = String::new();
s.append(foo());
s.append(format_args!("[{}]", bar()));

I think this is a nice ergonomic improvement.

14 Likes
2

See also discussion around related Clippy lint:

clippy::format_push_string

3

Yes, that's the reason why I posted this now. The mentioned clippy lint is allow-by-default now, because the alternative, using write!(), is controversial. See for exmple this issue.

If String::append existed, I think the lint could be enabled again, and recommend this method instead of write!().

4

The name append is usually used for (&mut self, other: &mut Self) methods.

Also there is an argument to be had whether the type should be <T: Display>(&mut self, value: &T) rather than <T: Display>(&mut self, value: T).

8 Likes
5

Maybe push_fmt or write_fmt then?

Since impl<T: Display> Display for &T exists, I don't see a reason to use a more restictive API and only allow references here.

1 Like
6

Yeah, I think a name that starts with push could be good.

It avoids users from accidentally giving up ownership of a value, and then using .clone() to fix it if they weren’t aware of the Display for &T implementation.

2 Likes
7

I'm not sure if that is enough justification to make the API less ergonomic. Accidentally giving away ownership is already possible with several APIs, such as PathBuf::push and OsString::push. More generally, it's possible with every function that accepts a value implementing a trait such as AsRef<_>, Into<_> or ToString. Helping the user to avoid allocations is, IMHO, the responsibility of clippy.

3 Likes
8

I am aware that some standard library APIs do it differently already; however at least for AsRef it’s perhaps “more obvious” that you can pass a reference by the trait name alone. As one point of comparison, e.g. something like to_string in serde_json - Rust takes &T even though there’s a generic implementation Serialize for &T. It think that <T: Display>(&mut self, value: &T) is easier to understand, because it doesn’t assume knowledge of the fact that Display only has a &self method and comes with a <T: Display> Display for &T implementation. (Instead, it effectively even teaches that Display works with &self, because it demonstrates that API can usefully work with &T for a Display type.) The only downside to the “less general” signature is that callers will need to sometimes write the & themselves. It’s not “less general” in any meaningfully limiting way. (You can even call it with (&{x}), using a block expression to move x, if you absolutely need to drop x with/after the call.)

I know that this design question is a question of style and people can have different opinions. My main point is that this is a point of discussion, I don’t insist on that it must be one way or another, just there’s a trade-off that needs to be considered. Hence my initial comment was “there is an argument to be had whether the type should be …”, not just “the type should be …”.

9 Likes
9

An even closer point of comparison since it’s using the Display trait, too: Serializer::collect_str

fn collect_str<T: ?Sized>(self, value: &T) -> Result<Self::Ok, Self::Error>
where
    T: Display,
10

One workaround for this is to add an inherent String::write_fmt method shadowing the fmt::Write one (since String does not implement io::Write). That would at least remove the need for the use.

I wonder if that inherent method could be infallible, not sure if the write! macro cares about the method return type or not. (Though adding it infallibly now would cause issues with existing code that is currently resolving to fmt::Write::write_fmt, so that's only something that could have been done when String was first created).

1 Like
11

That's an interesting idea.

No, it cannot, as that would break code that currently uses write!().

12

I think I've written push_str(&format!(..)) reasonably often, would be great to have a more ergonomic/efficient alternative.

5 Likes
13

That isn't necessarily a reason to repeat the same mistakes.

Taking impl AsRef without any other bounds is imho a clear antipattern with the power of hindsight. It's very convenient to have emulated autoderef coercions and automatic ref-to-ref conversions on function arguments, but I argue that this is optimizing for initial writing at the expense of reading/editing.

(However, we may be stuck with this for the nearterm future, since AsRef is not reflexive and can't be made so without, at a minimum, impl specialization, and will even then still cause a significant amount of inference breakage if no mitigation is provided.)

Being able to call fs::open("strings are paths") rather than fs::open(Path::new("now I'm sad")) is a desirable benefit. Giving an owning nonreference to a function which cannot take advantage of the ownership is an unfortunate consequence of the design of AsRef which is IIUC a result of a stylistic preference for writing fn f<T: AsRef<str>>(_: T) instead of fn f<T: ?Sized + AsRef<str>>(_: &T), avoiding the need for ?Sized.

3 Likes
14

I also find this to be a papercut; mainly the need to sprinkle unnecessary unwrap(), which also seems like it isn't optimized out today.

Adding a method push() to std's String presumably also could at least use unreachable_unchecked! for the Err case which can't happen (right?).

15

It could be unlikely at most -- I can write a Display that returns an error regardless of what happens to the stream, even if I probably shouldn't.

1 Like
16

It can happen if the Display implementation returns an error. Those are not supposed to produce any errors though, but merely forward errors from the Formatter. But of course, safe code can break this rule, and breaking the rules like this must not result in undefined behavior; so unreachable_unchecked is not an option.

When implementing a format trait for your own type, you will have to implement a method of the signature:

fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {

Your type will be passed as self by-reference, and then the function should emit output into the Formatter f which implements fmt::Write. It is up to each format trait implementation to correctly adhere to the requested formatting parameters. The values of these parameters can be accessed with methods of the Formatter struct. In order to help with this, the Formatter struct also provides some helper methods.

Additionally, the return value of this function is fmt::Result which is a type alias of Result<(), std::fmt::Error>. Formatting implementations should ensure that they propagate errors from the Formatter (e.g., when calling write!). However, they should never return errors spuriously. That is, a formatting implementation must and may only return an error if the passed-in Formatter returns an error. This is because, contrary to what the function signature might suggest, string formatting is an infallible operation. This function only returns a result because writing to the underlying stream might fail and it must provide a way to propagate the fact that an error has occurred back up the stack.

Emphasis, mine. Quotation from: std::fmt - Rust

5 Likes
17

Urgh, right. But I think basically ever time I've wanted to write a Display to a String it's on a type I know for sure has an infallible Display impl. Hmm. Having half-baked thoughts of a new InfallibleDisplay trait, but not clear how invasive and compatible that'd be. Or perhaps like we have ToString today, there'd be a new AppendToString.

18

Another workaround would be to implement "inherent traits", and then make fmt::Write an inherent trait of String.

Inherent traits have been a desire of the library team for a long time, and the lang team is on board with shipping them. There's already a draft RFC.

I've gone through that RFC and followed up on some of the feedback.

Would someone be interested in updating the RFC, and then asking for an FCP (which I'd be happy to kick off)?

3 Likes
19

Could we maybe add a push_to_string(&self, &mut String) to ToString itself (with the natural default implementation) and then reimplement to_string in terms of that?

20

I don't think that's possible in a backwards compatible way.

Also, it might not be desirable to have a new method added to so many types. This method would always be available, since ToString is in the prelude.

1 Like