diff options
| author | Yoshitomo Nakanishi <yurayura.rounin.3@gmail.com> | 2021-02-07 18:00:08 +0900 |
|---|---|---|
| committer | Yoshitomo Nakanishi <yurayura.rounin.3@gmail.com> | 2021-02-07 18:11:31 +0900 |
| commit | 22d4483dee55fbf0c9ba48a3ddfd8abe06805c07 (patch) | |
| tree | 933310d1c1c2539641087f2eeaabc23e8667c1a1 | |
| parent | 8b8b0a94b05f7189d028d91deda659ed266ef09d (diff) | |
| download | rust-22d4483dee55fbf0c9ba48a3ddfd8abe06805c07.tar.gz rust-22d4483dee55fbf0c9ba48a3ddfd8abe06805c07.zip | |
Simplify DefaultNumericFallback
| -rw-r--r-- | clippy_lints/src/default_numeric_fallback.rs | 394 | ||||
| -rw-r--r-- | clippy_lints/src/lib.rs | 2 | ||||
| -rw-r--r-- | tests/ui/default_numeric_fallback.rs | 101 | ||||
| -rw-r--r-- | tests/ui/default_numeric_fallback.stderr | 176 |
4 files changed, 61 insertions, 612 deletions
diff --git a/clippy_lints/src/default_numeric_fallback.rs b/clippy_lints/src/default_numeric_fallback.rs index 5f0ee1be2b9..b3b5a4a8251 100644 --- a/clippy_lints/src/default_numeric_fallback.rs +++ b/clippy_lints/src/default_numeric_fallback.rs @@ -1,17 +1,9 @@ -use rustc_ast::ast::{Label, LitFloatType, LitIntType, LitKind}; -use rustc_hir::{ - self as hir, - intravisit::{walk_expr, walk_stmt, walk_ty, FnKind, NestedVisitorMap, Visitor}, - Body, BodyId, Expr, ExprKind, FnDecl, FnRetTy, Guard, HirId, Lit, Stmt, StmtKind, -}; +use rustc_ast::{LitFloatType, LitIntType, LitKind}; +use rustc_data_structures::fx::FxHashSet; +use rustc_hir::{Expr, ExprKind, HirId, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::{ - hir::map::Map, - ty::{self, subst::GenericArgKind, FloatTy, IntTy, Ty, TyCtxt}, -}; -use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::Span; -use rustc_typeck::hir_ty_to_ty; +use rustc_middle::ty::{self, FloatTy, IntTy}; +use rustc_session::{declare_tool_lint, impl_lint_pass}; use if_chain::if_chain; @@ -41,363 +33,53 @@ declare_clippy_lint! { /// Use instead: /// ```rust /// let i = 10i32; - /// let f = 1.23f64; + /// let f: f64 = 1.23; /// ``` pub DEFAULT_NUMERIC_FALLBACK, restriction, "usage of unconstrained numeric literals which may cause default numeric fallback." } -declare_lint_pass!(DefaultNumericFallback => [DEFAULT_NUMERIC_FALLBACK]); - -/// Return the body that includes passed `hir_id` if exists. -fn enclosing_body_opt(tcx: TyCtxt<'_>, hir_id: HirId) -> Option<BodyId> { - let hir_map = tcx.hir(); - let mut trace = vec![(hir_id)]; - - for (parent, _) in hir_map.parent_iter(hir_id) { - trace.push(parent); - if let Some(body) = hir_map.maybe_body_owned_by(parent) { - if trace.iter().any(|hir_id| *hir_id == body.hir_id) { - return Some(body); - } - } - } - - None +#[derive(Default)] +pub struct DefaultNumericFallback { + /// Hold `init` in `Local` if `Local` has a type annotation. + bounded_inits: FxHashSet<HirId>, } -fn ty_from_hir_ty<'tcx>(cx: &LateContext<'tcx>, hir_ty: &hir::Ty<'tcx>) -> Option<Ty<'tcx>> { - if enclosing_body_opt(cx.tcx, hir_ty.hir_id).is_some() { - cx.typeck_results().node_type_opt(hir_ty.hir_id) - } else { - Some(hir_ty_to_ty(cx.tcx, hir_ty)) - } -} +impl_lint_pass!(DefaultNumericFallback => [DEFAULT_NUMERIC_FALLBACK]); impl LateLintPass<'_> for DefaultNumericFallback { - fn check_fn( - &mut self, - cx: &LateContext<'tcx>, - _: FnKind<'tcx>, - fn_decl: &'tcx FnDecl<'_>, - body: &'tcx Body<'_>, - _: Span, - _: HirId, - ) { - let ret_ty_bound = match fn_decl.output { - FnRetTy::DefaultReturn(_) => None, - FnRetTy::Return(ty) => Some(ty), - } - .and_then(|ty| { - if is_infer_included(ty) { - None - } else { - ty_from_hir_ty(cx, ty) + fn check_stmt(&mut self, _: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) { + if_chain! { + if let StmtKind::Local(local) = stmt.kind; + if local.ty.is_some(); + if let Some(init) = local.init; + then { + self.bounded_inits.insert(init.hir_id); } - }); - - let mut visitor = NumericFallbackVisitor::new(ret_ty_bound, 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<Option<Ty<'tcx>>>, - - /// Ret type bound. - ret_ty_bound: Option<Ty<'tcx>>, - - /// Break type bounds. - break_ty_bounds: Vec<(Option<Label>, Option<Ty<'tcx>>)>, - - cx: &'a LateContext<'tcx>, -} - -impl<'a, 'tcx> NumericFallbackVisitor<'a, 'tcx> { - fn new(ret_ty_bound: Option<Ty<'tcx>>, cx: &'a LateContext<'tcx>) -> Self { - Self { - ty_bounds: vec![ret_ty_bound], - ret_ty_bound, - break_ty_bounds: vec![], - cx, } } - /// Check whether a passed literal has potential to cause fallback or not. - fn check_lit(&self, lit: &Lit, lit_ty: Ty<'tcx>) { - let ty_bound = self.ty_bounds.last().unwrap(); - - let should_lint = match (&lit.node, lit_ty.kind()) { - (LitKind::Int(_, LitIntType::Unsuffixed), ty::Int(ty::IntTy::I32)) => { - // In case integer literal is explicitly bound to i32, then suppress lint. - ty_bound.map_or(true, |ty_bound| !matches!(ty_bound.kind(), ty::Int(IntTy::I32))) - }, - - (LitKind::Float(_, LitFloatType::Unsuffixed), ty::Float(ty::FloatTy::F64)) => { - // In case float literal is explicitly bound to f64, then suppress lint. - ty_bound.map_or(true, |ty_bound| !matches!(ty_bound.kind(), ty::Float(FloatTy::F64))) - }, - - _ => false, - }; - - if should_lint { - span_lint_and_help( - self.cx, - DEFAULT_NUMERIC_FALLBACK, - lit.span, - "default numeric fallback might occur", - None, - "consider adding suffix to avoid default numeric fallback", - ); - } - } -} - -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, *self.ty_bounds.last().unwrap()) { - (ExprKind::Array(_), Some(last_bound)) => { - if let ty::Array(ty, _) = last_bound.kind() { - self.ty_bounds.push(Some(ty)) - } else { - self.ty_bounds.push(None) - } - }, - - (ExprKind::Call(func, args), _) => { - if_chain! { - if let ExprKind::Path(ref func_path) = func.kind; - if let Some(def_id) = self.cx.qpath_res(func_path, func.hir_id).opt_def_id(); - then { - let fn_sig = self.cx.tcx.fn_sig(def_id).skip_binder(); - for (expr, bound) in args.iter().zip(fn_sig.inputs().iter()) { - // Push found arg type, then visit arg. - self.ty_bounds.push(Some(bound)); - self.visit_expr(expr); - self.ty_bounds.pop(); - } - return; - } else { - self.ty_bounds.push(None) - } - } - }, - - (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(Some(bound)); - self.visit_expr(expr); - self.ty_bounds.pop(); - } - return; - } - - self.ty_bounds.push(None) - }, - - (ExprKind::Tup(exprs), Some(last_bound)) => { - if let ty::Tuple(tys) = last_bound.kind() { - for (expr, bound) in exprs.iter().zip(tys.iter()) { - if let GenericArgKind::Type(ty) = bound.unpack() { - self.ty_bounds.push(Some(ty)); - } else { - self.ty_bounds.push(None); - } - - self.visit_expr(expr); - self.ty_bounds.pop(); - } - return; - } - - self.ty_bounds.push(None) - }, - - (ExprKind::Lit(lit), _) => { - let ty = self.cx.typeck_results().expr_ty(expr); - self.check_lit(lit, ty); - return; - }, - - (ExprKind::If(cond, then, else_), last_bound) => { - // Cond has no type bound in any situation. - self.ty_bounds.push(None); - self.visit_expr(cond); - self.ty_bounds.pop(); - - // Propagate current bound to childs. - self.ty_bounds.push(last_bound); - self.visit_expr(then); - if let Some(else_) = else_ { - self.visit_expr(else_); - } - self.ty_bounds.pop(); - return; - }, - - (ExprKind::Loop(_, label, ..), last_bound) => { - self.break_ty_bounds.push((*label, last_bound)); - walk_expr(self, expr); - self.break_ty_bounds.pop(); - return; - }, - - (ExprKind::Match(arg, arms, _), last_bound) => { - // Match argument has no type bound. - self.ty_bounds.push(None); - self.visit_expr(arg); - for arm in arms.iter() { - self.visit_pat(arm.pat); - if let Some(Guard::If(guard)) = arm.guard { - self.visit_expr(guard); - } - } - self.ty_bounds.pop(); - - // Propagate current bound. - self.ty_bounds.push(last_bound); - for arm in arms.iter() { - self.visit_expr(arm.body); - } - self.ty_bounds.pop(); - return; - }, - - (ExprKind::Block(..), last_bound) => self.ty_bounds.push(last_bound), - - (ExprKind::Break(destination, _), _) => { - let ty = destination.label.map_or_else( - || self.break_ty_bounds.last().unwrap().1, - |dest_label| { - self.break_ty_bounds - .iter() - .rev() - .find_map(|(loop_label, ty)| { - loop_label.map_or(None, |loop_label| { - if loop_label.ident == dest_label.ident { - Some(*ty) - } else { - None - } - }) - }) - .unwrap() - }, - ); - self.ty_bounds.push(ty); - }, - - (ExprKind::Ret(_), _) => self.ty_bounds.push(self.ret_ty_bound), - - (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 field_ty = - 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(field_ty); - self.visit_expr(field.expr); - self.ty_bounds.pop(); - } - - // Visit base with no bound. - if let Some(base) = base { - self.ty_bounds.push(None); - self.visit_expr(base); - self.ty_bounds.pop(); - } - return; - } - } - self.ty_bounds.push(None); - }, - - _ => self.ty_bounds.push(None), - } - - walk_expr(self, expr); - self.ty_bounds.pop(); - } - - fn visit_stmt(&mut self, stmt: &'tcx Stmt<'_>) { - match stmt.kind { - StmtKind::Local(local) => { - let ty = local.ty.and_then(|hir_ty| { - if is_infer_included(hir_ty) { - None - } else { - ty_from_hir_ty(self.cx, hir_ty) - } - }); - self.ty_bounds.push(ty); - }, - - _ => self.ty_bounds.push(None), - } - - walk_stmt(self, stmt); - self.ty_bounds.pop(); - } - - fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> { - NestedVisitorMap::None - } -} - -/// Return true if a given ty includes `hir::TyKind::Infer`. -fn is_infer_included(ty: &hir::Ty<'_>) -> bool { - let mut infer_ty_finder = InferTyFinder::new(); - infer_ty_finder.visit_ty(ty); - infer_ty_finder.found -} - -struct InferTyFinder { - found: bool, -} - -impl InferTyFinder { - fn new() -> Self { - Self { found: false } - } -} - -impl<'tcx> Visitor<'tcx> for InferTyFinder { - type Map = Map<'tcx>; - - fn visit_ty(&mut self, ty: &'tcx hir::Ty<'_>) { - match ty.kind { - hir::TyKind::Infer => { - self.found = true; - }, - _ => { - walk_ty(self, ty); - }, + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + let expr_ty = cx.typeck_results().expr_ty(expr); + let hir_id = expr.hir_id; + if_chain! { + if let ExprKind::Lit(ref lit) = expr.kind; + if matches!(lit.node, + LitKind::Int(_, LitIntType::Unsuffixed) | LitKind::Float(_, LitFloatType::Unsuffixed)); + if matches!(expr_ty.kind(), ty::Int(IntTy::I32) | ty::Float(FloatTy::F64)); + if !self.bounded_inits.contains(&hir_id); + if !cx.tcx.hir().parent_iter(hir_id).any(|(ref hir_id, _)| self.bounded_inits.contains(hir_id)); + then { + span_lint_and_help( + cx, + DEFAULT_NUMERIC_FALLBACK, + lit.span, + "default numeric fallback might occur", + None, + "consider adding suffix to avoid default numeric fallback", + ) + } } } - - fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> { - NestedVisitorMap::None - } } diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 642c1e68dac..6590613b93a 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1028,7 +1028,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); + store.register_late_pass(|| box default_numeric_fallback::DefaultNumericFallback::default()); let msrv = conf.msrv.as_ref().and_then(|s| { parse_msrv(s, None, None).or_else(|| { diff --git a/tests/ui/default_numeric_fallback.rs b/tests/ui/default_numeric_fallback.rs index 90ee8ea0399..5ca5bc1e907 100644 --- a/tests/ui/default_numeric_fallback.rs +++ b/tests/ui/default_numeric_fallback.rs @@ -1,99 +1,18 @@ #![warn(clippy::default_numeric_fallback)] #![allow(unused)] -#![allow(clippy::never_loop)] -#![allow(clippy::no_effect)] -#![allow(clippy::unnecessary_operation)] - -fn ret_i31() -> i32 { - 23 -} - -fn concrete_arg(x: i32) {} - -fn generic_arg<T>(t: T) {} - -struct ConcreteStruct { - x: i32, -} - -struct GenericStruct<T> { - x: T, -} - -struct StructForMethodCallTest { - x: i32, -} - -impl StructForMethodCallTest { - fn concrete_arg(&self, x: i32) {} - - fn generic_arg<T>(&self, t: T) {} -} fn main() { - let s = StructForMethodCallTest { x: 10_i32 }; - // Bad. - let x = 22; - let x = 0.12; - let x: _ = 13; - let x: [_; 3] = [1, 2, 3]; - let x: (_, i32) = (1, 2); - - let x = if true { (1, 2) } else { (3, 4) }; - - let x = match 1 { - 1 => 1, - _ => 2, - }; - - let x = loop { - break 1; - }; - - let x = 'outer0: loop { - { - 'inner0: loop { - break 3; - } - }; - break 2; - }; - - let x = GenericStruct { x: 1 }; - - generic_arg(10); - s.generic_arg(10); - let f = || -> _ { 1 }; + let x = 1; + let x = 0.1; + let x = if true { 1 } else { 2 }; // Good. - let x = 22_i32; - let x: f64 = 0.12; - let x = 0.12_f64; - let x: i32 = 13; - let x: [i32; 3] = [1, 2, 3]; - let x: (i32, i32) = (1, 2); - - let x: (i32, i32) = if true { (1, 2) } else { (3, 4) }; - - let x: i32 = match true { - true => 1, - _ => 2, - }; - - let x: i32 = loop { - break 1; - }; - - let x: i32 = 'outer1: loop { - 'inner1: loop { - break 'outer1 3; - } - }; - - let x = ConcreteStruct { x: 1 }; - - concrete_arg(10); - s.concrete_arg(10); - let f = || -> i32 { 1 }; + let x = 1_i32; + let x: i32 = 1; + let x: _ = 1; + let x = 0.1_f64; + let x: f64 = 0.1; + let x: _ = 0.1; + let x: _ = if true { 1 } else { 2 }; } diff --git a/tests/ui/default_numeric_fallback.stderr b/tests/ui/default_numeric_fallback.stderr index 27629b994be..c36409a052c 100644 --- a/tests/ui/default_numeric_fallback.stderr +++ b/tests/ui/default_numeric_fallback.stderr @@ -1,187 +1,35 @@ error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:37:13 + --> $DIR/default_numeric_fallback.rs:6:13 | -LL | let x = 22; - | ^^ +LL | let x = 1; + | ^ | = note: `-D clippy::default-numeric-fallback` implied by `-D warnings` = help: consider adding suffix to avoid default numeric fallback error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:38:13 + --> $DIR/default_numeric_fallback.rs:7:13 | -LL | let x = 0.12; - | ^^^^ +LL | let x = 0.1; + | ^^^ | = help: consider adding suffix to avoid default numeric fallback error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:39:16 + --> $DIR/default_numeric_fallback.rs:8:23 | -LL | let x: _ = 13; - | ^^ - | - = help: consider adding suffix to avoid default numeric fallback - -error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:40:22 - | -LL | let x: [_; 3] = [1, 2, 3]; - | ^ - | - = help: consider adding suffix to avoid default numeric fallback - -error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:40:25 - | -LL | let x: [_; 3] = [1, 2, 3]; - | ^ - | - = help: consider adding suffix to avoid default numeric fallback - -error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:40:28 - | -LL | let x: [_; 3] = [1, 2, 3]; - | ^ - | - = help: consider adding suffix to avoid default numeric fallback - -error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:41:24 - | -LL | let x: (_, i32) = (1, 2); - | ^ - | - = help: consider adding suffix to avoid default numeric fallback - -error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:41:27 - | -LL | let x: (_, i32) = (1, 2); - | ^ - | - = help: consider adding suffix to avoid default numeric fallback - -error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:43:24 - | -LL | let x = if true { (1, 2) } else { (3, 4) }; - | ^ - | - = help: consider adding suffix to avoid default numeric fallback - -error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:43:27 - | -LL | let x = if true { (1, 2) } else { (3, 4) }; - | ^ - | - = help: consider adding suffix to avoid default numeric fallback - -error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:43:40 - | -LL | let x = if true { (1, 2) } else { (3, 4) }; - | ^ - | - = help: consider adding suffix to avoid default numeric fallback - -error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:43:43 - | -LL | let x = if true { (1, 2) } else { (3, 4) }; - | ^ - | - = help: consider adding suffix to avoid default numeric fallback - -error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:45:19 - | -LL | let x = match 1 { - | ^ - | - = help: consider adding suffix to avoid default numeric fallback - -error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:46:9 - | -LL | 1 => 1, - | ^ - | - = help: consider adding suffix to avoid default numeric fallback - -error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:46:14 - | -LL | 1 => 1, - | ^ - | - = help: consider adding suffix to avoid default numeric fallback - -error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:47:14 - | -LL | _ => 2, - | ^ - | - = help: consider adding suffix to avoid default numeric fallback - -error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:51:15 - | -LL | break 1; - | ^ - | - = help: consider adding suffix to avoid default numeric fallback - -error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:57:23 - | -LL | break 3; +LL | let x = if true { 1 } else { 2 }; | ^ | = help: consider adding suffix to avoid default numeric fallback error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:60:15 - | -LL | break 2; - | ^ - | - = help: consider adding suffix to avoid default numeric fallback - -error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:63:32 - | -LL | let x = GenericStruct { x: 1 }; - | ^ - | - = help: consider adding suffix to avoid default numeric fallback - -error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:65:17 - | -LL | generic_arg(10); - | ^^ - | - = help: consider adding suffix to avoid default numeric fallback - -error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:66:19 + --> $DIR/default_numeric_fallback.rs:8:34 | -LL | s.generic_arg(10); - | ^^ - | - = help: consider adding suffix to avoid default numeric fallback - -error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:67:23 - | -LL | let f = || -> _ { 1 }; - | ^ +LL | let x = if true { 1 } else { 2 }; + | ^ | = help: consider adding suffix to avoid default numeric fallback -error: aborting due to 23 previous errors +error: aborting due to 4 previous errors |
