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

single-match lint expands println! macro in suggestion #2455

Closed
kimsnj opened this issue Feb 12, 2018 · 2 comments
Closed

single-match lint expands println! macro in suggestion #2455

kimsnj opened this issue Feb 12, 2018 · 2 comments
Assignees
Labels
C-bug Category: Clippy is not doing the correct thing T-macros Type: Issues with macros and macro expansion

Comments

@kimsnj
Copy link
Contributor

kimsnj commented Feb 12, 2018

The example in the README file:

    let x = Some(1u8);
    match x {
        Some(y) => println!("{:?}", y),
        _ => ()
    }

produces this suggestions:

help: try this: `if let Some(y) = x { $ crate :: io :: _print ( format_args ! ( $ ( $ arg ) * ) ) }`

Using current master (commit: 6b3487a)
And rustc 1.25.0-nightly (b8398d9 2018-02-11)

Note putting the statement inside a block { println!("{:?}", y) } works properly (and is the current case covered by ui tests).

@oli-obk oli-obk added the C-bug Category: Clippy is not doing the correct thing label Feb 12, 2018
@kimsnj
Copy link
Contributor Author

kimsnj commented Feb 12, 2018

The issue seems to be more general than just the single arm lint. for example:

4 | /     match x {
5 | |         true => {println!("yay")},
6 | |         false => println!("nay")
7 | |     }
  | |_____^ help: consider using an if/else expression: `if x {println!("yay")} else { $ crate :: io :: _print ( format_args ! ( $ ( $ arg ) * ) ) }`

The difference in behaviour between the two arms can be observed by directly calling utils::snippet with either arms[0].body.span or arms[1].body.span.
This calls a method on LintContext session…

Can this be fixed on clippy side or should it be on rustc ?
I am not quite sure where to go from here.

@phansch phansch added the T-macros Type: Issues with macros and macro expansion label Apr 4, 2018
@oli-obk
Copy link
Contributor

oli-obk commented Apr 24, 2018

We should be checking the span for macro expansions and not produce a snippet in that case (or change it to approximate_suggestion and insert a placeholder

@phansch phansch self-assigned this Oct 6, 2018
bors bot added a commit that referenced this issue Oct 27, 2018
3366: Don't expand macros in some suggestions r=oli-obk a=phansch

Fixes #1148 
Fixes #1628
Fixes #2455
Fixes #3023
Fixes #3333
Fixes #3360

Co-authored-by: Philipp Hansch <dev@phansch.net>
bors bot added a commit that referenced this issue Oct 28, 2018
3217: Fix string_lit_as_bytes lint for macros r=phansch a=yaahallo

Prior to this change, string_lit_as_bytes would trigger for constructs
like `include_str!("filename").as_bytes()` and would recommend fixing it
by rewriting as `binclude_str!("filename")`.

This change updates the lint to act as an EarlyLintPass lint. It then
differentiates between string literals and macros that have bytes
yielding alternatives.

Closes #3205

3366: Don't expand macros in some suggestions r=oli-obk a=phansch

Fixes #1148 
Fixes #1628
Fixes #2455
Fixes #3023
Fixes #3333
Fixes #3360

Co-authored-by: Jane Lusby <jlusby42@gmail.com>
Co-authored-by: Philipp Hansch <dev@phansch.net>
@bors bors bot closed this as completed in #3366 Oct 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing T-macros Type: Issues with macros and macro expansion
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants