Skip to content

false positive for unused_parens with binary op if rhs needs parens #110251

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

Closed
lukas-code opened this issue Apr 12, 2023 · 0 comments · Fixed by #110257
Closed

false positive for unused_parens with binary op if rhs needs parens #110251

lukas-code opened this issue Apr 12, 2023 · 0 comments · Fixed by #110257
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@lukas-code
Copy link
Member

I tried this very normal code: link

#![allow(unreachable_code)]

fn main() {
    if (() = return) {}
    if ((..{}) == ..{}) {}
}

The current output is the following, but removing any of the parens is an error:

   Compiling playground v0.0.1 (/playground)
warning: unnecessary parentheses around `if` condition
 --> src/main.rs:4:8
  |
4 |     if (() = return) {}
  |        ^           ^
  |
  = note: `#[warn(unused_parens)]` on by default
help: remove these parentheses
  |
4 -     if (() = return) {}
4 +     if () = return {}
  |

warning: unnecessary parentheses around `if` condition
 --> src/main.rs:5:8
  |
5 |     if ((..{}) == ..{}) {}
  |        ^              ^
  |
help: remove these parentheses
  |
5 -     if ((..{}) == ..{}) {}
5 +     if (..{}) == ..{} {}
  |

warning: `playground` (bin "playground") generated 2 warnings
    Finished dev [unoptimized + debuginfo] target(s) in 0.99s
     Running `target/debug/playground`

This also gets cargo fixed incorrectly.

This need to change to look at the rhs of binops:

|| (followed_by_block
&& match &inner.kind {
ExprKind::Ret(_)
| ExprKind::Break(..)
| ExprKind::Yield(..)
| ExprKind::Yeet(..) => true,
ExprKind::Range(_lhs, Some(rhs), _limits) => {
matches!(rhs.kind, ExprKind::Block(..))
}
_ => parser::contains_exterior_struct_lit(&inner),
})

@rustbot label T-compiler A-lint A-diagnostics

@rustbot rustbot added A-diagnostics Area: Messages for errors, warnings, and lints A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 12, 2023
@lukas-code lukas-code changed the title false positive for ununsed_parens with binary op if rhs needs parens false positive for unused_parens with binary op if rhs needs parens Apr 12, 2023
@notriddle notriddle added the D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. label Apr 12, 2023
@bors bors closed this as completed in 06d12f6 Apr 17, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants