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 [unconstrained_numeric_literal] #34

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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5434,6 +5434,7 @@ Released 2018-09-13
[`type_id_on_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_id_on_box
[`type_repetition_in_bounds`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_repetition_in_bounds
[`unchecked_duration_subtraction`]: https://rust-lang.github.io/rust-clippy/master/index.html#unchecked_duration_subtraction
[`unconstrained_numeric_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#unconstrained_numeric_literal
[`undocumented_unsafe_blocks`]: https://rust-lang.github.io/rust-clippy/master/index.html#undocumented_unsafe_blocks
[`undropped_manually_drops`]: https://rust-lang.github.io/rust-clippy/master/index.html#undropped_manually_drops
[`unicode_not_nfc`]: https://rust-lang.github.io/rust-clippy/master/index.html#unicode_not_nfc
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::guidelines::PASSING_STRING_TO_C_FUNCTIONS_INFO,
crate::guidelines::PTR_DOUBLE_FREE_INFO,
crate::guidelines::RETURN_STACK_ADDRESS_INFO,
crate::guidelines::UNCONSTRAINED_NUMERIC_LITERAL_INFO,
crate::guidelines::UNSAFE_BLOCK_IN_PROC_MACRO_INFO,
crate::guidelines::UNTRUSTED_LIB_LOADING_INFO,
crate::guidelines_early::IMPLICIT_ABI_INFO,
Expand Down
31 changes: 31 additions & 0 deletions clippy_lints/src/guidelines/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ mod invalid_char_range;
mod passing_string_to_c_functions;
mod ptr;
mod return_stack_address;
mod unconstrained_numeric_literal;
mod unsafe_block_in_proc_macro;
mod untrusted_lib_loading;

Expand Down Expand Up @@ -354,6 +355,34 @@ declare_clippy_lint! {
"converting to char from a out-of-range unsigned int"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for usage of unconstrained numeric literals in variable initialization.
///
/// This lint is differ from `default_numeric_fallback` in the following perspectives:
/// 1. It only checks numeric literals in a local binding.
/// 2. It lints all kinds of numeric literals rather than `i32` and `f64`.
///
/// ### Why is this bad?
/// Initializing a numeric type without labeling its type could cause default numeric fallback.
///
/// ### Example
/// ```rust
/// let i = 10;
/// let f = 1.23;
/// ```
///
/// Use instead:
/// ```rust
/// let i = 10i32;
/// let f = 1.23f64;
/// ```
#[clippy::version = "1.74.0"]
pub UNCONSTRAINED_NUMERIC_LITERAL,
nursery,
"usage of unconstrained numeric literals in variable initialization"
}

/// Helper struct with user configured path-like functions, such as `std::fs::read`,
/// and a set for `def_id`s which should be filled during checks.
///
Expand Down Expand Up @@ -430,6 +459,7 @@ impl_lint_pass!(LintGroup => [
DANGLING_PTR_DEREFERENCE,
RETURN_STACK_ADDRESS,
INVALID_CHAR_RANGE,
UNCONSTRAINED_NUMERIC_LITERAL,
]);

impl<'tcx> LateLintPass<'tcx> for LintGroup {
Expand Down Expand Up @@ -500,6 +530,7 @@ impl<'tcx> LateLintPass<'tcx> for LintGroup {

fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx hir::Local<'tcx>) {
ptr::check_local(cx, local);
unconstrained_numeric_literal::check_local(cx, local);
}

fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx hir::Block<'tcx>) {
Expand Down
96 changes: 96 additions & 0 deletions clippy_lints/src/guidelines/unconstrained_numeric_literal.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::is_lint_allowed;
use clippy_utils::source::snippet_opt;
use rustc_errors::{Applicability, MultiSpan};
use rustc_hir::intravisit::{walk_expr, Visitor};
use rustc_hir::{Expr, ExprKind, Local, TyKind};
use rustc_lint::{LateContext, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_span::Span;

use super::UNCONSTRAINED_NUMERIC_LITERAL;

pub(super) fn check_local<'tcx>(cx: &LateContext<'tcx>, local: &'tcx Local<'tcx>) {
if !is_lint_allowed(cx, UNCONSTRAINED_NUMERIC_LITERAL, local.hir_id)
&& let Some(init) = local.init
&& local_has_implicit_ty(local)
{
let mut visitor = LitVisitor::new(cx);
visitor.visit_expr(init);

// The type could be wildcard (`_`), therefore we need to include its span for suggestion.
let span = if let Some(ty) = local.ty {
local.pat.span.to(ty.span)
} else {
local.pat.span
};

if !visitor.unconstrained_lit_spans.is_empty() {
span_lint_and_then(
cx,
UNCONSTRAINED_NUMERIC_LITERAL,
span,
"type of this numeric variable is unconstrained",
|diag| {
let sugg = format!(
"{}: {}",
snippet_opt(cx, local.pat.span).unwrap_or("_".to_string()),
ty_suggestion(cx, init),
);
diag.span_suggestion(
span,
"either add suffix to above numeric literal(s) or label the type explicitly",
sugg,
Applicability::MachineApplicable
);
diag.span_note(
MultiSpan::from_spans(visitor.unconstrained_lit_spans),
"unconstrained numeric literals defined here",
);
}
);
}
}
}

fn local_has_implicit_ty(local: &Local<'_>) -> bool {
match local.ty {
Some(ty) if matches!(ty.kind, TyKind::Infer) => true,
None => true,
_ => false,
}
}

struct LitVisitor<'hir, 'tcx> {
cx: &'hir LateContext<'tcx>,
unconstrained_lit_spans: Vec<Span>,
}

impl<'hir, 'tcx> LitVisitor<'hir, 'tcx> {
fn new(cx: &'hir LateContext<'tcx>) -> Self {
Self {
cx,
unconstrained_lit_spans: vec![],
}
}
}

impl<'hir, 'tcx> Visitor<'hir> for LitVisitor<'hir, 'tcx> {
fn visit_expr(&mut self, ex: &'hir Expr<'hir>) {
if let ExprKind::Lit(lit) = ex.kind {
if lit.node.is_numeric() && lit.node.is_unsuffixed() && !in_external_macro(self.cx.sess(), lit.span) {
self.unconstrained_lit_spans.push(lit.span);
}
} else {
walk_expr(self, ex);
}
}

// Don't visit local in this visitor, `Local`s are handled in `check_local` call.
fn visit_local(&mut self, _: &'hir Local<'hir>) {}
}

fn ty_suggestion(cx: &LateContext<'_>, init: &Expr<'_>) -> String {
let ty = cx.typeck_results().expr_ty(init);
ty.to_string()
}
95 changes: 95 additions & 0 deletions tests/ui/guidelines/unconstrained_numeric_literal.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
//@aux-build:../auxiliary/proc_macros.rs

#![feature(lint_reasons)]
#![warn(clippy::unconstrained_numeric_literal)]
#![allow(clippy::let_with_type_underscore, clippy::let_and_return)]

extern crate proc_macros;
use proc_macros::{external, inline_macros};

mod basic_expr {
fn test() {
let x: i32 = 22;
//~^ ERROR: type of this numeric variable is unconstrained
//~| NOTE: `-D clippy::unconstrained-numeric-literal` implied by `-D warnings`
let x: f64 = 22.0;
//~^ ERROR: type of this numeric variable is unconstrained
let x: [i32; 3] = [1, 2, 3];
//~^ ERROR: type of this numeric variable is unconstrained
let x: (i32, i32) = if true { (1, 2) } else { (3, 4) };
//~^ ERROR: type of this numeric variable is unconstrained
let x: (f64, i32, f64) = if true { (1.0, 2, 3.0) } else { (3.0, 4, 5.0) };
//~^ ERROR: type of this numeric variable is unconstrained
let x: i32 = match 1 {
//~^ ERROR: type of this numeric variable is unconstrained
1 => 1,
_ => 2,
};
// Has type annotation but it's a wildcard.
let x: i32 = 1;
//~^ ERROR: type of this numeric variable is unconstrained

let x = 22_i32;
let x: [i32; 3] = [1, 2, 3];
let x: (i32, i32) = if true { (1, 2) } else { (3, 4) };
let x: u64 = 1;
const CONST_X: i8 = 1;
}
}

mod nested_local {
fn test() {
let x: i32 = {
//~^ ERROR: type of this numeric variable is unconstrained
let y: i32 = 1;
//~^ ERROR: type of this numeric variable is unconstrained
1
};

let x: i32 = {
let y: i32 = 1;
//~^ ERROR: type of this numeric variable is unconstrained
1
};

const CONST_X: i32 = {
let y: i32 = 1;
//~^ ERROR: type of this numeric variable is unconstrained
1
};
}
}

mod in_macro {
use super::*;

#[inline_macros]
fn internal() {
inline!(let x: i32 = 22;);
//~^ ERROR: type of this numeric variable is unconstrained
}

fn external() {
external!(let x = 22;);
}
}

fn check_expect_suppression() {
#[expect(clippy::unconstrained_numeric_literal)]
let x = 21;
}

#[allow(clippy::useless_vec)]
fn check_vac_macro() {
let x: std::vec::Vec<i32> = vec![1, 2, 3];
//~^ ERROR: type of this numeric variable is unconstrained
let x: std::vec::Vec<f64> = vec![1.0];
//~^ ERROR: type of this numeric variable is unconstrained

let y = vec![1_i32, 2_i32];
let y = vec![0_u8, 1_u8];
let y = vec![2.0_f64, 3.0_f64];
let y: Vec<i32> = vec![1, 2];
}

fn main() {}
95 changes: 95 additions & 0 deletions tests/ui/guidelines/unconstrained_numeric_literal.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
//@aux-build:../auxiliary/proc_macros.rs

#![feature(lint_reasons)]
#![warn(clippy::unconstrained_numeric_literal)]
#![allow(clippy::let_with_type_underscore, clippy::let_and_return)]

extern crate proc_macros;
use proc_macros::{external, inline_macros};

mod basic_expr {
fn test() {
let x = 22;
//~^ ERROR: type of this numeric variable is unconstrained
//~| NOTE: `-D clippy::unconstrained-numeric-literal` implied by `-D warnings`
let x = 22.0;
//~^ ERROR: type of this numeric variable is unconstrained
let x = [1, 2, 3];
//~^ ERROR: type of this numeric variable is unconstrained
let x = if true { (1, 2) } else { (3, 4) };
//~^ ERROR: type of this numeric variable is unconstrained
let x = if true { (1.0, 2, 3.0) } else { (3.0, 4, 5.0) };
//~^ ERROR: type of this numeric variable is unconstrained
let x = match 1 {
//~^ ERROR: type of this numeric variable is unconstrained
1 => 1,
_ => 2,
};
// Has type annotation but it's a wildcard.
let x: _ = 1;
//~^ ERROR: type of this numeric variable is unconstrained

let x = 22_i32;
let x: [i32; 3] = [1, 2, 3];
let x: (i32, i32) = if true { (1, 2) } else { (3, 4) };
let x: u64 = 1;
const CONST_X: i8 = 1;
}
}

mod nested_local {
fn test() {
let x = {
//~^ ERROR: type of this numeric variable is unconstrained
let y = 1;
//~^ ERROR: type of this numeric variable is unconstrained
1
};

let x: i32 = {
let y = 1;
//~^ ERROR: type of this numeric variable is unconstrained
1
};

const CONST_X: i32 = {
let y = 1;
//~^ ERROR: type of this numeric variable is unconstrained
1
};
}
}

mod in_macro {
use super::*;

#[inline_macros]
fn internal() {
inline!(let x = 22;);
//~^ ERROR: type of this numeric variable is unconstrained
}

fn external() {
external!(let x = 22;);
}
}

fn check_expect_suppression() {
#[expect(clippy::unconstrained_numeric_literal)]
let x = 21;
}

#[allow(clippy::useless_vec)]
fn check_vac_macro() {
let x = vec![1, 2, 3];
//~^ ERROR: type of this numeric variable is unconstrained
let x = vec![1.0];
//~^ ERROR: type of this numeric variable is unconstrained

let y = vec![1_i32, 2_i32];
let y = vec![0_u8, 1_u8];
let y = vec![2.0_f64, 3.0_f64];
let y: Vec<i32> = vec![1, 2];
}

fn main() {}
Loading