Add new literal_string_with_formatting_arg lint

[GuillaumeGomez]

Fixes #10195.

changelog: Add new literal_string_with_formatting_arg lint

[rustbot]

r? @Jarcho

rustbot has assigned @Jarcho.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[GuillaumeGomez]

I could emit a suggestion for both cases by wrapping the literal into &format!(). Not sure if it's a good idea since it could be wrong in a lot of cases so for now I didn't do it.

[matthiaskrgr]

Would like to see a test for malformed fmt attemps such as {y:?}} or "{y:?} y:?}" to make sure we don't lint there by accident

[emilk]

Suggested change

	println!("{y:?}");
	println!("{y:?}");
	let code = "fn main {\n\
	}";

[emilk]

I did a little bit of testing, and it seems to work like it should! ❤️ A few more unit tests would be nice though

[GuillaumeGomez]

I just found a much better idea: using the rustc format parser directly. :3

[GuillaumeGomez]

So it now uses the rustc format parser, making things much better overall: no more regex, less false positive, simpler code.

I also added all suggested test cases and some more.

[y21]

What does the lint do for the empty JSON object {} which is also a valid formatting specifier? Would probably be a good to have as a negative test too.

[emilk]

Maybe a less strong wording would be good, e.g. "this looks like a formatting argument…" ? Just in case of false positives

[GuillaumeGomez]

What does the lint do for the empty JSON object {} which is also a valid formatting specifier? Would probably be a good to have as a negative test too.

It lints (as expected 😄). There was a test for that with other characters but created a new one with just {} just in case.

Maybe a less strong wording would be good, e.g. "this looks like a formatting argument…" ? Just in case of false positives

Good idea! I changed the wording.

[y21]

It lints (as expected 😄).

Realistically speaking, I feel like {} would run into more false positives than true positives if we decide to lint that.

{foo}/{foo:?} etc. all make sense since they can work with format! alone (that is, format!("{foo}") works), but you can't make that argument for just {}. Not only is it missing the format macro, but it's also missing a formatting argument.

If someone writes let foo = "{}";, it seems fairly unlikely to me that they actually meant to use a formatting macro there, because there is nothing to format.

[GuillaumeGomez]

Good point. Should I only lint against non-empty {} like {y}/{:?}/{y:?} and eventually we can add an option to look for everything?

[GuillaumeGomez]

I removed the check for {} for the time being.

[bors]

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

[GuillaumeGomez]

Fixed merge conflict.