Skip to content

Commit e32e4de

Browse files
Y-NakY-Nak
committed
New lint: default_numeric_fallback
1 parent 876ffa4 commit e32e4de

File tree

5 files changed

+679
-0
lines changed

5 files changed

+679
-0
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1909,6 +1909,7 @@ Released 2018-09-13
19091909
[`debug_assert_with_mut_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#debug_assert_with_mut_call
19101910
[`decimal_literal_representation`]: https://rust-lang.github.io/rust-clippy/master/index.html#decimal_literal_representation
19111911
[`declare_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#declare_interior_mutable_const
1912+
[`default_numeric_fallback`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_numeric_fallback
19121913
[`default_trait_access`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_trait_access
19131914
[`deprecated_cfg_attr`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_cfg_attr
19141915
[`deprecated_semver`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_semver

‎clippy_lints/src/default_numeric_fallback.rs

Lines changed: 388 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,388 @@
1+
use rustc_ast::ast::{Label, LitFloatType, LitIntType, LitKind};
2+
use rustc_hir::{
3+
self as hir,
4+
intravisit::{walk_expr, walk_stmt, walk_ty, FnKind, NestedVisitorMap, Visitor},
5+
Body, Expr, ExprKind, FnDecl, FnRetTy, Guard, HirId, Lit, Stmt, StmtKind,
6+
};
7+
use rustc_lint::{LateContext, LateLintPass};
8+
use rustc_middle::{
9+
hir::map::Map,
10+
ty::{self, subst::GenericArgKind, FloatTy, IntTy, Ty, TyCtxt},
11+
};
12+
use rustc_session::{declare_lint_pass, declare_tool_lint};
13+
use rustc_span::Span;
14+
use rustc_typeck::hir_ty_to_ty;
15+
16+
use if_chain::if_chain;
17+
18+
use crate::utils::span_lint_and_help;
19+
20+
declare_clippy_lint! {
21+
/// **What it does:** Checks for usage of unconstrained numeric literals which may cause default numeric fallback in type
22+
/// inference.
23+
///
24+
/// Default numeric fallback means that if numeric types have not yet been bound to concrete
25+
/// types at the end of type inference, then integer type is bound to `i32`, and similarly
26+
/// floating type is bound to `f64`.
27+
///
28+
/// See [RFC0212](https://github.com/rust-lang/rfcs/blob/master/text/0212-restore-int-fallback.md) for more information about the fallback.
29+
///
30+
/// **Why is this bad?** For those who are very careful about types, default numeric fallback
31+
/// can be a pitfall that cause unexpected runtime behavior.
32+
///
33+
/// **Known problems:** None.
34+
///
35+
/// **Example:**
36+
/// ```rust
37+
/// let i = 10;
38+
/// let f = 1.23;
39+
/// ```
40+
///
41+
/// Use instead:
42+
/// ```rust
43+
/// let i = 10i32;
44+
/// let f = 1.23f64;
45+
/// ```
46+
pub DEFAULT_NUMERIC_FALLBACK,
47+
restriction,
48+
"usage of unconstrained numeric literals which may cause default numeric fallback."
49+
}
50+
51+
declare_lint_pass!(DefaultNumericFallback => [DEFAULT_NUMERIC_FALLBACK]);
52+
53+
fn enclosing_body_owner_opt(tcx: TyCtxt<'_>, hir_id: HirId) -> Option<HirId> {
54+
let hir_map = tcx.hir();
55+
for (parent, _) in hir_map.parent_iter(hir_id) {
56+
if let Some(body) = hir_map.maybe_body_owned_by(parent) {
57+
return Some(hir_map.body_owner(body));
58+
}
59+
}
60+
None
61+
}
62+
63+
impl LateLintPass<'_> for DefaultNumericFallback {
64+
fn check_fn(
65+
&mut self,
66+
cx: &LateContext<'tcx>,
67+
_: FnKind<'tcx>,
68+
fn_decl: &'tcx FnDecl<'_>,
69+
body: &'tcx Body<'_>,
70+
_: Span,
71+
hir_id: HirId,
72+
) {
73+
let ret_ty_bound = match fn_decl.output {
74+
FnRetTy::DefaultReturn(_) => None,
75+
FnRetTy::Return(ty) => Some(ty),
76+
}
77+
.and_then(|ty| {
78+
let mut infer_ty_finder = InferTyFinder::new();
79+
infer_ty_finder.visit_ty(ty);
80+
if infer_ty_finder.found {
81+
None
82+
} else if enclosing_body_owner_opt(cx.tcx, hir_id).is_some() {
83+
cx.typeck_results().node_type_opt(ty.hir_id)
84+
} else {
85+
Some(hir_ty_to_ty(cx.tcx, ty))
86+
}
87+
});
88+
89+
let mut visitor = NumericFallbackVisitor::new(ret_ty_bound, cx);
90+
visitor.visit_body(body);
91+
}
92+
}
93+
94+
struct NumericFallbackVisitor<'a, 'tcx> {
95+
/// Stack manages type bound of exprs. The top element holds current expr type.
96+
ty_bounds: Vec<Option<Ty<'tcx>>>,
97+
98+
/// Ret type bound.
99+
ret_ty_bound: Option<Ty<'tcx>>,
100+
101+
/// Break type bounds.
102+
break_ty_bounds: Vec<(Option<Label>, Option<Ty<'tcx>>)>,
103+
104+
cx: &'a LateContext<'tcx>,
105+
}
106+
107+
impl<'a, 'tcx> NumericFallbackVisitor<'a, 'tcx> {
108+
fn new(ret_ty_bound: Option<Ty<'tcx>>, cx: &'a LateContext<'tcx>) -> Self {
109+
Self {
110+
ty_bounds: vec![ret_ty_bound],
111+
ret_ty_bound,
112+
break_ty_bounds: vec![],
113+
cx,
114+
}
115+
}
116+
117+
/// Check whether lit cause fallback or not.
118+
fn check_lit(&self, lit: &Lit, lit_ty: Ty<'tcx>) {
119+
let ty_bound = self.ty_bounds.last().unwrap();
120+
121+
let should_lint = match (&lit.node, lit_ty.kind()) {
122+
(LitKind::Int(_, LitIntType::Unsuffixed), ty::Int(ty::IntTy::I32)) => {
123+
// In case integer literal is explicitly bound to i32, then suppress lint.
124+
ty_bound.map_or(true, |ty_bound| !matches!(ty_bound.kind(), ty::Int(IntTy::I32)))
125+
},
126+
127+
(LitKind::Float(_, LitFloatType::Unsuffixed), ty::Float(ty::FloatTy::F64)) => {
128+
// In case float literal is explicitly bound to f64, then suppress lint.
129+
ty_bound.map_or(true, |ty_bound| !matches!(ty_bound.kind(), ty::Float(FloatTy::F64)))
130+
},
131+
132+
_ => false,
133+
};
134+
135+
if should_lint {
136+
span_lint_and_help(
137+
self.cx,
138+
DEFAULT_NUMERIC_FALLBACK,
139+
lit.span,
140+
"default numeric fallback might occur",
141+
None,
142+
"consider adding suffix to avoid default numeric fallback",
143+
);
144+
}
145+
}
146+
}
147+
148+
impl<'a, 'tcx> Visitor<'tcx> for NumericFallbackVisitor<'a, 'tcx> {
149+
type Map = Map<'tcx>;
150+
151+
#[allow(clippy::too_many_lines)]
152+
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
153+
match (&expr.kind, *self.ty_bounds.last().unwrap()) {
154+
(ExprKind::Array(_), Some(last_bound)) => {
155+
if let ty::Array(ty, _) = last_bound.kind() {
156+
self.ty_bounds.push(Some(ty))
157+
} else {
158+
self.ty_bounds.push(None)
159+
}
160+
},
161+
162+
(ExprKind::Call(func, args), _) => {
163+
if_chain! {
164+
if let ExprKind::Path(ref func_path) = func.kind;
165+
if let Some(def_id) = self.cx.qpath_res(func_path, func.hir_id).opt_def_id();
166+
then {
167+
let fn_sig = self.cx.tcx.fn_sig(def_id).skip_binder();
168+
for (expr, bound) in args.iter().zip(fn_sig.inputs().iter()) {
169+
// Push found arg type, then visit arg.
170+
self.ty_bounds.push(Some(bound));
171+
self.visit_expr(expr);
172+
self.ty_bounds.pop();
173+
}
174+
return;
175+
} else {
176+
self.ty_bounds.push(None)
177+
}
178+
}
179+
},
180+
181+
(ExprKind::MethodCall(_, _, args, _), _) => {
182+
if let Some(def_id) = self.cx.typeck_results().type_dependent_def_id(expr.hir_id) {
183+
let fn_sig = self.cx.tcx.fn_sig(def_id).skip_binder();
184+
for (expr, bound) in args.iter().zip(fn_sig.inputs().iter()) {
185+
self.ty_bounds.push(Some(bound));
186+
self.visit_expr(expr);
187+
self.ty_bounds.pop();
188+
}
189+
return;
190+
}
191+
192+
self.ty_bounds.push(None)
193+
},
194+
195+
(ExprKind::Tup(exprs), Some(last_bound)) => {
196+
if let ty::Tuple(tys) = last_bound.kind() {
197+
for (expr, bound) in exprs.iter().zip(tys.iter()) {
198+
if let GenericArgKind::Type(ty) = bound.unpack() {
199+
self.ty_bounds.push(Some(ty));
200+
} else {
201+
self.ty_bounds.push(None);
202+
}
203+
204+
self.visit_expr(expr);
205+
self.ty_bounds.pop();
206+
}
207+
return;
208+
}
209+
210+
self.ty_bounds.push(None)
211+
},
212+
213+
(ExprKind::Lit(lit), _) => {
214+
let ty = self.cx.typeck_results().expr_ty(expr);
215+
self.check_lit(lit, ty);
216+
return;
217+
},
218+
219+
(ExprKind::If(cond, then, else_), last_bound) => {
220+
// Cond has no type bound in any situation.
221+
self.ty_bounds.push(None);
222+
self.visit_expr(cond);
223+
self.ty_bounds.pop();
224+
225+
// Propagate current bound to childs.
226+
self.ty_bounds.push(last_bound);
227+
self.visit_expr(then);
228+
if let Some(else_) = else_ {
229+
self.visit_expr(else_);
230+
}
231+
self.ty_bounds.pop();
232+
return;
233+
},
234+
235+
(ExprKind::Loop(_, label, ..), last_bound) => {
236+
self.break_ty_bounds.push((*label, last_bound));
237+
walk_expr(self, expr);
238+
self.break_ty_bounds.pop();
239+
return;
240+
},
241+
242+
(ExprKind::Match(arg, arms, _), last_bound) => {
243+
// Match argument has no type bound.
244+
self.ty_bounds.push(None);
245+
self.visit_expr(arg);
246+
for arm in arms.iter() {
247+
self.visit_pat(arm.pat);
248+
if let Some(Guard::If(guard)) = arm.guard {
249+
self.visit_expr(guard);
250+
}
251+
}
252+
self.ty_bounds.pop();
253+
254+
// Propagate current bound.
255+
self.ty_bounds.push(last_bound);
256+
for arm in arms.iter() {
257+
self.visit_expr(arm.body);
258+
}
259+
self.ty_bounds.pop();
260+
return;
261+
},
262+
263+
(ExprKind::Block(..), last_bound) => self.ty_bounds.push(last_bound),
264+
265+
(ExprKind::Break(destination, _), _) => {
266+
let ty = destination.label.map_or_else(
267+
|| self.break_ty_bounds.last().unwrap().1,
268+
|dest_label| {
269+
self.break_ty_bounds
270+
.iter()
271+
.rev()
272+
.find_map(|(loop_label, ty)| {
273+
loop_label.map_or(None, |loop_label| {
274+
if loop_label.ident == dest_label.ident {
275+
Some(*ty)
276+
} else {
277+
None
278+
}
279+
})
280+
})
281+
.unwrap()
282+
},
283+
);
284+
self.ty_bounds.push(ty);
285+
},
286+
287+
(ExprKind::Ret(_), _) => self.ty_bounds.push(self.ret_ty_bound),
288+
289+
(ExprKind::Struct(qpath, fields, base), _) => {
290+
if_chain! {
291+
if let Some(def_id) = self.cx.qpath_res(qpath, expr.hir_id).opt_def_id();
292+
let ty = self.cx.tcx.type_of(def_id);
293+
if let Some(adt_def) = ty.ty_adt_def();
294+
if adt_def.is_struct();
295+
if let Some(variant) = adt_def.variants.iter().next();
296+
then {
297+
let fields_def = &variant.fields;
298+
299+
// Push field type then visit each field expr.
300+
for field in fields.iter() {
301+
let field_ty =
302+
fields_def
303+
.iter()
304+
.find_map(|f_def| {
305+
if f_def.ident == field.ident
306+
{ Some(self.cx.tcx.type_of(f_def.did)) }
307+
else { None }
308+
});
309+
self.ty_bounds.push(field_ty);
310+
self.visit_expr(field.expr);
311+
self.ty_bounds.pop();
312+
}
313+
314+
// Visit base with no bound.
315+
if let Some(base) = base {
316+
self.ty_bounds.push(None);
317+
self.visit_expr(base);
318+
self.ty_bounds.pop();
319+
}
320+
return;
321+
}
322+
}
323+
self.ty_bounds.push(None);
324+
},
325+
326+
_ => self.ty_bounds.push(None),
327+
}
328+
329+
walk_expr(self, expr);
330+
self.ty_bounds.pop();
331+
}
332+
333+
fn visit_stmt(&mut self, stmt: &'tcx Stmt<'_>) {
334+
match stmt.kind {
335+
StmtKind::Local(local) => {
336+
let ty = local.ty.and_then(|hir_ty| {
337+
let mut infer_ty_finder = InferTyFinder::new();
338+
infer_ty_finder.visit_ty(hir_ty);
339+
if infer_ty_finder.found {
340+
None
341+
} else {
342+
self.cx.typeck_results().node_type_opt(hir_ty.hir_id)
343+
}
344+
});
345+
self.ty_bounds.push(ty);
346+
},
347+
348+
_ => self.ty_bounds.push(None),
349+
}
350+
351+
walk_stmt(self, stmt);
352+
self.ty_bounds.pop();
353+
}
354+
355+
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
356+
NestedVisitorMap::None
357+
}
358+
}
359+
360+
/// Find `hir::TyKind::Infer` is included in passed typed.
361+
struct InferTyFinder {
362+
found: bool,
363+
}
364+
365+
impl InferTyFinder {
366+
fn new() -> Self {
367+
Self { found: false }
368+
}
369+
}
370+
371+
impl<'tcx> Visitor<'tcx> for InferTyFinder {
372+
type Map = Map<'tcx>;
373+
374+
fn visit_ty(&mut self, ty: &'tcx hir::Ty<'_>) {
375+
match ty.kind {
376+
hir::TyKind::Infer => {
377+
self.found = true;
378+
},
379+
_ => {
380+
walk_ty(self, ty);
381+
},
382+
}
383+
}
384+
385+
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
386+
NestedVisitorMap::None
387+
}
388+
}

0 commit comments

Comments
 (0)