Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New lint: suggest using "implicit named arguments" #8368

Closed
Tracked by #79
stanislav-tkach opened this issue Jan 29, 2022 · 7 comments · Fixed by #9233
Closed
Tracked by #79

New lint: suggest using "implicit named arguments" #8368

stanislav-tkach opened this issue Jan 29, 2022 · 7 comments · Fixed by #9233
Labels
A-lint Area: New lints

Comments

@stanislav-tkach
Copy link
Contributor

stanislav-tkach commented Jan 29, 2022

What it does

This lint suggests using the feature added in the 1.58 version of Rust: implicit named arguments for formatting macros.

Lint Name

implicit_named_arguments

Category

style

Advantage

  • Slightly less verbose code.
  • (Arguably) less error prone code.

Drawbacks

The only drawback that I can see is that not everyone would probably be happy with this lint, so perhaps it should be opt-in (pedantic).

Example

println!("1 = {}, 2 = {}, 3 = {}", first, second, third);

Could be written as:

println!("1 = {first}, 2 = {second}, 3 = {third}");
@stanislav-tkach stanislav-tkach added the A-lint Area: New lints label Jan 29, 2022
@stanislav-tkach
Copy link
Contributor Author

Perhaps this issue duplicated #633.

@edmorley
Copy link
Contributor

added in the 1.18 version of Rust

I think this should say 1.58, rather than 1.18:
https://github.com/rust-lang/rust/blob/master/RELEASES.md#version-1580-2022-01-13

@nyurik
Copy link
Contributor

nyurik commented Jul 18, 2022

Initially I made a suggestion for this in rust-lang/rustfmt#5450 , but it seems clippy would be a better target. I do think that this issue should be expanded to cover all cases listed below. My adjusted original post:

Since 1.58 Rust allows captured identifiers in format calls, i.e. println!("Hello, {person}!"); style strings (documentations). I would like to propose cargo clippy to automatically convert println!("Hello, {}!", person); and other format!-like calls to the inlined versions.

Original code After cargo clippy Notes
println!("{}", a) println!("{a}") positional in-lining
println!("{a}", a=b) println!("{b}") named param
println!("{a:0$}", width) println!("{a:width$}") width param
println!("{a:.0$}", prec) println!("{a:.prec$}") precision param

nyurik added a commit to nyurik/rust-clippy that referenced this issue Jul 21, 2022
Implements first non-working draft of rust-lang#8368
@nyurik
Copy link
Contributor

nyurik commented Jul 22, 2022

I began av implementation of this lint based on the PR #8518 by @Alexendoo. See first pass at nyurik@99f3352. So far I got it to identify most cases, but I am still struggling with the internal API -- for some reason the suggestions do not modify the source code. I am also not certain how to parse the structs in case the same value is used multiple times, i.e. ("{0} {0}", var) case. Any help and feedback is greatly appreciated :)

@Alexendoo
Copy link
Member

You could do the same as in check_literal, using a map to only consider arguments that are used once

fn check_literal(cx: &LateContext<'_>, format_args: &FormatArgsExpn<'_>, args: &[FormatArgsArg<'_>], name: &str) {
let mut counts = HirIdMap::<usize>::default();
for arg in args {
*counts.entry(arg.value.hir_id).or_default() += 1;
}
for arg in args {
if counts[&arg.value.hir_id] == 1

@nyurik
Copy link
Contributor

nyurik commented Jul 24, 2022

I added some more implementation code, and some of it even works (kinda). Any feedback is welcome - probably best to do it on the #9233 PR. Thanks!

nyurik added a commit to nyurik/rust-clippy that referenced this issue Sep 14, 2022
Implement rust-lang#8368 - a new lint to inline format arguments such as `print!("{}", var)` into `print!("{var}")`.

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

code | suggestion | comment
---|---|---
`print!("{0}={1}", var, 1+2)` | `print!("{var}={0}", 1+2)` | Format string uses an indexed argument that cannot be inlined.  Supporting this case requires re-indexing of the format string.

changelog: [`inline-format-args`]: A new lint to inline format arguments, i.e. `print!("{}", var)` into `print!("{var}")`
nyurik added a commit to nyurik/rust-clippy that referenced this issue Sep 14, 2022
Implement rust-lang#8368 - a new lint to inline format arguments such as `print!("{}", var)` into `print!("{var}")`.

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

code | suggestion | comment
---|---|---
`print!("{0}={1}", var, 1+2)` | `print!("{var}={0}", 1+2)` | Format string uses an indexed argument that cannot be inlined.  Supporting this case requires re-indexing of the format string.

changelog: [`inline-format-args`]: A new lint to inline format arguments, i.e. `print!("{}", var)` into `print!("{var}")`
nyurik added a commit to nyurik/rust-clippy that referenced this issue Sep 15, 2022
Implement rust-lang#8368 - a new lint to inline format arguments such as `print!("{}", var)` into `print!("{var}")`.

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

code | suggestion | comment
---|---|---
`print!("{0}={1}", var, 1+2)` | `print!("{var}={0}", 1+2)` | Format string uses an indexed argument that cannot be inlined.  Supporting this case requires re-indexing of the format string.

changelog: [`inline-format-args`]: A new lint to inline format arguments, i.e. `print!("{}", var)` into `print!("{var}")`
nyurik added a commit to nyurik/rust-clippy that referenced this issue Sep 15, 2022
Implement rust-lang#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

### Unsupported cases

code | suggestion | comment
---|---|---
`print!("{0}={1}", var, 1+2)` | `print!("{var}={0}", 1+2)` | Format
string uses an indexed argument that cannot be inlined.  Supporting this
case requires re-indexing of the format string.

changelog: [`needless-format-args`]: A new lint to inline format
arguments, i.e. `print!("{}", var)` into `print!("{var}")`
nyurik added a commit to nyurik/rust-clippy that referenced this issue Sep 23, 2022
Implement rust-lang#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

### Unsupported cases

code | suggestion | comment
---|---|---
`print!("{0}={1}", var, 1+2)` | `print!("{var}={0}", 1+2)` | Format
string uses an indexed argument that cannot be inlined.  Supporting this
case requires re-indexing of the format string.

changelog: [`needless-format-args`]: A new lint to inline format
arguments, i.e. `print!("{}", var)` into `print!("{var}")`
nyurik added a commit to nyurik/rust-clippy that referenced this issue Sep 23, 2022
Implement rust-lang#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

### Unsupported cases

code | suggestion | comment
---|---|---
`print!("{0}={1}", var, 1+2)` | `print!("{var}={0}", 1+2)` | Format
string uses an indexed argument that cannot be inlined.  Supporting this
case requires re-indexing of the format string.

changelog: [`needless-format-args`]: A new lint to inline format
arguments, i.e. `print!("{}", var)` into `print!("{var}")`
nyurik added a commit to nyurik/rust-clippy that referenced this issue Sep 24, 2022
Implement rust-lang#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

### Unsupported cases

code | suggestion | comment
---|---|---
`print!("{0}={1}", var, 1+2)` | `print!("{var}={0}", 1+2)` | Format
string uses an indexed argument that cannot be inlined.  Supporting this
case requires re-indexing of the format string.

changelog: [`needless-format-args`]: A new lint to inline format
arguments, i.e. `print!("{}", var)` into `print!("{var}")`
nyurik added a commit to nyurik/rust-clippy that referenced this issue Sep 24, 2022
Implement rust-lang#8368 - a new
lint to inline format arguments such as `print!("{}", var)` into
`print!("{var}")`.

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

code | suggestion | comment
---|---|---
`print!("{0}={1}", var, 1+2)` | `print!("{var}={0}", 1+2)` | Format
string uses an indexed argument that cannot be inlined.  Supporting this
case requires re-indexing of the format string.

changelog: [`uninlined_format_args`]: A new lint to inline format
arguments, i.e. `print!("{}", var)` into `print!("{var}")`
nyurik added a commit to nyurik/rust-clippy that referenced this issue Sep 25, 2022
Implement rust-lang#8368 - a new
lint to inline format arguments such as `print!("{}", var)` into
`print!("{var}")`.

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

code | suggestion | comment
---|---|---
`print!("{0}={1}", var, 1+2)` | `print!("{var}={0}", 1+2)` | Format
string uses an indexed argument that cannot be inlined.  Supporting this
case requires re-indexing of the format string.

changelog: [`uninlined_format_args`]: A new lint to inline format
arguments, i.e. `print!("{}", var)` into `print!("{var}")`
bors added a commit that referenced this issue Sep 26, 2022
new uninlined_format_args lint to inline explicit arguments

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:
```rust
# 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}")`
@nyurik
Copy link
Contributor

nyurik commented Oct 13, 2022

Forgot to comment on this one... DONE! :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants