rust-lang
WIP format literal arg inlining
A rough draft of #10739 implementation. The goal is to allow any kind of literal string arguments to be inlined automatically.
format!("hello {}", "world")
// is replaced with
format!("hello world")
changelog: [`uninlined_format_args`]: support for inlining string literals
I'd say this should be under a new lint name, although it's similar what it's doing is different enough to the existing uninlined_format_args
The existing implementation can also be used, it handles the various raw/non-raw permutations of the literal and format string as well as some other literals. You'd have to change the part where it checks the macro names before it's called + do a lint rename for both print_literal and write_literal to the new one
@Alexendoo after some more thinking, it seems separating it into a new lint would create more problems than solve:
- multiple lints operating on the same expression tend to cause issues, esp in the
--fixmode - a format expression with numeric indexes would be ignored by both lints unless used together:
format!("{0} {1}", var, "str")will be ignored by both theuninlined_format_argsand the string inlining lint, but when used together it would suggest theformat!("{var} str") - the original lint name
uninlined_format_argsfits perfectly for the string inlining as well, so wouldn't cause any confusion
So in short, we would not really gain anything besides problems from multiple lints... Not worth it IMO :)
I don't see what problem that would cause here, overlapping suggestions would cause rustfix ui tests to fail (cargo fix can handle them though) but the suggestions here wouldn't have overlapping spans
The main thing is so that you can set the lint levels independently, e.g. print_literal and write_literal are style but uninlined_format_args is pedantic. It wouldn't be unusual to want to disable one independently if they were in the same group
It would also make the lint documentation more confusing to have to explain the two different kinds of inlining in one lint, and how allow-mixed-uninlined-format-args would only apply to one of those kinds
format!("{0} {1}", var, "str") doesn't seem likely enough to occur to make up for that
@Alexendoo you do raise important config point. I think we can actually achieve both goals if we track the type of inlining needed -- e.g. if both are enabled, and both are needed (the "literal", var case above), we can show just one suggestion (under either of the names, or even add a note saying that both were involved). If only one is enabled, it won't even consider the other in the inlining.
To me that sounds too complicated/novel considering the problem is relatively minor
Agreed, let's not complicate the solution. As long as both lints don't step on each other's toes by issuing incompatible suggestions, having two lints that don't care about each other is fine.
:umbrella: The latest upstream changes (presumably #10358) made this pull request unmergeable. Please resolve the merge conflicts.
:umbrella: The latest upstream changes (presumably #12306) made this pull request unmergeable. Please resolve the merge conflicts.
Hey @nyurik, this is a ping from triage. Do you still plan to continue this PR? If you need help, you're always welcome to ask questions here or on Zulip :)
Hey this is triage, I'm closing this due to inactivity. If you want to continue this implementation, you're welcome to create a new PR. Thank you for the time, you already put into this!
Interested parties are welcome to pick this implementation up as well :)
@rustbot label +S-inactive-closed -S-waiting-on-author -S-waiting-on-review