Skip to content

Commit ca7408b

Browse files
committed
Add wrong_any_coerce lint
1 parent 00ed705 commit ca7408b

File tree

8 files changed

+379
-3
lines changed

8 files changed

+379
-3
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -899,6 +899,7 @@ All notable changes to this project will be documented in this file.
899899
[`write_literal`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#write_literal
900900
[`write_with_newline`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#write_with_newline
901901
[`writeln_empty_string`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#writeln_empty_string
902+
[`wrong_any_coerce`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#wrong_any_coerce
902903
[`wrong_pub_self_convention`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#wrong_pub_self_convention
903904
[`wrong_self_convention`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#wrong_self_convention
904905
[`wrong_transmute`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#wrong_transmute

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ We are currently in the process of discussing Clippy 1.0 via the RFC process in
99

1010
A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
1111

12-
[There are 283 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)
12+
[There are 284 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)
1313

1414
We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
1515

clippy_lints/src/any_coerce.rs

Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution.
3+
//
4+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
5+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
6+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
7+
// option. This file may not be copied, modified, or distributed
8+
// except according to those terms.
9+
10+
use crate::rustc::hir::Expr;
11+
use crate::rustc::infer::InferCtxt;
12+
use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
13+
use crate::rustc::traits;
14+
use crate::rustc::ty::adjustment::Adjust;
15+
use crate::rustc::ty::{self, ToPolyTraitRef, Ty};
16+
use crate::rustc::{declare_tool_lint, lint_array};
17+
use crate::syntax_pos::symbol::Ident;
18+
use crate::utils::{match_def_path, paths, span_lint_and_then};
19+
use if_chain::if_chain;
20+
use std::collections::VecDeque;
21+
22+
/// **What it does:** Checks for coercing something that already contains a
23+
/// `dyn Any` to `dyn Any` itself.
24+
///
25+
/// **Why is this bad?** It's probably a mistake.
26+
///
27+
/// **Known problems:** None.
28+
///
29+
/// **Example:**
30+
/// ```rust
31+
/// let box_foo: Box<Foo> = Box::new(Foo);
32+
/// let mut box_any: Box<dyn Any> = box_foo;
33+
/// let bad: &mut dyn Any = &mut box_any;
34+
/// // you probably meant
35+
/// let ok: &mut dyn Any = &mut *box_any;
36+
/// ```
37+
declare_clippy_lint! {
38+
pub WRONG_ANY_COERCE,
39+
correctness,
40+
"coercing a type already containing `dyn Any` to `dyn Any` itself"
41+
}
42+
43+
#[derive(Default)]
44+
pub struct WrongAnyCoerce;
45+
46+
impl LintPass for WrongAnyCoerce {
47+
fn get_lints(&self) -> LintArray {
48+
lint_array!(WRONG_ANY_COERCE)
49+
}
50+
}
51+
52+
struct LintData<'tcx> {
53+
coerced_to_any: Ty<'tcx>,
54+
}
55+
56+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for WrongAnyCoerce {
57+
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
58+
let adjustments = cx.tables.expr_adjustments(expr);
59+
for (i, adj) in adjustments.iter().enumerate() {
60+
if let Adjust::Unsize = adj.kind {
61+
let src_ty = if i == 0 {
62+
cx.tables.expr_ty(expr)
63+
} else {
64+
adjustments[i - 1].target
65+
};
66+
cx.tcx.infer_ctxt().enter(|infcx| {
67+
let opt_lint_data = check_unsize_coercion(infcx, cx.param_env, src_ty, adj.target);
68+
if let Some(lint_data) = opt_lint_data {
69+
// TODO: we might be able to suggest dereferencing in some cases
70+
let cta_str = lint_data.coerced_to_any.to_string();
71+
span_lint_and_then(
72+
cx,
73+
WRONG_ANY_COERCE,
74+
expr.span,
75+
&format!("coercing `{}` to `dyn Any`", cta_str),
76+
|db| {
77+
if !cta_str.contains("Any") {
78+
db.note(&format!("`{}` dereferences to `dyn Any`", cta_str));
79+
}
80+
},
81+
)
82+
}
83+
});
84+
}
85+
}
86+
}
87+
}
88+
89+
/// Returns whether or not this coercion should be linted
90+
fn check_unsize_coercion<'tcx>(
91+
infcx: InferCtxt<'_, '_, 'tcx>,
92+
param_env: ty::ParamEnv<'tcx>,
93+
src_ty: Ty<'tcx>,
94+
tgt_ty: Ty<'tcx>,
95+
) -> Option<LintData<'tcx>> {
96+
// redo the typechecking for this coercion to see if it required unsizing something to `dyn Any`
97+
// see https://github.com/rust-lang/rust/blob/cae6efc37d70ab7d353e6ab9ce229d59a65ed643/src/librustc_typeck/check/coercion.rs#L454-L611
98+
let tcx = infcx.tcx;
99+
// don't report overflow errors
100+
let mut selcx = traits::SelectionContext::with_query_mode(&infcx, traits::TraitQueryMode::Canonical);
101+
let mut queue = VecDeque::new();
102+
queue.push_back(
103+
ty::TraitRef::new(
104+
tcx.lang_items().coerce_unsized_trait().unwrap(),
105+
tcx.mk_substs_trait(src_ty, &[tgt_ty.into()]),
106+
)
107+
.to_poly_trait_ref(),
108+
);
109+
while let Some(trait_ref) = queue.pop_front() {
110+
if match_def_path(tcx, trait_ref.def_id(), &paths::ANY_TRAIT) {
111+
// found something unsizing to `dyn Any`
112+
let coerced_to_any = trait_ref.self_ty();
113+
if type_contains_any(&mut selcx, param_env, coerced_to_any) {
114+
return Some(LintData { coerced_to_any });
115+
}
116+
}
117+
let select_result = selcx.select(&traits::Obligation::new(
118+
traits::ObligationCause::dummy(),
119+
param_env,
120+
trait_ref.to_poly_trait_predicate(),
121+
));
122+
if let Ok(Some(vtable)) = select_result {
123+
// we only care about trait predicates
124+
queue.extend(
125+
vtable
126+
.nested_obligations()
127+
.into_iter()
128+
.filter_map(|oblig| oblig.predicate.to_opt_poly_trait_ref()),
129+
);
130+
}
131+
}
132+
None
133+
}
134+
135+
fn type_contains_any<'tcx>(
136+
selcx: &mut traits::SelectionContext<'_, '_, 'tcx>,
137+
param_env: ty::ParamEnv<'tcx>,
138+
ty: Ty<'tcx>,
139+
) -> bool {
140+
// check if it derefs to `dyn Any`
141+
if_chain! {
142+
if let Some((any_src_deref_ty, _deref_count)) = fully_deref_type(selcx, param_env, ty);
143+
if let ty::TyKind::Dynamic(trait_list, _) = any_src_deref_ty.sty;
144+
if match_def_path(selcx.tcx(), trait_list.skip_binder().principal().def_id, &paths::ANY_TRAIT);
145+
then {
146+
// TODO: use deref_count to make a suggestion
147+
return true;
148+
}
149+
}
150+
// TODO: check for `RefCell<dyn Any>`?
151+
false
152+
}
153+
154+
/// Calls [deref_type] repeatedly
155+
fn fully_deref_type<'tcx>(
156+
selcx: &mut traits::SelectionContext<'_, '_, 'tcx>,
157+
param_env: ty::ParamEnv<'tcx>,
158+
src_ty: Ty<'tcx>,
159+
) -> Option<(Ty<'tcx>, usize)> {
160+
if let Some(deref_1) = deref_type(selcx, param_env, src_ty) {
161+
let mut deref_count = 1;
162+
let mut cur_ty = deref_1;
163+
while let Some(deref_n) = deref_type(selcx, param_env, cur_ty) {
164+
deref_count += 1;
165+
cur_ty = deref_n;
166+
}
167+
Some((cur_ty, deref_count))
168+
} else {
169+
None
170+
}
171+
}
172+
173+
/// Returns the type of `*expr`, where `expr` has type `src_ty`.
174+
/// This will go through `Deref` `impl`s if necessary.
175+
/// Returns `None` if `*expr` would not typecheck.
176+
fn deref_type<'tcx>(
177+
selcx: &mut traits::SelectionContext<'_, '_, 'tcx>,
178+
param_env: ty::ParamEnv<'tcx>,
179+
src_ty: Ty<'tcx>,
180+
) -> Option<Ty<'tcx>> {
181+
if let Some(ty::TypeAndMut { ty, .. }) = src_ty.builtin_deref(true) {
182+
Some(ty)
183+
} else {
184+
// compute `<T as Deref>::Target`
185+
let infcx = selcx.infcx();
186+
let tcx = selcx.tcx();
187+
let src_deref = ty::TraitRef::new(
188+
tcx.lang_items().deref_trait().unwrap(),
189+
tcx.mk_substs_trait(src_ty, &[]),
190+
);
191+
let mut obligations = Vec::new();
192+
let src_deref_ty = traits::normalize_projection_type(
193+
selcx,
194+
param_env,
195+
ty::ProjectionTy::from_ref_and_name(tcx, src_deref, Ident::from_str("Target")),
196+
traits::ObligationCause::dummy(),
197+
0,
198+
&mut obligations,
199+
);
200+
// only return something if all the obligations definitely hold
201+
let obligations_ok = obligations.iter().all(|oblig| infcx.predicate_must_hold(oblig));
202+
if obligations_ok {
203+
Some(infcx.resolve_type_vars_if_possible(&src_deref_ty))
204+
} else {
205+
None
206+
}
207+
}
208+
}

