From 2bb8c45026f0bfc244390f2d6cecc05cfa1f1171 Mon Sep 17 00:00:00 2001 From: kyoto7250 <50972773+kyoto7250@users.noreply.github.com> Date: Sun, 12 Jun 2022 21:30:55 +0900 Subject: [PATCH] feat(lint): add default_iter_empty Update description in clippy_lints/src/default_iter_empty.rs Co-authored-by: Fridtjof Stoldt Update clippy_lints/src/default_iter_empty.rs Co-authored-by: Alex Macleod Update clippy_lints/src/default_iter_empty.rs Co-authored-by: Alex Macleod renamed default_iter_empty to default_instead_of_iter_empty Avoid duplicate messages add tests for regression rewrite 'Why is this bad?' cargo dev fmt delete default_iter_empty lint in renamed_lint.rs rewrite a message in the suggestion cargo dev update_lints --check --- CHANGELOG.md | 1 + .../src/default_instead_of_iter_empty.rs | 68 +++++++++++++++++++ clippy_lints/src/lib.register_all.rs | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.register_style.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_utils/src/paths.rs | 1 + tests/ui/default_instead_of_iter_empty.fixed | 21 ++++++ tests/ui/default_instead_of_iter_empty.rs | 21 ++++++ tests/ui/default_instead_of_iter_empty.stderr | 22 ++++++ 10 files changed, 139 insertions(+) create mode 100644 clippy_lints/src/default_instead_of_iter_empty.rs create mode 100644 tests/ui/default_instead_of_iter_empty.fixed create mode 100644 tests/ui/default_instead_of_iter_empty.rs create mode 100644 tests/ui/default_instead_of_iter_empty.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 6aaf12ed9..d64dd772a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3348,6 +3348,7 @@ Released 2018-09-13 [`debug_assert_with_mut_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#debug_assert_with_mut_call [`decimal_literal_representation`]: https://rust-lang.github.io/rust-clippy/master/index.html#decimal_literal_representation [`declare_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#declare_interior_mutable_const +[`default_instead_of_iter_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_instead_of_iter_empty [`default_numeric_fallback`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_numeric_fallback [`default_trait_access`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_trait_access [`default_union_representation`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_union_representation diff --git a/clippy_lints/src/default_instead_of_iter_empty.rs b/clippy_lints/src/default_instead_of_iter_empty.rs new file mode 100644 index 000000000..3c996d3d2 --- /dev/null +++ b/clippy_lints/src/default_instead_of_iter_empty.rs @@ -0,0 +1,68 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::last_path_segment; +use clippy_utils::source::snippet_with_applicability; +use clippy_utils::{match_def_path, paths}; +use rustc_errors::Applicability; +use rustc_hir::{def, Expr, ExprKind, GenericArg, QPath, TyKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// ### What it does + /// It checks for `std::iter::Empty::default()` and suggests replacing it with + /// `std::iter::empty()`. + /// ### Why is this bad? + /// `std::iter::empty()` is the more idiomatic way. + /// ### Example + /// ```rust + /// let _ = std::iter::Empty::::default(); + /// let iter: std::iter::Empty = std::iter::Empty::default(); + /// ``` + /// Use instead: + /// ```rust + /// let _ = std::iter::empty::(); + /// let iter: std::iter::Empty = std::iter::empty(); + /// ``` + #[clippy::version = "1.63.0"] + pub DEFAULT_INSTEAD_OF_ITER_EMPTY, + style, + "check `std::iter::Empty::default()` and replace with `std::iter::empty()`" +} +declare_lint_pass!(DefaultIterEmpty => [DEFAULT_INSTEAD_OF_ITER_EMPTY]); + +impl<'tcx> LateLintPass<'tcx> for DefaultIterEmpty { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if let ExprKind::Call(iter_expr, []) = &expr.kind + && let ExprKind::Path(QPath::TypeRelative(ty, _)) = &iter_expr.kind + && let TyKind::Path(ty_path) = &ty.kind + && let QPath::Resolved(None, path) = ty_path + && let def::Res::Def(_, def_id) = &path.res + && match_def_path(cx, *def_id, &paths::ITER_EMPTY) + { + let mut applicability = Applicability::MachineApplicable; + let sugg = make_sugg(cx, ty_path, &mut applicability); + span_lint_and_sugg( + cx, + DEFAULT_INSTEAD_OF_ITER_EMPTY, + expr.span, + "`std::iter::empty()` is the more idiomatic way", + "try", + sugg, + applicability, + ); + } + } +} + +fn make_sugg(cx: &LateContext<'_>, ty_path: &rustc_hir::QPath<'_>, applicability: &mut Applicability) -> String { + if let Some(last) = last_path_segment(ty_path).args + && let Some(iter_ty) = last.args.iter().find_map(|arg| match arg { + GenericArg::Type(ty) => Some(ty), + _ => None, + }) + { + format!("std::iter::empty::<{}>()", snippet_with_applicability(cx, iter_ty.span, "..", applicability)) + } else { + "std::iter::empty()".to_owned() + } +} diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 8a2cfbff9..7876f21f6 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -43,6 +43,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(copies::IF_SAME_THEN_ELSE), LintId::of(crate_in_macro_def::CRATE_IN_MACRO_DEF), LintId::of(default::FIELD_REASSIGN_WITH_DEFAULT), + LintId::of(default_instead_of_iter_empty::DEFAULT_INSTEAD_OF_ITER_EMPTY), LintId::of(dereference::NEEDLESS_BORROW), LintId::of(derivable_impls::DERIVABLE_IMPLS), LintId::of(derive::DERIVE_HASH_XOR_EQ), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 8ad984c68..de22f50cf 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -105,6 +105,7 @@ store.register_lints(&[ dbg_macro::DBG_MACRO, default::DEFAULT_TRAIT_ACCESS, default::FIELD_REASSIGN_WITH_DEFAULT, + default_instead_of_iter_empty::DEFAULT_INSTEAD_OF_ITER_EMPTY, default_numeric_fallback::DEFAULT_NUMERIC_FALLBACK, default_union_representation::DEFAULT_UNION_REPRESENTATION, dereference::EXPLICIT_DEREF_METHODS, diff --git a/clippy_lints/src/lib.register_style.rs b/clippy_lints/src/lib.register_style.rs index b6992ae0a..d52ec50e5 100644 --- a/clippy_lints/src/lib.register_style.rs +++ b/clippy_lints/src/lib.register_style.rs @@ -14,6 +14,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![ LintId::of(collapsible_if::COLLAPSIBLE_IF), LintId::of(comparison_chain::COMPARISON_CHAIN), LintId::of(default::FIELD_REASSIGN_WITH_DEFAULT), + LintId::of(default_instead_of_iter_empty::DEFAULT_INSTEAD_OF_ITER_EMPTY), LintId::of(dereference::NEEDLESS_BORROW), LintId::of(derive::DERIVE_PARTIAL_EQ_WITHOUT_EQ), LintId::of(disallowed_methods::DISALLOWED_METHODS), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 84898eae0..dcee11cc2 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -199,6 +199,7 @@ mod crate_in_macro_def; mod create_dir; mod dbg_macro; mod default; +mod default_instead_of_iter_empty; mod default_numeric_fallback; mod default_union_representation; mod dereference; @@ -910,6 +911,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(mismatching_type_param_order::TypeParamMismatch)); store.register_late_pass(|| Box::new(as_underscore::AsUnderscore)); store.register_late_pass(|| Box::new(read_zero_byte_vec::ReadZeroByteVec)); + store.register_late_pass(|| Box::new(default_instead_of_iter_empty::DefaultIterEmpty)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index 89789c3d8..b31e2c1cd 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -62,6 +62,7 @@ pub const IO_WRITE: [&str; 3] = ["std", "io", "Write"]; pub const IPADDR_V4: [&str; 5] = ["std", "net", "ip", "IpAddr", "V4"]; pub const IPADDR_V6: [&str; 5] = ["std", "net", "ip", "IpAddr", "V6"]; pub const ITER_COUNT: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "count"]; +pub const ITER_EMPTY: [&str; 5] = ["core", "iter", "sources", "empty", "Empty"]; pub const ITER_REPEAT: [&str; 5] = ["core", "iter", "sources", "repeat", "repeat"]; #[expect(clippy::invalid_paths)] // internal lints do not know about all external crates pub const ITERTOOLS_NEXT_TUPLE: [&str; 3] = ["itertools", "Itertools", "next_tuple"]; diff --git a/tests/ui/default_instead_of_iter_empty.fixed b/tests/ui/default_instead_of_iter_empty.fixed new file mode 100644 index 000000000..f1abfdcd6 --- /dev/null +++ b/tests/ui/default_instead_of_iter_empty.fixed @@ -0,0 +1,21 @@ +// run-rustfix +#![warn(clippy::default_instead_of_iter_empty)] +#![allow(dead_code)] +use std::collections::HashMap; + +#[derive(Default)] +struct Iter { + iter: std::iter::Empty, +} + +fn main() { + // Do lint. + let _ = std::iter::empty::(); + let _ = std::iter::empty::>(); + let _foo: std::iter::Empty = std::iter::empty(); + + // Do not lint. + let _ = Vec::::default(); + let _ = String::default(); + let _ = Iter::default(); +} diff --git a/tests/ui/default_instead_of_iter_empty.rs b/tests/ui/default_instead_of_iter_empty.rs new file mode 100644 index 000000000..2630519c4 --- /dev/null +++ b/tests/ui/default_instead_of_iter_empty.rs @@ -0,0 +1,21 @@ +// run-rustfix +#![warn(clippy::default_instead_of_iter_empty)] +#![allow(dead_code)] +use std::collections::HashMap; + +#[derive(Default)] +struct Iter { + iter: std::iter::Empty, +} + +fn main() { + // Do lint. + let _ = std::iter::Empty::::default(); + let _ = std::iter::Empty::>::default(); + let _foo: std::iter::Empty = std::iter::Empty::default(); + + // Do not lint. + let _ = Vec::::default(); + let _ = String::default(); + let _ = Iter::default(); +} diff --git a/tests/ui/default_instead_of_iter_empty.stderr b/tests/ui/default_instead_of_iter_empty.stderr new file mode 100644 index 000000000..460fc84de --- /dev/null +++ b/tests/ui/default_instead_of_iter_empty.stderr @@ -0,0 +1,22 @@ +error: `std::iter::empty()` is the more idiomatic way + --> $DIR/default_instead_of_iter_empty.rs:13:13 + | +LL | let _ = std::iter::Empty::::default(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::iter::empty::()` + | + = note: `-D clippy::default-instead-of-iter-empty` implied by `-D warnings` + +error: `std::iter::empty()` is the more idiomatic way + --> $DIR/default_instead_of_iter_empty.rs:14:13 + | +LL | let _ = std::iter::Empty::>::default(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::iter::empty::>()` + +error: `std::iter::empty()` is the more idiomatic way + --> $DIR/default_instead_of_iter_empty.rs:15:41 + | +LL | let _foo: std::iter::Empty = std::iter::Empty::default(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::iter::empty()` + +error: aborting due to 3 previous errors +