From 62835c953190e7c6cadac1557156a47753a2a913 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 31 Aug 2023 19:11:58 -0700 Subject: [PATCH] diagnostics: avoid wrong `unused_parens` on `x as (T) < y` --- compiler/rustc_lint/src/early.rs | 1 + compiler/rustc_lint/src/passes.rs | 1 + compiler/rustc_lint/src/unused.rs | 29 +++- .../lint/unused/unused-parens-issue-106413.rs | 32 +++++ .../unused/unused-parens-issue-106413.stderr | 127 ++++++++++++++++++ 5 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 tests/ui/lint/unused/unused-parens-issue-106413.rs create mode 100644 tests/ui/lint/unused/unused-parens-issue-106413.stderr diff --git a/compiler/rustc_lint/src/early.rs b/compiler/rustc_lint/src/early.rs index 211ea8f4347..d102e3a6c15 100644 --- a/compiler/rustc_lint/src/early.rs +++ b/compiler/rustc_lint/src/early.rs @@ -228,6 +228,7 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T> }) => self.check_id(closure_id), _ => {} } + lint_callback!(self, check_expr_post, e); } fn visit_generic_arg(&mut self, arg: &'a ast::GenericArg) { diff --git a/compiler/rustc_lint/src/passes.rs b/compiler/rustc_lint/src/passes.rs index 16964565b01..fca0eeeecc4 100644 --- a/compiler/rustc_lint/src/passes.rs +++ b/compiler/rustc_lint/src/passes.rs @@ -153,6 +153,7 @@ macro_rules! early_lint_methods { fn check_pat(a: &ast::Pat); fn check_pat_post(a: &ast::Pat); fn check_expr(a: &ast::Expr); + fn check_expr_post(a: &ast::Expr); fn check_ty(a: &ast::Ty); fn check_generic_arg(a: &ast::GenericArg); fn check_generic_param(a: &ast::GenericParam); diff --git a/compiler/rustc_lint/src/unused.rs b/compiler/rustc_lint/src/unused.rs index 6041f80753b..5c9731cd8df 100644 --- a/compiler/rustc_lint/src/unused.rs +++ b/compiler/rustc_lint/src/unused.rs @@ -955,11 +955,14 @@ declare_lint! { pub struct UnusedParens { with_self_ty_parens: bool, + /// `1 as (i32) < 2` parses to ExprKind::Lt + /// `1 as i32 < 2` parses to i32::<2[missing angle bracket] + parens_in_cast_in_lt: Vec, } impl UnusedParens { pub fn new() -> Self { - Self { with_self_ty_parens: false } + Self { with_self_ty_parens: false, parens_in_cast_in_lt: Vec::new() } } } @@ -1055,6 +1058,14 @@ impl UnusedParens { impl EarlyLintPass for UnusedParens { #[inline] fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) { + if let ExprKind::Binary(op, lhs, _rhs) = &e.kind && + (op.node == ast::BinOpKind::Lt || op.node == ast::BinOpKind::Shl) && + let ExprKind::Cast(_expr, ty) = &lhs.kind && + let ast::TyKind::Paren(_) = &ty.kind + { + self.parens_in_cast_in_lt.push(ty.id); + } + match e.kind { ExprKind::Let(ref pat, _, _) | ExprKind::ForLoop(ref pat, ..) => { self.check_unused_parens_pat(cx, pat, false, false, (true, true)); @@ -1101,6 +1112,17 @@ impl EarlyLintPass for UnusedParens { ::check_expr(self, cx, e) } + fn check_expr_post(&mut self, _cx: &EarlyContext<'_>, e: &ast::Expr) { + if let ExprKind::Binary(op, lhs, _rhs) = &e.kind && + (op.node == ast::BinOpKind::Lt || op.node == ast::BinOpKind::Shl) && + let ExprKind::Cast(_expr, ty) = &lhs.kind && + let ast::TyKind::Paren(_) = &ty.kind + { + let id = self.parens_in_cast_in_lt.pop().expect("check_expr and check_expr_post must balance"); + assert_eq!(id, ty.id, "check_expr and check_expr_post is a depth-first tree traversal"); + } + } + fn check_pat(&mut self, cx: &EarlyContext<'_>, p: &ast::Pat) { use ast::{Mutability, PatKind::*}; let keep_space = (false, false); @@ -1141,6 +1163,11 @@ impl EarlyLintPass for UnusedParens { } fn check_ty(&mut self, cx: &EarlyContext<'_>, ty: &ast::Ty) { + if let ast::TyKind::Paren(_) = ty.kind && + Some(&ty.id) == self.parens_in_cast_in_lt.last() + { + return; + } match &ty.kind { ast::TyKind::Array(_, len) => { self.check_unused_delims_expr( diff --git a/tests/ui/lint/unused/unused-parens-issue-106413.rs b/tests/ui/lint/unused/unused-parens-issue-106413.rs new file mode 100644 index 00000000000..7e76ab073b4 --- /dev/null +++ b/tests/ui/lint/unused/unused-parens-issue-106413.rs @@ -0,0 +1,32 @@ +// check-pass +#![warn(unused_parens)] + +fn id(t: T) -> T { t } + +fn main() { + // This should not warn + let _ = 1 as (i32) < 2; + let _ = id(1 as (i32) < 2); + let _ = id(1 as i32) + as (i32) < 2; + // These should warn + let _ = 1 as ((i32)) < 2; //~WARN unnecessary parentheses + let _ = 1 as (i32); //~WARN unnecessary parentheses + let _ = 1 as (i32) > 2; //~WARN unnecessary parentheses + let _ = id(1 as (i32)) //~WARN unnecessary parentheses + as (i32) < 2; + let _ = id(1 as (i32)) < 2; //~WARN unnecessary parentheses + + // This should not warn + let _ = 1 as (i32) << 2; + let _ = id(1 as (i32) << 2); + let _ = id(1 as i32) + as (i32) << 2; + // These should warn + let _ = 1 as ((i32)) << 2; //~WARN unnecessary parentheses + let _ = 1 as (i32); //~WARN unnecessary parentheses + let _ = 1 as (i32) >> 2; //~WARN unnecessary parentheses + let _ = id(1 as (i32)) //~WARN unnecessary parentheses + as (i32) << 2; + let _ = id(1 as (i32)) << 2; //~WARN unnecessary parentheses +} diff --git a/tests/ui/lint/unused/unused-parens-issue-106413.stderr b/tests/ui/lint/unused/unused-parens-issue-106413.stderr new file mode 100644 index 00000000000..d2d8f76d967 --- /dev/null +++ b/tests/ui/lint/unused/unused-parens-issue-106413.stderr @@ -0,0 +1,127 @@ +warning: unnecessary parentheses around type + --> $DIR/unused-parens-issue-106413.rs:13:19 + | +LL | let _ = 1 as ((i32)) < 2; + | ^ ^ + | +note: the lint level is defined here + --> $DIR/unused-parens-issue-106413.rs:2:9 + | +LL | #![warn(unused_parens)] + | ^^^^^^^^^^^^^ +help: remove these parentheses + | +LL - let _ = 1 as ((i32)) < 2; +LL + let _ = 1 as (i32) < 2; + | + +warning: unnecessary parentheses around type + --> $DIR/unused-parens-issue-106413.rs:14:18 + | +LL | let _ = 1 as (i32); + | ^ ^ + | +help: remove these parentheses + | +LL - let _ = 1 as (i32); +LL + let _ = 1 as i32; + | + +warning: unnecessary parentheses around type + --> $DIR/unused-parens-issue-106413.rs:15:18 + | +LL | let _ = 1 as (i32) > 2; + | ^ ^ + | +help: remove these parentheses + | +LL - let _ = 1 as (i32) > 2; +LL + let _ = 1 as i32 > 2; + | + +warning: unnecessary parentheses around type + --> $DIR/unused-parens-issue-106413.rs:16:21 + | +LL | let _ = id(1 as (i32)) + | ^ ^ + | +help: remove these parentheses + | +LL - let _ = id(1 as (i32)) +LL + let _ = id(1 as i32) + | + +warning: unnecessary parentheses around type + --> $DIR/unused-parens-issue-106413.rs:18:21 + | +LL | let _ = id(1 as (i32)) < 2; + | ^ ^ + | +help: remove these parentheses + | +LL - let _ = id(1 as (i32)) < 2; +LL + let _ = id(1 as i32) < 2; + | + +warning: unnecessary parentheses around type + --> $DIR/unused-parens-issue-106413.rs:26:19 + | +LL | let _ = 1 as ((i32)) << 2; + | ^ ^ + | +help: remove these parentheses + | +LL - let _ = 1 as ((i32)) << 2; +LL + let _ = 1 as (i32) << 2; + | + +warning: unnecessary parentheses around type + --> $DIR/unused-parens-issue-106413.rs:27:18 + | +LL | let _ = 1 as (i32); + | ^ ^ + | +help: remove these parentheses + | +LL - let _ = 1 as (i32); +LL + let _ = 1 as i32; + | + +warning: unnecessary parentheses around type + --> $DIR/unused-parens-issue-106413.rs:28:18 + | +LL | let _ = 1 as (i32) >> 2; + | ^ ^ + | +help: remove these parentheses + | +LL - let _ = 1 as (i32) >> 2; +LL + let _ = 1 as i32 >> 2; + | + +warning: unnecessary parentheses around type + --> $DIR/unused-parens-issue-106413.rs:29:21 + | +LL | let _ = id(1 as (i32)) + | ^ ^ + | +help: remove these parentheses + | +LL - let _ = id(1 as (i32)) +LL + let _ = id(1 as i32) + | + +warning: unnecessary parentheses around type + --> $DIR/unused-parens-issue-106413.rs:31:21 + | +LL | let _ = id(1 as (i32)) << 2; + | ^ ^ + | +help: remove these parentheses + | +LL - let _ = id(1 as (i32)) << 2; +LL + let _ = id(1 as i32) << 2; + | + +warning: 10 warnings emitted +