clippy_lints/src/lib.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ mod consts;
8686
mod utils;
8787

8888
// begin lints modules, do not remove this comment, it’s used in `update_lints`
89+
pub mod any_coerce;
8990
pub mod approx_const;
9091
pub mod arithmetic;
9192
pub mod assign_ops;
@@ -454,6 +455,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
454455
reg.register_late_lint_pass(box non_copy_const::NonCopyConst);
455456
reg.register_late_lint_pass(box ptr_offset_with_cast::Pass);
456457
reg.register_late_lint_pass(box redundant_clone::RedundantClone);
458+
reg.register_late_lint_pass(box any_coerce::WrongAnyCoerce);
457459

458460
reg.register_lint_group("clippy::restriction", Some("clippy_restriction"), vec![
459461
arithmetic::FLOAT_ARITHMETIC,
@@ -527,6 +529,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
527529
]);
528530

529531
reg.register_lint_group("clippy::all", Some("clippy"), vec![
532+
any_coerce::WRONG_ANY_COERCE,
530533
approx_const::APPROX_CONSTANT,
531534
assign_ops::ASSIGN_OP_PATTERN,
532535
assign_ops::MISREFACTORED_ASSIGN_OP,
@@ -900,6 +903,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
900903
]);
901904

902905
reg.register_lint_group("clippy::correctness", Some("clippy_correctness"), vec![
906+
any_coerce::WRONG_ANY_COERCE,
903907
approx_const::APPROX_CONSTANT,
904908
attrs::DEPRECATED_SEMVER,
905909
attrs::USELESS_ATTRIBUTE,

clippy_lints/src/types.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ fn is_any_trait(t: &hir::Ty) -> bool {
362362
if traits.len() >= 1;
363363
// Only Send/Sync can be used as additional traits, so it is enough to
364364
// check only the first trait.
365-
if match_path(&traits[0].trait_ref.path, &paths::ANY_TRAIT);
365+
if match_path(&traits[0].trait_ref.path, &paths::ANY_TRAIT_STD);
366366
then {
367367
return true;
368368
}

clippy_lints/src/utils/paths.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@
1111
//! This module contains paths to types and functions Clippy needs to know
1212
//! about.
1313
14-
pub const ANY_TRAIT: [&str; 3] = ["std", "any", "Any"];
14+
pub const ANY_TRAIT: [&str; 3] = ["core", "any", "Any"];
15+
pub const ANY_TRAIT_STD: [&str; 3] = ["std", "any", "Any"];
1516
pub const ARC: [&str; 3] = ["alloc", "sync", "Arc"];
1617
pub const ASMUT_TRAIT: [&str; 3] = ["core", "convert", "AsMut"];
1718
pub const ASREF_TRAIT: [&str; 3] = ["core", "convert", "AsRef"];

tests/ui/any_coerce.rs

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution.
3+
//
4+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
5+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
6+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
7+
// option. This file may not be copied, modified, or distributed
8+
// except according to those terms.
9+
10+
#![feature(unsize, coerce_unsized)]
11+
#![deny(clippy::wrong_any_coerce)]
12+
#![deny(bare_trait_objects)]
13+
14+
use std::any::Any;
15+
use std::cell::RefCell;
16+
use std::fmt::Debug;
17+
use std::iter::Iterator;
18+
use std::marker::{Send, Unsize};
19+
use std::ops::CoerceUnsized;
20+
use std::ops::Deref;
21+
use std::rc::Rc;
22+
23+
struct Foo;
24+
25+
struct Unsizeable<T: ?Sized, U: ?Sized, V: ?Sized> {
26+
box_v: Box<V>,
27+
rc_t: Rc<T>,
28+
u: U,
29+
}
30+
31+
fn main() {
32+
let mut box_any: Box<dyn Any> = Box::new(Foo);
33+
let _: *mut dyn Any = &mut box_any; // LINT
34+
let _: *mut dyn Any = &mut *box_any; // ok
35+
36+
let rc_rc_any: Rc<Rc<dyn Any>> = Rc::new(Rc::new(Foo));
37+
let _: &dyn Any = &rc_rc_any; // LINT
38+
let _: &dyn Any = &*rc_rc_any; // LINT
39+
let _: &dyn Any = &**rc_rc_any; // ok
40+
let _: &Rc<dyn Any> = &*rc_rc_any; // ok
41+
42+
let refcell_box_any: RefCell<Box<dyn Any>> = RefCell::new(Box::new(Foo));
43+
let _: &RefCell<dyn Any> = &refcell_box_any; // LINT
44+
45+
let rc_unsizable_rc_any: Rc<Unsizeable<i32, Rc<dyn Any>, i32>> = Rc::new(Unsizeable {
46+
box_v: Box::new(0),
47+
rc_t: Rc::new(0),
48+
u: Rc::new(Foo),
49+
});
50+
let _: Rc<Unsizeable<i32, dyn Any, i32>> = rc_unsizable_rc_any.clone(); // LINT
51+
let _: &Unsizeable<i32, dyn Any, i32> = &*rc_unsizable_rc_any; // LINT
52+
let _: &Rc<Unsizeable<i32, Rc<dyn Any>, i32>> = &rc_unsizable_rc_any; // ok
53+
let _: &Unsizeable<i32, Rc<dyn Any>, i32> = &*rc_unsizable_rc_any; // ok
54+
55+
let ref_any: &dyn Any = &Foo;
56+
let _: &dyn Any = &ref_any; // LINT
57+
let _: &dyn Any = &*ref_any; // ok
58+
59+
let ref_refcell_any: &'static RefCell<dyn Any> = Box::leak(Box::new(RefCell::new(Foo)));
60+
let _: &dyn Any = &ref_refcell_any.borrow(); // LINT
61+
let _: &dyn Any = &*ref_refcell_any.borrow(); // ok
62+
}
63+
64+
fn very_generic<T, U>(t: &'static T)
65+
where
66+
T: Deref<Target = U> + 'static,
67+
U: Deref<Target = dyn Any + Send> + 'static,
68+
{
69+
let _: &dyn Any = t; // LINT
70+
let _: &dyn Any = &t; // LINT
71+
let _: &dyn Any = &*t; // LINT
72+
let _: &dyn Any = &**t; // LINT
73+
let _: &dyn Any = &***t; // ok
74+
}

0 commit comments

Comments
 (0)