diff options
| -rw-r--r-- | clippy_lints/src/types.rs | 44 | ||||
| -rw-r--r-- | tests/ui/borrow_box.rs | 16 | ||||
| -rw-r--r-- | tests/ui/borrow_box.stderr | 50 |
3 files changed, 98 insertions, 12 deletions
diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 6a33aaaaab2..45f3bc3ea85 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -10,9 +10,9 @@ use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc_hir as hir; use rustc_hir::intravisit::{walk_body, walk_expr, walk_ty, FnKind, NestedVisitorMap, Visitor}; use rustc_hir::{ - BinOpKind, Block, Body, Expr, ExprKind, FnDecl, FnRetTy, FnSig, GenericArg, GenericParamKind, HirId, ImplItem, - ImplItemKind, Item, ItemKind, Lifetime, Lit, Local, MatchSource, MutTy, Mutability, Node, QPath, Stmt, StmtKind, - TraitFn, TraitItem, TraitItemKind, TyKind, UnOp, + BinOpKind, Block, Body, Expr, ExprKind, FnDecl, FnRetTy, FnSig, GenericArg, GenericBounds, GenericParamKind, HirId, + ImplItem, ImplItemKind, Item, ItemKind, Lifetime, Lit, Local, MatchSource, MutTy, Mutability, Node, QPath, Stmt, + StmtKind, SyntheticTyParamKind, TraitFn, TraitItem, TraitItemKind, TyKind, UnOp, }; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::hir::map::Map; @@ -678,17 +678,30 @@ impl Types { // details. return; } + + // When trait objects or opaque types have lifetime or auto-trait bounds, + // we need to add parentheses to avoid a syntax error due to its ambiguity. + // Originally reported as the issue #3128. + let inner_snippet = snippet(cx, inner.span, ".."); + let suggestion = match &inner.kind { + TyKind::TraitObject(bounds, lt_bound) if bounds.len() > 1 || !lt_bound.is_elided() => { + format!("&{}({})", ltopt, &inner_snippet) + }, + TyKind::Path(qpath) + if get_bounds_if_impl_trait(cx, qpath, inner.hir_id) + .map_or(false, |bounds| bounds.len() > 1) => + { + format!("&{}({})", ltopt, &inner_snippet) + }, + _ => format!("&{}{}", ltopt, &inner_snippet), + }; span_lint_and_sugg( cx, BORROWED_BOX, hir_ty.span, "you seem to be trying to use `&Box<T>`. Consider using just `&T`", "try", - format!( - "&{}{}", - ltopt, - &snippet(cx, inner.span, "..") - ), + suggestion, // To make this `MachineApplicable`, at least one needs to check if it isn't a trait item // because the trait impls of it will break otherwise; // and there may be other cases that result in invalid code. @@ -721,6 +734,21 @@ fn is_any_trait(t: &hir::Ty<'_>) -> bool { false } +fn get_bounds_if_impl_trait<'tcx>(cx: &LateContext<'tcx>, qpath: &QPath<'_>, id: HirId) -> Option<GenericBounds<'tcx>> { + if_chain! { + if let Some(did) = qpath_res(cx, qpath, id).opt_def_id(); + if let Some(node) = cx.tcx.hir().get_if_local(did); + if let Node::GenericParam(generic_param) = node; + if let GenericParamKind::Type { synthetic, .. } = generic_param.kind; + if synthetic == Some(SyntheticTyParamKind::ImplTrait); + then { + Some(generic_param.bounds) + } else { + None + } + } +} + declare_clippy_lint! { /// **What it does:** Checks for binding a unit value. /// diff --git a/tests/ui/borrow_box.rs b/tests/ui/borrow_box.rs index 1901de46ca8..b606f773cfb 100644 --- a/tests/ui/borrow_box.rs +++ b/tests/ui/borrow_box.rs @@ -3,6 +3,8 @@ #![allow(unused_variables)] #![allow(dead_code)] +use std::fmt::Display; + pub fn test1(foo: &mut Box<bool>) { // Although this function could be changed to "&mut bool", // avoiding the Box, mutable references to boxes are not @@ -89,6 +91,20 @@ pub fn test13(boxed_slice: &mut Box<[i32]>) { *boxed_slice = data.into_boxed_slice(); } +// The suggestion should include proper parentheses to avoid a syntax error. +pub fn test14(_display: &Box<dyn Display>) {} +pub fn test15(_display: &Box<dyn Display + Send>) {} +pub fn test16<'a>(_display: &'a Box<dyn Display + 'a>) {} + +pub fn test17(_display: &Box<impl Display>) {} +pub fn test18(_display: &Box<impl Display + Send>) {} +pub fn test19<'a>(_display: &'a Box<impl Display + 'a>) {} + +// This exists only to check what happens when parentheses are already present. +// Even though the current implementation doesn't put extra parentheses, +// it's fine that unnecessary parentheses appear in the future for some reason. +pub fn test20(_display: &Box<(dyn Display + Send)>) {} + fn main() { test1(&mut Box::new(false)); test2(); diff --git a/tests/ui/borrow_box.stderr b/tests/ui/borrow_box.stderr index b5db691f89f..3eac32815be 100644 --- a/tests/ui/borrow_box.stderr +++ b/tests/ui/borrow_box.stderr @@ -1,5 +1,5 @@ error: you seem to be trying to use `&Box<T>`. Consider using just `&T` - --> $DIR/borrow_box.rs:19:14 + --> $DIR/borrow_box.rs:21:14 | LL | let foo: &Box<bool>; | ^^^^^^^^^^ help: try: `&bool` @@ -11,16 +11,58 @@ LL | #![deny(clippy::borrowed_box)] | ^^^^^^^^^^^^^^^^^^^^ error: you seem to be trying to use `&Box<T>`. Consider using just `&T` - --> $DIR/borrow_box.rs:23:10 + --> $DIR/borrow_box.rs:25:10 | LL | foo: &'a Box<bool>, | ^^^^^^^^^^^^^ help: try: `&'a bool` error: you seem to be trying to use `&Box<T>`. Consider using just `&T` - --> $DIR/borrow_box.rs:27:17 + --> $DIR/borrow_box.rs:29:17 | LL | fn test4(a: &Box<bool>); | ^^^^^^^^^^ help: try: `&bool` -error: aborting due to 3 previous errors +error: you seem to be trying to use `&Box<T>`. Consider using just `&T` + --> $DIR/borrow_box.rs:95:25 + | +LL | pub fn test14(_display: &Box<dyn Display>) {} + | ^^^^^^^^^^^^^^^^^ help: try: `&dyn Display` + +error: you seem to be trying to use `&Box<T>`. Consider using just `&T` + --> $DIR/borrow_box.rs:96:25 + | +LL | pub fn test15(_display: &Box<dyn Display + Send>) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&(dyn Display + Send)` + +error: you seem to be trying to use `&Box<T>`. Consider using just `&T` + --> $DIR/borrow_box.rs:97:29 + | +LL | pub fn test16<'a>(_display: &'a Box<dyn Display + 'a>) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&'a (dyn Display + 'a)` + +error: you seem to be trying to use `&Box<T>`. Consider using just `&T` + --> $DIR/borrow_box.rs:99:25 + | +LL | pub fn test17(_display: &Box<impl Display>) {} + | ^^^^^^^^^^^^^^^^^^ help: try: `&impl Display` + +error: you seem to be trying to use `&Box<T>`. Consider using just `&T` + --> $DIR/borrow_box.rs:100:25 + | +LL | pub fn test18(_display: &Box<impl Display + Send>) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&(impl Display + Send)` + +error: you seem to be trying to use `&Box<T>`. Consider using just `&T` + --> $DIR/borrow_box.rs:101:29 + | +LL | pub fn test19<'a>(_display: &'a Box<impl Display + 'a>) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&'a (impl Display + 'a)` + +error: you seem to be trying to use `&Box<T>`. Consider using just `&T` + --> $DIR/borrow_box.rs:106:25 + | +LL | pub fn test20(_display: &Box<(dyn Display + Send)>) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&(dyn Display + Send)` + +error: aborting due to 10 previous errors |