Skip to content

Diagnostic width span is not added when '0$' is used as width in format strings #99480

New issue

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

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

Already on GitHub? # to your account

Merged
merged 2 commits into from
Jul 20, 2022
Merged

Diagnostic width span is not added when '0$' is used as width in format strings #99480

merged 2 commits into from
Jul 20, 2022

Conversation

miam-miam
Copy link
Contributor

@miam-miam miam-miam commented Jul 19, 2022

When the following code is run rustc does not add diagnostic spans for the width argument. Such spans are necessary for a clippy lint that I am currently writing.

println!("Hello {1:0$}!", 5, "x");
//                 ^^
// Should have a span here

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 19, 2022
@rust-highfive
Copy link
Contributor

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

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 19, 2022
@miam-miam miam-miam changed the title Added diagnostic width span when when '0$' is used as width Diagnostic width span is not added when '0$' is used as width in format strings Jul 19, 2022
@miam-miam miam-miam marked this pull request as ready for review July 19, 2022 22:04
@oli-obk
Copy link
Contributor

oli-obk commented Jul 20, 2022

Is it possible to trigger any rustc error/lint that will show this change?

@miam-miam
Copy link
Contributor Author

I added a test which wouldn't have previously passed if that's what you mean?

@oli-obk
Copy link
Contributor

oli-obk commented Jul 20, 2022

Yea I saw that ^^ I was wondering about a user-visible change. It's not really important, just wondering

@miam-miam
Copy link
Contributor Author

miam-miam commented Jul 20, 2022

Yes there is!

fn main() {
    let test = 5;
    println!("Hello {test:1$}!");
}

Gives

error: invalid reference to positional argument 1 (no arguments were given)
 --> src/main.rs:3:21
  |
3 |     println!("Hello {test:1$}!");
  |                     ^^^^^^--^
  |                           |
  |                           this width flag expects an `usize` argument at position 1, but no arguments were given
  |
  = note: positional arguments are zero-based
  = note: for information about formatting flags, visit https://doc.rust-lang.org/std/fmt/index.html

But

fn main() {
  let test = 5;
  println!("Hello {test:0$}!");
}

Only gives this

error: invalid reference to positional argument 0 (no arguments were given)
 --> src/main.rs:3:21
  |
3 |     println!("Hello {test:0$}!");
  |                     ^^^^^^^^^
  |
  = note: positional arguments are zero-based

error: could not compile `playground` due to previous error

@oli-obk
Copy link
Contributor

oli-obk commented Jul 20, 2022

cool! Please add such a test to the src/test/ui test suite. There are existing tests around wrong format strings, so you can just add your snippet to one of those existing tests and rebless that test with ./x.py test src/test/ui --bless --test-arg name_of_the_test

@oli-obk
Copy link
Contributor

oli-obk commented Jul 20, 2022

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 20, 2022

📌 Commit f8dfc4b has been approved by oli-obk

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 20, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Jul 20, 2022

@bors rollup

bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 20, 2022
…askrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#99212 (introduce `implied_by` in `#[unstable]` attribute)
 - rust-lang#99352 (Use `typeck_results` to avoid duplicate `ast_ty_to_ty` call)
 - rust-lang#99355 (better error for bad depth parameter on macro metavar expr)
 - rust-lang#99480 (Diagnostic width span is not added when '0$' is used as width in format strings)
 - rust-lang#99488 (compiletest: Allow using revisions with debuginfo tests.)
 - rust-lang#99489 (rustdoc UI fixes)
 - rust-lang#99508 (Avoid `Symbol` to `String` conversions)
 - rust-lang#99510 (adapt assembly/static-relocation-model test for LLVM change)
 - rust-lang#99516 (Use new tracking issue for proc_macro::tracked_*.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 9e197b7 into rust-lang:master Jul 20, 2022
@rustbot rustbot added this to the 1.64.0 milestone Jul 20, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants