From 7b2fa2077fa48826fa98fab4402de0c5037de065 Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Sun, 18 Mar 2018 22:27:15 +0200 Subject: [PATCH 1/3] Add duration_subsec lint Closes #2543 --- clippy_lints/src/duration_subsec.rs | 64 +++++++++++++++++++++++++++++ clippy_lints/src/lib.rs | 3 ++ clippy_lints/src/utils/paths.rs | 1 + tests/ui/duration_subsec.rs | 26 ++++++++++++ tests/ui/duration_subsec.stderr | 28 +++++++++++++ tests/ui/duration_subsec.stdout | 0 6 files changed, 122 insertions(+) create mode 100644 clippy_lints/src/duration_subsec.rs create mode 100644 tests/ui/duration_subsec.rs create mode 100644 tests/ui/duration_subsec.stderr create mode 100644 tests/ui/duration_subsec.stdout diff --git a/clippy_lints/src/duration_subsec.rs b/clippy_lints/src/duration_subsec.rs new file mode 100644 index 000000000000..de75276ada7b --- /dev/null +++ b/clippy_lints/src/duration_subsec.rs @@ -0,0 +1,64 @@ +use rustc::hir::*; +use rustc::lint::*; +use syntax::codemap::Spanned; + +use crate::consts::{constant, Constant}; +use crate::utils::paths; +use crate::utils::{match_type, snippet, span_lint_and_sugg, walk_ptrs_ty}; + +/// **What it does:** Checks for calculation of subsecond microseconds or milliseconds from +/// `Duration::subsec_nanos()`. +/// +/// **Why is this bad?** It's more concise to call `Duration::subsec_micros()` or +/// `Duration::subsec_millis()`. +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust +/// let dur = Duration::new(5, 0); +/// let _micros = dur.subsec_nanos() / 1_000; +/// let _millis = dur.subsec_nanos() / 1_000_000; +/// ``` +declare_lint! { + pub DURATION_SUBSEC, + Warn, + "checks for `dur.subsec_nanos() / 1_000` or `dur.subsec_nanos() / 1_000_000`" +} + +#[derive(Copy, Clone)] +pub struct DurationSubsec; + +impl LintPass for DurationSubsec { + fn get_lints(&self) -> LintArray { + lint_array!(DURATION_SUBSEC) + } +} + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DurationSubsec { + fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { + if_chain! { + if let ExprBinary(Spanned { node: BiDiv, .. }, ref left, ref right) = expr.node; + if let ExprMethodCall(ref method_path, _ , ref args) = left.node; + if method_path.name == "subsec_nanos"; + if match_type(cx, walk_ptrs_ty(cx.tables.expr_ty(&args[0])), &paths::DURATION); + if let Some((Constant::Int(divisor), _)) = constant(cx, cx.tables, right); + then { + let suggested_fn = match divisor { + 1_000 => "subsec_micros", + 1_000_000 => "subsec_millis", + _ => return, + }; + + span_lint_and_sugg( + cx, + DURATION_SUBSEC, + expr.span, + &format!("Calling `{}()` is more concise than this calculation", suggested_fn), + "try", + format!("{}.{}()", snippet(cx, args[0].span, "_"), suggested_fn), + ); + } + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index c4dfc1f0168d..8188dd87836b 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -116,6 +116,7 @@ pub mod doc; pub mod double_comparison; pub mod double_parens; pub mod drop_forget_ref; +pub mod duration_subsec; pub mod else_if_without_else; pub mod empty_enum; pub mod entry; @@ -423,6 +424,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { reg.register_late_lint_pass(box inherent_impl::Pass::default()); reg.register_late_lint_pass(box neg_cmp_op_on_partial_ord::NoNegCompOpForPartialOrd); reg.register_late_lint_pass(box unwrap::Pass); + reg.register_late_lint_pass(box duration_subsec::DurationSubsec); reg.register_lint_group("clippy_restriction", vec![ @@ -518,6 +520,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { drop_forget_ref::DROP_REF, drop_forget_ref::FORGET_COPY, drop_forget_ref::FORGET_REF, + duration_subsec::DURATION_SUBSEC, entry::MAP_ENTRY, enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT, enum_variants::ENUM_VARIANT_NAMES, diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index ab62346ea7ef..5af1a151c8ca 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -27,6 +27,7 @@ pub const DEFAULT_TRAIT: [&str; 3] = ["core", "default", "Default"]; pub const DISPLAY_FMT_METHOD: [&str; 4] = ["core", "fmt", "Display", "fmt"]; pub const DOUBLE_ENDED_ITERATOR: [&str; 4] = ["core", "iter", "traits", "DoubleEndedIterator"]; pub const DROP: [&str; 3] = ["core", "mem", "drop"]; +pub const DURATION: [&str; 3] = ["core", "time", "Duration"]; pub const FMT_ARGUMENTS_NEWV1: [&str; 4] = ["core", "fmt", "Arguments", "new_v1"]; pub const FMT_ARGUMENTS_NEWV1FORMATTED: [&str; 4] = ["core", "fmt", "Arguments", "new_v1_formatted"]; pub const FMT_ARGUMENTV1_NEW: [&str; 4] = ["core", "fmt", "ArgumentV1", "new"]; diff --git a/tests/ui/duration_subsec.rs b/tests/ui/duration_subsec.rs new file mode 100644 index 000000000000..5b87d69646e7 --- /dev/null +++ b/tests/ui/duration_subsec.rs @@ -0,0 +1,26 @@ +#![warn(duration_subsec)] + +use std::time::Duration; + +fn main() { + let dur = Duration::new(5, 0); + + let bad_micros = dur.subsec_nanos() / 1_000; + let good_micros = dur.subsec_micros(); + assert_eq!(bad_micros, good_micros); + + let bad_millis = dur.subsec_nanos() / 1_000_000; + let good_millis = dur.subsec_millis(); + assert_eq!(bad_millis, good_millis); + + // Handle refs + let _ = (&dur).subsec_nanos() / 1_000; + + // Handle constants + const NANOS_IN_MICRO: u32 = 1_000; + let _ = dur.subsec_nanos() / NANOS_IN_MICRO; + + // Other literals aren't linted + let _ = dur.subsec_nanos() / 699; + +} diff --git a/tests/ui/duration_subsec.stderr b/tests/ui/duration_subsec.stderr new file mode 100644 index 000000000000..f77aa2172aa6 --- /dev/null +++ b/tests/ui/duration_subsec.stderr @@ -0,0 +1,28 @@ +error: Calling `subsec_micros()` is more concise than this calculation + --> $DIR/duration_subsec.rs:8:22 + | +8 | let bad_micros = dur.subsec_nanos() / 1_000; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `dur.subsec_micros()` + | + = note: `-D duration-subsec` implied by `-D warnings` + +error: Calling `subsec_millis()` is more concise than this calculation + --> $DIR/duration_subsec.rs:12:22 + | +12 | let bad_millis = dur.subsec_nanos() / 1_000_000; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `dur.subsec_millis()` + +error: Calling `subsec_micros()` is more concise than this calculation + --> $DIR/duration_subsec.rs:17:13 + | +17 | let _ = (&dur).subsec_nanos() / 1_000; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(&dur).subsec_micros()` + +error: Calling `subsec_micros()` is more concise than this calculation + --> $DIR/duration_subsec.rs:21:13 + | +21 | let _ = dur.subsec_nanos() / NANOS_IN_MICRO; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `dur.subsec_micros()` + +error: aborting due to 4 previous errors + diff --git a/tests/ui/duration_subsec.stdout b/tests/ui/duration_subsec.stdout new file mode 100644 index 000000000000..e69de29bb2d1 From b0d364cb3e4e646591c5b3e84595415ba295fa0f Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Sat, 9 Jun 2018 11:04:21 +0200 Subject: [PATCH 2/3] duration_subsec: fix declaration; correctly classify --- clippy_lints/src/duration_subsec.rs | 4 ++-- clippy_lints/src/lib.rs | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/duration_subsec.rs b/clippy_lints/src/duration_subsec.rs index de75276ada7b..fced237b66f8 100644 --- a/clippy_lints/src/duration_subsec.rs +++ b/clippy_lints/src/duration_subsec.rs @@ -20,9 +20,9 @@ use crate::utils::{match_type, snippet, span_lint_and_sugg, walk_ptrs_ty}; /// let _micros = dur.subsec_nanos() / 1_000; /// let _millis = dur.subsec_nanos() / 1_000_000; /// ``` -declare_lint! { +declare_clippy_lint! { pub DURATION_SUBSEC, - Warn, + complexity, "checks for `dur.subsec_nanos() / 1_000` or `dur.subsec_nanos() / 1_000_000`" } diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 8188dd87836b..553a41c066c3 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -789,6 +789,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { cyclomatic_complexity::CYCLOMATIC_COMPLEXITY, double_comparison::DOUBLE_COMPARISONS, double_parens::DOUBLE_PARENS, + duration_subsec::DURATION_SUBSEC, eval_order_dependence::DIVERGING_SUB_EXPRESSION, eval_order_dependence::EVAL_ORDER_DEPENDENCE, explicit_write::EXPLICIT_WRITE, From 725e9621d0ea1f07ad75d5f26193dfba72ee73b2 Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Tue, 12 Jun 2018 08:25:10 +0200 Subject: [PATCH 3/3] duration_subsec: Add check for `subsec_micros` --- clippy_lints/src/duration_subsec.rs | 17 ++++++++-------- tests/ui/duration_subsec.rs | 11 ++++++----- tests/ui/duration_subsec.stderr | 30 +++++++++++++++++------------ 3 files changed, 32 insertions(+), 26 deletions(-) diff --git a/clippy_lints/src/duration_subsec.rs b/clippy_lints/src/duration_subsec.rs index fced237b66f8..5f34803c8137 100644 --- a/clippy_lints/src/duration_subsec.rs +++ b/clippy_lints/src/duration_subsec.rs @@ -6,11 +6,11 @@ use crate::consts::{constant, Constant}; use crate::utils::paths; use crate::utils::{match_type, snippet, span_lint_and_sugg, walk_ptrs_ty}; -/// **What it does:** Checks for calculation of subsecond microseconds or milliseconds from -/// `Duration::subsec_nanos()`. +/// **What it does:** Checks for calculation of subsecond microseconds or milliseconds +/// from other `Duration` methods. /// /// **Why is this bad?** It's more concise to call `Duration::subsec_micros()` or -/// `Duration::subsec_millis()`. +/// `Duration::subsec_millis()` than to calculate them. /// /// **Known problems:** None. /// @@ -23,7 +23,7 @@ use crate::utils::{match_type, snippet, span_lint_and_sugg, walk_ptrs_ty}; declare_clippy_lint! { pub DURATION_SUBSEC, complexity, - "checks for `dur.subsec_nanos() / 1_000` or `dur.subsec_nanos() / 1_000_000`" + "checks for calculation of subsecond microseconds or milliseconds" } #[derive(Copy, Clone)] @@ -40,16 +40,15 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DurationSubsec { if_chain! { if let ExprBinary(Spanned { node: BiDiv, .. }, ref left, ref right) = expr.node; if let ExprMethodCall(ref method_path, _ , ref args) = left.node; - if method_path.name == "subsec_nanos"; if match_type(cx, walk_ptrs_ty(cx.tables.expr_ty(&args[0])), &paths::DURATION); if let Some((Constant::Int(divisor), _)) = constant(cx, cx.tables, right); then { - let suggested_fn = match divisor { - 1_000 => "subsec_micros", - 1_000_000 => "subsec_millis", + let suggested_fn = match (method_path.name.as_str().as_ref(), divisor) { + ("subsec_micros", 1_000) => "subsec_millis", + ("subsec_nanos", 1_000) => "subsec_micros", + ("subsec_nanos", 1_000_000) => "subsec_millis", _ => return, }; - span_lint_and_sugg( cx, DURATION_SUBSEC, diff --git a/tests/ui/duration_subsec.rs b/tests/ui/duration_subsec.rs index 5b87d69646e7..8c75c5f2fcd8 100644 --- a/tests/ui/duration_subsec.rs +++ b/tests/ui/duration_subsec.rs @@ -5,14 +5,16 @@ use std::time::Duration; fn main() { let dur = Duration::new(5, 0); + let bad_millis_1 = dur.subsec_micros() / 1_000; + let bad_millis_2 = dur.subsec_nanos() / 1_000_000; + let good_millis = dur.subsec_millis(); + assert_eq!(bad_millis_1, good_millis); + assert_eq!(bad_millis_2, good_millis); + let bad_micros = dur.subsec_nanos() / 1_000; let good_micros = dur.subsec_micros(); assert_eq!(bad_micros, good_micros); - let bad_millis = dur.subsec_nanos() / 1_000_000; - let good_millis = dur.subsec_millis(); - assert_eq!(bad_millis, good_millis); - // Handle refs let _ = (&dur).subsec_nanos() / 1_000; @@ -22,5 +24,4 @@ fn main() { // Other literals aren't linted let _ = dur.subsec_nanos() / 699; - } diff --git a/tests/ui/duration_subsec.stderr b/tests/ui/duration_subsec.stderr index f77aa2172aa6..a1aacec3a75a 100644 --- a/tests/ui/duration_subsec.stderr +++ b/tests/ui/duration_subsec.stderr @@ -1,28 +1,34 @@ -error: Calling `subsec_micros()` is more concise than this calculation - --> $DIR/duration_subsec.rs:8:22 +error: Calling `subsec_millis()` is more concise than this calculation + --> $DIR/duration_subsec.rs:8:24 | -8 | let bad_micros = dur.subsec_nanos() / 1_000; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `dur.subsec_micros()` +8 | let bad_millis_1 = dur.subsec_micros() / 1_000; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `dur.subsec_millis()` | = note: `-D duration-subsec` implied by `-D warnings` error: Calling `subsec_millis()` is more concise than this calculation - --> $DIR/duration_subsec.rs:12:22 + --> $DIR/duration_subsec.rs:9:24 + | +9 | let bad_millis_2 = dur.subsec_nanos() / 1_000_000; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `dur.subsec_millis()` + +error: Calling `subsec_micros()` is more concise than this calculation + --> $DIR/duration_subsec.rs:14:22 | -12 | let bad_millis = dur.subsec_nanos() / 1_000_000; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `dur.subsec_millis()` +14 | let bad_micros = dur.subsec_nanos() / 1_000; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `dur.subsec_micros()` error: Calling `subsec_micros()` is more concise than this calculation - --> $DIR/duration_subsec.rs:17:13 + --> $DIR/duration_subsec.rs:19:13 | -17 | let _ = (&dur).subsec_nanos() / 1_000; +19 | let _ = (&dur).subsec_nanos() / 1_000; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(&dur).subsec_micros()` error: Calling `subsec_micros()` is more concise than this calculation - --> $DIR/duration_subsec.rs:21:13 + --> $DIR/duration_subsec.rs:23:13 | -21 | let _ = dur.subsec_nanos() / NANOS_IN_MICRO; +23 | let _ = dur.subsec_nanos() / NANOS_IN_MICRO; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `dur.subsec_micros()` -error: aborting due to 4 previous errors +error: aborting due to 5 previous errors