From 074c8ca8f181718fdd0f3831e0d53ab05802ae19 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Mon, 4 Jan 2021 00:20:48 -0500 Subject: [PATCH 1/4] Add lint: int_min_max_value --- CHANGELOG.md | 1 + clippy_lints/src/int_min_max_value.rs | 84 +++++++++++++++++++++ clippy_lints/src/lib.rs | 5 ++ tests/ui/checked_conversions.fixed | 1 + tests/ui/checked_conversions.rs | 1 + tests/ui/checked_conversions.stderr | 32 ++++---- tests/ui/int_min_max_value.fixed | 12 +++ tests/ui/int_min_max_value.rs | 12 +++ tests/ui/int_min_max_value.stderr | 22 ++++++ tests/ui/manual_saturating_arithmetic.fixed | 2 +- tests/ui/manual_saturating_arithmetic.rs | 2 +- tests/ui/min_rust_version_attr.rs | 6 ++ tests/ui/min_rust_version_attr.stderr | 8 +- tests/ui/suspicious_arithmetic_impl.rs | 1 + tests/ui/suspicious_arithmetic_impl.stderr | 18 ++--- 15 files changed, 176 insertions(+), 31 deletions(-) create mode 100644 clippy_lints/src/int_min_max_value.rs create mode 100644 tests/ui/int_min_max_value.fixed create mode 100644 tests/ui/int_min_max_value.rs create mode 100644 tests/ui/int_min_max_value.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 64864c2e2780..ce5c62061f31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/clippy_lints/src/int_min_max_value.rs b/clippy_lints/src/int_min_max_value.rs new file mode 100644 index 000000000000..65d43085e3d6 --- /dev/null +++ b/clippy_lints/src/int_min_max_value.rs @@ -0,0 +1,84 @@ +use crate::utils::{meets_msrv, snippet_with_applicability, span_lint_and_sugg}; +use if_chain::if_chain; +use rustc_ast::{IntTy, UintTy}; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind, QPath}; +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 std::borrow::Cow; + +declare_clippy_lint! { + /// **What it does:** Checks for uses of `min_value()` and `max_value()` functions of the + /// primitive integer types. + /// + /// **Why is this bad?** Both functions 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(); + /// ``` + /// Use instead: + /// ```rust + /// let min = i32::MIN; + /// let max = i32::MAX; + /// ``` + pub INT_MIN_MAX_VALUE, + style, + "default lint description" +} + +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, +} +impl IntMinMaxValue { + #[must_use] + pub fn new(msrv: Option) -> Self { + Self { msrv } + } +} + +impl LateLintPass<'_> for IntMinMaxValue { + extract_msrv_attr!(LateContext); + + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if_chain! { + if meets_msrv(self.msrv.as_ref(), &INT_MIN_MAX_VALUE_MSRV); + if !in_external_macro(cx.sess(), expr.span); + if let ExprKind::Call(func, []) = expr.kind; + 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()) + } + x => x, + }; + span_lint_and_sugg(cx, INT_MIN_MAX_VALUE, expr.span, msg, "use constant instead", format!("{}::{}", sugg_path, new_name), app); + } + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 37a56bc20c8c..8a0b10067903 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -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; @@ -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, @@ -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); @@ -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), @@ -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), diff --git a/tests/ui/checked_conversions.fixed b/tests/ui/checked_conversions.fixed index 12290db3dcf5..ffb37b0b3c30 100644 --- a/tests/ui/checked_conversions.fixed +++ b/tests/ui/checked_conversions.fixed @@ -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)] diff --git a/tests/ui/checked_conversions.rs b/tests/ui/checked_conversions.rs index 895a301e8212..d0ffe19a817e 100644 --- a/tests/ui/checked_conversions.rs +++ b/tests/ui/checked_conversions.rs @@ -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)] diff --git a/tests/ui/checked_conversions.stderr b/tests/ui/checked_conversions.stderr index 18518def0acb..0b88d10f3d6c 100644 --- a/tests/ui/checked_conversions.stderr +++ b/tests/ui/checked_conversions.stderr @@ -1,5 +1,5 @@ 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()` @@ -7,91 +7,91 @@ LL | let _ = value <= (u32::max_value() as i64) && value >= 0; = 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()` diff --git a/tests/ui/int_min_max_value.fixed b/tests/ui/int_min_max_value.fixed new file mode 100644 index 000000000000..6018dbe1ef8d --- /dev/null +++ b/tests/ui/int_min_max_value.fixed @@ -0,0 +1,12 @@ +// run-rustfix + +#![allow(unused)] +#![warn(clippy::int_min_max_value)] + +type Uint = u8; + +fn main() { + let min = u32::MIN; + let max = isize::MAX; + let min = crate::Uint::MIN; +} diff --git a/tests/ui/int_min_max_value.rs b/tests/ui/int_min_max_value.rs new file mode 100644 index 000000000000..f6e9c1fd7fb6 --- /dev/null +++ b/tests/ui/int_min_max_value.rs @@ -0,0 +1,12 @@ +// run-rustfix + +#![allow(unused)] +#![warn(clippy::int_min_max_value)] + +type Uint = u8; + +fn main() { + let min = u32::min_value(); + let max = isize::max_value(); + let min = crate::Uint::min_value(); +} diff --git a/tests/ui/int_min_max_value.stderr b/tests/ui/int_min_max_value.stderr new file mode 100644 index 000000000000..ad70794f4fec --- /dev/null +++ b/tests/ui/int_min_max_value.stderr @@ -0,0 +1,22 @@ +error: `min_value` is soft-deprecated + --> $DIR/int_min_max_value.rs:9:15 + | +LL | let min = u32::min_value(); + | ^^^^^^^^^^^^^^^^ help: use constant instead: `u32::MIN` + | + = note: `-D clippy::int-min-max-value` implied by `-D warnings` + +error: `max_value` is soft-deprecated + --> $DIR/int_min_max_value.rs:10:15 + | +LL | let max = isize::max_value(); + | ^^^^^^^^^^^^^^^^^^ help: use constant instead: `isize::MAX` + +error: `min_value` is soft-deprecated + --> $DIR/int_min_max_value.rs:11:15 + | +LL | let min = crate::Uint::min_value(); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use constant instead: `crate::Uint::MIN` + +error: aborting due to 3 previous errors + diff --git a/tests/ui/manual_saturating_arithmetic.fixed b/tests/ui/manual_saturating_arithmetic.fixed index c4f53c446c9f..c58974507e39 100644 --- a/tests/ui/manual_saturating_arithmetic.fixed +++ b/tests/ui/manual_saturating_arithmetic.fixed @@ -1,6 +1,6 @@ // run-rustfix -#![allow(unused_imports)] +#![allow(unused_imports, clippy::int_min_max_value)] use std::{i128, i32, u128, u32}; diff --git a/tests/ui/manual_saturating_arithmetic.rs b/tests/ui/manual_saturating_arithmetic.rs index cd83cf6e65e9..5543bec9c7eb 100644 --- a/tests/ui/manual_saturating_arithmetic.rs +++ b/tests/ui/manual_saturating_arithmetic.rs @@ -1,6 +1,6 @@ // run-rustfix -#![allow(unused_imports)] +#![allow(unused_imports, clippy::int_min_max_value)] use std::{i128, i32, u128, u32}; diff --git a/tests/ui/min_rust_version_attr.rs b/tests/ui/min_rust_version_attr.rs index 0f47f1cbc402..225ed2f47891 100644 --- a/tests/ui/min_rust_version_attr.rs +++ b/tests/ui/min_rust_version_attr.rs @@ -123,6 +123,11 @@ fn missing_const_for_fn() -> i32 { 1 } +fn int_min_max_value() { + let min = u32::min_value(); + let max = isize::max_value(); +} + fn main() { filter_map_next(); checked_conversion(); @@ -138,6 +143,7 @@ fn main() { replace_with_default(); map_unwrap_or(); missing_const_for_fn(); + int_min_max_value(); } mod meets_msrv { diff --git a/tests/ui/min_rust_version_attr.stderr b/tests/ui/min_rust_version_attr.stderr index e3e3b335cbe1..8d3575c2da83 100644 --- a/tests/ui/min_rust_version_attr.stderr +++ b/tests/ui/min_rust_version_attr.stderr @@ -1,12 +1,12 @@ error: stripping a prefix manually - --> $DIR/min_rust_version_attr.rs:150:24 + --> $DIR/min_rust_version_attr.rs:156:24 | LL | assert_eq!(s["hello, ".len()..].to_uppercase(), "WORLD!"); | ^^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::manual-strip` implied by `-D warnings` note: the prefix was tested here - --> $DIR/min_rust_version_attr.rs:149:9 + --> $DIR/min_rust_version_attr.rs:155:9 | LL | if s.starts_with("hello, ") { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -17,13 +17,13 @@ LL | assert_eq!(.to_uppercase(), "WORLD!"); | error: stripping a prefix manually - --> $DIR/min_rust_version_attr.rs:162:24 + --> $DIR/min_rust_version_attr.rs:168:24 | LL | assert_eq!(s["hello, ".len()..].to_uppercase(), "WORLD!"); | ^^^^^^^^^^^^^^^^^^^^ | note: the prefix was tested here - --> $DIR/min_rust_version_attr.rs:161:9 + --> $DIR/min_rust_version_attr.rs:167:9 | LL | if s.starts_with("hello, ") { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/suspicious_arithmetic_impl.rs b/tests/ui/suspicious_arithmetic_impl.rs index 5c280efac1a8..00ed661ba98b 100644 --- a/tests/ui/suspicious_arithmetic_impl.rs +++ b/tests/ui/suspicious_arithmetic_impl.rs @@ -1,4 +1,5 @@ #![warn(clippy::suspicious_arithmetic_impl)] +#![allow(clippy::int_min_max_value)] use std::ops::{ Add, AddAssign, BitAnd, BitOr, BitOrAssign, BitXor, Div, DivAssign, Mul, MulAssign, Rem, Shl, Shr, Sub, }; diff --git a/tests/ui/suspicious_arithmetic_impl.stderr b/tests/ui/suspicious_arithmetic_impl.stderr index 388fc7400820..60df78eb3a59 100644 --- a/tests/ui/suspicious_arithmetic_impl.stderr +++ b/tests/ui/suspicious_arithmetic_impl.stderr @@ -1,5 +1,5 @@ error: suspicious use of binary operator in `Add` impl - --> $DIR/suspicious_arithmetic_impl.rs:13:20 + --> $DIR/suspicious_arithmetic_impl.rs:14:20 | LL | Foo(self.0 - other.0) | ^ @@ -7,7 +7,7 @@ LL | Foo(self.0 - other.0) = note: `-D clippy::suspicious-arithmetic-impl` implied by `-D warnings` error: suspicious use of binary operator in `AddAssign` impl - --> $DIR/suspicious_arithmetic_impl.rs:19:23 + --> $DIR/suspicious_arithmetic_impl.rs:20:23 | LL | *self = *self - other; | ^ @@ -15,43 +15,43 @@ LL | *self = *self - other; = note: `#[deny(clippy::suspicious_op_assign_impl)]` on by default error: suspicious use of binary operator in `MulAssign` impl - --> $DIR/suspicious_arithmetic_impl.rs:32:16 + --> $DIR/suspicious_arithmetic_impl.rs:33:16 | LL | self.0 /= other.0; | ^^ error: suspicious use of binary operator in `Rem` impl - --> $DIR/suspicious_arithmetic_impl.rs:70:20 + --> $DIR/suspicious_arithmetic_impl.rs:71:20 | LL | Foo(self.0 / other.0) | ^ error: suspicious use of binary operator in `BitAnd` impl - --> $DIR/suspicious_arithmetic_impl.rs:78:20 + --> $DIR/suspicious_arithmetic_impl.rs:79:20 | LL | Foo(self.0 | other.0) | ^ error: suspicious use of binary operator in `BitOr` impl - --> $DIR/suspicious_arithmetic_impl.rs:86:20 + --> $DIR/suspicious_arithmetic_impl.rs:87:20 | LL | Foo(self.0 ^ other.0) | ^ error: suspicious use of binary operator in `BitXor` impl - --> $DIR/suspicious_arithmetic_impl.rs:94:20 + --> $DIR/suspicious_arithmetic_impl.rs:95:20 | LL | Foo(self.0 & other.0) | ^ error: suspicious use of binary operator in `Shl` impl - --> $DIR/suspicious_arithmetic_impl.rs:102:20 + --> $DIR/suspicious_arithmetic_impl.rs:103:20 | LL | Foo(self.0 >> other.0) | ^^ error: suspicious use of binary operator in `Shr` impl - --> $DIR/suspicious_arithmetic_impl.rs:110:20 + --> $DIR/suspicious_arithmetic_impl.rs:111:20 | LL | Foo(self.0 << other.0) | ^^ From 101d3822fb10bff36185c61fed0ce7cb15d3ab93 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Tue, 5 Jan 2021 10:02:46 -0500 Subject: [PATCH 2/4] Fix lint errors --- clippy_lints/src/int_min_max_value.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/int_min_max_value.rs b/clippy_lints/src/int_min_max_value.rs index 65d43085e3d6..488131fdd22d 100644 --- a/clippy_lints/src/int_min_max_value.rs +++ b/clippy_lints/src/int_min_max_value.rs @@ -1,6 +1,5 @@ use crate::utils::{meets_msrv, snippet_with_applicability, span_lint_and_sugg}; use if_chain::if_chain; -use rustc_ast::{IntTy, UintTy}; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, QPath}; use rustc_lint::{LateContext, LateLintPass, LintContext}; @@ -31,7 +30,7 @@ declare_clippy_lint! { /// ``` pub INT_MIN_MAX_VALUE, style, - "default lint description" + "use of `min_value()` and `max_value()` for primitive integer types" } impl_lint_pass!(IntMinMaxValue => [INT_MIN_MAX_VALUE]); @@ -75,7 +74,7 @@ impl LateLintPass<'_> for IntMinMaxValue { Cow::Owned(x) => { Cow::Owned(x.rsplitn(2, "::").nth(1).unwrap_or("_").into()) } - x => x, + 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); } From e177e6c18e26824b1b22e63d9d992b742e212c5b Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Fri, 8 Jan 2021 19:38:39 -0500 Subject: [PATCH 3/4] Add: Warn on the use of module constants --- clippy_lints/src/int_min_max_value.rs | 153 +++++++++++++++++++++----- tests/ui/int_min_max_value.fixed | 12 -- tests/ui/int_min_max_value.rs | 8 +- tests/ui/int_min_max_value.stderr | 19 +++- 4 files changed, 148 insertions(+), 44 deletions(-) delete mode 100644 tests/ui/int_min_max_value.fixed diff --git a/clippy_lints/src/int_min_max_value.rs b/clippy_lints/src/int_min_max_value.rs index 488131fdd22d..d1e716d8c000 100644 --- a/clippy_lints/src/int_min_max_value.rs +++ b/clippy_lints/src/int_min_max_value.rs @@ -1,19 +1,20 @@ -use crate::utils::{meets_msrv, snippet_with_applicability, span_lint_and_sugg}; +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, QPath}; +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. + /// primitive integer types, and the `MIN` and `MAX` constants in their respective modules. /// - /// **Why is this bad?** Both functions are soft-deprecated with the use of the `MIN` and `MAX` - /// constants recommended instead. + /// **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. /// @@ -22,11 +23,15 @@ declare_clippy_lint! { /// ```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, @@ -50,34 +55,128 @@ impl IntMinMaxValue { impl LateLintPass<'_> for IntMinMaxValue { extract_msrv_attr!(LateContext); - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + 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(), expr.span); - if let ExprKind::Call(func, []) = expr.kind; - 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(); + 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 { - 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 { + if !(crate_name.ident.as_str() == "std" || crate_name.ident.as_str() == "core") { 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); + 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)) } diff --git a/tests/ui/int_min_max_value.fixed b/tests/ui/int_min_max_value.fixed deleted file mode 100644 index 6018dbe1ef8d..000000000000 --- a/tests/ui/int_min_max_value.fixed +++ /dev/null @@ -1,12 +0,0 @@ -// run-rustfix - -#![allow(unused)] -#![warn(clippy::int_min_max_value)] - -type Uint = u8; - -fn main() { - let min = u32::MIN; - let max = isize::MAX; - let min = crate::Uint::MIN; -} diff --git a/tests/ui/int_min_max_value.rs b/tests/ui/int_min_max_value.rs index f6e9c1fd7fb6..35f83f96c88c 100644 --- a/tests/ui/int_min_max_value.rs +++ b/tests/ui/int_min_max_value.rs @@ -1,12 +1,16 @@ -// run-rustfix - #![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; } diff --git a/tests/ui/int_min_max_value.stderr b/tests/ui/int_min_max_value.stderr index ad70794f4fec..4ef25a4ecf40 100644 --- a/tests/ui/int_min_max_value.stderr +++ b/tests/ui/int_min_max_value.stderr @@ -1,10 +1,17 @@ +error: use of the module constants `MIN` and `MAX` for primitive integer types is soft-deprecated + --> $DIR/int_min_max_value.rs:4:1 + | +LL | use std::i32; + | ^^^^^^^^^^^^^ + | + = note: `-D clippy::int-min-max-value` implied by `-D warnings` + = help: remove this import + error: `min_value` is soft-deprecated --> $DIR/int_min_max_value.rs:9:15 | LL | let min = u32::min_value(); | ^^^^^^^^^^^^^^^^ help: use constant instead: `u32::MIN` - | - = note: `-D clippy::int-min-max-value` implied by `-D warnings` error: `max_value` is soft-deprecated --> $DIR/int_min_max_value.rs:10:15 @@ -18,5 +25,11 @@ error: `min_value` is soft-deprecated LL | let min = crate::Uint::min_value(); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use constant instead: `crate::Uint::MIN` -error: aborting due to 3 previous errors +error: `std::u32::MAX` is soft-deprecated + --> $DIR/int_min_max_value.rs:12:15 + | +LL | let max = std::u32::MAX; + | ^^^^^^^^^^^^^ help: use associated constant instead: `u32::MAX` + +error: aborting due to 5 previous errors From f562c9d8b81f680c5588f63cba6ad2a6f2906adc Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Sun, 10 Jan 2021 12:34:25 -0500 Subject: [PATCH 4/4] Fix: symbol as string comparison --- clippy_lints/src/int_min_max_value.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/int_min_max_value.rs b/clippy_lints/src/int_min_max_value.rs index d1e716d8c000..a26a977dfa70 100644 --- a/clippy_lints/src/int_min_max_value.rs +++ b/clippy_lints/src/int_min_max_value.rs @@ -62,7 +62,7 @@ impl LateLintPass<'_> for IntMinMaxValue { if let ItemKind::Use(path, UseKind::Single) = item.kind; if let [crate_name, mod_name] = path.segments; then { - if !(crate_name.ident.as_str() == "std" || crate_name.ident.as_str() == "core") { + if !(crate_name.ident.name == sym::std || crate_name.ident.name == sym::core) { return; }