Skip to content

Detect likely attempts to use interpolation in string literals #100584

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

Closed
estebank opened this issue Aug 15, 2022 · 6 comments
Closed

Detect likely attempts to use interpolation in string literals #100584

estebank opened this issue Aug 15, 2022 · 6 comments
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-papercut Diagnostics: An error or lint that needs small tweaks. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@estebank
Copy link
Contributor

estebank commented Aug 15, 2022

Given

fn foo(x: &str) {
    let _ = "{x}";
}

we currently emit a warning about x being unused.

We should additionally point at the x inside the string literal and mention that if we wanted string interpolation, it's not gonna work because it needs to be inside of a format!() invocation.

https://www.reddit.com/r/rust/comments/wo3id0/media_noob_question_why_are_my_parameters_unused/

@estebank estebank added A-diagnostics Area: Messages for errors, warnings, and lints P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-papercut Diagnostics: An error or lint that needs small tweaks. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. labels Aug 15, 2022
@lyming2007
Copy link

Do you mean, we should emit like this?

warning: unused variable: `x`
 --> main.rs:1:8
  |
1 | fn foo(x: &str) {
  |        let _ = "{x}";
  |                     ======= x needs to be inside of a format!() invocation
  |
  = note: `#[warn(unused_variables)]` on by default

@estebank
Copy link
Contributor Author

Ideally, it would look something like this:

warning: unused variable: `x`
 --> main.rs:1:8
  |
1 | fn foo(x: &str) {
  |        ^ unused variable
2 |        let _ = "{x}";
  |                  - you might have meant to use string interpolation in this string literal
help: string interpolation only works in `format!` invocations
  |
2 |        let _ = format!("{x}");
  |                ++++++++     +
  |
  = note: `#[warn(unused_variables)]` on by default

@lyming2007
Copy link

@rustbot claim

@lyming2007
Copy link

cat main.rs
fn foo(xyza: &str) {
    let _ = "{xyza}";
}


fn main() {
  foo("aaa");
}

Current output is:

warning: unused variable: `xyza`
 --> main.rs:1:8
  |
1 | fn foo(xyza: &str) {
  |        ^^^^ help: if this is intentional, prefix it with an underscore: `_xyza`
  |
  = note: `#[warn(unused_variables)]` on by default

warning: 1 warning emitted

With my fix https://github.com/lyming2007/rust/pull/new/issue-100584

The output looks like:

warning: unused variable: `xyza`
 --> main.rs:1:8
  |
1 | fn foo(xyza: &str) {
  |        ^^^^
  |        |
  |        unused variable
  |        help: if this is intentional, prefix it with an underscore: `_xyza`
2 |     let _ = "{xyza}";
  |             -------- you might have meant to use string interpolation in this string literal
  |
  = note: `#[warn(unused_variables)]` on by default
  = help: string interpolation only works in `format!` invocations

warning: 1 warning emitted

There are still 2 issues for me.
#1 I am not able to emit something like

 |
2 |        let _ = format!("{x}");
  |                ++++++++     +
  |

please give me an example, then I can look how the compiler deal with it.

#2 I can't upload a specific test case for this issue, because the test script will opt out the unused variable. Here is the script I got:

/home/lym/rust3/rust/build/x86_64-unknown-linux-gnu/stage1/bin/rustc /home/lym/rust3/rust/src/test/ui/type/issue-100584.rs -Zthreads=1 --target=x86_64-unknown-linux-gnu --error-format json --json future-incompat -Ccodegen-units=1 -Zui-testing -Zdeduplicate-diagnostics=no -Cstrip=debuginfo --emit metadata -C prefer-dynamic --out-dir /home/lym/rust3/rust/build/x86_64-unknown-linux-gnu/test/ui/type/issue-100584 -A unused -Crpath -Cdebuginfo=0 -Lnative=/home/lym/rust3/rust/build/x86_64-unknown-linux-gnu/native/rust-test-helpers -L /home/lym/rust3/rust/build/x86_64-unknown-linux-gnu/test/ui/type/issue-100584/auxiliary

So it will always output nothing to stderr and make the test failed.
@estebank please advise

@estebank
Copy link
Contributor Author

You'd have to use:

err.multipart_suggestion(
    vec![
        (lit_expr.span.shrink_to_lo(), "format!(".to_string()),
        (lit_expr.span.shrink_to_hi(), ")".to_string()),
    ],
    "if you meant to use string interpolation in the string literal, use the `format!` marco",
    Applicability::MachineApplicable,
);

Have you run python3 x.py test src/test/ui/type/issue-100584.rs --bless?

@lyming2007
Copy link

thanks. please see my fix:
#100941
@estebank

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-papercut Diagnostics: An error or lint that needs small tweaks. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

2 participants