Skip to content

New lint: int_min_max_value #6570

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

Closed
wants to merge 4 commits into from
Closed
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 @@ -1996,6 +1996,7 @@ Released 2018-09-13
[`inline_asm_x86_att_syntax`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_asm_x86_att_syntax
[`inline_asm_x86_intel_syntax`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_asm_x86_intel_syntax
[`inline_fn_without_body`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_fn_without_body
[`int_min_max_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#int_min_max_value
[`int_plus_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#int_plus_one
[`integer_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#integer_arithmetic
[`integer_division`]: https://rust-lang.github.io/rust-clippy/master/index.html#integer_division
Expand Down
182 changes: 182 additions & 0 deletions clippy_lints/src/int_min_max_value.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
use crate::utils::{meets_msrv, snippet_with_applicability, span_lint_and_help, span_lint_and_sugg};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, Item, ItemKind, QPath, UseKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::{lint::in_external_macro, ty};
use rustc_semver::RustcVersion;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::symbol::{sym, Symbol};
use std::borrow::Cow;

declare_clippy_lint! {
/// **What it does:** Checks for uses of `min_value()` and `max_value()` functions of the
/// primitive integer types, and the `MIN` and `MAX` constants in their respective modules.
///
/// **Why is this bad?** Both functions and the module constants are soft-deprecated with the
/// use of the `MIN` and `MAX` constants recommended instead.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// let min = i32::min_value();
/// let max = i32::max_value();
/// let min = std::i32::MIN;
/// let min = std::i32::MAX;
/// ```
/// Use instead:
/// ```rust
/// let min = i32::MIN;
/// let max = i32::MAX;
/// let min = i32::MIN;
/// let max = i32::MAX;
/// ```
pub INT_MIN_MAX_VALUE,
style,
"use of `min_value()` and `max_value()` for primitive integer types"
}

impl_lint_pass!(IntMinMaxValue => [INT_MIN_MAX_VALUE]);

const INT_MIN_MAX_VALUE_MSRV: RustcVersion = RustcVersion::new(1, 43, 0);

pub struct IntMinMaxValue {
msrv: Option<RustcVersion>,
}
impl IntMinMaxValue {
#[must_use]
pub fn new(msrv: Option<RustcVersion>) -> Self {
Self { msrv }
}
}

impl LateLintPass<'_> for IntMinMaxValue {
extract_msrv_attr!(LateContext);

fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
if_chain! {
if meets_msrv(self.msrv.as_ref(), &INT_MIN_MAX_VALUE_MSRV);
if !in_external_macro(cx.sess(), item.span);
if let ItemKind::Use(path, UseKind::Single) = item.kind;
if let [crate_name, mod_name] = path.segments;
then {
if !(crate_name.ident.name == sym::std || crate_name.ident.name == sym::core) {
return;
}

let mod_name = mod_name.ident.as_str();
if mod_name == "i8" || mod_name == "i16"
|| mod_name == "i32" || mod_name == "i64"
|| mod_name == "i128" || mod_name == "isize"
|| mod_name == "u8" || mod_name == "u16"
|| mod_name == "u32" || mod_name == "u64"
|| mod_name == "u128" || mod_name == "usize"
{
span_lint_and_help(
cx,
INT_MIN_MAX_VALUE,
item.span,
"use of the module constants `MIN` and `MAX` for primitive integer types is soft-deprecated",
None,
"remove this import",
);
}
}
}
}

fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if !meets_msrv(self.msrv.as_ref(), &INT_MIN_MAX_VALUE_MSRV) || in_external_macro(cx.sess(), expr.span) {
return;
}

match expr.kind {
ExprKind::Call(func, []) => if_chain! {
if let ExprKind::Path(QPath::TypeRelative(ty, name)) = func.kind;
let res_ty = cx.typeck_results().node_type(ty.hir_id);
if let ty::Int(_) | ty::Uint(_) = res_ty.kind();
then {
let (msg, new_name) = if name.ident.as_str() == "max_value" {
("`max_value` is soft-deprecated", "MAX")
} else if name.ident.as_str() == "min_value" {
("`min_value` is soft-deprecated", "MIN")
} else {
return;
};


let mut app = Applicability::MachineApplicable;
let sugg_path = match snippet_with_applicability(cx, ty.span, "_", &mut app) {
// the span for the type includes the method name for some reason, strip it off
Cow::Owned(x) => {
Cow::Owned(x.rsplitn(2, "::").nth(1).unwrap_or("_").into())
}
Cow::Borrowed(x) => Cow::Borrowed(x),
};
span_lint_and_sugg(
cx,
INT_MIN_MAX_VALUE,
expr.span,
msg,
"use constant instead",
format!("{}::{}", sugg_path, new_name),
app,
);
}
},
ExprKind::Path(QPath::Resolved(None, path)) => if_chain! {
if let [crate_name, _, _] = path.segments;
if crate_name.ident.name == sym::std || crate_name.ident.name == sym::core;
if let Some(id) = path.res.opt_def_id();
if let Some((mod_name, constant_name)) = get_prim_module_constant(&cx.get_def_path(id));
if path.segments[1].ident.as_str() == mod_name && path.segments[2].ident.as_str() == constant_name;
then {
span_lint_and_sugg(
cx,
INT_MIN_MAX_VALUE,
expr.span,
&format!("`{}::{}::{}` is soft-deprecated", crate_name.ident.as_str(), mod_name, constant_name),
"use associated constant instead",
format!("{}::{}", mod_name, constant_name),
Applicability::MachineApplicable,
);
}
},
_ => (),
}
}
}

fn get_prim_module_constant(path: &[Symbol]) -> Option<(&'static str, &'static str)> {
if path.len() != 3 {
return None;
}
if !(path[0] == sym::core || path[0] == sym::std) {
return None;
}
let mod_name = match path[1] {
sym::u8 => "u8",
sym::u16 => "u16",
sym::u32 => "u32",
sym::u64 => "u64",
sym::u128 => "u128",
sym::usize => "usize",
sym::i8 => "i8",
sym::i16 => "i16",
sym::i32 => "i32",
sym::i64 => "i64",
sym::i128 => "i128",
sym::isize => "isize",
_ => return None,
};
let constant_name = if path[2].as_str() == "MIN" {
"MIN"
} else if path[2].as_str() == "MAX" {
"MAX"
} else {
return None;
};
Some((mod_name, constant_name))
}
5 changes: 5 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ mod infinite_iter;
mod inherent_impl;
mod inherent_to_string;
mod inline_fn_without_body;
mod int_min_max_value;
mod int_plus_one;
mod integer_division;
mod items_after_statements;
Expand Down Expand Up @@ -642,6 +643,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&inherent_to_string::INHERENT_TO_STRING,
&inherent_to_string::INHERENT_TO_STRING_SHADOW_DISPLAY,
&inline_fn_without_body::INLINE_FN_WITHOUT_BODY,
&int_min_max_value::INT_MIN_MAX_VALUE,
&int_plus_one::INT_PLUS_ONE,
&integer_division::INTEGER_DIVISION,
&items_after_statements::ITEMS_AFTER_STATEMENTS,
Expand Down Expand Up @@ -1026,6 +1028,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(move || box use_self::UseSelf::new(msrv));
store.register_late_pass(move || box missing_const_for_fn::MissingConstForFn::new(msrv));
store.register_late_pass(move || box needless_question_mark::NeedlessQuestionMark::new(msrv));
store.register_late_pass(move || box int_min_max_value::IntMinMaxValue::new(msrv));

store.register_late_pass(|| box size_of_in_element_count::SizeOfInElementCount);
store.register_late_pass(|| box map_clone::MapClone);
Expand Down Expand Up @@ -1446,6 +1449,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&inherent_to_string::INHERENT_TO_STRING),
LintId::of(&inherent_to_string::INHERENT_TO_STRING_SHADOW_DISPLAY),
LintId::of(&inline_fn_without_body::INLINE_FN_WITHOUT_BODY),
LintId::of(&int_min_max_value::INT_MIN_MAX_VALUE),
LintId::of(&int_plus_one::INT_PLUS_ONE),
LintId::of(&large_const_arrays::LARGE_CONST_ARRAYS),
LintId::of(&large_enum_variant::LARGE_ENUM_VARIANT),
Expand Down Expand Up @@ -1687,6 +1691,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&functions::RESULT_UNIT_ERR),
LintId::of(&if_let_some_result::IF_LET_SOME_RESULT),
LintId::of(&inherent_to_string::INHERENT_TO_STRING),
LintId::of(&int_min_max_value::INT_MIN_MAX_VALUE),
LintId::of(&len_zero::COMPARISON_TO_EMPTY),
LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY),
LintId::of(&len_zero::LEN_ZERO),
Expand Down
1 change: 1 addition & 0 deletions tests/ui/checked_conversions.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
clippy::cast_lossless,
// Int::max_value will be deprecated in the future
deprecated,
clippy::int_min_max_value,
)]
#![warn(clippy::checked_conversions)]

