Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the from_str_radix_10 lint #6717

Merged
merged 10 commits into from
Feb 20, 2021
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
74 changes: 74 additions & 0 deletions clippy_lints/src/from_str_radix_10.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
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;

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::<primitive>()`
/// (and in most cases, the turbofish can be removed), which reduces code length
/// and complexity.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```ignore
/// let input: &str = get_input();
/// let num = u16::from_str_radix(input, 10)?;
/// ```
/// Use instead:
/// ```ignore
/// let input: &str = get_input();
/// let num: u16 = input.parse()?;
/// ```
pub FROM_STR_RADIX_10,
style,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think style is OK, as I don't expect any false positives here.

"from_str_radix with radix 10"
}

declare_lint_pass!(FromStrRadix10 => [FROM_STR_RADIX_10]);

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 matches!(prim_ty, PrimTy::Int(_) | PrimTy::Uint(_));

// 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 is a primitive `10`
if arguments.len() == 2;
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 replaced with a call to `str::parse`",
"try",
format!("({}).parse()", orig_string),
booleancoercion marked this conversation as resolved.
Show resolved Hide resolved
Applicability::MaybeIncorrect
);
}
}
}
}
5 changes: 5 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -1256,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),
Expand Down Expand Up @@ -1468,6 +1471,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),
Expand Down Expand Up @@ -1724,6 +1728,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),
Expand Down
36 changes: 36 additions & 0 deletions tests/ui/from_str_radix_10.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#![warn(clippy::from_str_radix_10)]

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<dyn std::error::Error>> {
// 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)?;

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)?;
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(())
}
34 changes: 34 additions & 0 deletions tests/ui/from_str_radix_10.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
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)?;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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
|
LL | i64::from_str_radix("24", 10)?;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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
|
LL | isize::from_str_radix("100", 10)?;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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
|
LL | u8::from_str_radix("7", 10)?;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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
|
LL | i32::from_str_radix(string, 10)?;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(string).parse()`

error: aborting due to 5 previous errors