Skip to content

Commit e2753f9

Browse files
committed
Auto merge of #6662 - Y-Nak:default-numeric-fallback, r=flip1995
New lint: default_numeric_fallback fixes #6064 r? `@flip1995` As we discussed in [here](https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/Issue.20.236064/near/224647188) and [here](https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/Issue.20clippy.236064/near/224746333), I start implementing this lint from the strictest version. In this PR, I'll allow the below two cases to pass the lint to reduce FPs. 1. Appearances of unsuffixed numeric literals in `Local` if `Local` has a type annotation, for example: ```rust // Good. let x: i32 = 1; // Also good. let x: (i32, i32) = if cond { (1, 2) } else { (2, 3) }; ``` 2. Appearances of unsuffixed numeric literals in args of `Call` or `MethodCall` if corresponding arguments of their signature have concrete types, for example: ```rust fn foo_mono(x: i32) -> i32 { x } fn foo_poly<T>(t: T) -> t { t } // Good. let x = foo_mono(13); // Still bad. let x: i32 = foo_poly(13); ``` changelog: Added restriction lint: `default_numeric_fallback`
2 parents f28c54c + 9b0c1eb commit e2753f9

File tree

5 files changed

+525
-0
lines changed

5 files changed

+525
-0
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -2027,6 +2027,7 @@ Released 2018-09-13
20272027
[`debug_assert_with_mut_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#debug_assert_with_mut_call
20282028
[`decimal_literal_representation`]: https://rust-lang.github.io/rust-clippy/master/index.html#decimal_literal_representation
20292029
[`declare_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#declare_interior_mutable_const
2030+
[`default_numeric_fallback`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_numeric_fallback
20302031
[`default_trait_access`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_trait_access
20312032
[`deprecated_cfg_attr`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_cfg_attr
20322033
[`deprecated_semver`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_semver
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,237 @@
1+
use rustc_ast::ast::{LitFloatType, LitIntType, LitKind};
2+
use rustc_errors::Applicability;
3+
use rustc_hir::{
4+
intravisit::{walk_expr, walk_stmt, NestedVisitorMap, Visitor},
5+
Body, Expr, ExprKind, HirId, Lit, Stmt, StmtKind,
6+
};
7+
use rustc_lint::{LateContext, LateLintPass};
8+
use rustc_middle::{
9+
hir::map::Map,
10+
ty::{self, FloatTy, IntTy, PolyFnSig, Ty},
11+
};
12+
use rustc_session::{declare_lint_pass, declare_tool_lint};
13+
14+
use if_chain::if_chain;
15+
16+
use crate::utils::{snippet, span_lint_and_sugg};
17+
18+
declare_clippy_lint! {
19+
/// **What it does:** Checks for usage of unconstrained numeric literals which may cause default numeric fallback in type
20+
/// inference.
21+
///
22+
/// Default numeric fallback means that if numeric types have not yet been bound to concrete
23+
/// types at the end of type inference, then integer type is bound to `i32`, and similarly
24+
/// floating type is bound to `f64`.
25+
///
26+
/// See [RFC0212](https://github.com/rust-lang/rfcs/blob/master/text/0212-restore-int-fallback.md) for more information about the fallback.
27+
///
28+
/// **Why is this bad?** For those who are very careful about types, default numeric fallback
29+
/// can be a pitfall that cause unexpected runtime behavior.
30+
///
31+
/// **Known problems:** This lint can only be allowed at the function level or above.
32+
///
33+
/// **Example:**
34+
/// ```rust
35+
/// let i = 10;
36+
/// let f = 1.23;
37+
/// ```
38+
///
39+
/// Use instead:
40+
/// ```rust
41+
/// let i = 10i32;
42+
/// let f = 1.23f64;
43+
/// ```
44+
pub DEFAULT_NUMERIC_FALLBACK,
45+
restriction,
46+
"usage of unconstrained numeric literals which may cause default numeric fallback."
47+
}
48+
49+
declare_lint_pass!(DefaultNumericFallback => [DEFAULT_NUMERIC_FALLBACK]);
50+
51+
impl LateLintPass<'_> for DefaultNumericFallback {
52+
fn check_body(&mut self, cx: &LateContext<'tcx>, body: &'tcx Body<'_>) {
53+
let mut visitor = NumericFallbackVisitor::new(cx);
54+
visitor.visit_body(body);
55+
}
56+
}
57+
58+
struct NumericFallbackVisitor<'a, 'tcx> {
59+
/// Stack manages type bound of exprs. The top element holds current expr type.
60+
ty_bounds: Vec<TyBound<'tcx>>,
61+
62+
cx: &'a LateContext<'tcx>,
63+
}
64+
65+
impl<'a, 'tcx> NumericFallbackVisitor<'a, 'tcx> {
66+
fn new(cx: &'a LateContext<'tcx>) -> Self {
67+
Self {
68+
ty_bounds: vec![TyBound::Nothing],
69+
cx,
70+
}
71+
}
72+
73+
/// Check whether a passed literal has potential to cause fallback or not.
74+
fn check_lit(&self, lit: &Lit, lit_ty: Ty<'tcx>) {
75+
if_chain! {
76+
if let Some(ty_bound) = self.ty_bounds.last();
77+
if matches!(lit.node,
78+
LitKind::Int(_, LitIntType::Unsuffixed) | LitKind::Float(_, LitFloatType::Unsuffixed));
79+
if !ty_bound.is_integral();
80+
then {
81+
let suffix = match lit_ty.kind() {
82+
ty::Int(IntTy::I32) => "i32",
83+
ty::Float(FloatTy::F64) => "f64",
84+
// Default numeric fallback never results in other types.
85+
_ => return,
86+
};
87+
88+
let sugg = format!("{}_{}", snippet(self.cx, lit.span, ""), suffix);
89+
span_lint_and_sugg(
90+
self.cx,
91+
DEFAULT_NUMERIC_FALLBACK,
92+
lit.span,
93+
"default numeric fallback might occur",
94+
"consider adding suffix",
95+
sugg,
96+
Applicability::MaybeIncorrect,
97+
);
98+
}
99+
}
100+
}
101+
}
102+
103+
impl<'a, 'tcx> Visitor<'tcx> for NumericFallbackVisitor<'a, 'tcx> {
104+
type Map = Map<'tcx>;
105+
106+
#[allow(clippy::too_many_lines)]
107+
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
108+
match &expr.kind {
109+
ExprKind::Call(func, args) => {
110+
if let Some(fn_sig) = fn_sig_opt(self.cx, func.hir_id) {
111+
for (expr, bound) in args.iter().zip(fn_sig.skip_binder().inputs().iter()) {
112+
// Push found arg type, then visit arg.
113+
self.ty_bounds.push(TyBound::Ty(bound));
114+
self.visit_expr(expr);
115+
self.ty_bounds.pop();
116+
}
117+
return;
118+
}
119+
},
120+
121+
ExprKind::MethodCall(_, _, args, _) => {
122+
if let Some(def_id) = self.cx.typeck_results().type_dependent_def_id(expr.hir_id) {
123+
let fn_sig = self.cx.tcx.fn_sig(def_id).skip_binder();
124+
for (expr, bound) in args.iter().zip(fn_sig.inputs().iter()) {
125+
self.ty_bounds.push(TyBound::Ty(bound));
126+
self.visit_expr(expr);
127+
self.ty_bounds.pop();
128+
}
129+
return;
130+
}
131+
},
132+
133+
ExprKind::Struct(qpath, fields, base) => {
134+
if_chain! {
135+
if let Some(def_id) = self.cx.qpath_res(qpath, expr.hir_id).opt_def_id();
136+
let ty = self.cx.tcx.type_of(def_id);
137+
if let Some(adt_def) = ty.ty_adt_def();
138+
if adt_def.is_struct();
139+
if let Some(variant) = adt_def.variants.iter().next();
140+
then {
141+
let fields_def = &variant.fields;
142+
143+
// Push field type then visit each field expr.
144+
for field in fields.iter() {
145+
let bound =
146+
fields_def
147+
.iter()
148+
.find_map(|f_def| {
149+
if f_def.ident == field.ident
150+
{ Some(self.cx.tcx.type_of(f_def.did)) }
151+
else { None }
152+
});
153+
self.ty_bounds.push(bound.into());
154+
self.visit_expr(field.expr);
155+
self.ty_bounds.pop();
156+
}
157+
158+
// Visit base with no bound.
159+
if let Some(base) = base {
160+
self.ty_bounds.push(TyBound::Nothing);
161+
self.visit_expr(base);
162+
self.ty_bounds.pop();
163+
}
164+
return;
165+
}
166+
}
167+
},
168+
169+
ExprKind::Lit(lit) => {
170+
let ty = self.cx.typeck_results().expr_ty(expr);
171+
self.check_lit(lit, ty);
172+
return;
173+
},
174+
175+
_ => {},
176+
}
177+
178+
walk_expr(self, expr);
179+
}
180+
181+
fn visit_stmt(&mut self, stmt: &'tcx Stmt<'_>) {
182+
match stmt.kind {
183+
StmtKind::Local(local) => {
184+
if local.ty.is_some() {
185+
self.ty_bounds.push(TyBound::Any)
186+
} else {
187+
self.ty_bounds.push(TyBound::Nothing)
188+
}
189+
},
190+
191+
_ => self.ty_bounds.push(TyBound::Nothing),
192+
}
193+
194+
walk_stmt(self, stmt);
195+
self.ty_bounds.pop();
196+
}
197+
198+
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
199+
NestedVisitorMap::None
200+
}
201+
}
202+
203+
fn fn_sig_opt<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId) -> Option<PolyFnSig<'tcx>> {
204+
let node_ty = cx.typeck_results().node_type_opt(hir_id)?;
205+
// We can't use `TyS::fn_sig` because it automatically performs substs, this may result in FNs.
206+
match node_ty.kind() {
207+
ty::FnDef(def_id, _) => Some(cx.tcx.fn_sig(*def_id)),
208+
ty::FnPtr(fn_sig) => Some(*fn_sig),
209+
_ => None,
210+
}
211+
}
212+
213+
#[derive(Debug, Clone, Copy)]
214+
enum TyBound<'tcx> {
215+
Any,
216+
Ty(Ty<'tcx>),
217+
Nothing,
218+
}
219+
220+
impl<'tcx> TyBound<'tcx> {
221+
fn is_integral(self) -> bool {
222+
match self {
223+
TyBound::Any => true,
224+
TyBound::Ty(t) => t.is_integral(),
225+
TyBound::Nothing => false,
226+
}
227+
}
228+
}
229+
230+
impl<'tcx> From<Option<Ty<'tcx>>> for TyBound<'tcx> {
231+
fn from(v: Option<Ty<'tcx>>) -> Self {
232+
match v {
233+
Some(t) => TyBound::Ty(t),
234+
None => TyBound::Nothing,
235+
}
236+
}
237+
}

