Skip to content

remove false positives of unused_braces #70789

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

Merged
merged 1 commit into from
Apr 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions src/librustc_lint/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,19 @@ impl From<UnusedDelimsCtx> for &'static str {
trait UnusedDelimLint {
const DELIM_STR: &'static str;

/// Due to `ref` pattern, there can be a difference between using
/// `{ expr }` and `expr` in pattern-matching contexts. This means
/// that we should only lint `unused_parens` and not `unused_braces`
/// in this case.
///
/// ```rust
/// let mut a = 7;
/// let ref b = { a }; // We actually borrow a copy of `a` here.
/// a += 1; // By mutating `a` we invalidate any borrows of `a`.
/// assert_eq!(b + 1, a); // `b` does not borrow `a`, so we can still use it here.
/// ```
const LINT_EXPR_IN_PATTERN_MATCHING_CTX: bool;

// this cannot be a constant is it refers to a static.
fn lint(&self) -> &'static Lint;

Expand Down Expand Up @@ -454,7 +467,10 @@ trait UnusedDelimLint {
fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) {
use rustc_ast::ast::ExprKind::*;
let (value, ctx, followed_by_block, left_pos, right_pos) = match e.kind {
If(ref cond, ref block, ..) => {
// Do not lint `unused_braces` in `if let` expressions.
If(ref cond, ref block, ..)
if !matches!(cond.kind, Let(_, _)) || Self::LINT_EXPR_IN_PATTERN_MATCHING_CTX =>
{
let left = e.span.lo() + rustc_span::BytePos(2);
let right = block.span.lo();
(cond, UnusedDelimsCtx::IfCond, true, Some(left), Some(right))
Expand All @@ -470,7 +486,7 @@ trait UnusedDelimLint {
(cond, UnusedDelimsCtx::ForIterExpr, true, None, Some(block.span.lo()))
}

Match(ref head, _) => {
Match(ref head, _) if Self::LINT_EXPR_IN_PATTERN_MATCHING_CTX => {
let left = e.span.lo() + rustc_span::BytePos(5);
(head, UnusedDelimsCtx::MatchScrutineeExpr, true, Some(left), None)
}
Expand Down Expand Up @@ -512,7 +528,7 @@ trait UnusedDelimLint {

fn check_stmt(&mut self, cx: &EarlyContext<'_>, s: &ast::Stmt) {
match s.kind {
StmtKind::Local(ref local) => {
StmtKind::Local(ref local) if Self::LINT_EXPR_IN_PATTERN_MATCHING_CTX => {
if let Some(ref value) = local.init {
self.check_unused_delims_expr(
cx,
Expand Down Expand Up @@ -565,6 +581,8 @@ declare_lint_pass!(UnusedParens => [UNUSED_PARENS]);
impl UnusedDelimLint for UnusedParens {
const DELIM_STR: &'static str = "parentheses";

const LINT_EXPR_IN_PATTERN_MATCHING_CTX: bool = true;

fn lint(&self) -> &'static Lint {
UNUSED_PARENS
}
Expand Down Expand Up @@ -736,6 +754,8 @@ declare_lint_pass!(UnusedBraces => [UNUSED_BRACES]);
impl UnusedDelimLint for UnusedBraces {
const DELIM_STR: &'static str = "braces";

const LINT_EXPR_IN_PATTERN_MATCHING_CTX: bool = false;

fn lint(&self) -> &'static Lint {
UNUSED_BRACES
}
Expand Down
31 changes: 25 additions & 6 deletions src/test/ui/lint/unused_braces.rs
Original file line number Diff line number Diff line change
@@ -1,29 +1,48 @@
// check-pass
#![warn(unused_braces, unused_parens)]

fn consume<T>(_: T) {}

fn main() {
let _ = (7);
//~^WARN unnecessary parentheses

let _ = { 7 };
//~^ WARN unnecessary braces
// Do not emit a lint in these cases,
// as we have to be careful with
// `ref` patterns.
{
let _ = { 7 };

if let 7 = { 7 } { }

match { 7 } {
_ => (),
}
}

if let 7 = { 7 } {
if { true } {
//~^ WARN unnecessary braces
}

while { false } {
//~^ WARN unnecessary braces
}

let _: [u8; { 3 }];
//~^ WARN unnecessary braces

// do not emit error for multiline blocks.
consume({ 7 });
//~^ WARN unnecessary braces

// Do not emit lint for multiline blocks.
let _ = {
7
};

// do not emit error for unsafe blocks.
// Do not emit lint for unsafe blocks.
let _ = unsafe { 7 };

// do not emit error, as the `{` would then
// Do not emit lint, as the `{` would then
// be parsed as part of the `return`.
if { return } {

Expand Down
26 changes: 16 additions & 10 deletions src/test/ui/lint/unused_braces.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
warning: unnecessary parentheses around assigned value
--> $DIR/unused_braces.rs:5:13
--> $DIR/unused_braces.rs:7:13
|
LL | let _ = (7);
| ^^^ help: remove these parentheses
Expand All @@ -10,27 +10,33 @@ note: the lint level is defined here
LL | #![warn(unused_braces, unused_parens)]
| ^^^^^^^^^^^^^

warning: unnecessary braces around assigned value
--> $DIR/unused_braces.rs:8:13
warning: unnecessary braces around `if` condition
--> $DIR/unused_braces.rs:23:8
|
LL | let _ = { 7 };
| ^^^^^ help: remove these braces
LL | if { true } {
| ^^^^^^^^ help: remove these braces
|
note: the lint level is defined here
--> $DIR/unused_braces.rs:2:9
|
LL | #![warn(unused_braces, unused_parens)]
| ^^^^^^^^^^^^^

warning: unnecessary braces around `let` scrutinee expression
--> $DIR/unused_braces.rs:11:16
warning: unnecessary braces around `while` condition
--> $DIR/unused_braces.rs:27:11
|
LL | if let 7 = { 7 } {
| ^^^^^ help: remove these braces
LL | while { false } {
| ^^^^^^^^^ help: remove these braces

warning: unnecessary braces around const expression
--> $DIR/unused_braces.rs:15:17
--> $DIR/unused_braces.rs:31:17
|
LL | let _: [u8; { 3 }];
| ^^^^^ help: remove these braces

warning: unnecessary braces around function argument
--> $DIR/unused_braces.rs:34:13
|
LL | consume({ 7 });
| ^^^^^ help: remove these braces

Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@ struct A {
b: u32,
}

fn consume<T>(_: T) {}

fn main() {
let a = A {
a: 42,
b: 1729,
};

let _ = &{ a.b };
let _ = { a.b };
consume(&{ a.b });
consume({ a.b });
//~^ WARN unnecessary braces
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
warning: unnecessary braces around assigned value
--> $DIR/unused_parens_borrow.rs:20:13
warning: unnecessary braces around function argument
--> $DIR/unused_braces_borrow.rs:22:13
|
LL | let _ = { a.b };
LL | consume({ a.b });
| ^^^^^^^ help: remove these braces
|
note: the lint level is defined here
--> $DIR/unused_parens_borrow.rs:2:9
--> $DIR/unused_braces_borrow.rs:2:9
|
LL | #![warn(unused_braces)]
| ^^^^^^^^^^^^^
Expand Down