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

format! allows named arguments to be referenced by position #69110

Closed
Amanieu opened this issue Feb 12, 2020 · 5 comments
Closed

format! allows named arguments to be referenced by position #69110

Amanieu opened this issue Feb 12, 2020 · 5 comments
Labels
C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@Amanieu
Copy link
Member

Amanieu commented Feb 12, 2020

The following example compiles, but shouldn't.

println!("{0} {foo}", foo = 2);

This seems to be an unintentional regression that was introduced somewhere between 1.11.0 and 1.12.0, most likely by #33642.

However, this code has been accepted on stable for multiple years now (1.12 was in 2016), so fixing this might break existing code. We could still change this back to an error if crater reports no regressions since it is reasonable to say that the code in question is clearly incorrect and was not caught by the compiler by mistake.

@jonas-schievink jonas-schievink added C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Feb 12, 2020
@estebank
Copy link
Contributor

This should be reintroduced as a warn by default lint, regardless of crater results. We know this is relatively easy to stumble and rely on and rust is no longer close to 100% open source. Furthermore this is relatively benign mistake, a warning seems to me more than enough.

@ollie27
Copy link
Member

ollie27 commented Feb 13, 2020

This is pretty much a duplicate of #45256.

@pnkfelix
Copy link
Member

This is pretty much a duplicate of #45256.

(yeah but clearly no one acted on it back then, since even then the official response was "oh, we should lint against this.")

@Nemo157
Copy link
Member

Nemo157 commented Jan 14, 2022

This has also leaked into the captured formatting parameter arguments in some situations (I could not come up with a situation where capturing a value parameter would do something similar):

fn main() {
    let a = 5;
    println!("{:-<a$}");
}

prints

5----

@Enselic
Copy link
Member

Enselic commented Jan 6, 2024

Triage: #45256 has been closed as fixed, and the above code gives this warning now:

warning: named argument `foo` is not used by name
 --> src/main.rs:2:27
  |
2 |     println!("{0} {foo}", foo = 2);
  |               ---         ^^^ this named argument is referred to by position in formatting string
  |               |
  |               this formatting argument uses named argument `foo` by position
  |
  = note: `#[warn(named_arguments_used_positionally)]` on by default
help: use the named argument by name to avoid ambiguity
  |
2 |     println!("{foo} {foo}", foo = 2);
  |                ~~~

So we can close this issue too now.

@Enselic Enselic closed this as completed Jan 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants