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

[default_numeric_fallback] do not lint on constants #9636

Merged
merged 3 commits into from
Oct 16, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
65 changes: 29 additions & 36 deletions clippy_lints/src/default_numeric_fallback.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use clippy_utils::diagnostics::span_lint_hir_and_then;
use clippy_utils::numeric_literal;
use clippy_utils::source::snippet_opt;
use clippy_utils::{get_parent_node, numeric_literal};
use if_chain::if_chain;
use rustc_ast::ast::{LitFloatType, LitIntType, LitKind};
use rustc_errors::Applicability;
use rustc_hir::{
intravisit::{walk_expr, walk_stmt, Visitor},
Body, Expr, ExprKind, HirId, Lit, Stmt, StmtKind,
Body, Expr, ExprKind, HirId, ItemKind, Lit, Node, Stmt, StmtKind,
};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::{
Expand Down Expand Up @@ -55,22 +55,31 @@ declare_lint_pass!(DefaultNumericFallback => [DEFAULT_NUMERIC_FALLBACK]);

impl<'tcx> LateLintPass<'tcx> for DefaultNumericFallback {
fn check_body(&mut self, cx: &LateContext<'tcx>, body: &'tcx Body<'_>) {
let mut visitor = NumericFallbackVisitor::new(cx);
let is_parent_const = if let Some(Node::Item(item)) = get_parent_node(cx.tcx, body.id().hir_id) {
matches!(item.kind, ItemKind::Const(..))
} else {
false
};
let mut visitor = NumericFallbackVisitor::new(cx, is_parent_const);
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>>,
ty_bounds: Vec<ExplicitTyBound>,

cx: &'a LateContext<'tcx>,
}

impl<'a, 'tcx> NumericFallbackVisitor<'a, 'tcx> {
fn new(cx: &'a LateContext<'tcx>) -> Self {
fn new(cx: &'a LateContext<'tcx>, is_parent_const: bool) -> Self {
Self {
ty_bounds: vec![TyBound::Nothing],
ty_bounds: vec![if is_parent_const {
ExplicitTyBound(true)
} else {
ExplicitTyBound(false)
}],
cx,
}
}
Expand All @@ -79,10 +88,9 @@ impl<'a, 'tcx> NumericFallbackVisitor<'a, 'tcx> {
fn check_lit(&self, lit: &Lit, lit_ty: Ty<'tcx>, emit_hir_id: HirId) {
if_chain! {
if !in_external_macro(self.cx.sess(), lit.span);
if let Some(ty_bound) = self.ty_bounds.last();
if matches!(self.ty_bounds.last(), Some(ExplicitTyBound(false)));
if matches!(lit.node,
LitKind::Int(_, LitIntType::Unsuffixed) | LitKind::Float(_, LitFloatType::Unsuffixed));
if !ty_bound.is_numeric();
then {
let (suffix, is_float) = match lit_ty.kind() {
ty::Int(IntTy::I32) => ("i32", false),
Expand Down Expand Up @@ -123,7 +131,7 @@ impl<'a, 'tcx> Visitor<'tcx> for NumericFallbackVisitor<'a, 'tcx> {
if let Some(fn_sig) = fn_sig_opt(self.cx, func.hir_id) {
for (expr, bound) in iter::zip(*args, fn_sig.skip_binder().inputs()) {
// Push found arg type, then visit arg.
self.ty_bounds.push(TyBound::Ty(*bound));
self.ty_bounds.push((*bound).into());
self.visit_expr(expr);
self.ty_bounds.pop();
}
Expand All @@ -135,7 +143,7 @@ impl<'a, 'tcx> Visitor<'tcx> for NumericFallbackVisitor<'a, 'tcx> {
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 iter::zip(std::iter::once(*receiver).chain(args.iter()), fn_sig.inputs()) {
self.ty_bounds.push(TyBound::Ty(*bound));
self.ty_bounds.push((*bound).into());
self.visit_expr(expr);
self.ty_bounds.pop();
}
Expand Down Expand Up @@ -169,7 +177,7 @@ impl<'a, 'tcx> Visitor<'tcx> for NumericFallbackVisitor<'a, 'tcx> {

// Visit base with no bound.
if let Some(base) = base {
self.ty_bounds.push(TyBound::Nothing);
self.ty_bounds.push(ExplicitTyBound(false));
self.visit_expr(base);
self.ty_bounds.pop();
}
Expand All @@ -192,15 +200,10 @@ impl<'a, 'tcx> Visitor<'tcx> for NumericFallbackVisitor<'a, 'tcx> {

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);
}
},
// we cannot check the exact type since it's a hir::Ty which does not implement `is_numeric`
StmtKind::Local(local) => self.ty_bounds.push(ExplicitTyBound(local.ty.is_some())),

_ => self.ty_bounds.push(TyBound::Nothing),
_ => self.ty_bounds.push(ExplicitTyBound(false)),
}

walk_stmt(self, stmt);
Expand All @@ -218,28 +221,18 @@ fn fn_sig_opt<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId) -> Option<PolyFnSig<'
}
}

/// Wrapper around a `bool` to make the meaning of the value clearer
#[derive(Debug, Clone, Copy)]
enum TyBound<'tcx> {
Any,
Ty(Ty<'tcx>),
Nothing,
}
struct ExplicitTyBound(pub bool);

impl<'tcx> TyBound<'tcx> {
fn is_numeric(self) -> bool {
match self {
TyBound::Any => true,
TyBound::Ty(t) => t.is_numeric(),
TyBound::Nothing => false,
}
impl<'tcx> From<Ty<'tcx>> for ExplicitTyBound {
fn from(v: Ty<'tcx>) -> Self {
Self(v.is_numeric())
}
}

impl<'tcx> From<Option<Ty<'tcx>>> for TyBound<'tcx> {
impl<'tcx> From<Option<Ty<'tcx>>> for ExplicitTyBound {
fn from(v: Option<Ty<'tcx>>) -> Self {
match v {
Some(t) => TyBound::Ty(t),
None => TyBound::Nothing,
}
Self(v.map_or(false, Ty::is_numeric))
}
}
9 changes: 9 additions & 0 deletions tests/ui/default_numeric_fallback_f64.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ mod basic_expr {
let x: [f64; 3] = [1., 2., 3.];
let x: (f64, f64) = if true { (1., 2.) } else { (3., 4.) };
let x: _ = 1.;
const X: f32 = 1.;
}
}

Expand All @@ -59,6 +60,14 @@ mod nested_local {
// Should NOT lint this because this literal is bound to `_` of outer `Local`.
2.
};

const X: f32 = {
// Should lint this because this literal is not bound to any types.
let y = 1.0_f64;

// Should NOT lint this because this literal is bound to `_` of outer `Local`.
1.
};
}
}

Expand Down
9 changes: 9 additions & 0 deletions tests/ui/default_numeric_fallback_f64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ mod basic_expr {
let x: [f64; 3] = [1., 2., 3.];
let x: (f64, f64) = if true { (1., 2.) } else { (3., 4.) };
let x: _ = 1.;
const X: f32 = 1.;
}
}

Expand All @@ -59,6 +60,14 @@ mod nested_local {
// Should NOT lint this because this literal is bound to `_` of outer `Local`.
2.
};

const X: f32 = {
// 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.
};
}
}

