From 472134fb9c1f149795fe96a9923c888d51132ac0 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 Requested in issue #2543 --- clippy_lints/src/duration_subsec.rs | 65 +++++++++++++++++++++++++++++ clippy_lints/src/lib.rs | 3 ++ clippy_lints/src/utils/paths.rs | 1 + tests/ui/duration_subsec.rs | 23 ++++++++++ tests/ui/duration_subsec.stderr | 22 ++++++++++ tests/ui/duration_subsec.stdout | 0 6 files changed, 114 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..3d5a0a73d267 --- /dev/null +++ b/clippy_lints/src/duration_subsec.rs @@ -0,0 +1,65 @@ +use rustc::lint::*; +use rustc::hir::*; +use syntax::ast::LitKind; +use syntax::codemap::Spanned; +use utils::{match_type, snippet, span_lint_and_sugg, walk_ptrs_ty}; +use utils::paths; + +/// **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, + "checking `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 ExprLit(ref lit) = right.node; + then { + if let Spanned { node: LitKind::Int(divisor, _), .. } = **lit { + 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 calcuation", 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 d508a17a2645..7fc901873908 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -88,6 +88,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; @@ -376,6 +377,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { reg.register_late_lint_pass(box question_mark::QuestionMarkPass); reg.register_late_lint_pass(box suspicious_trait_impl::SuspiciousImpl); reg.register_late_lint_pass(box redundant_field_names::RedundantFieldNames); + reg.register_late_lint_pass(box duration_subsec::DurationSubsec); reg.register_lint_group("clippy_restrictions", vec![ @@ -471,6 +473,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 81c402ab49c9..b5ec70ea8c53 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -25,6 +25,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_ARGUMENTV1_NEW: [&str; 4] = ["core", "fmt", "ArgumentV1", "new"]; pub const FROM_FROM: [&str; 4] = ["core", "convert", "From", "from"]; diff --git a/tests/ui/duration_subsec.rs b/tests/ui/duration_subsec.rs new file mode 100644 index 000000000000..ca0f71873330 --- /dev/null +++ b/tests/ui/duration_subsec.rs @@ -0,0 +1,23 @@ +#![warn(duration_subsec)] + +#![feature(duration_extras)] + +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; + + // 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..17b545b088c1 --- /dev/null +++ b/tests/ui/duration_subsec.stderr @@ -0,0 +1,22 @@ +error: Calling `subsec_micros()` is more concise than this calcuation + --> $DIR/duration_subsec.rs:10:22 + | +10 | 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 calcuation + --> $DIR/duration_subsec.rs:14:22 + | +14 | let bad_millis = dur.subsec_nanos() / 1_000_000; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `dur.subsec_millis()` + +error: Calling `subsec_micros()` is more concise than this calcuation + --> $DIR/duration_subsec.rs:19:13 + | +19 | let _ = (&dur).subsec_nanos() / 1_000; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(&dur).subsec_micros()` + +error: aborting due to 3 previous errors + diff --git a/tests/ui/duration_subsec.stdout b/tests/ui/duration_subsec.stdout new file mode 100644 index 000000000000..e69de29bb2d1 From 4b79895464fae9abf016441223d620ff1d8ca4cc Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Thu, 22 Mar 2018 20:39:44 +0200 Subject: [PATCH 2/3] duration_subsec: Handle constant divisors --- clippy_lints/src/duration_subsec.rs | 32 ++++++++++++++--------------- tests/ui/duration_subsec.rs | 5 +++++ tests/ui/duration_subsec.stderr | 8 +++++++- 3 files changed, 27 insertions(+), 18 deletions(-) diff --git a/clippy_lints/src/duration_subsec.rs b/clippy_lints/src/duration_subsec.rs index 3d5a0a73d267..4532efd4a14b 100644 --- a/clippy_lints/src/duration_subsec.rs +++ b/clippy_lints/src/duration_subsec.rs @@ -1,7 +1,7 @@ use rustc::lint::*; use rustc::hir::*; -use syntax::ast::LitKind; use syntax::codemap::Spanned; +use consts::{constant, Constant}; use utils::{match_type, snippet, span_lint_and_sugg, walk_ptrs_ty}; use utils::paths; @@ -41,24 +41,22 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DurationSubsec { 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 ExprLit(ref lit) = right.node; + if let Some((Constant::Int(divisor), _)) = constant(cx, right); then { - if let Spanned { node: LitKind::Int(divisor, _), .. } = **lit { - let suggested_fn = match divisor { - 1_000 => "subsec_micros", - 1_000_000 => "subsec_millis", - _ => return, - }; + 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 calcuation", suggested_fn), - "try", - format!("{}.{}()", snippet(cx, args[0].span, "_"), suggested_fn), - ); - } + span_lint_and_sugg( + cx, + DURATION_SUBSEC, + expr.span, + &format!("Calling `{}()` is more concise than this calcuation", suggested_fn), + "try", + format!("{}.{}()", snippet(cx, args[0].span, "_"), suggested_fn), + ); } } } diff --git a/tests/ui/duration_subsec.rs b/tests/ui/duration_subsec.rs index ca0f71873330..c69773152585 100644 --- a/tests/ui/duration_subsec.rs +++ b/tests/ui/duration_subsec.rs @@ -18,6 +18,11 @@ fn main() { // Handle refs let _ = (&dur).subsec_nanos() / 1_000; + // Handle simple 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 index 17b545b088c1..c7405362d0f3 100644 --- a/tests/ui/duration_subsec.stderr +++ b/tests/ui/duration_subsec.stderr @@ -18,5 +18,11 @@ error: Calling `subsec_micros()` is more concise than this calcuation 19 | let _ = (&dur).subsec_nanos() / 1_000; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(&dur).subsec_micros()` -error: aborting due to 3 previous errors +error: Calling `subsec_micros()` is more concise than this calcuation + --> $DIR/duration_subsec.rs:23:13 + | +23 | let _ = dur.subsec_nanos() / NANOS_IN_MICRO; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `dur.subsec_micros()` + +error: aborting due to 4 previous errors From 6385770a4d3a70dc26be8cc7a63efdc6bc2c075f Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Sun, 25 Mar 2018 10:44:47 +0200 Subject: [PATCH 3/3] duration_subsec: Correct spelling error --- clippy_lints/src/duration_subsec.rs | 2 +- tests/ui/duration_subsec.stderr | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/duration_subsec.rs b/clippy_lints/src/duration_subsec.rs index 4532efd4a14b..0e9213496e23 100644 --- a/clippy_lints/src/duration_subsec.rs +++ b/clippy_lints/src/duration_subsec.rs @@ -53,7 +53,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DurationSubsec { cx, DURATION_SUBSEC, expr.span, - &format!("Calling `{}()` is more concise than this calcuation", suggested_fn), + &format!("Calling `{}()` is more concise than this calculation", suggested_fn), "try", format!("{}.{}()", snippet(cx, args[0].span, "_"), suggested_fn), ); diff --git a/tests/ui/duration_subsec.stderr b/tests/ui/duration_subsec.stderr index c7405362d0f3..19e143314c8d 100644 --- a/tests/ui/duration_subsec.stderr +++ b/tests/ui/duration_subsec.stderr @@ -1,4 +1,4 @@ -error: Calling `subsec_micros()` is more concise than this calcuation +error: Calling `subsec_micros()` is more concise than this calculation --> $DIR/duration_subsec.rs:10:22 | 10 | let bad_micros = dur.subsec_nanos() / 1_000; @@ -6,19 +6,19 @@ error: Calling `subsec_micros()` is more concise than this calcuation | = note: `-D duration-subsec` implied by `-D warnings` -error: Calling `subsec_millis()` is more concise than this calcuation +error: Calling `subsec_millis()` is more concise than this calculation --> $DIR/duration_subsec.rs:14:22 | 14 | let bad_millis = dur.subsec_nanos() / 1_000_000; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `dur.subsec_millis()` -error: Calling `subsec_micros()` is more concise than this calcuation +error: Calling `subsec_micros()` is more concise than this calculation --> $DIR/duration_subsec.rs:19:13 | 19 | let _ = (&dur).subsec_nanos() / 1_000; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(&dur).subsec_micros()` -error: Calling `subsec_micros()` is more concise than this calcuation +error: Calling `subsec_micros()` is more concise than this calculation --> $DIR/duration_subsec.rs:23:13 | 23 | let _ = dur.subsec_nanos() / NANOS_IN_MICRO;