diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | clippy_lints/src/default_numeric_fallback.rs | 237 | ||||
| -rw-r--r-- | clippy_lints/src/lib.rs | 4 | ||||
| -rw-r--r-- | tests/ui/default_numeric_fallback.rs | 135 | ||||
| -rw-r--r-- | tests/ui/default_numeric_fallback.stderr | 148 |
5 files changed, 525 insertions, 0 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 4dcf9c3529b..c59eae5d1c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2027,6 +2027,7 @@ Released 2018-09-13 [`debug_assert_with_mut_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#debug_assert_with_mut_call [`decimal_literal_representation`]: https://rust-lang.github.io/rust-clippy/master/index.html#decimal_literal_representation [`declare_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#declare_interior_mutable_const +[`default_numeric_fallback`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_numeric_fallback [`default_trait_access`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_trait_access [`deprecated_cfg_attr`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_cfg_attr [`deprecated_semver`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_semver diff --git a/clippy_lints/src/default_numeric_fallback.rs b/clippy_lints/src/default_numeric_fallback.rs new file mode 100644 index 00000000000..6ace9aa6bdf --- /dev/null +++ b/clippy_lints/src/default_numeric_fallback.rs @@ -0,0 +1,237 @@ +use rustc_ast::ast::{LitFloatType, LitIntType, LitKind}; +use rustc_errors::Applicability; +use rustc_hir::{ + intravisit::{walk_expr, walk_stmt, NestedVisitorMap, Visitor}, + Body, Expr, ExprKind, HirId, Lit, Stmt, StmtKind, +}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::{ + hir::map::Map, + ty::{self, FloatTy, IntTy, PolyFnSig, Ty}, +}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +use if_chain::if_chain; + +use crate::utils::{snippet, span_lint_and_sugg}; + +declare_clippy_lint! { + /// **What it does:** Checks for usage of unconstrained numeric literals which may cause default numeric fallback in type + /// inference. + /// + /// Default numeric fallback means that if numeric types have not yet been bound to concrete + /// types at the end of type inference, then integer type is bound to `i32`, and similarly + /// floating type is bound to `f64`. + /// + /// See [RFC0212](https://github.com/rust-lang/rfcs/blob/master/text/0212-restore-int-fallback.md) for more information about the fallback. + /// + /// **Why is this bad?** For those who are very careful about types, default numeric fallback + /// can be a pitfall that cause unexpected runtime behavior. + /// + /// **Known problems:** This lint can only be allowed at the function level or above. + /// + /// **Example:** + /// ```rust + /// let i = 10; + /// let f = 1.23; + /// ``` + /// + /// Use instead: + /// ```rust + /// let i = 10i32; + /// let f = 1.23f64; + /// ``` + pub DEFAULT_NUMERIC_FALLBACK, + restriction, + "usage of unconstrained numeric literals which may cause default numeric fallback." +} + +declare_lint_pass!(DefaultNumericFallback => [DEFAULT_NUMERIC_FALLBACK]); + +impl LateLintPass<'_> for DefaultNumericFallback { + fn check_body(&mut self, cx: &LateContext<'tcx>, body: &'tcx Body<'_>) { + let mut visitor = NumericFallbackVisitor::new(cx); + visitor.visit_body(body); + } +} + +struct NumericFallbackVisitor<'a, 'tcx> { + /// Stack manages type bound of exprs. The top element holds current expr type. + ty_bounds: Vec<TyBound<'tcx>>, + + cx: &'a LateContext<'tcx>, +} + +impl<'a, 'tcx> NumericFallbackVisitor<'a, 'tcx> { + fn new(cx: &'a LateContext<'tcx>) -> Self { + Self { + ty_bounds: vec![TyBound::Nothing], + cx, + } + } + + /// Check whether a passed literal has potential to cause fallback or not. + fn check_lit(&self, lit: &Lit, lit_ty: Ty<'tcx>) { + if_chain! { + if let Some(ty_bound) = self.ty_bounds.last(); + if matches!(lit.node, + LitKind::Int(_, LitIntType::Unsuffixed) | LitKind::Float(_, LitFloatType::Unsuffixed)); + if !ty_bound.is_integral(); + then { + let suffix = match lit_ty.kind() { + ty::Int(IntTy::I32) => "i32", + ty::Float(FloatTy::F64) => "f64", + // Default numeric fallback never results in other types. + _ => return, + }; + + let sugg = format!("{}_{}", snippet(self.cx, lit.span, ""), suffix); + span_lint_and_sugg( + self.cx, + DEFAULT_NUMERIC_FALLBACK, + lit.span, + "default numeric fallback might occur", + "consider adding suffix", + sugg, + Applicability::MaybeIncorrect, + ); + } + } + } +} + +impl<'a, 'tcx> Visitor<'tcx> for NumericFallbackVisitor<'a, 'tcx> { + type Map = Map<'tcx>; + + #[allow(clippy::too_many_lines)] + fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { + match &expr.kind { + ExprKind::Call(func, args) => { + if let Some(fn_sig) = fn_sig_opt(self.cx, func.hir_id) { + for (expr, bound) in args.iter().zip(fn_sig.skip_binder().inputs().iter()) { + // Push found arg type, then visit arg. + self.ty_bounds.push(TyBound::Ty(bound)); + self.visit_expr(expr); + self.ty_bounds.pop(); + } + return; + } + }, + + ExprKind::MethodCall(_, _, args, _) => { + if let Some(def_id) = self.cx.typeck_results().type_dependent_def_id(expr.hir_id) { + let fn_sig = self.cx.tcx.fn_sig(def_id).skip_binder(); + for (expr, bound) in args.iter().zip(fn_sig.inputs().iter()) { + self.ty_bounds.push(TyBound::Ty(bound)); + self.visit_expr(expr); + self.ty_bounds.pop(); + } + return; + } + }, + + ExprKind::Struct(qpath, fields, base) => { + if_chain! { + if let Some(def_id) = self.cx.qpath_res(qpath, expr.hir_id).opt_def_id(); + let ty = self.cx.tcx.type_of(def_id); + if let Some(adt_def) = ty.ty_adt_def(); + if adt_def.is_struct(); + if let Some(variant) = adt_def.variants.iter().next(); + then { + let fields_def = &variant.fields; + + // Push field type then visit each field expr. + for field in fields.iter() { + let bound = + fields_def + .iter() + .find_map(|f_def| { + if f_def.ident == field.ident + { Some(self.cx.tcx.type_of(f_def.did)) } + else { None } + }); + self.ty_bounds.push(bound.into()); + self.visit_expr(field.expr); + self.ty_bounds.pop(); + } + + // Visit base with no bound. + if let Some(base) = base { + self.ty_bounds.push(TyBound::Nothing); + self.visit_expr(base); + self.ty_bounds.pop(); + } + return; + } + } + }, + + ExprKind::Lit(lit) => { + let ty = self.cx.typeck_results().expr_ty(expr); + self.check_lit(lit, ty); + return; + }, + + _ => {}, + } + + walk_expr(self, expr); + } + + fn visit_stmt(&mut self, stmt: &'tcx Stmt<'_>) { + match stmt.kind { + StmtKind::Local(local) => { + if local.ty.is_some() { + self.ty_bounds.push(TyBound::Any) + } else { + self.ty_bounds.push(TyBound::Nothing) + } + }, + + _ => self.ty_bounds.push(TyBound::Nothing), + } + + walk_stmt(self, stmt); + self.ty_bounds.pop(); + } + + fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> { + NestedVisitorMap::None + } +} + +fn fn_sig_opt<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId) -> Option<PolyFnSig<'tcx>> { + let node_ty = cx.typeck_results().node_type_opt(hir_id)?; + // We can't use `TyS::fn_sig` because it automatically performs substs, this may result in FNs. + match node_ty.kind() { + ty::FnDef(def_id, _) => Some(cx.tcx.fn_sig(*def_id)), + ty::FnPtr(fn_sig) => Some(*fn_sig), + _ => None, + } +} + +#[derive(Debug, Clone, Copy)] +enum TyBound<'tcx> { + Any, + Ty(Ty<'tcx>), + Nothing, +} + +impl<'tcx> TyBound<'tcx> { + fn is_integral(self) -> bool { + match self { + TyBound::Any => true, + TyBound::Ty(t) => t.is_integral(), + TyBound::Nothing => false, + } + } +} + +impl<'tcx> From<Option<Ty<'tcx>>> for TyBound<'tcx> { + fn from(v: Option<Ty<'tcx>>) -> Self { + match v { + Some(t) => TyBound::Ty(t), + None => TyBound::Nothing, + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index d96911fac1a..40727980693 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -181,6 +181,7 @@ mod copy_iterator; mod create_dir; mod dbg_macro; mod default; +mod default_numeric_fallback; mod dereference; mod derive; mod disallowed_method; @@ -585,6 +586,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &dbg_macro::DBG_MACRO, &default::DEFAULT_TRAIT_ACCESS, &default::FIELD_REASSIGN_WITH_DEFAULT, + &default_numeric_fallback::DEFAULT_NUMERIC_FALLBACK, &dereference::EXPLICIT_DEREF_METHODS, &derive::DERIVE_HASH_XOR_EQ, &derive::DERIVE_ORD_XOR_PARTIAL_ORD, @@ -1031,6 +1033,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box strings::StringAdd); store.register_late_pass(|| box implicit_return::ImplicitReturn); store.register_late_pass(|| box implicit_saturating_sub::ImplicitSaturatingSub); + store.register_late_pass(|| box default_numeric_fallback::DefaultNumericFallback); let msrv = conf.msrv.as_ref().and_then(|s| { parse_msrv(s, None, None).or_else(|| { @@ -1265,6 +1268,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&asm_syntax::INLINE_ASM_X86_INTEL_SYNTAX), LintId::of(&create_dir::CREATE_DIR), LintId::of(&dbg_macro::DBG_MACRO), + LintId::of(&default_numeric_fallback::DEFAULT_NUMERIC_FALLBACK), LintId::of(&else_if_without_else::ELSE_IF_WITHOUT_ELSE), LintId::of(&exhaustive_items::EXHAUSTIVE_ENUMS), LintId::of(&exhaustive_items::EXHAUSTIVE_STRUCTS), diff --git a/tests/ui/default_numeric_fallback.rs b/tests/ui/default_numeric_fallback.rs new file mode 100644 index 00000000000..0b3758952ac --- /dev/null +++ b/tests/ui/default_numeric_fallback.rs @@ -0,0 +1,135 @@ +#![warn(clippy::default_numeric_fallback)] +#![allow(unused)] +#![allow(clippy::never_loop)] +#![allow(clippy::no_effect)] +#![allow(clippy::unnecessary_operation)] + +mod basic_expr { + fn test() { + // Should lint unsuffixed literals typed `i32`. + let x = 22; + let x = [1, 2, 3]; + let x = if true { (1, 2) } else { (3, 4) }; + let x = match 1 { + 1 => 1, + _ => 2, + }; + + // Should lint unsuffixed literals typed `f64`. + let x = 0.12; + + // Should NOT lint suffixed literals. + let x = 22_i32; + let x = 0.12_f64; + + // Should NOT lint literals in init expr if `Local` has a type annotation. + let x: f64 = 0.1; + let x: [i32; 3] = [1, 2, 3]; + let x: (i32, i32) = if true { (1, 2) } else { (3, 4) }; + let x: _ = 1; + } +} + +mod nested_local { + fn test() { + let x: _ = { + // Should lint this because this literal is not bound to any types. + let y = 1; + + // Should NOT lint this because this literal is bound to `_` of outer `Local`. + 1 + }; + + let x: _ = if true { + // Should lint this because this literal is not bound to any types. + let y = 1; + + // Should NOT lint this because this literal is bound to `_` of outer `Local`. + 1 + } else { + // Should lint this because this literal is not bound to any types. + let y = 1; + + // Should NOT lint this because this literal is bound to `_` of outer `Local`. + 2 + }; + } +} + +mod function_def { + fn ret_i32() -> i32 { + // Even though the output type is specified, + // this unsuffixed literal is linted to reduce heuristics and keep codebase simple. + 1 + } + + fn test() { + // Should lint this because return type is inferred to `i32` and NOT bound to a concrete + // type. + let f = || -> _ { 1 }; + + // Even though the output type is specified, + // this unsuffixed literal is linted to reduce heuristics and keep codebase simple. + let f = || -> i32 { 1 }; + } +} + +mod function_calls { + fn concrete_arg(x: i32) {} + + fn generic_arg<T>(t: T) {} + + fn test() { + // Should NOT lint this because the argument type is bound to a concrete type. + concrete_arg(1); + + // Should lint this because the argument type is inferred to `i32` and NOT bound to a concrete type. + generic_arg(1); + + // Should lint this because the argument type is inferred to `i32` and NOT bound to a concrete type. + let x: _ = generic_arg(1); + } +} + +mod struct_ctor { + struct ConcreteStruct { + x: i32, + } + + struct GenericStruct<T> { + x: T, + } + + fn test() { + // Should NOT lint this because the field type is bound to a concrete type. + ConcreteStruct { x: 1 }; + + // Should lint this because the field type is inferred to `i32` and NOT bound to a concrete type. + GenericStruct { x: 1 }; + + // Should lint this because the field type is inferred to `i32` and NOT bound to a concrete type. + let _ = GenericStruct { x: 1 }; + } +} + +mod method_calls { + struct StructForMethodCallTest {} + + impl StructForMethodCallTest { + fn concrete_arg(&self, x: i32) {} + + fn generic_arg<T>(&self, t: T) {} + } + + fn test() { + let s = StructForMethodCallTest {}; + + // Should NOT lint this because the argument type is bound to a concrete type. + s.concrete_arg(1); + + // Should lint this because the argument type is bound to a concrete type. + s.generic_arg(1); + } +} + +fn main() {} diff --git a/tests/ui/default_numeric_fallback.stderr b/tests/ui/default_numeric_fallback.stderr new file mode 100644 index 00000000000..b31aa4ebcf8 --- /dev/null +++ b/tests/ui/default_numeric_fallback.stderr @@ -0,0 +1,148 @@ +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback.rs:10:17 + | +LL | let x = 22; + | ^^ help: consider adding suffix: `22_i32` + | + = note: `-D clippy::default-numeric-fallback` implied by `-D warnings` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback.rs:11:18 + | +LL | let x = [1, 2, 3]; + | ^ help: consider adding suffix: `1_i32` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback.rs:11:21 + | +LL | let x = [1, 2, 3]; + | ^ help: consider adding suffix: `2_i32` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback.rs:11:24 + | +LL | let x = [1, 2, 3]; + | ^ help: consider adding suffix: `3_i32` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback.rs:12:28 + | +LL | let x = if true { (1, 2) } else { (3, 4) }; + | ^ help: consider adding suffix: `1_i32` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback.rs:12:31 + | +LL | let x = if true { (1, 2) } else { (3, 4) }; + | ^ help: consider adding suffix: `2_i32` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback.rs:12:44 + | +LL | let x = if true { (1, 2) } else { (3, 4) }; + | ^ help: consider adding suffix: `3_i32` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback.rs:12:47 + | +LL | let x = if true { (1, 2) } else { (3, 4) }; + | ^ help: consider adding suffix: `4_i32` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback.rs:13:23 + | +LL | let x = match 1 { + | ^ help: consider adding suffix: `1_i32` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback.rs:14:13 + | +LL | 1 => 1, + | ^ help: consider adding suffix: `1_i32` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback.rs:14:18 + | +LL | 1 => 1, + | ^ help: consider adding suffix: `1_i32` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback.rs:15:18 + | +LL | _ => 2, + | ^ help: consider adding suffix: `2_i32` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback.rs:19:17 + | +LL | let x = 0.12; + | ^^^^ help: consider adding suffix: `0.12_f64` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback.rs:37:21 + | +LL | let y = 1; + | ^ help: consider adding suffix: `1_i32` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback.rs:45:21 + | +LL | let y = 1; + | ^ help: consider adding suffix: `1_i32` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback.rs:51:21 + | +LL | let y = 1; + | ^ help: consider adding suffix: `1_i32` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback.rs:63:9 + | +LL | 1 + | ^ help: consider adding suffix: `1_i32` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback.rs:69:27 + | +LL | let f = || -> _ { 1 }; + | ^ help: consider adding suffix: `1_i32` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback.rs:73:29 + | +LL | let f = || -> i32 { 1 }; + | ^ help: consider adding suffix: `1_i32` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback.rs:87:21 + | +LL | generic_arg(1); + | ^ help: consider adding suffix: `1_i32` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback.rs:90:32 + | +LL | let x: _ = generic_arg(1); + | ^ help: consider adding suffix: `1_i32` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback.rs:108:28 + | +LL | GenericStruct { x: 1 }; + | ^ help: consider adding suffix: `1_i32` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback.rs:111:36 + | +LL | let _ = GenericStruct { x: 1 }; + | ^ help: consider adding suffix: `1_i32` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback.rs:131:23 + | +LL | s.generic_arg(1); + | ^ help: consider adding suffix: `1_i32` + +error: aborting due to 24 previous errors + |