diff --git a/clippy_lints/src/duration_subsec.rs b/clippy_lints/src/duration_subsec.rs new file mode 100644 index 00000000000..de75276ada7 --- /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 c4dfc1f0168..8188dd87836 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 ab62346ea7e..5af1a151c8c 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 00000000000..5b87d69646e --- /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 00000000000..f77aa2172aa --- /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 00000000000..e69de29bb2d