add write_fmt method to String, to make write! macro work without imports

[RalfJung]

Proposal

Problem statement

It would be great if this code compiled:

fn main() {
    let mut s = String::new();
    write!(s, "hello").unwrap();
}

Using write! to append to a String is one of these neat Rust things that are not obvious to discover, but once you know about it it is amazingly useful.

Solution sketch

The code currently fails saying that io::Write or fmt::Write needs to be imported or a write_fmt method needs to be added. (The right trait to import is fmt::Write, as I found out by trial-and-error.)

From the error message it sounds like that could be avoided if we simply added a write_fmt inherent method to String. That would just get one minor roadblock out of the way which sounds like a win to me. :)

Alternatives

We could decide the roadblock isn't bad enough to warrant doing anything.

What happens now?

This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

• We think this problem seems worth solving, and the standard library might be the right place to solve it.
• We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

• We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
• We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.

[pitaj]

Is there precedence for this on any other types?

[RalfJung]

Not that I am aware of. But String is a very fundamental type and at the same time it is far from obvious that it would implement fmt::Write or io::Write. People writing to a File will probably have io::Write imported anyway, or will at least have std::io around and be generally looking in the right direction.

[pitaj]

One problem I could see is that the implementation of fmt_write will take precedence over any trait impl. So it would prevent using this macro with io::Write on String, if we wanted to add an impl for that.

[RalfJung]

Even if we added such an impl, of course its write_fmt would behave exactly the same as fmt::Write::write_fmt so that would not be a problem. There's no reasonable world in which io::Write and fmt::Write are both implemented on String but their write_fmt behave differently.

[shepmaster]

If I understand the problem at hand, this could use inherent traits, if those ever existed.

[Veykril]

To add, r-a does this so frequently that we have a similar macro in our code base https://github.com/rust-lang/rust-analyzer/blob/62268e474e9165de0cdb08d3794eec4b6ef1c6cd/crates/stdx/src/macros.rs#L13-L20

