Skip to content

Commit 8e9c8f6

Browse files
committed
Add wrong_any_coerce lint
1 parent f0edfab commit 8e9c8f6

File tree

9 files changed

+404
-4
lines changed

9 files changed

+404
-4
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1176,6 +1176,7 @@ Released 2018-09-13
11761176
[`write_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#write_literal
11771177
[`write_with_newline`]: https://rust-lang.github.io/rust-clippy/master/index.html#write_with_newline
11781178
[`writeln_empty_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#writeln_empty_string
1179+
[`wrong_any_coerce`]: https://rust-lang.github.io/rust-clippy/master/index.html#wrong_any_coerce
11791180
[`wrong_pub_self_convention`]: https://rust-lang.github.io/rust-clippy/master/index.html#wrong_pub_self_convention
11801181
[`wrong_self_convention`]: https://rust-lang.github.io/rust-clippy/master/index.html#wrong_self_convention
11811182
[`wrong_transmute`]: https://rust-lang.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
@@ -6,7 +6,7 @@
66

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

9-
[There are 305 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
9+
[There are 306 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
1010

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

clippy_lints/src/any_coerce.rs

Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,225 @@
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, PointerCast};
15+
use crate::rustc::ty::{self, ToPolyTraitRef, Ty};
16+
use crate::rustc::{declare_lint_pass, declare_tool_lint};
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+
declare_clippy_lint! {
23+
/// **What it does:** Checks for coercing something that already contains a
24+
/// `dyn Any` to `dyn Any` itself.
25+
///
26+
/// **Why is this bad?** It's probably a mistake.
27+
///
28+
/// **Known problems:** None.
29+
///
30+
/// **Example:**
31+
/// ```rust
32+
/// let box_foo: Box<Foo> = Box::new(Foo);
33+
/// let mut box_any: Box<dyn Any> = box_foo;
34+
/// let bad: &mut dyn Any = &mut box_any;
35+
/// // you probably meant
36+
/// let ok: &mut dyn Any = &mut *box_any;
37+
/// ```
38+
pub WRONG_ANY_COERCE,
39+
correctness,
40+
"coercing a type already containing `dyn Any` to `dyn Any` itself"
41+
}
42+
43+
declare_lint_pass!(WrongAnyCoerce => [WRONG_ANY_COERCE]);
44+
45+
struct LintData<'tcx> {
46+
coerced_to_any: Ty<'tcx>,
47+
}
48+
49+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for WrongAnyCoerce {
50+
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
51+
let adjustments = cx.tables.expr_adjustments(expr);
52+
for (i, adj) in adjustments.iter().enumerate() {
53+
if let Adjust::Pointer(PointerCast::Unsize) = adj.kind {
54+
let src_ty = if i == 0 {
55+
cx.tables.expr_ty(expr)
56+
} else {
57+
adjustments[i - 1].target
58+
};
59+
cx.tcx.infer_ctxt().enter(|infcx| {
60+
let opt_lint_data = check_unsize_coercion(cx, &infcx, cx.param_env, src_ty, adj.target);
61+
if let Some(lint_data) = opt_lint_data {
62+
// TODO: we might be able to suggest dereferencing in some cases
63+
let cta_str = lint_data.coerced_to_any.to_string();
64+
span_lint_and_then(
65+
cx,
66+
WRONG_ANY_COERCE,
67+
expr.span,
68+
&format!("coercing `{}` to `dyn Any`", cta_str),
69+
|db| {
70+
if !cta_str.contains("Any") {
71+
db.note(&format!("`{}` dereferences to `dyn Any`", cta_str));
72+
}
73+
},
74+
)
75+
}
76+
});
77+
}
78+
}
79+
}
80+
}
81+
82+
/// Returns whether or not this coercion should be linted
83+
fn check_unsize_coercion<'tcx>(
84+
cx: &LateContext<'_, 'tcx>,
85+
infcx: &InferCtxt<'_, 'tcx>,
86+
param_env: ty::ParamEnv<'tcx>,
87+
src_ty: Ty<'tcx>,
88+
tgt_ty: Ty<'tcx>,
89+
) -> Option<LintData<'tcx>> {
90+
// redo the typechecking for this coercion to see if it required unsizing something to `dyn Any`
91+
// see https://github.com/rust-lang/rust/blob/cae6efc37d70ab7d353e6ab9ce229d59a65ed643/src/librustc_typeck/check/coercion.rs#L454-L611
92+
let tcx = infcx.tcx;
93+
let coerce_unsized_trait_did = tcx.lang_items().coerce_unsized_trait().unwrap();
94+
let unsize_trait_did = tcx.lang_items().unsize_trait().unwrap();
95+
96+
// don't report overflow errors
97+
let mut selcx = traits::SelectionContext::with_query_mode(&infcx, traits::TraitQueryMode::Canonical);
98+
let mut queue = VecDeque::new();
99+
queue.push_back(
100+
ty::TraitRef::new(
101+
coerce_unsized_trait_did,
102+
tcx.mk_substs_trait(src_ty, &[tgt_ty.into()]),
103+
)
104+
.to_poly_trait_ref(),
105+
);
106+
while let Some(trait_ref) = queue.pop_front() {
107+
if_chain! {
108+
if trait_ref.def_id() == unsize_trait_did;
109+
if is_type_dyn_any(cx, trait_ref.skip_binder().input_types().nth(1).unwrap());
110+
// found something unsizing to `dyn Any`
111+
let coerced_to_any = trait_ref.self_ty();
112+
if type_contains_any(cx, &mut selcx, param_env, coerced_to_any);
113+
then {
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 for these traits
124+
let traits = [coerce_unsized_trait_did, unsize_trait_did];
125+
queue.extend(
126+
vtable
127+
.nested_obligations()
128+
.into_iter()
129+
.filter_map(|oblig| oblig.predicate.to_opt_poly_trait_ref())
130+
.filter(|tr| traits.contains(&tr.def_id())),
131+
);
132+
}
133+
}
134+
None
135+
}
136+
137+
fn type_contains_any<'tcx>(
138+
cx: &LateContext<'_, 'tcx>,
139+
selcx: &mut traits::SelectionContext<'_, 'tcx>,
140+
param_env: ty::ParamEnv<'tcx>,
141+
ty: Ty<'tcx>,
142+
) -> bool {
143+
// check if it derefs to `dyn Any`
144+
if_chain! {
145+
if let Some((any_src_deref_ty, _deref_count)) = fully_deref_type(selcx, param_env, ty);
146+
if is_type_dyn_any(cx, any_src_deref_ty);
147+
then {
148+
// TODO: use deref_count to make a suggestion
149+
return true;
150+
}
151+
}
152+
// TODO: check for `RefCell<dyn Any>`?
153+
false
154+
}
155+
156+
fn is_type_dyn_any<'tcx>(
157+
cx: &LateContext<'_, 'tcx>,
158+
ty: Ty<'tcx>,
159+
) -> bool {
160+
if_chain! {
161+
if let ty::Dynamic(trait_list, _) = ty.sty;
162+
if let Some(principal_trait) = trait_list.skip_binder().principal();
163+
if match_def_path(cx, principal_trait.def_id, &paths::ANY_TRAIT);
164+
then {
165+
return true;
166+
}
167+
}
168+
false
169+
}
170+
171+
/// Calls [`deref_type`] repeatedly
172+
fn fully_deref_type<'tcx>(
173+
selcx: &mut traits::SelectionContext<'_, 'tcx>,
174+
param_env: ty::ParamEnv<'tcx>,
175+
src_ty: Ty<'tcx>,
176+
) -> Option<(Ty<'tcx>, usize)> {
177+
if let Some(deref_1) = deref_type(selcx, param_env, src_ty) {
178+
let mut deref_count = 1;
179+
let mut cur_ty = deref_1;
180+
while let Some(deref_n) = deref_type(selcx, param_env, cur_ty) {
181+
deref_count += 1;
182+
cur_ty = deref_n;
183+
}
184+
Some((cur_ty, deref_count))
185+
} else {
186+
None
187+
}
188+
}
189+
190+
/// Returns the type of `*expr`, where `expr` has type `src_ty`.
191+
/// This will go through `Deref` `impl`s if necessary.
192+
/// Returns `None` if `*expr` would not typecheck.
193+
fn deref_type<'tcx>(
194+
selcx: &mut traits::SelectionContext<'_, 'tcx>,
195+
param_env: ty::ParamEnv<'tcx>,
196+
src_ty: Ty<'tcx>,
197+
) -> Option<Ty<'tcx>> {
198+
if let Some(ty::TypeAndMut { ty, .. }) = src_ty.builtin_deref(true) {
199+
Some(ty)
200+
} else {
201+
// compute `<T as Deref>::Target`
202+
let infcx = selcx.infcx();
203+
let tcx = selcx.tcx();
204+
let src_deref = ty::TraitRef::new(
205+
tcx.lang_items().deref_trait().unwrap(),
206+
tcx.mk_substs_trait(src_ty, &[]),
207+
);
208+
let mut obligations = Vec::new();
209+
let src_deref_ty = traits::normalize_projection_type(
210+
selcx,
211+
param_env,
212+
ty::ProjectionTy::from_ref_and_name(tcx, src_deref, Ident::from_str("Target")),
213+
traits::ObligationCause::dummy(),
214+
0,
215+
&mut obligations,
216+
);
217+
// only return something if all the obligations definitely hold
218+
let obligations_ok = obligations.iter().all(|oblig| infcx.predicate_must_hold_considering_regions(oblig));
219+
if obligations_ok {
220+
Some(infcx.resolve_vars_if_possible(&src_deref_ty))
221+
} else {
222+
None
223+
}
224+
}
225+
}

