Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2024-11-28 07:30:57 +00:00
Code Issues Projects Releases Packages Wiki Activity

Simplify DefaultNumericFallback

Browse source
This commit is contained in:
Yoshitomo Nakanishi 2021-02-07 18:00:08 +09:00
parent 8b8b0a94b0
commit 22d4483dee
4 changed files with 61 additions and 612 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

394
clippy_lints/src/default_numeric_fallback.rs
View file

@ -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",
);
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",
)
}
}
}
}
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 nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}
}

2
clippy_lints/src/lib.rs
View file

@ -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(|| {

101
tests/ui/default_numeric_fallback.rs
View file

@ -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 };
}

176
tests/ui/default_numeric_fallback.stderr
View file

@ -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
--> $DIR/default_numeric_fallback.rs:8:34
|
LL | break 2;
| ^
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: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
|
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 };
| ^
|
= help: consider adding suffix to avoid default numeric fallback
error: aborting due to 23 previous errors
error: aborting due to 4 previous errors