(though we discard the result since it doesn't ever fail for us)

[RalfJung]

Yeah writing to a String is never supposed to fail, but I prefer a panic over silently throwing away an error.

[Veykril]

Given a custom Debug/Display impl can fail even when writing to strings this proposal returning a Result is appropriate imo.
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=c5e2ef87f493f8e15dafea9a967f2aa8

struct S;

impl std::fmt::Display for S {
    fn fmt(&self, _: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
        Result::Err(std::fmt::Error)
    }
}

fn main() {
    use std::fmt::Write as _;
    write!(String::new(), "{}", S).unwrap();
}

[RalfJung]

Given a custom Debug/Display impl can fail even when writing to strings this proposal returning a Result is appropriate imo.

The documentation says they are not allowed to do that, so this is a buggy Display impl. (I don't know where those docs are, but I am sure I have seen docs saying that Debug-Display are only allowed to forward errors from the formatter, and not raise their own errors. The ToString impl relies on that.)

[Veykril]

TIL

https://doc.rust-lang.org/std/fmt/struct.Error.html states

This type does not support transmission of an error other than that an error occurred. Any extra information must be arranged to be transmitted through some other means.

but I'll have to say that is very much not discoverable, nor does it express that I guess (was the closest I could find). I assume this is just describing the fact that the error has no payload. So ye I can't really find where this is described, aside from the comment on the ToString impl

[RalfJung]

I don't think those docs are meant to imply "no new errors during Debug/Display". I thought I had seen a fairly clear statement of that rule but I don't remember where...

[thomcc]

I thought I had seen a fairly clear statement of that rule but I don't remember where...

Possibly here https://github.com/rust-lang/rust/blob/cedbe5c715c1fa9359683c5f108bed2054ac258b/library/alloc/src/string.rs#L2441-L2446. I'm not sure it's normative though, since it's so buried away. Notably we panic in these cases deliberately, rather than ignoring the error, because such an error may exist. I would be opposed to changing that, for example.

[mkroening]

I think you mean this? https://doc.rust-lang.org/std/fmt/index.html#formatting-traits

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.

[kpreid]

Given the premise that formatting implementations "must and may only return an error if…", it would be really nice if String::write_fmt() returned Result<(), Infallible> to communicate that it can't actually fail and not require the caller to use either _ = or .unwrap() to silence the unused_must_use warning. To my eyes, that's the one big inelegance of using write! on strings.

Unfortunately, that would cause a type error in any existing code that tries to propagate std::fmt::Error out (though adding impl From<Infallible> for fmt::Error would solve that for ?s) when the inherent method shadows fmt::Write. But perhaps there's a solution I haven't thought of, or a crater run will show that it's OK.

(And ignoring a Result<(), Infallible> or Result<(), !> still prompts an unused_must_use warning, but that could be fixed.)

[thomcc]

I think if we do this it should just return fmt::Error. The slight convenience factor isn't worth the potential breakage. I'd be willing to try a crater run, but I expect this breakage does exist.

[kornelski]

BTW, String does not, and can not, implement io::Write, because io::Write allows writing non-UTF-8 data.

However, there is an edge case that could break:

trait CustomWrite {
    fn write_fmt(&self, a: std::fmt::Arguments<'_>) -> () {}
}

impl CustomWrite for String {}

write!(&mut s, "");

[pitaj]

One thought I had was to add a new trait for infallible write-to-string in the prelude:

mod prelude {
    pub trait WriteStringInfallible {
        fn write_fmt(&mut self, args: std::fmt::Arguments<'_>);
    }

    impl WriteStringInfallible for String {
        fn write_fmt(&mut self, args: std::fmt::Arguments<'_>) {
            let _ = <Self as std::fmt::Write>::write_fmt(self, args);
        }
    }
}

use prelude::*;

fn main() {
    let mut s = String::new();
    write!(s, "Hello, world!");

    println!("{s}");
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=a6c94ee049cc04400eb963c16bf86925

Currently this results in an ambiguity error. Could we change the resolver to automatically pick an explicitly imported trait method if all of the other options are glob-imported?

Then you could still do

use std::fmt::Write;

To use write! with that trait instead.

But I think an easier solution would be a way to suppress must_use. Then we can put that attribute on String::write_fmt:

impl String {
     #[suppress_must_use]
     fn write_fmt(&mut self, args: fmt::Arguments<'_>) -> fmt::Result {
         <Self as fmt::Write>::write_fmt(self, args)
     }
}

[RalfJung]

On IRLO, some alternatives have been suggested to achieving the goal of "easily and efficiently extending a string with formatting":

s += format_args!("hello {world}");
s.push_display(&format_args!("hello {world}"));

I feel like something like this is probably preferable over my original proposal here.

[matklad]

Is there precedence for this on any other types?

Formatter::write_fmt is also an inherent method:

https://doc.rust-lang.org/stable/std/fmt/struct.Formatter.html#method.write_fmt

[m-ou-se]

Making s += format_args!("hello {world}"); work seems very harmless. I see no reason not to add this.

The only open question for that is how to handle a misbehaving (error-returning) Display/Debug/... implementation. Panicking seems a bit cleaner than just ignoring, but might result in more generated code, which might not be worth given that the docs say:

contrary to what the function signature might suggest, string formatting is an infallible operation

[cuviper]

Making s += format_args!("hello {world}"); work seems very harmless.

It might lead to inference failures, because right now there's only one RHS type for AddAssign<&str>. So for example, with two String values, s += s2.as_ref() has to infer that using AsRef<str>, and I'm not sure that will happen if there's another AddAssign possibility. (Maybe it's saved if the other RHS isn't a reference?)

At the very least, we'll need a crater run to gauge this.

(edit): This point was also raised here: https://internals.rust-lang.org/t/mut-string-format-args/19461/18

[RalfJung]

IMO we should definitely panic on errors when debug assertions are enabled. There's no good reason to silently ignore what is clearly a bug.

I have less of a strong opinion for builds without debug assertions.

[m-ou-se]

We discussed this in a recent libs-api meeting.

We don't think we should be adding an inherent String::write_fmt that returns a fmt::Result, as the Err case is not useful. We'd love to find an effective way to append a fmt::Arguments to a String without having to (needlessly) handle fmt/io errors.

A few of the possible directions are a += operator implementation, some edition-dependent infallible write_fmt method, or a new trait (with write_fmt method) for infallible formatting.

Unfortunately, implementing the += operator currently breaks a lot of things, as pointed out above: #261 (comment)

The other possible directions might be made impossible or much trickier if we were to add an inherent String::fmt with the same signature as fmt::Write::write_fmt. We should not close off these possibilities until we have figured out what path forward to take.

[dtolnay]

I wouldn't rule out += just yet. #261 (comment) says that it breaks s += s2.as_ref() (AsRef<str>) but I don't think it does. See playground.