clippy_lints/src/lib.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ mod consts;
143143
mod utils;
144144

145145
// begin lints modules, do not remove this comment, it’s used in `update_lints`
146+
pub mod any_coerce;
146147
pub mod approx_const;
147148
pub mod arithmetic;
148149
pub mod assertions_on_constants;
@@ -582,6 +583,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
582583
reg.register_late_lint_pass(box path_buf_push_overwrite::PathBufPushOverwrite);
583584
reg.register_late_lint_pass(box checked_conversions::CheckedConversions);
584585
reg.register_late_lint_pass(box integer_division::IntegerDivision);
586+
reg.register_late_lint_pass(box any_coerce::WrongAnyCoerce);
585587

586588
reg.register_lint_group("clippy::restriction", Some("clippy_restriction"), vec![
587589
arithmetic::FLOAT_ARITHMETIC,
@@ -667,6 +669,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
667669
]);
668670

669671
reg.register_lint_group("clippy::all", Some("clippy"), vec![
672+
any_coerce::WRONG_ANY_COERCE,
670673
approx_const::APPROX_CONSTANT,
671674
assertions_on_constants::ASSERTIONS_ON_CONSTANTS,
672675
assign_ops::ASSIGN_OP_PATTERN,
@@ -1049,6 +1052,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
10491052
]);
10501053

