Skip to content

Commit

Permalink
new lint legacy_integral_constants
Browse files Browse the repository at this point in the history
  • Loading branch information
Centri3 committed Jun 20, 2023
1 parent 62972ae commit da08b7f
Show file tree
Hide file tree
Showing 17 changed files with 311 additions and 43 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4887,6 +4887,7 @@ Released 2018-09-13
[`large_stack_arrays`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_stack_arrays
[`large_stack_frames`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_stack_frames
[`large_types_passed_by_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_types_passed_by_value
[`legacy_integral_constants`]: https://rust-lang.github.io/rust-clippy/master/index.html#legacy_integral_constants
[`len_without_is_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#len_without_is_empty
[`len_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#len_zero
[`let_and_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/casts/cast_possible_truncation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ fn apply_reductions(cx: &LateContext<'_>, nbits: u64, expr: &Expr<'_>, signed: b
})
},
BinOpKind::Rem | BinOpKind::BitAnd => get_constant_bits(cx, right)
.unwrap_or(u64::max_value())
.unwrap_or(u64::MAX)
.min(apply_reductions(cx, nbits, left, signed)),
BinOpKind::Shr => apply_reductions(cx, nbits, left, signed)
.saturating_sub(constant_int(cx, right).map_or(0, |s| u64::try_from(s).expect("shift too high"))),
Expand All @@ -56,7 +56,7 @@ fn apply_reductions(cx: &LateContext<'_>, nbits: u64, expr: &Expr<'_>, signed: b
} else {
None
};
apply_reductions(cx, nbits, left, signed).min(max_bits.unwrap_or(u64::max_value()))
apply_reductions(cx, nbits, left, signed).min(max_bits.unwrap_or(u64::MAX))
},
ExprKind::MethodCall(method, _, [lo, hi], _) => {
if method.ident.as_str() == "clamp" {
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 @@ -230,6 +230,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::large_include_file::LARGE_INCLUDE_FILE_INFO,
crate::large_stack_arrays::LARGE_STACK_ARRAYS_INFO,
crate::large_stack_frames::LARGE_STACK_FRAMES_INFO,
crate::legacy_integral_constants::LEGACY_INTEGRAL_CONSTANTS_INFO,
crate::len_zero::COMPARISON_TO_EMPTY_INFO,
crate::len_zero::LEN_WITHOUT_IS_EMPTY_INFO,
crate::len_zero::LEN_ZERO_INFO,
Expand Down
24 changes: 12 additions & 12 deletions clippy_lints/src/implicit_saturating_add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,18 +82,18 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitSaturatingAdd {

fn get_int_max(ty: Ty<'_>) -> Option<u128> {
match ty.peel_refs().kind() {
Int(IntTy::I8) => i8::max_value().try_into().ok(),
Int(IntTy::I16) => i16::max_value().try_into().ok(),
Int(IntTy::I32) => i32::max_value().try_into().ok(),
Int(IntTy::I64) => i64::max_value().try_into().ok(),
Int(IntTy::I128) => i128::max_value().try_into().ok(),
Int(IntTy::Isize) => isize::max_value().try_into().ok(),
Uint(UintTy::U8) => u8::max_value().try_into().ok(),
Uint(UintTy::U16) => u16::max_value().try_into().ok(),
Uint(UintTy::U32) => u32::max_value().try_into().ok(),
Uint(UintTy::U64) => u64::max_value().try_into().ok(),
Uint(UintTy::U128) => Some(u128::max_value()),
Uint(UintTy::Usize) => usize::max_value().try_into().ok(),
Int(IntTy::I8) => i8::MAX.try_into().ok(),
Int(IntTy::I16) => i16::MAX.try_into().ok(),
Int(IntTy::I32) => i32::MAX.try_into().ok(),
Int(IntTy::I64) => i64::MAX.try_into().ok(),
Int(IntTy::I128) => i128::MAX.try_into().ok(),
Int(IntTy::Isize) => isize::MAX.try_into().ok(),
Uint(UintTy::U8) => u8::MAX.try_into().ok(),
Uint(UintTy::U16) => u16::MAX.try_into().ok(),
Uint(UintTy::U32) => u32::MAX.try_into().ok(),
Uint(UintTy::U64) => u64::MAX.try_into().ok(),
Uint(UintTy::U128) => Some(u128::MAX),
Uint(UintTy::Usize) => usize::MAX.try_into().ok(),
_ => None,
}
}
Expand Down
135 changes: 135 additions & 0 deletions clippy_lints/src/legacy_integral_constants.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
use clippy_utils::{
diagnostics::span_lint_and_sugg,
get_parent_expr, is_from_proc_macro, last_path_segment,
msrvs::{self, Msrv},
};
use rustc_errors::Applicability;
use rustc_hir::{def::Res, def_id::DefId};
use rustc_hir::{Expr, ExprKind, PrimTy, QPath, TyKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::{sym, Symbol};

declare_clippy_lint! {
/// ### What it does
/// Checks for usage of `<integer>::max_value()`, `std::<integer>::MAX`,
/// `std::<float>::EPSILON`, etc.
///
/// ### Why is this bad?
/// All of these have been superceded by the associated constants on their respective types,
/// such as `i128::MAX`. These legacy constants may be deprecated in a future version of rust.
///
/// ### Example
/// ```rust
/// let eps = std::f32::EPSILON;
/// ```
/// Use instead:
/// ```rust
/// let eps = f32::EPSILON;
/// ```
#[clippy::version = "1.72.0"]
pub LEGACY_INTEGRAL_CONSTANTS,
style,
"checks for usage of legacy std integral constants"
}
pub struct LegacyIntegralConstants {
msrv: Msrv,
}

impl LegacyIntegralConstants {
#[must_use]
pub fn new(msrv: Msrv) -> Self {
Self { msrv }
}
}

impl_lint_pass!(LegacyIntegralConstants => [LEGACY_INTEGRAL_CONSTANTS]);

impl<'tcx> LateLintPass<'tcx> for LegacyIntegralConstants {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
if !self.msrv.meets(msrvs::STD_INTEGRAL_CONSTANTS) || in_external_macro(cx.sess(), expr.span) {
return;
}
let ExprKind::Path(qpath) = expr.kind else {
return;
};

// `std::<integer>::<CONST>` check
let (span, sugg, is_method) = if let QPath::Resolved(_, path) = qpath
&& let Some(def_id) = path.res.opt_def_id()
&& let Some(name) = path.segments.iter().last().map(|segment| segment.ident.name)
&& let Some(module_name) = is_path_in_integral_module(cx, def_id)
{
(
expr.span,
format!("{module_name}::{name}"),
false,
)
// `<integer>::xxx_value` check
} else if let QPath::TypeRelative(ty, _) = qpath
&& let TyKind::Path(ty_qpath) = ty.kind
&& let Res::PrimTy(PrimTy::Int(_) | PrimTy::Uint(_)) = cx.qpath_res(&ty_qpath, ty.hir_id)
&& let last_segment = last_path_segment(&qpath)
&& let name = last_segment.ident.name.as_str()
&& (name == "max_value" || name == "min_value")
// Also remove the `()`
&& let Some(par_expr) = get_parent_expr(cx, expr)
&& let ExprKind::Call(_, _) = par_expr.kind
{
(
qpath.last_segment_span().with_hi(par_expr.span.hi()),
name[..=2].to_ascii_uppercase(),
true,
)
} else {
return;
};

if !is_from_proc_macro(cx, expr) {
let msg = if is_method {
"usage of a legacy integral constant method"
} else {
"usage of a legacy integral constant"
};

span_lint_and_sugg(
cx,
LEGACY_INTEGRAL_CONSTANTS,
span,
msg,
"try using the associated constant instead",
sugg,
Applicability::MachineApplicable,
);
}
}

extract_msrv_attr!(LateContext);
}

fn is_path_in_integral_module(cx: &LateContext<'_>, def_id: DefId) -> Option<Symbol> {
if let [
sym::core,
module @ (sym::u8
| sym::i8
| sym::u16
| sym::i16
| sym::u32
| sym::i32
| sym::u64
| sym::i64
| sym::u128
| sym::i128
| sym::usize
| sym::isize
| sym::f32
| sym::f64),
_,
] = &*cx.get_def_path(def_id)
{
return Some(*module);
}

None
}
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ mod large_futures;
mod large_include_file;
mod large_stack_arrays;
mod large_stack_frames;
mod legacy_integral_constants;
mod len_zero;
mod let_if_seq;
mod let_underscore;
Expand Down Expand Up @@ -1063,6 +1064,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
def_id_to_usage: rustc_data_structures::fx::FxHashMap::default(),
})
});
store.register_late_pass(move |_| Box::new(legacy_integral_constants::LegacyIntegralConstants::new(msrv())));
// add lints here, do not remove this comment, it's used in `new_lint`
}