Expand Down
34 changes: 20 additions & 14 deletions tests/ui/default_numeric_fallback_f64.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -61,79 +61,85 @@ LL | _ => 1.,
| ^^ help: consider adding suffix: `1.0_f64`

error: default numeric fallback might occur
--> $DIR/default_numeric_fallback_f64.rs:43:21
--> $DIR/default_numeric_fallback_f64.rs:44:21
|
LL | let y = 1.;
| ^^ help: consider adding suffix: `1.0_f64`

error: default numeric fallback might occur
--> $DIR/default_numeric_fallback_f64.rs:51:21
--> $DIR/default_numeric_fallback_f64.rs:52:21
|
LL | let y = 1.;
| ^^ help: consider adding suffix: `1.0_f64`

error: default numeric fallback might occur
--> $DIR/default_numeric_fallback_f64.rs:57:21
--> $DIR/default_numeric_fallback_f64.rs:58:21
|
LL | let y = 1.;
| ^^ help: consider adding suffix: `1.0_f64`

error: default numeric fallback might occur
--> $DIR/default_numeric_fallback_f64.rs:69:9
--> $DIR/default_numeric_fallback_f64.rs:66:21
|
LL | let y = 1.;
| ^^ help: consider adding suffix: `1.0_f64`

error: default numeric fallback might occur
--> $DIR/default_numeric_fallback_f64.rs:78:9
|
LL | 1.
| ^^ help: consider adding suffix: `1.0_f64`

