Merge pull request #2839 from mikerite/duration_subsec_pr_2

Add duration_subsec lint

Author: oli-obk

clippy_lints/src/duration_subsec.rs

@@ -0,0 +1,63 @@
+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 other `Duration` methods.
+///
+/// **Why is this bad?** It's more concise to call `Duration::subsec_micros()` or
+/// `Duration::subsec_millis()` than to calculate them.
+///
+/// **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_clippy_lint! {
+    pub DURATION_SUBSEC,
+    complexity,
+    "checks for calculation of subsecond microseconds or milliseconds"
+}
+
+#[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 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 (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,
+                    expr.span,
+                    &format!("Calling `{}()` is more concise than this calculation", suggested_fn),
+                    "try",
+                    format!("{}.{}()", snippet(cx, args[0].span, "_"), suggested_fn),
+                );
+            }
+        }
+    }
+}

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![
@@ -517,6 +519,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,
@@ -789,6 +792,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,

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"];

tests/ui/duration_subsec.rs

@@ -0,0 +1,27 @@
+#![warn(duration_subsec)]
+
+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);
+
+    // 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;
+}

tests/ui/duration_subsec.stderr

@@ -0,0 +1,34 @@
+error: Calling `subsec_millis()` is more concise than this calculation
+ --> $DIR/duration_subsec.rs:8:24
+  |
+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: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
+   |
+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:19:13
+   |
+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:23:13
+   |
+23 |     let _ = dur.subsec_nanos() / NANOS_IN_MICRO;
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `dur.subsec_micros()`
+
+error: aborting due to 5 previous errors
+