println! is completely useless in doctests, since it doesn't actually output anything into the documentation, since it doesn't actually test anything, and since it leads documentation writers to avoid using something like assert_eq! that does show example results (which is useful information) in the documentation.
Can println! and similar please be banned from standard library doctests?
Those are examples before they are tests. Sometimes it makes sense to use println! in an example.
Here's one such case: https://doc.rust-lang.org/std/iter/trait.Iterator.html#examples-12
If you’re doing some sort of side effect, prefer
fortomap():// don't do this: (0..5).map(|x| println!("{x}")); // it won't even execute, as it is lazy. Rust will warn you about this. // Instead, use for: for x in 0..5 { println!("{x}"); }
What is the value of showing println!() in this code? The most it can convey is that some values have a Display or Debug instance.
A much better way to illustrate the point of this example could be the following:
let mut sum = 0;
// this does nothing (besides creating and dropping an iterator):
(0..5).map(|x| sum += x);
assert_eq!(sum, 0);
// instead do it like this:
for x in 0..5 {
sum += x;
}
assert_eq!(sum, 15);
The println! version is better than the sum version in explaining map versus for. Printing is the more trivial operation and adds no cognitive load to the example, while the sum is a distraction from the main point of the example.
I'm not so sure. The example as it stands only makes sense because of the comment that explains the .map() won't be evaluated. The assertions demonstrate that in code.
println! adds a lot of work to the example, as the people who need it are new to Rust, and they might have to take the time to find some place to run the example to see exactly what it does. In some cases, that can't be done with the Rust Playground, and so they have to take the time to make a new crate and so forth just to see an example of a value produced by a method. assert_eq! immediately shows an example value.
I'd also point to the fact that a large number of examples use assert_eq!, and I don't think that that should be changed to println! to reduce potential cognitive load.
I think a slightly better example would be:
// don't do this:
let mut sum = 0;
(0..3).map(|x| sum += x);
assert_eq!(sum, 0);
// it won't even execute, as it is lazy. Rust will warn you about this.
// Instead, use for:
let mut sum = 0;
for x in 0..3 {
sum += x;
}
assert_eq!(sum, 0 + 1 + 2 + 3);
It's just an example of an operation done for its side effect, it's also short and something everyone reading will be familiar with.
I don't really understand this.
I think the sum examples have the problem that they're telling the user ("do this instead") to sum a range using a for loop instead of the more natural (at least in my opinion) (0..3).sum(). Especially since we are talking about the Iterator docs, it feels awkward. That said, this is just one example and you may agree or disagree with the sum() point. inspect is another good example where I think println is very natural and probably matches how the average programmer would use the method. And I'm sure there are more like that, I just opened the Iterator docs because there are a lot of methods and ctrl+f'd println.
If there are specific docs you want to improve I'm sure that would be welcomed, but I think a ban would be unjustified. Surely at least println's own docs should be able to use println?
I think the sum examples have the problem that they're telling the user ("do this instead") to sum a range using a for loop instead of the more natural (at least in my opinion)
(0..3).sum().
I don't think that that's a fatal problem with it, as inspect has an example of using fold to sum and you can replace summing with something else that doesn't have a premade iterator method.
And I'm sure there are more like that, I just opened the
Iteratordocs because there are a lot of methods and ctrl+f'dprintln.
Yes, but here are two more examples of where println! doesn't help:
#![feature(iter_collect_into)]
let a = [1, 2, 3];
let mut vec: Vec::<i32> = Vec::with_capacity(6);
a.iter().map(|&x| x * 2).collect_into(&mut vec);
a.iter().map(|&x| x * 10).collect_into(&mut vec);
assert_eq!(6, vec.capacity());
println!("{:?}", vec);
There's no reason to have a println! when assert_eq! effectively prints the result into the documentation.
let a = [1, 2, 3];
let mut vec: Vec::<i32> = Vec::with_capacity(6);
let count = a.iter().collect_into(&mut vec).iter().count();
assert_eq!(count, vec.len());
println!("Vec len is {}", count);
let count = a.iter().collect_into(&mut vec).iter().count();
assert_eq!(count, vec.len());
println!("Vec len now is {}", count);
This also has the same problem. Replacing the last line with assert_eq!(count, 6); or assert_eq!(vec.len(), 6); would be a better explanation of what's going. It has the same benefit as the println!, but the result is included in the documentation rather than requiring you to run the code.
inspect is another good example where I think
printlnis very natural and probably matches how the average programmer would use the method.
Surely at least
println's own docs should be able to useprintln?
Yes, but this would probably be done by a lint, and that lint could deny by default for the standard library and allow for println!'s own documentation or for places where println! is what's used in the vast majority of use cases for some function.
I kind of agree with the central thesis here: that println! is generally not as useful in a doc example than something like an assert_eq!. I certainly almost never use println! in my own crate docs for exactly the complaint lodged here: it doesn't show the actual output. With that said, there are two mitigating things to consider with std:
println! is largely mitigated IMO.I don't think there should be an outright ban. That's way too strong. Sometimes println! is the right thing. Whether we'd enforce that through a lint (assuming we adopted this posture) is something I'd defer on personally. But as a rule of thumb, yes, I'd agree we should lean towards assert_eq! instead of println! in doc examples.
BTW, maybe there could be some kind of test assertion for testing the output? The test harness already captures output by default.
```rust,should_print="foo"
println!("foo");
```
How about, instead of squeezing the assertion into the first line (limiting possible formatting), making the output part of the example text:
/// ```rust
/// println!("foo");
/// ```
///
/// This prints:
///
/// ```output_of_previous
/// foo
/// ```
This way, readers get to see the output, it can be multiple lines, and is tested. In some cases, I think this might allow writing examples that are much less compromised by also being tests.
I once worked with a testing system whose primary means of making assertions was to compare printed output — essentially checking a REPL transcript. This wasn't always good, but it often made tests very clear and concise because it eliminates assert_eq! boilerplate (at the cost of only comparing the printed representation).
In that case I’d use a more trivial side effect (like exit(1)), as println! is quite a complex operation that involves multiple concepts.
I was actually serious and meant it literally when asking “what does println!() show?” It shows a side effect, done by a macro invocation, with formatting magic, involving the Debug or Display traits (typically). None of this is required by the example in question apart from the side effect.
Sorry for the rant, but here you focus on the least interesting part of my message in order to “counter” it. I chose the repeated mutation of a variable as the side effect to demonstrate whether the computation occurs. You are right in that the addition is not necessary to show what we want to show, so a better example would be
let mut called = false;
(0..1).map(|x| called = true);
assert!(!called);
for x in 0..1 {
called = true;
}
assert!(called);
This is the essence of the demonstration, comments may be added as extra bells and whistles.
I stand by my point that println!() is exceedingly complex as a side effect, the reader of the documentation does not see the result, and thus it should mostly occur in the documentation of println!() (of course; your questioning this in the last paragraph feels like shoddy discussion tactics to me). Your point about inspect is also a good one, since logging or tracing is the most likely reason for using this method.
This is kind of a logical fallacy. What does assert_eq! do? It shows an expected output, done by macro magic, with custom equality defined by the PartialEq and Eq traits, producing a panic on failure, which might be caught somewhere higher up the call stack in the general case (unless you have configured your project to not do that).
println! is easy to understand, even if you don’t know how it works. assert_eq! is also pretty easy to understand, and is better for testability. I think it’d be reasonable to reconsider uses of println! in the standard library docs, but as others have said these are examples first (runnable examples, no less) and tests second.
(As to which makes a better example, that depends! If the output is important, probably assert_eq!. But for just reading code, I think most people—though definitely not everyone—find printing easier to reason about than assertions, because (a) it’s the first thing you learn in most imperative language tutorials, including the Rust book, and (b) you don’t have to think about what properties being asserted are important, because there are none.)
EDIT: To summarize, I generally agree with you, but think the hyperbole is hurting your cause.
For the purposes of documentation, the fact that this expected output is written down (so I can read it while reading the documentation) is quite nice. For the purposes of rust education, the fact that if I copy/paste this code (or run it in playground) it won't print anything kind of hurts the presentation. And introducing extra variables and operations is definitely a little more obfuscated than printing. I think the ideal operation here would allow for writing
// don't do this:
(0..5).map(|x| println!("{x}"));
// This prints:
//
// it won't even execute, as it is lazy. Rust will warn you about this.
// Instead, use for:
for x in 0..3 {
println!("{x}");
}
// This prints:
// 0
// 1
// 2
Where // This prints:[1] is understood by the doctest harness to assert on the output. Because now people learning rust (possibly familiar with println?) can run the code (and edit it and run it again), everyone else can see the output already in a comment, and the doctest author gets their documentation tested.
Or some annotation; it's possible that a default phrase in only English is less accessible than it could be. ↩︎
That seems like it would be a nice alternative, though I think that the output should be separated from the code. This could be helped by putting:
/// ```rust
/// // don't do this:
/// println!("`map` output:");
/// (0..5).map(|x| println!("{x}"));
///
/// // it won't even execute, as it is lazy. Rust will warn you about this.
///
/// // Instead, use for:
/// println!("`for` output:");
/// for x in 0..3 {
/// println!("{x}");
/// }
/// ```
///
/// ```output
/// `map` output:
/// `for` output:
/// 0
/// 1
/// 2
/// ```Reformatted a bit, I would probably suggest splitting it into separate blocks:
Don't use `map()` for side effects like printing:
```rust
(0..5).map(|x| print!("{x}"));
```
as `map()` is lazy, and this won't print anything:
```stdout
```
Rust will warn you about this. Instead, use `for`:
```rust
for x in 0..5 {
print!("{x}");
}
```
This will print as expected:
```stdout
01234
```
Rendered:
Don't use
map()for side effects like printing:(0..5).map(|x| print!("{x}"));as
map()is lazy, and this won't print anything:Rust will warn you about this. Instead, use
for:for x in 0..5 { print!("{x}"); }This will print as expected:
01234
(An empty code block is invisible inside a blockquote here, but much more visible in rustdoc or even outside a blockquote here, e.g.)
I'm the author of collect_into. I don't remember why I used printlns there, but asserts are way better for those examples, I opened a PR.
I also don't think banning prints is the best way to fix the issue. Maybe a recommendation to prefer asserts over printlns (when possible) written somewhere should be added, I don't remember having seen one at the time I wrote collect_into docs.
There's a lint for this Clippy Lints.
Could enable it with some cfg(doc).
It seems like sometimes you would want the output inline, sometimes at the end, sometimes in separate blocks... I'm not sure if being overly restrictive here is conducive to the main goal of making nice looking/correct doctests. That said, if this existed in any form I would probably use it.