From 1c3033d5cf903d19d8f3dcdfc01ba1dc55debf71 Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Mon, 8 Feb 2021 01:34:59 +0900 Subject: [PATCH 1/3] add a new lint `bytes_nth` --- CHANGELOG.md | 1 + clippy_lints/src/lib.rs | 3 +++ clippy_lints/src/methods/bytes_nth.rs | 39 +++++++++++++++++++++++++++ clippy_lints/src/methods/mod.rs | 25 +++++++++++++++++ tests/ui/bytes_nth.fixed | 9 +++++++ tests/ui/bytes_nth.rs | 9 +++++++ tests/ui/bytes_nth.stderr | 16 +++++++++++ 7 files changed, 102 insertions(+) create mode 100644 clippy_lints/src/methods/bytes_nth.rs create mode 100644 tests/ui/bytes_nth.fixed create mode 100644 tests/ui/bytes_nth.rs create mode 100644 tests/ui/bytes_nth.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index b74841c77..7c79fe888 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1877,6 +1877,7 @@ Released 2018-09-13 [`box_vec`]: https://rust-lang.github.io/rust-clippy/master/index.html#box_vec [`boxed_local`]: https://rust-lang.github.io/rust-clippy/master/index.html#boxed_local [`builtin_type_shadow`]: https://rust-lang.github.io/rust-clippy/master/index.html#builtin_type_shadow +[`bytes_nth`]: https://rust-lang.github.io/rust-clippy/master/index.html#bytes_nth [`cargo_common_metadata`]: https://rust-lang.github.io/rust-clippy/master/index.html#cargo_common_metadata [`case_sensitive_file_extension_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#case_sensitive_file_extension_comparisons [`cast_lossless`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_lossless diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index fe4aa584b..22d91c9d4 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -734,6 +734,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &mem_replace::MEM_REPLACE_WITH_DEFAULT, &mem_replace::MEM_REPLACE_WITH_UNINIT, &methods::BIND_INSTEAD_OF_MAP, + &methods::BYTES_NTH, &methods::CHARS_LAST_CMP, &methods::CHARS_NEXT_CMP, &methods::CLONE_DOUBLE_REF, @@ -1531,6 +1532,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&mem_replace::MEM_REPLACE_WITH_DEFAULT), LintId::of(&mem_replace::MEM_REPLACE_WITH_UNINIT), LintId::of(&methods::BIND_INSTEAD_OF_MAP), + LintId::of(&methods::BYTES_NTH), LintId::of(&methods::CHARS_LAST_CMP), LintId::of(&methods::CHARS_NEXT_CMP), LintId::of(&methods::CLONE_DOUBLE_REF), @@ -1749,6 +1751,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&matches::SINGLE_MATCH), LintId::of(&mem_replace::MEM_REPLACE_OPTION_WITH_NONE), LintId::of(&mem_replace::MEM_REPLACE_WITH_DEFAULT), + LintId::of(&methods::BYTES_NTH), LintId::of(&methods::CHARS_LAST_CMP), LintId::of(&methods::CHARS_NEXT_CMP), LintId::of(&methods::FROM_ITER_INSTEAD_OF_COLLECT), diff --git a/clippy_lints/src/methods/bytes_nth.rs b/clippy_lints/src/methods/bytes_nth.rs new file mode 100644 index 000000000..4f38db06c --- /dev/null +++ b/clippy_lints/src/methods/bytes_nth.rs @@ -0,0 +1,39 @@ +use crate::utils::{is_type_diagnostic_item, snippet_with_applicability, span_lint_and_sugg}; +use if_chain::if_chain; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::LateContext; +use rustc_span::sym; + +use super::BYTES_NTH; + +pub(super) fn lints<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, iter_args: &'tcx [Expr<'tcx>]) { + if_chain! { + if let ExprKind::MethodCall(_, _, ref args, _) = expr.kind; + let ty = cx.typeck_results().expr_ty(&iter_args[0]).peel_refs(); + let caller_type = if is_type_diagnostic_item(cx, ty, sym::string_type) { + Some("String") + } else if ty.is_str() { + Some("str") + } else { + None + }; + if let Some(caller_type) = caller_type; + then { + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( + cx, + BYTES_NTH, + expr.span, + &format!("called `.byte().nth()` on a `{}`", caller_type), + "try calling `.as_bytes().get()`", + format!( + "{}.as_bytes().get({})", + snippet_with_applicability(cx, iter_args[0].span, "..", &mut applicability), + snippet_with_applicability(cx, args[1].span, "..", &mut applicability) + ), + applicability, + ); + } + } +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 3e34fc1ae..4cb3a8585 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -1,4 +1,5 @@ mod bind_instead_of_map; +mod bytes_nth; mod filter_map_identity; mod inefficient_to_string; mod inspect_for_each; @@ -1490,6 +1491,28 @@ declare_clippy_lint! { "call to `filter_map` where `flatten` is sufficient" } +declare_clippy_lint! { + /// **What it does:** Checks for the use of `.bytes().nth()`. + /// + /// **Why is this bad?** `.as_bytes().get()` is more efficient and more + /// readable. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust + /// // Bad + /// let _ = "Hello".bytes().nth(3);; + /// + /// // Good + /// let _ = "Hello".as_bytes().get(3); + /// ``` + pub BYTES_NTH, + style, + "replace `.bytes().nth()` with `.as_bytes().get()`" +} + pub struct Methods { msrv: Option, } @@ -1537,6 +1560,7 @@ impl_lint_pass!(Methods => [ ITER_NEXT_SLICE, ITER_NTH, ITER_NTH_ZERO, + BYTES_NTH, ITER_SKIP_NEXT, GET_UNWRAP, STRING_EXTEND_CHARS, @@ -1614,6 +1638,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods { ["extend", ..] => lint_extend(cx, expr, arg_lists[0]), ["nth", "iter"] => lint_iter_nth(cx, expr, &arg_lists, false), ["nth", "iter_mut"] => lint_iter_nth(cx, expr, &arg_lists, true), + ["nth", "bytes"] => bytes_nth::lints(cx, expr, &arg_lists[1]), ["nth", ..] => lint_iter_nth_zero(cx, expr, arg_lists[0]), ["step_by", ..] => lint_step_by(cx, expr, arg_lists[0]), ["next", "skip"] => lint_iter_skip_next(cx, expr, arg_lists[1]), diff --git a/tests/ui/bytes_nth.fixed b/tests/ui/bytes_nth.fixed new file mode 100644 index 000000000..36bf8660a --- /dev/null +++ b/tests/ui/bytes_nth.fixed @@ -0,0 +1,9 @@ +// run-rustfix + +#![warn(clippy::bytes_nth)] + +fn main() { + let _ = "Hello".as_bytes().get(3); + + let _ = String::from("Hello").as_bytes().get(3); +} diff --git a/tests/ui/bytes_nth.rs b/tests/ui/bytes_nth.rs new file mode 100644 index 000000000..257344c2d --- /dev/null +++ b/tests/ui/bytes_nth.rs @@ -0,0 +1,9 @@ +// run-rustfix + +#![warn(clippy::bytes_nth)] + +fn main() { + let _ = "Hello".bytes().nth(3); + + let _ = String::from("Hello").bytes().nth(3); +} diff --git a/tests/ui/bytes_nth.stderr b/tests/ui/bytes_nth.stderr new file mode 100644 index 000000000..b46a07364 --- /dev/null +++ b/tests/ui/bytes_nth.stderr @@ -0,0 +1,16 @@ +error: called `.byte().nth()` on a `str` + --> $DIR/bytes_nth.rs:6:13 + | +LL | let _ = "Hello".bytes().nth(3); + | ^^^^^^^^^^^^^^^^^^^^^^ help: try calling `.as_bytes().get()`: `"Hello".as_bytes().get(3)` + | + = note: `-D clippy::bytes-nth` implied by `-D warnings` + +error: called `.byte().nth()` on a `String` + --> $DIR/bytes_nth.rs:8:13 + | +LL | let _ = String::from("Hello").bytes().nth(3); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try calling `.as_bytes().get()`: `String::from("Hello").as_bytes().get(3)` + +error: aborting due to 2 previous errors + From 932cc085e6ac277d58fbb6bebe05252b75d5dc54 Mon Sep 17 00:00:00 2001 From: Takayuki Maeda <41065217+TaKO8Ki@users.noreply.github.com> Date: Wed, 10 Feb 2021 15:49:07 +0900 Subject: [PATCH 2/3] Update clippy_lints/src/methods/bytes_nth.rs Co-authored-by: Phil Hansch --- clippy_lints/src/methods/bytes_nth.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/methods/bytes_nth.rs b/clippy_lints/src/methods/bytes_nth.rs index 4f38db06c..defc50ede 100644 --- a/clippy_lints/src/methods/bytes_nth.rs +++ b/clippy_lints/src/methods/bytes_nth.rs @@ -26,7 +26,7 @@ pub(super) fn lints<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, iter_args: &' BYTES_NTH, expr.span, &format!("called `.byte().nth()` on a `{}`", caller_type), - "try calling `.as_bytes().get()`", + "try", format!( "{}.as_bytes().get({})", snippet_with_applicability(cx, iter_args[0].span, "..", &mut applicability), From 5996ae1cfca619145ffb5e041592efdd097e6391 Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Wed, 10 Feb 2021 16:15:29 +0900 Subject: [PATCH 3/3] add some test cases --- tests/ui/bytes_nth.fixed | 8 +++++--- tests/ui/bytes_nth.rs | 8 +++++--- tests/ui/bytes_nth.stderr | 22 ++++++++++++++-------- 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/tests/ui/bytes_nth.fixed b/tests/ui/bytes_nth.fixed index 36bf8660a..bf68a7bbb 100644 --- a/tests/ui/bytes_nth.fixed +++ b/tests/ui/bytes_nth.fixed @@ -1,9 +1,11 @@ // run-rustfix +#![allow(clippy::unnecessary_operation)] #![warn(clippy::bytes_nth)] fn main() { - let _ = "Hello".as_bytes().get(3); - - let _ = String::from("Hello").as_bytes().get(3); + let s = String::from("String"); + s.as_bytes().get(3); + &s.as_bytes().get(3); + s[..].as_bytes().get(3); } diff --git a/tests/ui/bytes_nth.rs b/tests/ui/bytes_nth.rs index 257344c2d..629812cc0 100644 --- a/tests/ui/bytes_nth.rs +++ b/tests/ui/bytes_nth.rs @@ -1,9 +1,11 @@ // run-rustfix +#![allow(clippy::unnecessary_operation)] #![warn(clippy::bytes_nth)] fn main() { - let _ = "Hello".bytes().nth(3); - - let _ = String::from("Hello").bytes().nth(3); + let s = String::from("String"); + s.bytes().nth(3); + &s.bytes().nth(3); + s[..].bytes().nth(3); } diff --git a/tests/ui/bytes_nth.stderr b/tests/ui/bytes_nth.stderr index b46a07364..9a5742928 100644 --- a/tests/ui/bytes_nth.stderr +++ b/tests/ui/bytes_nth.stderr @@ -1,16 +1,22 @@ -error: called `.byte().nth()` on a `str` - --> $DIR/bytes_nth.rs:6:13 +error: called `.byte().nth()` on a `String` + --> $DIR/bytes_nth.rs:8:5 | -LL | let _ = "Hello".bytes().nth(3); - | ^^^^^^^^^^^^^^^^^^^^^^ help: try calling `.as_bytes().get()`: `"Hello".as_bytes().get(3)` +LL | s.bytes().nth(3); + | ^^^^^^^^^^^^^^^^ help: try: `s.as_bytes().get(3)` | = note: `-D clippy::bytes-nth` implied by `-D warnings` error: called `.byte().nth()` on a `String` - --> $DIR/bytes_nth.rs:8:13 + --> $DIR/bytes_nth.rs:9:6 | -LL | let _ = String::from("Hello").bytes().nth(3); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try calling `.as_bytes().get()`: `String::from("Hello").as_bytes().get(3)` +LL | &s.bytes().nth(3); + | ^^^^^^^^^^^^^^^^ help: try: `s.as_bytes().get(3)` -error: aborting due to 2 previous errors +error: called `.byte().nth()` on a `str` + --> $DIR/bytes_nth.rs:10:5 + | +LL | s[..].bytes().nth(3); + | ^^^^^^^^^^^^^^^^^^^^ help: try: `s[..].as_bytes().get(3)` + +error: aborting due to 3 previous errors