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.

Sign up for GitHub

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

Jump to bottom

[default_numeric_fallback] do not lint on constants #9636

Merged
merged 3 commits into from
Oct 16, 2022
Merged

[default_numeric_fallback] do not lint on constants #9636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e51e930
`default_numeric_fallback` do not lint on constants
kraktus Oct 12, 2022
36b2685
refactor `default_numeric_fallback`
kraktus Oct 12, 2022
d38175f
`explicit_ty_bound` code golf
kraktus Oct 15, 2022
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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