From b80ac2af9c0939428054e7d018f19fc16b920b05 Mon Sep 17 00:00:00 2001 From: boolean_coercion Date: Wed, 10 Feb 2021 21:47:04 +0200 Subject: [PATCH 01/10] Added boilerplate --- CHANGELOG.md | 1 + clippy_lints/src/from_str_radix_10.rs | 34 +++++++++++++++++++++++++++ clippy_lints/src/lib.rs | 4 ++++ tests/ui/from_str_radix_10.rs | 5 ++++ 4 files changed, 44 insertions(+) create mode 100644 clippy_lints/src/from_str_radix_10.rs create mode 100644 tests/ui/from_str_radix_10.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 4dcf9c3529bb..56b74a7d0ab6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2092,6 +2092,7 @@ Released 2018-09-13 [`forget_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_ref [`from_iter_instead_of_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_iter_instead_of_collect [`from_over_into`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_over_into +[`from_str_radix_10`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_str_radix_10 [`future_not_send`]: https://rust-lang.github.io/rust-clippy/master/index.html#future_not_send [`get_last_with_len`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_last_with_len [`get_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_unwrap diff --git a/clippy_lints/src/from_str_radix_10.rs b/clippy_lints/src/from_str_radix_10.rs new file mode 100644 index 000000000000..ec2a60ec47c5 --- /dev/null +++ b/clippy_lints/src/from_str_radix_10.rs @@ -0,0 +1,34 @@ +use rustc_lint::{EarlyLintPass, EarlyContext}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_ast::ast::*; + +declare_clippy_lint! { + /// **What it does:** + /// Checks for function invocations of the form `primitive::from_str_radix(s, 10)` + /// + /// **Why is this bad?** + /// This specific common use case can be rewritten as `s.parse::()` + /// (and in most cases, the turbofish can be removed), which reduces code length + /// and complexity. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust + /// let input: &str = get_input(); + /// let num = u16::from_str_radix(input, 10)?; + /// ``` + /// Use instead: + /// ```rust + /// let input: &str = get_input(); + /// let num: u16 = input.parse()?; + /// ``` + pub FROM_STR_RADIX_10, + style, + "default lint description" +} + +declare_lint_pass!(FromStrRadix10 => [FROM_STR_RADIX_10]); + +impl EarlyLintPass for FromStrRadix10 {} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index d96911fac1a0..5b84422458fb 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -210,6 +210,7 @@ mod floating_point_arithmetic; mod format; mod formatting; mod from_over_into; +mod from_str_radix_10; mod functions; mod future_not_send; mod get_last_with_len; @@ -637,6 +638,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &formatting::SUSPICIOUS_ELSE_FORMATTING, &formatting::SUSPICIOUS_UNARY_OP_FORMATTING, &from_over_into::FROM_OVER_INTO, + &from_str_radix_10::FROM_STR_RADIX_10, &functions::DOUBLE_MUST_USE, &functions::MUST_USE_CANDIDATE, &functions::MUST_USE_UNIT, @@ -1468,6 +1470,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&formatting::SUSPICIOUS_ELSE_FORMATTING), LintId::of(&formatting::SUSPICIOUS_UNARY_OP_FORMATTING), LintId::of(&from_over_into::FROM_OVER_INTO), + LintId::of(&from_str_radix_10::FROM_STR_RADIX_10), LintId::of(&functions::DOUBLE_MUST_USE), LintId::of(&functions::MUST_USE_UNIT), LintId::of(&functions::NOT_UNSAFE_PTR_ARG_DEREF), @@ -1724,6 +1727,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&formatting::SUSPICIOUS_ELSE_FORMATTING), LintId::of(&formatting::SUSPICIOUS_UNARY_OP_FORMATTING), LintId::of(&from_over_into::FROM_OVER_INTO), + LintId::of(&from_str_radix_10::FROM_STR_RADIX_10), LintId::of(&functions::DOUBLE_MUST_USE), LintId::of(&functions::MUST_USE_UNIT), LintId::of(&functions::RESULT_UNIT_ERR), diff --git a/tests/ui/from_str_radix_10.rs b/tests/ui/from_str_radix_10.rs new file mode 100644 index 000000000000..70eaa8d666c0 --- /dev/null +++ b/tests/ui/from_str_radix_10.rs @@ -0,0 +1,5 @@ +#![warn(clippy::from_str_radix_10)] + +fn main() { + // test code goes here +} From 64729390a1b2b6b05f8a4407658163ddff4d017e Mon Sep 17 00:00:00 2001 From: boolean_coercion Date: Thu, 11 Feb 2021 02:30:37 +0200 Subject: [PATCH 02/10] Implemented majority of from_str_radix_10 --- clippy_lints/src/from_str_radix_10.rs | 59 +++++++++++++++++++++++++-- clippy_lints/src/lib.rs | 1 + tests/ui/from_str_radix_10.rs | 32 ++++++++++++++- 3 files changed, 86 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/from_str_radix_10.rs b/clippy_lints/src/from_str_radix_10.rs index ec2a60ec47c5..612ea9ae62c7 100644 --- a/clippy_lints/src/from_str_radix_10.rs +++ b/clippy_lints/src/from_str_radix_10.rs @@ -1,6 +1,10 @@ -use rustc_lint::{EarlyLintPass, EarlyContext}; +use rustc_lint::{LateLintPass, LateContext}; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_ast::ast::*; +use rustc_hir::*; +use rustc_errors::Applicability; +use if_chain::if_chain; + +use crate::utils::span_lint_and_sugg; declare_clippy_lint! { /// **What it does:** @@ -26,9 +30,56 @@ declare_clippy_lint! { /// ``` pub FROM_STR_RADIX_10, style, - "default lint description" + "from_str_radix with radix 10" } declare_lint_pass!(FromStrRadix10 => [FROM_STR_RADIX_10]); -impl EarlyLintPass for FromStrRadix10 {} +impl LateLintPass<'tcx> for FromStrRadix10 { + fn check_expr(&mut self, cx: &LateContext<'tcx>, exp: &Expr<'tcx>) { + if_chain! { + if let ExprKind::Call(maybe_path, arguments) = &exp.kind; + if let ExprKind::Path(qpath) = &maybe_path.kind; + if let QPath::TypeRelative(ty, pathseg) = &qpath; + + // check if the first part of the path is some integer primitive + if let TyKind::Path(ty_qpath) = &ty.kind; + let ty_res = cx.qpath_res(ty_qpath, ty.hir_id); + if let def::Res::PrimTy(prim_ty) = ty_res; + if is_primitive_integer_ty(prim_ty); + + // check if the second part of the path indeed calls the associated + // function `from_str_radix` + if pathseg.ident.name.as_str() == "from_str_radix"; + + // check if the second argument resolves to a constant `10` + if arguments.len() == 2; + if is_constant_10(&arguments[1]); + + then { + span_lint_and_sugg( + cx, + FROM_STR_RADIX_10, + exp.span, + "This call to `from_str_radix` can be shortened to a call to str::parse", + "try", + format!("TODO"), + Applicability::MachineApplicable + ); + } + } + } +} + +fn is_primitive_integer_ty(ty: PrimTy) -> bool { + match ty { + PrimTy::Int(_) => true, + PrimTy::Uint(_) => true, + _ => false + } +} + +fn is_constant_10<'tcx>(expr: &Expr<'tcx>) -> bool { + // TODO + true +} \ No newline at end of file diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 5b84422458fb..dae686b1229d 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1258,6 +1258,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || box types::PtrAsPtr::new(msrv)); store.register_late_pass(|| box case_sensitive_file_extension_comparisons::CaseSensitiveFileExtensionComparisons); store.register_late_pass(|| box redundant_slicing::RedundantSlicing); + store.register_late_pass(|| box from_str_radix_10::FromStrRadix10); store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![ LintId::of(&arithmetic::FLOAT_ARITHMETIC), diff --git a/tests/ui/from_str_radix_10.rs b/tests/ui/from_str_radix_10.rs index 70eaa8d666c0..086616d09ff8 100644 --- a/tests/ui/from_str_radix_10.rs +++ b/tests/ui/from_str_radix_10.rs @@ -1,5 +1,33 @@ #![warn(clippy::from_str_radix_10)] -fn main() { - // test code goes here +mod some_mod { + // fake function that shouldn't trigger the lint + pub fn from_str_radix(_: &str, _: u32) -> Result<(), std::num::ParseIntError> { + unimplemented!() + } } + +// fake function that shouldn't trigger the lint +fn from_str_radix(_: &str, _: u32) -> Result<(), std::num::ParseIntError> { + unimplemented!() +} + +fn main() -> Result<(), Box> { + // all of these should trigger the lint + u32::from_str_radix("30", 10)?; + i64::from_str_radix("24", 10)?; + isize::from_str_radix("100", 10)?; + u8::from_str_radix("7", 10)?; + + // none of these should trigger the lint + u16::from_str_radix("20", 3)?; + i32::from_str_radix("45", 12)?; + usize::from_str_radix("10", 16)?; + i128::from_str_radix("10", 13)?; + some_mod::from_str_radix("50", 10)?; + some_mod::from_str_radix("50", 6)?; + from_str_radix("50", 10)?; + from_str_radix("50", 6)?; + + Ok(()) +} \ No newline at end of file From a389c02461107a71da042c1a9dbf4c0e597df3b4 Mon Sep 17 00:00:00 2001 From: boolean_coercion Date: Thu, 11 Feb 2021 12:30:23 +0200 Subject: [PATCH 03/10] from_str_radix_10 should be done --- clippy_lints/src/from_str_radix_10.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/clippy_lints/src/from_str_radix_10.rs b/clippy_lints/src/from_str_radix_10.rs index 612ea9ae62c7..57661e0f9bca 100644 --- a/clippy_lints/src/from_str_radix_10.rs +++ b/clippy_lints/src/from_str_radix_10.rs @@ -52,18 +52,20 @@ impl LateLintPass<'tcx> for FromStrRadix10 { // function `from_str_radix` if pathseg.ident.name.as_str() == "from_str_radix"; - // check if the second argument resolves to a constant `10` + // check if the second argument is a primitive `10` if arguments.len() == 2; - if is_constant_10(&arguments[1]); + if let ExprKind::Lit(lit) = &arguments[1].kind; + if let rustc_ast::ast::LitKind::Int(10, _) = lit.node; then { + let orig_string = crate::utils::snippet(cx, arguments[0].span, "string"); span_lint_and_sugg( cx, FROM_STR_RADIX_10, exp.span, "This call to `from_str_radix` can be shortened to a call to str::parse", "try", - format!("TODO"), + format!("({}).parse()", orig_string), Applicability::MachineApplicable ); } @@ -77,9 +79,4 @@ fn is_primitive_integer_ty(ty: PrimTy) -> bool { PrimTy::Uint(_) => true, _ => false } -} - -fn is_constant_10<'tcx>(expr: &Expr<'tcx>) -> bool { - // TODO - true } \ No newline at end of file From 0b31b470ad743fc758a1c47a17d7241948cd4a3b Mon Sep 17 00:00:00 2001 From: boolean_coercion Date: Thu, 11 Feb 2021 12:42:20 +0200 Subject: [PATCH 04/10] Changed applicability to MaybeIncorrect because of surrounding braces --- clippy_lints/src/from_str_radix_10.rs | 2 +- tests/ui/from_str_radix_10.rs | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/from_str_radix_10.rs b/clippy_lints/src/from_str_radix_10.rs index 57661e0f9bca..9371104bf9b2 100644 --- a/clippy_lints/src/from_str_radix_10.rs +++ b/clippy_lints/src/from_str_radix_10.rs @@ -66,7 +66,7 @@ impl LateLintPass<'tcx> for FromStrRadix10 { "This call to `from_str_radix` can be shortened to a call to str::parse", "try", format!("({}).parse()", orig_string), - Applicability::MachineApplicable + Applicability::MaybeIncorrect ); } } diff --git a/tests/ui/from_str_radix_10.rs b/tests/ui/from_str_radix_10.rs index 086616d09ff8..795e795fcb3a 100644 --- a/tests/ui/from_str_radix_10.rs +++ b/tests/ui/from_str_radix_10.rs @@ -19,6 +19,9 @@ fn main() -> Result<(), Box> { isize::from_str_radix("100", 10)?; u8::from_str_radix("7", 10)?; + let string = "300"; + i32::from_str_radix(string, 10)?; + // none of these should trigger the lint u16::from_str_radix("20", 3)?; i32::from_str_radix("45", 12)?; From d1a627ab3bd801879a565cc9e68a322d8d28bcbf Mon Sep 17 00:00:00 2001 From: boolean_coercion Date: Thu, 11 Feb 2021 12:46:11 +0200 Subject: [PATCH 05/10] Ran bless and rustfmt --- clippy_lints/src/from_str_radix_10.rs | 14 +++++------ tests/ui/from_str_radix_10.rs | 2 +- tests/ui/from_str_radix_10.stderr | 34 +++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 8 deletions(-) create mode 100644 tests/ui/from_str_radix_10.stderr diff --git a/clippy_lints/src/from_str_radix_10.rs b/clippy_lints/src/from_str_radix_10.rs index 9371104bf9b2..9a8d4542616d 100644 --- a/clippy_lints/src/from_str_radix_10.rs +++ b/clippy_lints/src/from_str_radix_10.rs @@ -1,8 +1,8 @@ -use rustc_lint::{LateLintPass, LateContext}; -use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_hir::*; -use rustc_errors::Applicability; use if_chain::if_chain; +use rustc_errors::Applicability; +use rustc_hir::*; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; use crate::utils::span_lint_and_sugg; @@ -16,7 +16,7 @@ declare_clippy_lint! { /// and complexity. /// /// **Known problems:** None. - /// + /// /// **Example:** /// /// ```rust @@ -77,6 +77,6 @@ fn is_primitive_integer_ty(ty: PrimTy) -> bool { match ty { PrimTy::Int(_) => true, PrimTy::Uint(_) => true, - _ => false + _ => false, } -} \ No newline at end of file +} diff --git a/tests/ui/from_str_radix_10.rs b/tests/ui/from_str_radix_10.rs index 795e795fcb3a..2d8106da7ba3 100644 --- a/tests/ui/from_str_radix_10.rs +++ b/tests/ui/from_str_radix_10.rs @@ -33,4 +33,4 @@ fn main() -> Result<(), Box> { from_str_radix("50", 6)?; Ok(()) -} \ No newline at end of file +} diff --git a/tests/ui/from_str_radix_10.stderr b/tests/ui/from_str_radix_10.stderr new file mode 100644 index 000000000000..376d0dd56eab --- /dev/null +++ b/tests/ui/from_str_radix_10.stderr @@ -0,0 +1,34 @@ +error: This call to `from_str_radix` can be shortened to a call to str::parse + --> $DIR/from_str_radix_10.rs:17:5 + | +LL | u32::from_str_radix("30", 10)?; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `("30").parse()` + | + = note: `-D clippy::from-str-radix-10` implied by `-D warnings` + +error: This call to `from_str_radix` can be shortened to a call to str::parse + --> $DIR/from_str_radix_10.rs:18:5 + | +LL | i64::from_str_radix("24", 10)?; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `("24").parse()` + +error: This call to `from_str_radix` can be shortened to a call to str::parse + --> $DIR/from_str_radix_10.rs:19:5 + | +LL | isize::from_str_radix("100", 10)?; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `("100").parse()` + +error: This call to `from_str_radix` can be shortened to a call to str::parse + --> $DIR/from_str_radix_10.rs:20:5 + | +LL | u8::from_str_radix("7", 10)?; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `("7").parse()` + +error: This call to `from_str_radix` can be shortened to a call to str::parse + --> $DIR/from_str_radix_10.rs:23:5 + | +LL | i32::from_str_radix(string, 10)?; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(string).parse()` + +error: aborting due to 5 previous errors + From 9194c11d69aa98c9d85fd0979bf9b93d3d7de809 Mon Sep 17 00:00:00 2001 From: boolean_coercion Date: Thu, 11 Feb 2021 13:15:06 +0200 Subject: [PATCH 06/10] Fixed doctests that shouldn't have been compiled --- clippy_lints/src/from_str_radix_10.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/from_str_radix_10.rs b/clippy_lints/src/from_str_radix_10.rs index 9a8d4542616d..53cb1e3ecd98 100644 --- a/clippy_lints/src/from_str_radix_10.rs +++ b/clippy_lints/src/from_str_radix_10.rs @@ -19,12 +19,12 @@ declare_clippy_lint! { /// /// **Example:** /// - /// ```rust + /// ```ignore /// let input: &str = get_input(); /// let num = u16::from_str_radix(input, 10)?; /// ``` /// Use instead: - /// ```rust + /// ```ignore /// let input: &str = get_input(); /// let num: u16 = input.parse()?; /// ``` From 642efabfbbba6577a3698a305bac8ce0c693e67f Mon Sep 17 00:00:00 2001 From: boolean_coercion Date: Fri, 12 Feb 2021 11:53:52 +0200 Subject: [PATCH 07/10] Fixed typos and updated to matches! where applicable --- clippy_lints/src/from_str_radix_10.rs | 12 ++---------- tests/ui/from_str_radix_10.stderr | 10 +++++----- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/clippy_lints/src/from_str_radix_10.rs b/clippy_lints/src/from_str_radix_10.rs index 53cb1e3ecd98..de9add4b6b6b 100644 --- a/clippy_lints/src/from_str_radix_10.rs +++ b/clippy_lints/src/from_str_radix_10.rs @@ -46,7 +46,7 @@ impl LateLintPass<'tcx> for FromStrRadix10 { if let TyKind::Path(ty_qpath) = &ty.kind; let ty_res = cx.qpath_res(ty_qpath, ty.hir_id); if let def::Res::PrimTy(prim_ty) = ty_res; - if is_primitive_integer_ty(prim_ty); + if matches!(prim_ty, PrimTy::Int(_) | PrimTy::Uint(_)); // check if the second part of the path indeed calls the associated // function `from_str_radix` @@ -63,7 +63,7 @@ impl LateLintPass<'tcx> for FromStrRadix10 { cx, FROM_STR_RADIX_10, exp.span, - "This call to `from_str_radix` can be shortened to a call to str::parse", + "this call to `from_str_radix` can be replaced with a call to `str::parse`", "try", format!("({}).parse()", orig_string), Applicability::MaybeIncorrect @@ -72,11 +72,3 @@ impl LateLintPass<'tcx> for FromStrRadix10 { } } } - -fn is_primitive_integer_ty(ty: PrimTy) -> bool { - match ty { - PrimTy::Int(_) => true, - PrimTy::Uint(_) => true, - _ => false, - } -} diff --git a/tests/ui/from_str_radix_10.stderr b/tests/ui/from_str_radix_10.stderr index 376d0dd56eab..5557cd3b9eff 100644 --- a/tests/ui/from_str_radix_10.stderr +++ b/tests/ui/from_str_radix_10.stderr @@ -1,4 +1,4 @@ -error: This call to `from_str_radix` can be shortened to a call to str::parse +error: this call to `from_str_radix` can be replaced with a call to `str::parse` --> $DIR/from_str_radix_10.rs:17:5 | LL | u32::from_str_radix("30", 10)?; @@ -6,25 +6,25 @@ LL | u32::from_str_radix("30", 10)?; | = note: `-D clippy::from-str-radix-10` implied by `-D warnings` -error: This call to `from_str_radix` can be shortened to a call to str::parse +error: this call to `from_str_radix` can be replaced with a call to `str::parse` --> $DIR/from_str_radix_10.rs:18:5 | LL | i64::from_str_radix("24", 10)?; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `("24").parse()` -error: This call to `from_str_radix` can be shortened to a call to str::parse +error: this call to `from_str_radix` can be replaced with a call to `str::parse` --> $DIR/from_str_radix_10.rs:19:5 | LL | isize::from_str_radix("100", 10)?; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `("100").parse()` -error: This call to `from_str_radix` can be shortened to a call to str::parse +error: this call to `from_str_radix` can be replaced with a call to `str::parse` --> $DIR/from_str_radix_10.rs:20:5 | LL | u8::from_str_radix("7", 10)?; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `("7").parse()` -error: This call to `from_str_radix` can be shortened to a call to str::parse +error: this call to `from_str_radix` can be replaced with a call to `str::parse` --> $DIR/from_str_radix_10.rs:23:5 | LL | i32::from_str_radix(string, 10)?; From d36fe8556962467545f5e92bf896b6672f4e88ae Mon Sep 17 00:00:00 2001 From: boolean_coercion Date: Sat, 13 Feb 2021 00:33:08 +0200 Subject: [PATCH 08/10] Made parens addition smarter and added tests with bless --- clippy_lints/src/from_str_radix_10.rs | 11 +++++++-- tests/ui/from_str_radix_10.rs | 13 ++++++++++ tests/ui/from_str_radix_10.stderr | 34 ++++++++++++++++++--------- 3 files changed, 45 insertions(+), 13 deletions(-) diff --git a/clippy_lints/src/from_str_radix_10.rs b/clippy_lints/src/from_str_radix_10.rs index de9add4b6b6b..993b85ed9989 100644 --- a/clippy_lints/src/from_str_radix_10.rs +++ b/clippy_lints/src/from_str_radix_10.rs @@ -5,6 +5,7 @@ use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use crate::utils::span_lint_and_sugg; +use crate::utils::sugg::Sugg; declare_clippy_lint! { /// **What it does:** @@ -58,14 +59,20 @@ impl LateLintPass<'tcx> for FromStrRadix10 { if let rustc_ast::ast::LitKind::Int(10, _) = lit.node; then { - let orig_string = crate::utils::snippet(cx, arguments[0].span, "string"); + let sugg = Sugg::hir_with_applicability( + cx, + &arguments[0], + "", + &mut Applicability::MachineApplicable + ).maybe_par(); + span_lint_and_sugg( cx, FROM_STR_RADIX_10, exp.span, "this call to `from_str_radix` can be replaced with a call to `str::parse`", "try", - format!("({}).parse()", orig_string), + format!("{}.parse::<{}>()", sugg, prim_ty.name_str()), Applicability::MaybeIncorrect ); } diff --git a/tests/ui/from_str_radix_10.rs b/tests/ui/from_str_radix_10.rs index 2d8106da7ba3..0a9731286643 100644 --- a/tests/ui/from_str_radix_10.rs +++ b/tests/ui/from_str_radix_10.rs @@ -12,12 +12,25 @@ fn from_str_radix(_: &str, _: u32) -> Result<(), std::num::ParseIntError> { unimplemented!() } +// to test parenthesis addition +struct Test; + +impl std::ops::Add for Test { + type Output = &'static str; + + fn add(self, _: Self) -> Self::Output { + "304" + } +} + fn main() -> Result<(), Box> { // all of these should trigger the lint u32::from_str_radix("30", 10)?; i64::from_str_radix("24", 10)?; isize::from_str_radix("100", 10)?; u8::from_str_radix("7", 10)?; + u16::from_str_radix(&("10".to_owned() + "5"), 10)?; + i128::from_str_radix(Test + Test, 10)?; let string = "300"; i32::from_str_radix(string, 10)?; diff --git a/tests/ui/from_str_radix_10.stderr b/tests/ui/from_str_radix_10.stderr index 5557cd3b9eff..d66690019158 100644 --- a/tests/ui/from_str_radix_10.stderr +++ b/tests/ui/from_str_radix_10.stderr @@ -1,34 +1,46 @@ error: this call to `from_str_radix` can be replaced with a call to `str::parse` - --> $DIR/from_str_radix_10.rs:17:5 + --> $DIR/from_str_radix_10.rs:28:5 | LL | u32::from_str_radix("30", 10)?; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `("30").parse()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `"30".parse::()` | = note: `-D clippy::from-str-radix-10` implied by `-D warnings` error: this call to `from_str_radix` can be replaced with a call to `str::parse` - --> $DIR/from_str_radix_10.rs:18:5 + --> $DIR/from_str_radix_10.rs:29:5 | LL | i64::from_str_radix("24", 10)?; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `("24").parse()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `"24".parse::()` error: this call to `from_str_radix` can be replaced with a call to `str::parse` - --> $DIR/from_str_radix_10.rs:19:5 + --> $DIR/from_str_radix_10.rs:30:5 | LL | isize::from_str_radix("100", 10)?; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `("100").parse()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `"100".parse::()` error: this call to `from_str_radix` can be replaced with a call to `str::parse` - --> $DIR/from_str_radix_10.rs:20:5 + --> $DIR/from_str_radix_10.rs:31:5 | LL | u8::from_str_radix("7", 10)?; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `("7").parse()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `"7".parse::()` error: this call to `from_str_radix` can be replaced with a call to `str::parse` - --> $DIR/from_str_radix_10.rs:23:5 + --> $DIR/from_str_radix_10.rs:32:5 + | +LL | u16::from_str_radix(&("10".to_owned() + "5"), 10)?; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(&("10".to_owned() + "5")).parse::()` + +error: this call to `from_str_radix` can be replaced with a call to `str::parse` + --> $DIR/from_str_radix_10.rs:33:5 + | +LL | i128::from_str_radix(Test + Test, 10)?; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(Test + Test).parse::()` + +error: this call to `from_str_radix` can be replaced with a call to `str::parse` + --> $DIR/from_str_radix_10.rs:36:5 | LL | i32::from_str_radix(string, 10)?; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(string).parse()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `string.parse::()` -error: aborting due to 5 previous errors +error: aborting due to 7 previous errors From bf55aee7b142ad14d102de3260e314a84bf8c35c Mon Sep 17 00:00:00 2001 From: bool Date: Fri, 19 Feb 2021 19:36:28 +0200 Subject: [PATCH 09/10] Updated from_str_radix_10 sugg to be slightly smarter and ran bless --- clippy_lints/src/from_str_radix_10.rs | 26 ++++++++++++++++++++++---- tests/ui/from_str_radix_10.rs | 3 +++ tests/ui/from_str_radix_10.stderr | 10 ++++++++-- 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/from_str_radix_10.rs b/clippy_lints/src/from_str_radix_10.rs index 993b85ed9989..d0a170acb4fe 100644 --- a/clippy_lints/src/from_str_radix_10.rs +++ b/clippy_lints/src/from_str_radix_10.rs @@ -1,9 +1,12 @@ use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::*; +use rustc_hir::{def, Expr, ExprKind, PrimTy, QPath, TyKind}; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::Ty; use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::symbol::sym; +use crate::utils::is_type_diagnostic_item; use crate::utils::span_lint_and_sugg; use crate::utils::sugg::Sugg; @@ -40,8 +43,7 @@ impl LateLintPass<'tcx> for FromStrRadix10 { fn check_expr(&mut self, cx: &LateContext<'tcx>, exp: &Expr<'tcx>) { if_chain! { if let ExprKind::Call(maybe_path, arguments) = &exp.kind; - if let ExprKind::Path(qpath) = &maybe_path.kind; - if let QPath::TypeRelative(ty, pathseg) = &qpath; + if let ExprKind::Path(QPath::TypeRelative(ty, pathseg)) = &maybe_path.kind; // check if the first part of the path is some integer primitive if let TyKind::Path(ty_qpath) = &ty.kind; @@ -59,9 +61,20 @@ impl LateLintPass<'tcx> for FromStrRadix10 { if let rustc_ast::ast::LitKind::Int(10, _) = lit.node; then { + let expr = if let ExprKind::AddrOf(_, _, expr) = &arguments[0].kind { + let ty = cx.typeck_results().expr_ty(expr); + if is_ty_stringish(cx, ty) { + expr + } else { + &arguments[0] + } + } else { + &arguments[0] + }; + let sugg = Sugg::hir_with_applicability( cx, - &arguments[0], + expr, "", &mut Applicability::MachineApplicable ).maybe_par(); @@ -79,3 +92,8 @@ impl LateLintPass<'tcx> for FromStrRadix10 { } } } + +/// Checks if a Ty is `String` or `&str` +fn is_ty_stringish(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { + is_type_diagnostic_item(cx, ty, sym::string_type) || is_type_diagnostic_item(cx, ty, sym::str) +} diff --git a/tests/ui/from_str_radix_10.rs b/tests/ui/from_str_radix_10.rs index 0a9731286643..2f2ea04847a9 100644 --- a/tests/ui/from_str_radix_10.rs +++ b/tests/ui/from_str_radix_10.rs @@ -35,6 +35,9 @@ fn main() -> Result<(), Box> { let string = "300"; i32::from_str_radix(string, 10)?; + let stringier = "400".to_string(); + i32::from_str_radix(&stringier, 10)?; + // none of these should trigger the lint u16::from_str_radix("20", 3)?; i32::from_str_radix("45", 12)?; diff --git a/tests/ui/from_str_radix_10.stderr b/tests/ui/from_str_radix_10.stderr index d66690019158..471bf52a9a7e 100644 --- a/tests/ui/from_str_radix_10.stderr +++ b/tests/ui/from_str_radix_10.stderr @@ -28,7 +28,7 @@ error: this call to `from_str_radix` can be replaced with a call to `str::parse` --> $DIR/from_str_radix_10.rs:32:5 | LL | u16::from_str_radix(&("10".to_owned() + "5"), 10)?; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(&("10".to_owned() + "5")).parse::()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(("10".to_owned() + "5")).parse::()` error: this call to `from_str_radix` can be replaced with a call to `str::parse` --> $DIR/from_str_radix_10.rs:33:5 @@ -42,5 +42,11 @@ error: this call to `from_str_radix` can be replaced with a call to `str::parse` LL | i32::from_str_radix(string, 10)?; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `string.parse::()` -error: aborting due to 7 previous errors +error: this call to `from_str_radix` can be replaced with a call to `str::parse` + --> $DIR/from_str_radix_10.rs:39:5 + | +LL | i32::from_str_radix(&stringier, 10)?; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `stringier.parse::()` + +error: aborting due to 8 previous errors From c4b8d87ab96cb6ef44685d928adeb9915b06f71f Mon Sep 17 00:00:00 2001 From: bool Date: Fri, 19 Feb 2021 22:00:23 +0200 Subject: [PATCH 10/10] Fixed the known problems section --- clippy_lints/src/from_str_radix_10.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/from_str_radix_10.rs b/clippy_lints/src/from_str_radix_10.rs index d0a170acb4fe..0933f9830147 100644 --- a/clippy_lints/src/from_str_radix_10.rs +++ b/clippy_lints/src/from_str_radix_10.rs @@ -19,7 +19,9 @@ declare_clippy_lint! { /// (and in most cases, the turbofish can be removed), which reduces code length /// and complexity. /// - /// **Known problems:** None. + /// **Known problems:** + /// This lint may suggest using (&).parse() instead of .parse() directly + /// in some cases, which is correct but adds unnecessary complexity to the code. /// /// **Example:** ///