10511054
reg.register_lint_group("clippy::correctness", Some("clippy_correctness"), vec![
1055+
any_coerce::WRONG_ANY_COERCE,
10521056
approx_const::APPROX_CONSTANT,
10531057
attrs::DEPRECATED_SEMVER,
10541058
attrs::USELESS_ATTRIBUTE,

clippy_lints/src/types.rs

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

clippy_lints/src/utils/paths.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
//! This module contains paths to types and functions Clippy needs to know
22
//! about.
33
4-
pub const ANY_TRAIT: [&str; 3] = ["std", "any", "Any"];
4+
pub const ANY_TRAIT: [&str; 3] = ["core", "any", "Any"];
5+
pub const ANY_TRAIT_STD: [&str; 3] = ["std", "any", "Any"];
56
pub const ARC: [&str; 3] = ["alloc", "sync", "Arc"];
67
pub const ASMUT_TRAIT: [&str; 3] = ["core", "convert", "AsMut"];
78
pub const ASREF_TRAIT: [&str; 3] = ["core", "convert", "AsRef"];

src/lintlist/mod.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ pub use lint::Lint;
66
pub use lint::LINT_LEVELS;
77

88
// begin lint list, do not remove this comment, it’s used in `update_lints`
9-
pub const ALL_LINTS: [Lint; 305] = [
9+
pub const ALL_LINTS: [Lint; 306] = [
1010
Lint {
1111
name: "absurd_extreme_comparisons",
1212
group: "correctness",
@@ -2093,6 +2093,13 @@ pub const ALL_LINTS: [Lint; 305] = [
20932093
deprecation: None,
20942094
module: "write",
20952095
},
2096+
Lint {
2097+
name: "wrong_any_coerce",
2098+
group: "correctness",
2099+
desc: "coercing a type already containing `dyn Any` to `dyn Any` itself",
2100+
deprecation: None,
2101+
module: "any_coerce",
2102+
},
20962103
Lint {
20972104
name: "wrong_pub_self_convention",
20982105
group: "restriction",

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 + Send> = 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)