Expand Down
1 change: 1 addition & 0 deletions tests/ui/checked_conversions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
clippy::cast_lossless,
// Int::max_value will be deprecated in the future
deprecated,
clippy::int_min_max_value,
)]
#![warn(clippy::checked_conversions)]

Expand Down
32 changes: 16 additions & 16 deletions tests/ui/checked_conversions.stderr
Original file line number Diff line number Diff line change
@@ -1,97 +1,97 @@
error: checked cast can be simplified
--> $DIR/checked_conversions.rs:17:13
--> $DIR/checked_conversions.rs:18:13
|
LL | let _ = value <= (u32::max_value() as i64) && value >= 0;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u32::try_from(value).is_ok()`
|
= note: `-D clippy::checked-conversions` implied by `-D warnings`

error: checked cast can be simplified
--> $DIR/checked_conversions.rs:18:13
--> $DIR/checked_conversions.rs:19:13
|
LL | let _ = value <= (u32::MAX as i64) && value >= 0;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u32::try_from(value).is_ok()`

error: checked cast can be simplified
--> $DIR/checked_conversions.rs:22:13
--> $DIR/checked_conversions.rs:23:13
|
LL | let _ = value <= i64::from(u16::max_value()) && value >= 0;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u16::try_from(value).is_ok()`

error: checked cast can be simplified
--> $DIR/checked_conversions.rs:23:13
--> $DIR/checked_conversions.rs:24:13
|
LL | let _ = value <= i64::from(u16::MAX) && value >= 0;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u16::try_from(value).is_ok()`

