From 8f88acdbfa6073eefbfcd773619d460fdfec122a Mon Sep 17 00:00:00 2001 From: Labelray Date: Tue, 7 Sep 2021 09:59:21 +0800 Subject: [PATCH] add new lint `iter_not_returning_iterator` --- CHANGELOG.md | 1 + .../src/iter_not_returning_iterator.rs | 64 +++++++++++++++++++ clippy_lints/src/lib.rs | 4 ++ tests/ui/iter_not_returning_iterator.rs | 47 ++++++++++++++ tests/ui/iter_not_returning_iterator.stderr | 16 +++++ 5 files changed, 132 insertions(+) create mode 100644 clippy_lints/src/iter_not_returning_iterator.rs create mode 100644 tests/ui/iter_not_returning_iterator.rs create mode 100644 tests/ui/iter_not_returning_iterator.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index f5ac2f7c9f8..71383e8116b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2722,6 +2722,7 @@ Released 2018-09-13 [`iter_count`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_count [`iter_next_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_next_loop [`iter_next_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_next_slice +[`iter_not_returning_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_not_returning_iterator [`iter_nth`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_nth [`iter_nth_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_nth_zero [`iter_skip_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_skip_next diff --git a/clippy_lints/src/iter_not_returning_iterator.rs b/clippy_lints/src/iter_not_returning_iterator.rs new file mode 100644 index 00000000000..6c779348ca2 --- /dev/null +++ b/clippy_lints/src/iter_not_returning_iterator.rs @@ -0,0 +1,64 @@ +use clippy_utils::{diagnostics::span_lint, return_ty, ty::implements_trait}; +use rustc_hir::{ImplItem, ImplItemKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::symbol::kw; +use rustc_span::symbol::sym; + +declare_clippy_lint! { + /// ### What it does + /// Detects methods named `iter` or `iter_mut` that do not have a return type that implements `Iterator`. + /// + /// ### Why is this bad? + /// Methods named `iter` or `iter_mut` conventionally return an `Iterator`. + /// + /// ### Example + /// ```rust + /// // `String` does not implement `Iterator` + /// struct Data {} + /// impl Data { + /// fn iter(&self) -> String { + /// todo!() + /// } + /// } + /// ``` + /// Use instead: + /// ```rust + /// use std::str::Chars; + /// struct Data {} + /// impl Data { + /// fn iter(&self) -> Chars<'static> { + /// todo!() + /// } + /// } + /// ``` + pub ITER_NOT_RETURNING_ITERATOR, + pedantic, + "methods named `iter` or `iter_mut` that do not return an `Iterator`" +} + +declare_lint_pass!(IterNotReturningIterator => [ITER_NOT_RETURNING_ITERATOR]); + +impl LateLintPass<'_> for IterNotReturningIterator { + fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx ImplItem<'tcx>) { + let name: &str = &impl_item.ident.name.as_str(); + if_chain! { + if let ImplItemKind::Fn(fn_sig, _) = &impl_item.kind; + let ret_ty = return_ty(cx, impl_item.hir_id()); + if matches!(name, "iter" | "iter_mut"); + if let [param] = cx.tcx.fn_arg_names(impl_item.def_id); + if param.name == kw::SelfLower; + if let Some(iter_trait_id) = cx.tcx.get_diagnostic_item(sym::Iterator); + if !implements_trait(cx, ret_ty, iter_trait_id, &[]); + + then { + span_lint( + cx, + ITER_NOT_RETURNING_ITERATOR, + fn_sig.span, + &format!("this method is named `{}` but its return type does not implement `Iterator`", name), + ); + } + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 92a13be81f4..63deb46dc73 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -241,6 +241,7 @@ mod int_plus_one; mod integer_division; mod invalid_upcast_comparisons; mod items_after_statements; +mod iter_not_returning_iterator; mod large_const_arrays; mod large_enum_variant; mod large_stack_arrays; @@ -674,6 +675,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: integer_division::INTEGER_DIVISION, invalid_upcast_comparisons::INVALID_UPCAST_COMPARISONS, items_after_statements::ITEMS_AFTER_STATEMENTS, + iter_not_returning_iterator::ITER_NOT_RETURNING_ITERATOR, large_const_arrays::LARGE_CONST_ARRAYS, large_enum_variant::LARGE_ENUM_VARIANT, large_stack_arrays::LARGE_STACK_ARRAYS, @@ -1104,6 +1106,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(infinite_iter::MAYBE_INFINITE_ITER), LintId::of(invalid_upcast_comparisons::INVALID_UPCAST_COMPARISONS), LintId::of(items_after_statements::ITEMS_AFTER_STATEMENTS), + LintId::of(iter_not_returning_iterator::ITER_NOT_RETURNING_ITERATOR), LintId::of(large_stack_arrays::LARGE_STACK_ARRAYS), LintId::of(let_underscore::LET_UNDERSCORE_DROP), LintId::of(literal_representation::LARGE_DIGIT_GROUPS), @@ -2131,6 +2134,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(strlen_on_c_strings::StrlenOnCStrings)); store.register_late_pass(move || Box::new(self_named_constructors::SelfNamedConstructors)); store.register_late_pass(move || Box::new(feature_name::FeatureName)); + store.register_late_pass(move || Box::new(iter_not_returning_iterator::IterNotReturningIterator)); } #[rustfmt::skip] diff --git a/tests/ui/iter_not_returning_iterator.rs b/tests/ui/iter_not_returning_iterator.rs new file mode 100644 index 00000000000..377f760b3c4 --- /dev/null +++ b/tests/ui/iter_not_returning_iterator.rs @@ -0,0 +1,47 @@ +#![warn(clippy::iter_not_returning_iterator)] + +struct Data { + begin: u32, +} + +struct Counter { + count: u32, +} + +impl Data { + fn iter(&self) -> Counter { + todo!() + } + + fn iter_mut(&self) -> Counter { + todo!() + } +} + +struct Data2 { + begin: u32, +} + +struct Counter2 { + count: u32, +} + +impl Data2 { + fn iter(&self) -> Counter2 { + todo!() + } + + fn iter_mut(&self) -> Counter2 { + todo!() + } +} + +impl Iterator for Counter { + type Item = u32; + + fn next(&mut self) -> Option { + todo!() + } +} + +fn main() {} diff --git a/tests/ui/iter_not_returning_iterator.stderr b/tests/ui/iter_not_returning_iterator.stderr new file mode 100644 index 00000000000..2273cd0be66 --- /dev/null +++ b/tests/ui/iter_not_returning_iterator.stderr @@ -0,0 +1,16 @@ +error: this method is named `iter` but its return type does not implement `Iterator` + --> $DIR/iter_not_returning_iterator.rs:30:5 + | +LL | fn iter(&self) -> Counter2 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::iter-not-returning-iterator` implied by `-D warnings` + +error: this method is named `iter_mut` but its return type does not implement `Iterator` + --> $DIR/iter_not_returning_iterator.rs:34:5 + | +LL | fn iter_mut(&self) -> Counter2 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors +