more clippy fixes

[matthiaskrgr]

* remove redundant returns (clippy::needless_return)
* remove redundant import (clippy::single_component_path_imports)
* remove redundant format!() call (clippy::useless_format)
* don't use ok() before calling expect() (clippy::ok_expect)

[ecstatic-morse]

Most of this is a clear improvement IMO. However, I don't like eliding lifetimes everywhere possible, since some lifetime names ('tcx) have a specific meaning in rustc.

@matthiaskrgr I assume that your ultimate goal is to enable clippy as part of x.py fmt, which I am generally supportive of. However, I think the needless_lifetimes lint in specific may not be a good fit for the compiler. Obviously this is subjective. Perhaps enabling and configuring clippy should go through the major change process so people can weigh in? In any case, I'll defer to someone with more seniority.

[ecstatic-morse]

r? @Centril

[Dylan-DPC-zz]

@bors r+

[bors]

📌 Commit 85e608c503a8abf7fdb1c4613f000409354570a1 has been approved by Dylan-DPC

[Mark-Simulacrum]

@bors r-

I don't think eliding 'tcx is a good idea for now, or at least we should have a broader discussion.

[matthiaskrgr]

I removed the lifetime elision commit.

[Mark-Simulacrum]

Thanks!

@bors r+ rollup

[bors]

📌 Commit ad00e91 has been approved by Mark-Simulacrum

[matthiaskrgr]

@ecstatic-morse I personally don't think clippy is ready to be part of x.py fmt or something like that.
There are a lot of false positives (also involving #[cfg(..)], and some lints just suggest "hey, you should check that out just in case" and do not provides an actual clue on what to change.
I don't think scattering #![allow(clippy::...)] around the codebase is something people would want.
When I check rustc with clippy, I actually explicitly disable around 70 lints before having a detailed look at the findings.

I'm sifting through the clippy logs, can report false positives to clippy and fix the true positives in the codebase so I guess it's a win win.

[Centril]

For what it's worth, I would have r+ed this PR as well with the lifetime elision. It just seems like standard practice, and I don't think there's anything special about the compiler as compared to other codebases here. The fact that there might be arena allocation involved can often be inferred by e.g. Ty<'_> alone, e.g. you know it because that type is allocated on an arena. Also, ask yourself... is the fact that something is arena allocated important semantic information as compared to e.g., the type checking logic? Having 'tcx just adds unnecessary noise. Similarly, for new code, I don't think we should e.g., reject code with explicit named lifetimes, and so it seems strange that we would reject it when old code is improved too.

In some cases however, e.g., involving unsafe code in the standard library, or unsafe code in general, we should be more careful wrt. lifetime elision. But most of the time, there's no unsafe code in sight.

r? @Mark-Simulacrum