error: checked cast can be simplified
--> $DIR/checked_conversions.rs:27:13
--> $DIR/checked_conversions.rs:28:13
|
LL | let _ = value <= (u8::max_value() as isize) && value >= 0;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u8::try_from(value).is_ok()`

error: checked cast can be simplified
--> $DIR/checked_conversions.rs:28:13
--> $DIR/checked_conversions.rs:29:13
|
LL | let _ = value <= (u8::MAX as isize) && value >= 0;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u8::try_from(value).is_ok()`

error: checked cast can be simplified
--> $DIR/checked_conversions.rs:34:13
--> $DIR/checked_conversions.rs:35:13
|
LL | let _ = value <= (i32::max_value() as i64) && value >= (i32::min_value() as i64);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i32::try_from(value).is_ok()`

error: checked cast can be simplified
--> $DIR/checked_conversions.rs:35:13
--> $DIR/checked_conversions.rs:36:13
|
LL | let _ = value <= (i32::MAX as i64) && value >= (i32::MIN as i64);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i32::try_from(value).is_ok()`

error: checked cast can be simplified
--> $DIR/checked_conversions.rs:39:13
--> $DIR/checked_conversions.rs:40:13
|
LL | let _ = value <= i64::from(i16::max_value()) && value >= i64::from(i16::min_value());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i16::try_from(value).is_ok()`

error: checked cast can be simplified
--> $DIR/checked_conversions.rs:40:13
--> $DIR/checked_conversions.rs:41:13
|
LL | let _ = value <= i64::from(i16::MAX) && value >= i64::from(i16::MIN);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i16::try_from(value).is_ok()`

error: checked cast can be simplified
--> $DIR/checked_conversions.rs:46:13
--> $DIR/checked_conversions.rs:47:13
|
LL | let _ = value <= i32::max_value() as u32;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i32::try_from(value).is_ok()`

error: checked cast can be simplified
--> $DIR/checked_conversions.rs:47:13
--> $DIR/checked_conversions.rs:48:13
|
LL | let _ = value <= i32::MAX as u32;
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `i32::try_from(value).is_ok()`

error: checked cast can be simplified
--> $DIR/checked_conversions.rs:51:13
--> $DIR/checked_conversions.rs:52:13
|
LL | let _ = value <= isize::max_value() as usize && value as i32 == 5;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `isize::try_from(value).is_ok()`

error: checked cast can be simplified
--> $DIR/checked_conversions.rs:52:13
--> $DIR/checked_conversions.rs:53:13
|
LL | let _ = value <= isize::MAX as usize && value as i32 == 5;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `isize::try_from(value).is_ok()`

error: checked cast can be simplified
--> $DIR/checked_conversions.rs:56:13
--> $DIR/checked_conversions.rs:57:13
|
LL | let _ = value <= u16::max_value() as u32 && value as i32 == 5;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u16::try_from(value).is_ok()`

error: checked cast can be simplified
--> $DIR/checked_conversions.rs:57:13
--> $DIR/checked_conversions.rs:58:13
|
LL | let _ = value <= u16::MAX as u32 && value as i32 == 5;
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u16::try_from(value).is_ok()`
Expand Down
16 changes: 16 additions & 0 deletions tests/ui/int_min_max_value.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#![allow(unused)]
#![warn(clippy::int_min_max_value)]

use std::i32;

type Uint = u8;

fn main() {
let min = u32::min_value();
let max = isize::max_value();
let min = crate::Uint::min_value();
let max = std::u32::MAX;

let min = isize::MIN;
let max = usize::MAX;
}
Loading