From bf55aee7b142ad14d102de3260e314a84bf8c35c Mon Sep 17 00:00:00 2001 From: bool Date: Fri, 19 Feb 2021 19:36:28 +0200 Subject: [PATCH] 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 993b85ed9..d0a170acb 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 0a9731286..2f2ea0484 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 d66690019..471bf52a9 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