new uninlined_format_args lint to inline explicit arguments

[nyurik]

Implement #8368 - a new lint to inline format arguments such as print!("{}", var) into print!("{var}").

Supported cases

code	suggestion	comment
print!("{}", var)	print!("{var}")	simple variables
print!("{0}", var)	print!("{var}")	positional variables
print!("{v}", v=var)	print!("{var}")	named variables
print!("{0} {0}", var)	print!("{var} {var}")	aliased variables
print!("{0:1$}", var, width)	print!("{var:width$}")	width support
print!("{0:.1$}", var, prec)	print!("{var:.prec$}")	precision support
print!("{:.*}", prec, var)	print!("{var:.prec$}")	asterisk support

Known Problems

• There may be a false positive if the format string is wrapped in a macro call:

# let var = 42;
macro_rules! no_param_str { () => { "{}" }; }
macro_rules! pass_through { ($expr:expr) => { $expr }; }
println!(no_param_str!(), var);
println!(pass_through!("{}"), var);

• Format string uses an indexed argument that cannot be inlined.
  Supporting this case requires re-indexing of the format string.
  Until implemented, print!("{0}={1}", var, 1+2) should be changed to print!("{var}={0}", 1+2) by hand.

changelog: [uninlined_format_args]: A new lint to inline format arguments, i.e. print!("{}", var) into print!("{var}")

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @xFrednet (or someone else) soon.

Please see the contribution instructions for more information.

[Alexendoo]

👍

I had a look through most of the TODOs. Also worth mentioning you can use let chains rather than if_chain, it plays better with rust_analyzer (though also doesn't get automatically formatted yet)

[nyurik]

Thanks @Alexendoo for the review! I made a number of changes based on that, but still a few todos. My biggest concern is how to detect if an argument is also used as precision and/or width. I know some format-related work is being done in the rustc, and there seem to be at least 3 devs involved, so I wonder who/how can coordinate these efforts to be consistent and reuse each other's work.

[est31]

If it's not in your TODOs yet, the lint should be integrated into the MSRV system of clippy and be gated on 1.58.0, as that's when format capturing was added.

[bors]

☔ The latest upstream changes (presumably #9243) made this pull request unmergeable. Please resolve the merge conflicts.

[nyurik]

@est31 thx, fixed. I think I am done with this lint -- it now solves the majority of cases, and it avoids expanding width/precision arguments by checking if the argument span contains a $ symbol. Not ideal, but much better than nothing.

[xFrednet]

@Alexendoo Do you want to take over the full review? I believe you know the format macro better than me. If you don't have the time, feel free to give it back. I should have more time for Clippy again 🙃

[Alexendoo]

Sure! Sounds good to me. I'll take a full look at it once it's unblocked

[xFrednet]

Awesome, thank you!

r? @Alexendoo

[nyurik]

Thx @Alexendoo ! Is there something i can do to help the #8518 ? Are there any fundamental blocks on that, or is it juts a matter of patiently waiting? :)

[Alexendoo]

Yeah just a matter of waiting for the large amount of reviewer bandwidth needed

[bors]

☔ The latest upstream changes (presumably #9264) made this pull request unmergeable. Please resolve the merge conflicts.

[flip1995]

Thanks for this PR! I learned something about the format macro and what is possible with it.

The impl LGTM overall, but some housekeeping, like documentation, has to be done still.

[nyurik]

@flip1995 thanks for the review! I addressed some of your points, and made a few clarifications. I am still in a bit of a limbo state - waiting for the upstream #8518 conflicts to be resolved, plus now there is #9349 - so I am not certain at which point I should start resolving conflicts and getting it back to the "ready to merge" state again.

[flip1995]

No worries. Let's get the other PRs merged first. You can work on this as you see fit. But also feel free to just let this PR sit as-is until we resolved the other issues.

[Alexendoo]

Personally I'm inclined to leave it as a known issue for now. It's not limited to just this lint, any other lint working with string literals may have a problem with this style of proc macro, print_literal is affected at least

I plan to replace expand_past_previous_comma with something more robust, I can probably catch this issue at the same time with that

[nyurik]

@Alexendoo ok, thx, I squashed and cleaned up the PR - should be in a good state now. I also added a few more unit tests that would also fail until your changes.

[Alexendoo]

Looks good! Just some documentation nits and then it's good for merging

Thank you for the endurance 😄

[Alexendoo]

Good ol' update_lints :D

Could give it one final squash too

[nyurik]

Bummer, I can't, I'm at a playground))))

[nyurik]

I'll do it in a bit. Thanks for all the help!!!

[nyurik]

@Alexendoo ok, I think it's finally ready 😀

[Alexendoo]

Awesome, thanks @nyurik!

@bors r+

[bors]

📌 Commit 5a71bbd has been approved by Alexendoo

It is now in the queue for this repository.

[bors]

⌛ Testing commit 5a71bbd with merge cf93865...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Alexendoo
Pushing cf93865 to master...