Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New lint: default_numeric_fallback #6662

Merged
merged 8 commits into from
Feb 16, 2021
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1909,6 +1909,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
Expand Down
228 changes: 228 additions & 0 deletions clippy_lints/src/default_numeric_fallback.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,228 @@
use rustc_ast::ast::{LitFloatType, LitIntType, LitKind};
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::span_lint_and_help;

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 matches!(lit_ty.kind(), ty::Int(IntTy::I32) | ty::Float(FloatTy::F64));
if !ty_bound.is_integral();
then {
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 {
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,
}
}
}
4 changes: 4 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -584,6 +585,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,
Expand Down Expand Up @@ -1026,6 +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);

let msrv = conf.msrv.as_ref().and_then(|s| {
parse_msrv(s, None, None).or_else(|| {
Expand Down Expand Up @@ -1258,6 +1261,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),
Expand Down
Loading