diff --git a/CHANGELOG.md b/CHANGELOG.md index 84a3bd491cb8..30d3108e0be4 100644 --- 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 index 87ef441eadd5..922dbcd11380 100644 --- 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 index dae1c429b7d5..c108290e4782 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -272,6 +272,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; @@ -713,6 +714,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, @@ -944,6 +946,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), @@ -1238,6 +1241,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), @@ -1363,6 +1367,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 index 18ac8f0f4736..8ddd54bb7a38 100644 --- 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 new file mode 100644 index 000000000000..a40d92f38332 --- /dev/null +++ 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", "", "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 index 5c92d69a4993..0b301b6be963 100644 --- 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 new file mode 100644 index 000000000000..19184df0becb --- /dev/null +++ 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 new file mode 100644 index 000000000000..45a6728ebf57 --- /dev/null +++ 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 new file mode 100644 index 000000000000..177d3ccd3e23 --- /dev/null +++ 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 +