clippy_lints/src/lib.rs

+4
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ mod copy_iterator;
181181
mod create_dir;
182182
mod dbg_macro;
183183
mod default;
184+
mod default_numeric_fallback;
184185
mod dereference;
185186
mod derive;
186187
mod disallowed_method;
@@ -585,6 +586,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
585586
&dbg_macro::DBG_MACRO,
586587
&default::DEFAULT_TRAIT_ACCESS,
587588
&default::FIELD_REASSIGN_WITH_DEFAULT,
589+
&default_numeric_fallback::DEFAULT_NUMERIC_FALLBACK,
588590
&dereference::EXPLICIT_DEREF_METHODS,
589591
&derive::DERIVE_HASH_XOR_EQ,
590592
&derive::DERIVE_ORD_XOR_PARTIAL_ORD,
@@ -1031,6 +1033,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10311033
store.register_late_pass(|| box strings::StringAdd);
10321034
store.register_late_pass(|| box implicit_return::ImplicitReturn);
10331035
store.register_late_pass(|| box implicit_saturating_sub::ImplicitSaturatingSub);
1036+
store.register_late_pass(|| box default_numeric_fallback::DefaultNumericFallback);
10341037

10351038
let msrv = conf.msrv.as_ref().and_then(|s| {
10361039
parse_msrv(s, None, None).or_else(|| {
@@ -1265,6 +1268,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
12651268
LintId::of(&asm_syntax::INLINE_ASM_X86_INTEL_SYNTAX),
12661269
LintId::of(&create_dir::CREATE_DIR),
12671270
LintId::of(&dbg_macro::DBG_MACRO),
1271+
LintId::of(&default_numeric_fallback::DEFAULT_NUMERIC_FALLBACK),
12681272
LintId::of(&else_if_without_else::ELSE_IF_WITHOUT_ELSE),
12691273
LintId::of(&exhaustive_items::EXHAUSTIVE_ENUMS),
12701274
LintId::of(&exhaustive_items::EXHAUSTIVE_STRUCTS),

0 commit comments

Comments
 (0)