Expand Down
2 changes: 1 addition & 1 deletion clippy_utils/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ msrv_aliases! {
1,47,0 { TAU, IS_ASCII_DIGIT_CONST, ARRAY_IMPL_ANY_LEN }
1,46,0 { CONST_IF_MATCH }
1,45,0 { STR_STRIP_PREFIX }
1,43,0 { LOG2_10, LOG10_2 }
1,43,0 { LOG2_10, LOG10_2, STD_INTEGRAL_CONSTANTS }
1,42,0 { MATCHES_MACRO, SLICE_PATTERNS, PTR_SLICE_RAW_PARTS }
1,41,0 { RE_REBALANCING_COHERENCE, RESULT_MAP_OR_ELSE }
1,40,0 { MEM_TAKE, NON_EXHAUSTIVE, OPTION_AS_DEREF }
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 @@ -2,6 +2,7 @@

#![allow(
clippy::cast_lossless,
clippy::legacy_integral_constants,
unused,
// Int::max_value will be deprecated in the future
deprecated,
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 @@ -2,6 +2,7 @@

#![allow(
clippy::cast_lossless,
clippy::legacy_integral_constants,
unused,
// Int::max_value will be deprecated in the future
deprecated,
Expand Down
34 changes: 17 additions & 17 deletions tests/ui/checked_conversions.stderr
Original file line number Diff line number Diff line change
@@ -1,103 +1,103 @@
error: checked cast can be simplified
--> $DIR/checked_conversions.rs:16:13
--> $DIR/checked_conversions.rs:17: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:17:13
--> $DIR/checked_conversions.rs:18: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:21:13
--> $DIR/checked_conversions.rs:22: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:22:13
--> $DIR/checked_conversions.rs:23: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:26:13
--> $DIR/checked_conversions.rs:27: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:27:13
--> $DIR/checked_conversions.rs:28: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:33:13
--> $DIR/checked_conversions.rs:34: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:34:13
--> $DIR/checked_conversions.rs:35: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:38:13
--> $DIR/checked_conversions.rs:39: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:39:13
--> $DIR/checked_conversions.rs:40: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:45:13
--> $DIR/checked_conversions.rs:46: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:46:13
--> $DIR/checked_conversions.rs:47: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:50:13
--> $DIR/checked_conversions.rs:51: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:51:13
--> $DIR/checked_conversions.rs:52: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:55:13
--> $DIR/checked_conversions.rs:56: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:56:13
--> $DIR/checked_conversions.rs:57:13
|
LL | let _ = value <= u16::MAX as u32 && value as i32 == 5;
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u16::try_from(value).is_ok()`

error: checked cast can be simplified
--> $DIR/checked_conversions.rs:89:13
--> $DIR/checked_conversions.rs:90:13
|
LL | let _ = value <= (u32::MAX as i64) && value >= 0;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `u32::try_from(value).is_ok()`
Expand Down
31 changes: 31 additions & 0 deletions tests/ui/legacy_integral_constants.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//@run-rustfix
//@aux-build:proc_macros.rs
#![allow(clippy::no_effect, deprecated, unused)]
#![warn(clippy::legacy_integral_constants)]

#[macro_use]
extern crate proc_macros;

fn main() {
f32::EPSILON;
u8::MIN;
usize::MIN;
u32::MAX;
use std::u32::MAX;
u32::MAX;
u32::MAX;
u8::MAX;
u8::MIN;
::std::primitive::u8::MIN;
::std::u8::MIN;
::std::primitive::u8::MIN;
std::primitive::u32::MAX;
// Don't lint
f32::EPSILON;
u8::MIN;
external! {
::std::primitive::u8::MIN;
::std::u8::MIN;
::std::primitive::u8::min_value();
}
}
Loading

0 comments on commit da08b7f

Please sign in to comment.