squiid/rust
1
0
Fork 0
Code Issues Pull requests Projects Activity

fix the error-causing suggestion of 'borrowed_box'

Browse source 
fix the error-causing suggestion of 'borrowed_box',
which missed parentheses and was ambiguous.
This commit is contained in:
rail 2020-10-21 09:42:00 +13:00
parent afbac8906e
commit e568a328f9
3 changed files with 98 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

44
clippy_lints/src/types.rs
View file

@ -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.
///

16
tests/ui/borrow_box.rs
View file

@ -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();

50
tests/ui/borrow_box.stderr
View file

@ -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