error: default numeric fallback might occur
--> $DIR/default_numeric_fallback_f64.rs:75:27
--> $DIR/default_numeric_fallback_f64.rs:84:27
|
LL | let f = || -> _ { 1. };
| ^^ help: consider adding suffix: `1.0_f64`

error: default numeric fallback might occur
--> $DIR/default_numeric_fallback_f64.rs:79:29
--> $DIR/default_numeric_fallback_f64.rs:88:29
|
LL | let f = || -> f64 { 1. };
| ^^ help: consider adding suffix: `1.0_f64`

error: default numeric fallback might occur
--> $DIR/default_numeric_fallback_f64.rs:93:21
--> $DIR/default_numeric_fallback_f64.rs:102:21
|
LL | generic_arg(1.);
| ^^ help: consider adding suffix: `1.0_f64`

error: default numeric fallback might occur
--> $DIR/default_numeric_fallback_f64.rs:96:32
--> $DIR/default_numeric_fallback_f64.rs:105:32
|
LL | let x: _ = generic_arg(1.);
| ^^ help: consider adding suffix: `1.0_f64`

error: default numeric fallback might occur
--> $DIR/default_numeric_fallback_f64.rs:114:28
--> $DIR/default_numeric_fallback_f64.rs:123:28
|
LL | GenericStruct { x: 1. };
| ^^ help: consider adding suffix: `1.0_f64`

error: default numeric fallback might occur
--> $DIR/default_numeric_fallback_f64.rs:117:36
--> $DIR/default_numeric_fallback_f64.rs:126:36
|
LL | let _ = GenericStruct { x: 1. };
| ^^ help: consider adding suffix: `1.0_f64`

error: default numeric fallback might occur
--> $DIR/default_numeric_fallback_f64.rs:135:24
--> $DIR/default_numeric_fallback_f64.rs:144:24
|
LL | GenericEnum::X(1.);
| ^^ help: consider adding suffix: `1.0_f64`

error: default numeric fallback might occur
--> $DIR/default_numeric_fallback_f64.rs:155:23
--> $DIR/default_numeric_fallback_f64.rs:164:23
|
LL | s.generic_arg(1.);
| ^^ help: consider adding suffix: `1.0_f64`

error: default numeric fallback might occur
--> $DIR/default_numeric_fallback_f64.rs:162:21
--> $DIR/default_numeric_fallback_f64.rs:171:21
|
LL | let x = 22.;
| ^^^ help: consider adding suffix: `22.0_f64`
Expand All @@ -143,5 +149,5 @@ LL | internal_macro!();
|
= note: this error originates in the macro `internal_macro` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 23 previous errors
error: aborting due to 24 previous errors

10 changes: 10 additions & 0 deletions tests/ui/default_numeric_fallback_i32.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ mod basic_expr {
let x: [i32; 3] = [1, 2, 3];
let x: (i32, i32) = if true { (1, 2) } else { (3, 4) };
let x: _ = 1;
let x: u64 = 1;
const CONST_X: i8 = 1;
}
}

Expand All @@ -59,6 +61,14 @@ mod nested_local {
// Should NOT lint this because this literal is bound to `_` of outer `Local`.
2
};

const CONST_X: i32 = {
// Should lint this because this literal is not bound to any types.
let y = 1_i32;

// Should NOT lint this because this literal is bound to `_` of outer `Local`.
1
};
}
}

Expand Down
10 changes: 10 additions & 0 deletions tests/ui/default_numeric_fallback_i32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ mod basic_expr {
let x: [i32; 3] = [1, 2, 3];
let x: (i32, i32) = if true { (1, 2) } else { (3, 4) };
let x: _ = 1;
let x: u64 = 1;
const CONST_X: i8 = 1;
}
}

Expand All @@ -59,6 +61,14 @@ mod nested_local {
// Should NOT lint this because this literal is bound to `_` of outer `Local`.
2
};

const CONST_X: i32 = {
// 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
};
}
}

Expand Down
Loading