More efficient formatting for assert!

[ssokolow]

As requested...

Currently, one of the wasteful patterns that I've been git gui cherry-picking against whenever I run cargo fmt is this sort of inefficient multi-line layout with the "short, long, short" argument patterns typical of assert!:

assert!(some_short_expr,
        "Arr! Here be a reasonably long format string with an argument: {}",
        short);

The pattern I'm used to using instead, is this:

assert!(some_short_expr,
        "Arr! Here be a reasonably long format string with an argument: {}", short);

I feel that it has the following advantages:

1. Less wasted space for the common case of one or two short arguments for the format string.
2. It helps to visually group the statement into two parts:
   1. The assert! itself and the expression to be tested
   2. The format string and its arguments

...and, for things where the arguments can't all fit on the same line, something like this:

assert!(result,
        "Arr! While plunderin' the hold, we got '{}' when given '{}' (we expected '{}')",
         result, input, expected);

Again, limiting wasted space and producing intuitive "assertion, message, arguments" grouping.

NOTE: The indent style used in my examples is unimportant. The detail that I'm arguing for is a more helpful line-breaking algorithm for assert!.

[joshtriplett]

In #65 , we also discussed having special-case formatting for format!, println!, and other things that take format strings. Including assert! and family in that list seems appropriate.

[joshtriplett]

I'm hesitant to support putting the format arguments on the same line as the format string when breaking across multiple lines; that seems like it'd tend to let them disappear more easily. Also, with a long format string and everything else short, this seems like it'd produce an oddly unbalanced result.

However, the idea of putting the format arguments on the same line (perhaps when they're "simple", per #47 ) does seem appealing. And in particular, having the format string on one line and then the format arguments on one line makes it easy to pair up the arguments with the placeholders.

I don't have a strong opinion about this, but I personally wouldn't have any objections to a rule that said something like "for functions/macros that take a format string and format arguments, group the format arguments onto one line if and only if they're simple" (per the definition of "simple").

That would produce results like this:

assert!(
    result,
    "Arr! While plunderin' the hold, we got '{}' when given '{}' (we expected '{}')",
    result, input, expected,
);

println!(
    "Arr! While plunderin' the hold, we got '{}' when given '{}' (we expected '{}')",
    result, input, expected,
);

Separately from that, we could talk about whether it makes sense to put the first argument on the same line as the function. If we were only talking about functions like println! and format! that take a format string as their first argument, that would produce a nicely consistent result. However, with functions like writeln! or assert! that take a format string as their second argument, that produces a much less satisfying result, in my opinion.

[ssokolow]

I'm hesitant to support putting the format arguments on the same line as the format string when breaking across multiple lines; that seems like it'd tend to let them disappear more easily.

I don't really see that being a bigger problem than with the single-line form.

In fact, if keeping the format arguments from getting lost is your concern, I think it'd be better on that metric than the single-line form, since it's semantically equivalent to this uglier markup:

    if result != expected {
        panic!("Arr! We failed, for we got '{}'", result) }

...plus, you always have to think about what people will do with it in real-world scenarios.

I don't know how representative I am, but, given how many asserts I tend to write in close proximity in my unit tests, If rustfmt tries to force me into wasting five lines when it would fit in two or three, It'll likely tip the scale enough for me to leverage a macro to accomplish one of these single-line forms, even if it requires a little uncomfortable shoe-horning (or assert_eq3! and so on) in some places:

assert_eq2!(result == expected, "short, grep-only string", result, input, expected);
assert_eq2!(r == e, "Some longer failure message with unidiomatic var names", r, i, e);

...and notice that such a "best of bad options" use of single-line forms forces either the expression or the message to be less amenable to visual scanning, since one of them won't form a neatly-aligned column.

However, the idea of putting the format arguments on the same line (perhaps when they're "simple", per #47 ) does seem appealing. And in particular, having the format string on one line and then the format arguments on one line makes it easy to pair up the arguments with the placeholders.

I don't disagree there when the message string is long enough for them to not fit on the same line.

I get the impression that this may come down to differences in subjective weighting of various advantages and disadvantages.

Separately from that, we could talk about whether it makes sense to put the first argument on the same line as the function. If we were only talking about functions like println! and format! that take a format string as their first argument, that would produce a nicely consistent result. However, with functions like writeln! or assert! that take a format string as their second argument, that produces a much less satisfying result, in my opinion.

I see your point about writeln!, but I refer back to my point about substituting an if statement and panic! for assert!.

If you see assert! as more "an if with a body carried along" rather than "a panic! that happens to be conditional", then the grouping makes more sense.

assert that X holds true
    otherwise, panic with Y

[nrc]

for functions/macros that take a format string and format arguments, group the format arguments onto one line if and only if they're simple

This is a nice formulation of the rough idea I had in mind. A few examples of expected formatting:

println!(
    " a very {} long {} format {} string",
    arg1, arg2, arg3
);

assert_eq!(
    arg1, arg2,
    "a message"
);

write!(
    out,
    " a very {} long {} format {} string",
    arg1, arg2, arg3
);

To clarify, I think all args need to be simple for them to be single-lined, i.e, we would not do this:

println!(
    " a very {} long {} format {} string",
    foo == bar,
    arg2, arg3
);

We would put arg2 and arg3 on separate lines.

I'm also not sure how to define a format string - just a string with {...} in it? Could be both false negatives and positives. We could white-list a bunch of macros to apply this to. OTOH, I would like to apply this formatting to at least one of my own (custom) macros, so it would be nice to have a general rule.

[joshtriplett]

@ssokolow Thanks for pointing out that we already put format strings and their arguments on the same line, when they're short enough. I hadn't even thought about that, but you're right, I don't see anything wrong with:

println!("some {} string {} {} arguments", "format", "with", 3);

So given that, I also don't see a problem with doing the same for the assert family of macros.

@nrc I do like the idea of trying to generalize this, but I wouldn't mind just having a way to tag such macros with an attribute or similar. Probably a good idea to support that anyway, to improve warning handling (matching format strings with format arguments), and we can key off the same thing.

[joshtriplett]

We discussed this in the style team meeting, and came to the following conclusion:

We should not put arguments (assert conditions, or format strings) on the same line as the macro name.

We should recognize things that look like a formatting macro (macro, has a string containing apparent format-argument templates like {}, has following arguments), and special-case their formatting, by putting the format arguments all on the same line if the line containing the format arguments qualifies as "simple". (We should not put the format arguments on the same line as the format string.)

[ssokolow]

While I'm not happy about that conclusion, I appreciate the process that came to it and will respect the decision as it applies to others.

That said, I have enough of an obstinate individualist streak that I suspect I'll eventually wind up maintaining a patched rustfmt to extend rustfmt.toml's powers on this issue.

[joshtriplett]

@ssokolow FWIW, I don't think any objections were raised to the idea of configuration for this point. If you ended up writing patches to add such configuration, they might well get accepted. Our discussion solely regarded the default behavior.

[ssokolow]

That's even better. :)

At the moment, it's not essential enough to justify, but I'll definitely keep it on my TODO list as a successor to "just run rustfmt infrequently and use git gui to revert the bits I disagree with".

(I said "eventually" specifically because, at the moment, I can't justify any coding not essential to significantly improving the efficiency of time-sensitive things like making archival-quality backups of a library of 20+ year old audio CDs where at least one already fails the EAC/Whipper "get the same data twice" accuracy test.)

[nrc]

Entering FCP based on #86 (comment)