[rustdoc] Replace print methods with functions to improve code readability

[GuillaumeGomez]

We have a lot of print methods, making it quite tricky to know what the types we're manipulating are.

We did something similar with Clean trait a few years ago (the first PR was #99638, followed by a lot of them).

Each commit replaces one type for easier review.

r? @yotamofek

[rustbot]

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[GuillaumeGomez]

I somehow replaced a write!("{} ") with a write_str. That was a bad move. :)

[yotamofek]

I dunno, personally, I kinda prefer the method scheme over free fns. term.print() is better than print_term(term), IMHO. But then again, not all the calls to print are on a receiver that's named so descriptively.
So 🤷
If it's more consistent with the rest of the codebase, then yeah, it's probably a good change.
I do think that it's a bit too many commits though? Maybe squash 'em down into one (or more than one if you can find a logical way to group some of the changed items together?)

On an unrelated note, I wish there was a nice way to reduce all the impl Display boilerplate, e.g. something like:

#[display_fn]
fn print_term(term: ..., f: &mut Formatter) -> fmt::Result {
    // ...
}

and that would be expanded into a fn print_term(term: ...) -> impl Display.
But I really don't like the idea of a macro changing an fn's signature, sounds very confusing.

[GuillaumeGomez]

Not a big fan either of changing a signature like that.

I'll squash down commits once you're ok with the code to make it easier for you to review.

And having methods is better... until you have a lot of types implementing the same method and then you're completely lost when tracking down a bug. ^^'

[yotamofek]

Not directly related to your changes, but a drive-by thought while reviewing this. (also, this is probably something I wrote, originally).

This can be re-written as:

Suggested change

	let mut real_params = generics.params.iter().filter(|p| !p.is_synthetic_param()).peekable();
	if real_params.peek().is_none() {
	None
	} else {
	Some(Wrapped::with_angle_brackets().wrap_fn(move |f| {
	real_params.clone().map(|g| print_generic_param_def(g, cx)).joined(", ", f)
	}))
	}
	.maybe_display()
	let mut real_params = self.params.iter().filter(|p| !p.is_synthetic_param()).peekable();
	Wrapped::with_angle_brackets()
	.when(real_params.peek().is_some())
	.wrap_fn(move |f| real_params.clone().map(|g| g.print(cx)).joined(", ", f))

WDYT?

[yotamofek]

I'll squash down commits once you're ok with the code to make it easier for you to review.

👍
Might take me a bit though :)

And having methods is better... until you have a lot of types implementing the same method and then you're completely lost when tracking down a bug. ^^'

Yeah I agree, at some point it's sorta like a poor-man's trait.

[yotamofek]

Nit - maybe?

Suggested change

	clean::GenericArg::Lifetime(lt) => print_lifetime(lt).fmt(f),
	clean::GenericArg::Lifetime(lt) => f.write_str(print_lifetime(lt)),

[yotamofek]

You've already made this change elsewhere

[yotamofek]

Nit (unrelated to PR):

Suggested change

	clean::GenericArg::Infer => Display::fmt("_", f),
	clean::GenericArg::Infer => f.write_char('_'),

[GuillaumeGomez]

Nice catch!

[yotamofek]

Okay, turned out to be a pretty easy review :)
r=me after addressing (or ignoring) nits, and squashing commits

[GuillaumeGomez]

Squashed commits and applied suggestions. Thanks for the review!

@bors r=yotamofek rollup

[bors]

📌 Commit 0f03429 has been approved by yotamofek

It is now in the queue for this repository.