Auto merge of #4801 - mikerite:to_digit_is_some, r=flip1995

bors · bors · commit 79d3b30cd772 · 2019-11-11T11:28:27.000Z
To digit is some

Add a lint that recommends replacing `to_digit().is_some()` with `is_digit()` on `char`s

changelog: Add lint `to_digit_is_some`
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1203,6 +1203,7 @@ Released 2018-09-13
 [`suspicious_unary_op_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_unary_op_formatting
 [`temporary_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_assignment
 [`temporary_cstring_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_cstring_as_ptr
+[`to_digit_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_digit_is_some
 [`todo`]: https://rust-lang.github.io/rust-clippy/master/index.html#todo
 [`too_many_arguments`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments
 [`too_many_lines`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_lines
diff --git a/README.md b/README.md
@@ -6,7 +6,7 @@
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are 332 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are 333 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
 
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -274,6 +274,7 @@ pub mod strings;
 pub mod suspicious_trait_impl;
 pub mod swap;
 pub mod temporary_assignment;
+pub mod to_digit_is_some;
 pub mod trait_bounds;
 pub mod transmute;
 pub mod transmuting_null;
@@ -715,6 +716,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
         &swap::ALMOST_SWAPPED,
         &swap::MANUAL_SWAP,
         &temporary_assignment::TEMPORARY_ASSIGNMENT,
+        &to_digit_is_some::TO_DIGIT_IS_SOME,
         &trait_bounds::TYPE_REPETITION_IN_BOUNDS,
         &transmute::CROSSPOINTER_TRANSMUTE,
         &transmute::TRANSMUTE_BYTES_TO_STR,
@@ -946,6 +948,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
     store.register_late_pass(|| box unused_self::UnusedSelf);
     store.register_late_pass(|| box mutable_debug_assertion::DebugAssertWithMutCall);
     store.register_late_pass(|| box exit::Exit);
+    store.register_late_pass(|| box to_digit_is_some::ToDigitIsSome);
 
     store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
         LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1240,6 +1243,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
         LintId::of(&swap::ALMOST_SWAPPED),
         LintId::of(&swap::MANUAL_SWAP),
         LintId::of(&temporary_assignment::TEMPORARY_ASSIGNMENT),
+        LintId::of(&to_digit_is_some::TO_DIGIT_IS_SOME),
         LintId::of(&transmute::CROSSPOINTER_TRANSMUTE),
         LintId::of(&transmute::TRANSMUTE_BYTES_TO_STR),
         LintId::of(&transmute::TRANSMUTE_INT_TO_BOOL),
@@ -1365,6 +1369,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
         LintId::of(&returns::NEEDLESS_RETURN),
         LintId::of(&returns::UNUSED_UNIT),
         LintId::of(&strings::STRING_LIT_AS_BYTES),
+        LintId::of(&to_digit_is_some::TO_DIGIT_IS_SOME),
         LintId::of(&try_err::TRY_ERR),
         LintId::of(&types::FN_TO_NUMERIC_CAST),
         LintId::of(&types::FN_TO_NUMERIC_CAST_WITH_TRUNCATION),
diff --git a/clippy_lints/src/literal_representation.rs b/clippy_lints/src/literal_representation.rs
@@ -364,7 +364,7 @@ impl LiteralDigitGrouping {
                 if_chain! {
                     if let Some(src) = snippet_opt(cx, lit.span);
                     if let Some(firstch) = src.chars().next();
-                    if char::to_digit(firstch, 10).is_some();
+                    if char::is_digit(firstch, 10);
                     then {
                         let digit_info = DigitInfo::new(&src, false);
                         let _ = Self::do_lint(digit_info.digits, digit_info.suffix, in_macro).map_err(|warning_type| {
@@ -378,7 +378,7 @@ impl LiteralDigitGrouping {
                 if_chain! {
                     if let Some(src) = snippet_opt(cx, lit.span);
                     if let Some(firstch) = src.chars().next();
-                    if char::to_digit(firstch, 10).is_some();
+                    if char::is_digit(firstch, 10);
                     then {
                         let digit_info = DigitInfo::new(&src, true);
                         // Separate digits into integral and fractional parts.
@@ -512,7 +512,7 @@ impl DecimalLiteralRepresentation {
             if let LitKind::Int(val, _) = lit.kind;
             if let Some(src) = snippet_opt(cx, lit.span);
             if let Some(firstch) = src.chars().next();
-            if char::to_digit(firstch, 10).is_some();
+            if char::is_digit(firstch, 10);
             let digit_info = DigitInfo::new(&src, false);
             if digit_info.radix == Radix::Decimal;
             if val >= u128::from(self.threshold);
diff --git a/clippy_lints/src/to_digit_is_some.rs b/clippy_lints/src/to_digit_is_some.rs
@@ -0,0 +1,94 @@
+use crate::utils::{match_def_path, snippet_with_applicability, span_lint_and_sugg};
+use if_chain::if_chain;
+use rustc::hir;
+use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
+use rustc::ty;
+use rustc::{declare_lint_pass, declare_tool_lint};
+use rustc_errors::Applicability;
+
+declare_clippy_lint! {
+    /// **What it does:** Checks for `.to_digit(..).is_some()` on `char`s.
+    ///
+    /// **Why is this bad?** This is a convoluted way of checking if a `char` is a digit. It's
+    /// more straight forward to use the dedicated `is_digit` method.
+    ///
+    /// **Example:**
+    /// ```rust
+    /// # let c = 'c';
+    /// # let radix = 10;
+    /// let is_digit = c.to_digit(radix).is_some();
+    /// ```
+    /// can be written as:
+    /// ```
+    /// # let c = 'c';
+    /// # let radix = 10;
+    /// let is_digit = c.is_digit(radix);
+    /// ```
+    pub TO_DIGIT_IS_SOME,
+    style,
+    "`char.is_digit()` is clearer"
+}
+
+declare_lint_pass!(ToDigitIsSome => [TO_DIGIT_IS_SOME]);
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ToDigitIsSome {
+    fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr) {
+        if_chain! {
+            if let hir::ExprKind::MethodCall(is_some_path, _, is_some_args) = &expr.kind;
+            if is_some_path.ident.name.as_str() == "is_some";
+            if let [to_digit_expr] = &**is_some_args;
+            then {
+                let match_result = match &to_digit_expr.kind {
+                    hir::ExprKind::MethodCall(to_digits_path, _, to_digit_args) => {
+                        if_chain! {
+                            if let [char_arg, radix_arg] = &**to_digit_args;
+                            if to_digits_path.ident.name.as_str() == "to_digit";
+                            let char_arg_ty = cx.tables.expr_ty_adjusted(char_arg);
+                            if char_arg_ty.kind == ty::Char;
+                            then {
+                                Some((true, char_arg, radix_arg))
+                            } else {
+                                None
+                            }
+                        }
+                    }
+                    hir::ExprKind::Call(to_digits_call, to_digit_args) => {
+                        if_chain! {
+                            if let [char_arg, radix_arg] = &**to_digit_args;
+                            if let hir::ExprKind::Path(to_digits_path) = &to_digits_call.kind;
+                            if let to_digits_call_res = cx.tables.qpath_res(to_digits_path, to_digits_call.hir_id);
+                            if let Some(to_digits_def_id) = to_digits_call_res.opt_def_id();
+                            if match_def_path(cx, to_digits_def_id, &["core", "char", "methods", "<impl char>", "to_digit"]);
+                            then {
+                                Some((false, char_arg, radix_arg))
+                            } else {
+                                None
+                            }
+                        }
+                    }
+                    _ => None
+                };
+
+                if let Some((is_method_call, char_arg, radix_arg)) = match_result {
+                    let mut applicability = Applicability::MachineApplicable;
+                    let char_arg_snip = snippet_with_applicability(cx, char_arg.span, "_", &mut applicability);
+                    let radix_snip = snippet_with_applicability(cx, radix_arg.span, "_", &mut applicability);
+
+                    span_lint_and_sugg(
+                        cx,
+                        TO_DIGIT_IS_SOME,
+                        expr.span,
+                        "use of `.to_digit(..).is_some()`",
+                        "try this",
+                        if is_method_call {
+                            format!("{}.is_digit({})", char_arg_snip, radix_snip)
+                        } else {
+                            format!("char::is_digit({}, {})", char_arg_snip, radix_snip)
+                        },
+                        applicability,
+                    );
+                }
+            }
+        }
+    }
+}
diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
@@ -6,7 +6,7 @@ pub use lint::Lint;
 pub use lint::LINT_LEVELS;
 
 // begin lint list, do not remove this comment, it’s used in `update_lints`
-pub const ALL_LINTS: [Lint; 332] = [
+pub const ALL_LINTS: [Lint; 333] = [
     Lint {
         name: "absurd_extreme_comparisons",
         group: "correctness",
@@ -1876,6 +1876,13 @@ pub const ALL_LINTS: [Lint; 332] = [
         deprecation: None,
         module: "methods",
     },
+    Lint {
+        name: "to_digit_is_some",
+        group: "style",
+        desc: "`char.is_digit()` is clearer",
+        deprecation: None,
+        module: "to_digit_is_some",
+    },
     Lint {
         name: "todo",
         group: "restriction",
diff --git a/tests/ui/to_digit_is_some.fixed b/tests/ui/to_digit_is_some.fixed
@@ -0,0 +1,11 @@
+//run-rustfix
+
+#![warn(clippy::to_digit_is_some)]
+
+fn main() {
+    let c = 'x';
+    let d = &c;
+
+    let _ = d.is_digit(10);
+    let _ = char::is_digit(c, 10);
+}
diff --git a/tests/ui/to_digit_is_some.rs b/tests/ui/to_digit_is_some.rs
@@ -0,0 +1,11 @@
+//run-rustfix
+
+#![warn(clippy::to_digit_is_some)]
+
+fn main() {
+    let c = 'x';
+    let d = &c;
+
+    let _ = d.to_digit(10).is_some();
+    let _ = char::to_digit(c, 10).is_some();
+}
diff --git a/tests/ui/to_digit_is_some.stderr b/tests/ui/to_digit_is_some.stderr
@@ -0,0 +1,16 @@
+error: use of `.to_digit(..).is_some()`
+  --> $DIR/to_digit_is_some.rs:9:13
+   |
+LL |     let _ = d.to_digit(10).is_some();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `d.is_digit(10)`
+   |
+   = note: `-D clippy::to-digit-is-some` implied by `-D warnings`
+
+error: use of `.to_digit(..).is_some()`
+  --> $DIR/to_digit_is_some.rs:10:13
+   |
+LL |     let _ = char::to_digit(c, 10).is_some();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `char::is_digit(c, 10)`
+
+error: aborting due to 2 previous errors
+
