new lint: iter_without_into_iter

This commit is contained in:
y21 2023-09-18 00:19:34 +02:00
parent 91997a4df4
commit 330ebbb9f9
6 changed files with 409 additions and 0 deletions

View file

@ -5036,6 +5036,7 @@ Released 2018-09-13
[`iter_skip_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_skip_next
[`iter_skip_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_skip_zero
[`iter_with_drain`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_with_drain
[`iter_without_into_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_without_into_iter
[`iterator_step_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iterator_step_by_zero
[`just_underscores_and_digits`]: https://rust-lang.github.io/rust-clippy/master/index.html#just_underscores_and_digits
[`large_const_arrays`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_const_arrays

View file

@ -229,6 +229,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::items_after_statements::ITEMS_AFTER_STATEMENTS_INFO,
crate::items_after_test_module::ITEMS_AFTER_TEST_MODULE_INFO,
crate::iter_not_returning_iterator::ITER_NOT_RETURNING_ITERATOR_INFO,
crate::iter_without_into_iter::ITER_WITHOUT_INTO_ITER_INFO,
crate::large_const_arrays::LARGE_CONST_ARRAYS_INFO,
crate::large_enum_variant::LARGE_ENUM_VARIANT_INFO,
crate::large_futures::LARGE_FUTURES_INFO,

View file

@ -0,0 +1,135 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::get_parent_as_impl;
use clippy_utils::source::snippet;
use clippy_utils::ty::{implements_trait, make_normalized_projection};
use rustc_errors::Applicability;
use rustc_hir::{FnRetTy, ImplItemKind, ImplicitSelfKind, TyKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::sym;
declare_clippy_lint! {
/// ### What it does
/// Looks for `iter` and `iter_mut` methods without an associated `IntoIterator for (&|&mut) Type` implementation.
///
/// ### Why is this bad?
/// It's not bad, but having them is idiomatic and allows the type to be used in for loops directly
/// (`for val in &iter {}`), without having to first call `iter()` or `iter_mut()`.
///
/// ### Example
/// ```rust
/// struct MySlice<'a>(&'a [u8]);
/// impl<'a> MySlice<'a> {
/// pub fn iter(&self) -> std::slice::Iter<'a, u8> {
/// self.0.iter()
/// }
/// }
/// ```
/// Use instead:
/// ```rust
/// struct MySlice<'a>(&'a [u8]);
/// impl<'a> MySlice<'a> {
/// pub fn iter(&self) -> std::slice::Iter<'a, u8> {
/// self.0.iter()
/// }
/// }
/// impl<'a> IntoIterator for &MySlice<'a> {
/// type Item = &'a u8;
/// type IntoIter = std::slice::Iter<'a, u8>;
/// fn into_iter(self) -> Self::IntoIter {
/// self.iter()
/// }
/// }
/// ```
#[clippy::version = "1.74.0"]
pub ITER_WITHOUT_INTO_ITER,
pedantic,
"implementing `iter(_mut)` without an associated `IntoIterator for (&|&mut) Type` impl"
}
declare_lint_pass!(IterWithoutIntoIter => [ITER_WITHOUT_INTO_ITER]);
/// Checks if a given type is nameable in a trait (impl).
/// RPIT is stable, but impl Trait in traits is not (yet), so when we have
/// a function such as `fn iter(&self) -> impl IntoIterator`, we can't
/// suggest `type IntoIter = impl IntoIterator`.
fn is_nameable_in_impl_trait(ty: &rustc_hir::Ty<'_>) -> bool {
!matches!(ty.kind, TyKind::OpaqueDef(..))
}
impl LateLintPass<'_> for IterWithoutIntoIter {
fn check_impl_item(&mut self, cx: &LateContext<'_>, item: &rustc_hir::ImplItem<'_>) {
let item_did = item.owner_id.to_def_id();
let (borrow_prefix, expected_implicit_self) = match item.ident.name {
sym::iter => ("&", ImplicitSelfKind::ImmRef),
sym::iter_mut => ("&mut ", ImplicitSelfKind::MutRef),
_ => return,
};
if let ImplItemKind::Fn(sig, _) = item.kind
&& let FnRetTy::Return(ret) = sig.decl.output
&& is_nameable_in_impl_trait(ret)
&& cx.tcx.generics_of(item_did).params.is_empty()
&& sig.decl.implicit_self == expected_implicit_self
&& sig.decl.inputs.len() == 1
&& let Some(imp) = get_parent_as_impl(cx.tcx, item.hir_id())
&& imp.of_trait.is_none()
&& let sig = cx.tcx.liberate_late_bound_regions(
item_did,
cx.tcx.fn_sig(item_did).instantiate_identity()
)
&& let ref_ty = sig.inputs()[0]
&& let Some(into_iter_did) = cx.tcx.get_diagnostic_item(sym::IntoIterator)
&& let Some(iterator_did) = cx.tcx.get_diagnostic_item(sym::Iterator)
&& let ret_ty = sig.output()
// Order is important here, we need to check that the `fn iter` return type actually implements `IntoIterator`
// *before* normalizing `<_ as IntoIterator>::Item` (otherwise make_normalized_projection ICEs)
&& implements_trait(cx, ret_ty, iterator_did, &[])
&& let Some(iter_ty) = make_normalized_projection(
cx.tcx,
cx.param_env,
iterator_did,
sym!(Item),
[ret_ty],
)
// Only lint if the `IntoIterator` impl doesn't actually exist
&& !implements_trait(cx, ref_ty, into_iter_did, &[])
{
let self_ty_snippet = format!("{borrow_prefix}{}", snippet(cx, imp.self_ty.span, ".."));
span_lint_and_then(
cx,
ITER_WITHOUT_INTO_ITER,
item.span,
&format!("`{}` method without an `IntoIterator` impl for `{self_ty_snippet}`", item.ident),
|diag| {
// Get the lower span of the `impl` block, and insert the suggestion right before it:
// impl X {
// ^ fn iter(&self) -> impl IntoIterator { ... }
// }
let span_behind_impl = cx.tcx
.def_span(cx.tcx.hir().parent_id(item.hir_id()).owner.def_id)
.shrink_to_lo();
let sugg = format!(
"
impl IntoIterator for {self_ty_snippet} {{
type IntoIter = {ret_ty};
type Iter = {iter_ty};
fn into_iter() -> Self::IntoIter {{
self.iter()
}}
}}
"
);
diag.span_suggestion_verbose(
span_behind_impl,
format!("consider implementing `IntoIterator` for `{self_ty_snippet}`"),
sugg,
// Suggestion is on a best effort basis, might need some adjustments by the user
// such as adding some lifetimes in the associated types, or importing types.
Applicability::Unspecified,
);
});
}
}
}

View file

@ -169,6 +169,7 @@ mod invalid_upcast_comparisons;
mod items_after_statements;
mod items_after_test_module;
mod iter_not_returning_iterator;
mod iter_without_into_iter;
mod large_const_arrays;
mod large_enum_variant;
mod large_futures;
@ -1121,6 +1122,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
))
});
store.register_late_pass(move |_| Box::new(manual_hash_one::ManualHashOne::new(msrv())));
store.register_late_pass(|_| Box::new(iter_without_into_iter::IterWithoutIntoIter));
// add lints here, do not remove this comment, it's used in `new_lint`
}

View file

@ -0,0 +1,120 @@
//@no-rustfix
#![warn(clippy::iter_without_into_iter)]
fn main() {
{
struct S;
impl S {
pub fn iter(&self) -> std::slice::Iter<'_, u8> {
//~^ ERROR: `iter` method without an `IntoIterator` impl
[].iter()
}
pub fn iter_mut(&mut self) -> std::slice::IterMut<'_, u8> {
//~^ ERROR: `iter_mut` method without an `IntoIterator` impl
[].iter_mut()
}
}
}
{
struct S;
impl S {
pub fn iter(&self) -> impl Iterator<Item = &u8> {
// RPITIT is not stable, so we can't generally suggest it here yet
[].iter()
}
}
}
{
struct S<'a>(&'a mut [u8]);
impl<'a> S<'a> {
pub fn iter(&self) -> std::slice::Iter<'_, u8> {
//~^ ERROR: `iter` method without an `IntoIterator` impl
self.0.iter()
}
pub fn iter_mut(&mut self) -> std::slice::IterMut<'_, u8> {
//~^ ERROR: `iter_mut` method without an `IntoIterator` impl
self.0.iter_mut()
}
}
}
{
// Incompatible signatures
struct S;
impl S {
pub fn iter(self) -> std::slice::Iter<'static, u8> {
todo!()
}
}
struct S2;
impl S2 {
pub async fn iter(&self) -> std::slice::Iter<'static, u8> {
todo!()
}
}
struct S3;
impl S3 {
pub fn iter(&self, _additional_param: ()) -> std::slice::Iter<'static, u8> {
todo!()
}
}
struct S4<T>(T);
impl<T> S4<T> {
pub fn iter<U>(&self) -> std::slice::Iter<'static, (T, U)> {
todo!()
}
}
struct S5<T>(T);
impl<T> S5<T> {
pub fn iter(&self) -> std::slice::Iter<'static, T> {
todo!()
}
}
}
{
struct S<T>(T);
impl<T> S<T> {
pub fn iter(&self) -> std::slice::Iter<'_, T> {
//~^ ERROR: `iter` method without an `IntoIterator` impl
todo!()
}
pub fn iter_mut(&mut self) -> std::slice::IterMut<'_, T> {
//~^ ERROR: `iter_mut` method without an `IntoIterator` impl
todo!()
}
}
}
{
struct S<T>(T);
impl<T> S<T> {
pub fn iter(&self) -> std::slice::Iter<'_, T> {
// Don't lint, there's an existing (wrong) IntoIterator impl
todo!()
}
}
impl<'a, T> IntoIterator for &'a S<T> {
type Item = &'a String;
type IntoIter = std::slice::Iter<'a, String>;
fn into_iter(self) -> Self::IntoIter {
todo!()
}
}
}
{
struct S<T>(T);
impl<T> S<T> {
pub fn iter_mut(&self) -> std::slice::IterMut<'_, T> {
// Don't lint, there's an existing (wrong) IntoIterator impl
todo!()
}
}
impl<'a, T> IntoIterator for &'a mut S<T> {
type Item = &'a mut String;
type IntoIter = std::slice::IterMut<'a, String>;
fn into_iter(self) -> Self::IntoIter {
todo!()
}
}
}
}

View file

@ -0,0 +1,150 @@
error: `iter` method without an `IntoIterator` impl for `&S`
--> $DIR/iter_without_into_iter.rs:8:13
|
LL | / pub fn iter(&self) -> std::slice::Iter<'_, u8> {
LL | |
LL | | [].iter()
LL | | }
| |_____________^
|
= note: `-D clippy::iter-without-into-iter` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::iter_without_into_iter)]`
help: consider implementing `IntoIterator` for `&S`
|
LL ~
LL + impl IntoIterator for &S {
LL + type IntoIter = std::slice::Iter<'_, u8>;
LL + type Iter = &u8;
LL + fn into_iter() -> Self::IntoIter {
LL + self.iter()
LL + }
LL + }
|
error: `iter_mut` method without an `IntoIterator` impl for `&mut S`
--> $DIR/iter_without_into_iter.rs:12:13
|
LL | / pub fn iter_mut(&mut self) -> std::slice::IterMut<'_, u8> {
LL | |
LL | | [].iter_mut()
LL | | }
| |_____________^
|
help: consider implementing `IntoIterator` for `&mut S`
|
LL ~
LL + impl IntoIterator for &mut S {
LL + type IntoIter = std::slice::IterMut<'_, u8>;
LL + type Iter = &mut u8;
LL + fn into_iter() -> Self::IntoIter {
LL + self.iter()
LL + }
LL + }
|
error: `iter` method without an `IntoIterator` impl for `&S<'a>`
--> $DIR/iter_without_into_iter.rs:30:13
|
LL | / pub fn iter(&self) -> std::slice::Iter<'_, u8> {
LL | |
LL | | self.0.iter()
LL | | }
| |_____________^
|
help: consider implementing `IntoIterator` for `&S<'a>`
|
LL ~
LL + impl IntoIterator for &S<'a> {
LL + type IntoIter = std::slice::Iter<'_, u8>;
LL + type Iter = &u8;
LL + fn into_iter() -> Self::IntoIter {
LL + self.iter()
LL + }
LL + }
|
error: `iter_mut` method without an `IntoIterator` impl for `&mut S<'a>`
--> $DIR/iter_without_into_iter.rs:34:13
|
LL | / pub fn iter_mut(&mut self) -> std::slice::IterMut<'_, u8> {
LL | |
LL | | self.0.iter_mut()
LL | | }
| |_____________^
|
help: consider implementing `IntoIterator` for `&mut S<'a>`
|
LL ~
LL + impl IntoIterator for &mut S<'a> {
LL + type IntoIter = std::slice::IterMut<'_, u8>;
LL + type Iter = &mut u8;
LL + fn into_iter() -> Self::IntoIter {
LL + self.iter()
LL + }
LL + }
|
error: `iter` method without an `IntoIterator` impl for `&S5<T>`
--> $DIR/iter_without_into_iter.rs:68:13
|
LL | / pub fn iter(&self) -> std::slice::Iter<'static, T> {
LL | | todo!()
LL | | }
| |_____________^
|
help: consider implementing `IntoIterator` for `&S5<T>`
|
LL ~
LL + impl IntoIterator for &S5<T> {
LL + type IntoIter = std::slice::Iter<'static, T>;
LL + type Iter = &T;
LL + fn into_iter() -> Self::IntoIter {
LL + self.iter()
LL + }
LL + }
|
error: `iter` method without an `IntoIterator` impl for `&S<T>`
--> $DIR/iter_without_into_iter.rs:76:13
|
LL | / pub fn iter(&self) -> std::slice::Iter<'_, T> {
LL | |
LL | | todo!()
LL | | }
| |_____________^
|
help: consider implementing `IntoIterator` for `&S<T>`
|
LL ~
LL + impl IntoIterator for &S<T> {
LL + type IntoIter = std::slice::Iter<'_, T>;
LL + type Iter = &T;
LL + fn into_iter() -> Self::IntoIter {
LL + self.iter()
LL + }
LL + }
|
error: `iter_mut` method without an `IntoIterator` impl for `&mut S<T>`
--> $DIR/iter_without_into_iter.rs:80:13
|
LL | / pub fn iter_mut(&mut self) -> std::slice::IterMut<'_, T> {
LL | |
LL | | todo!()
LL | | }
| |_____________^
|
help: consider implementing `IntoIterator` for `&mut S<T>`
|
LL ~
LL + impl IntoIterator for &mut S<T> {
LL + type IntoIter = std::slice::IterMut<'_, T>;
LL + type Iter = &mut T;
LL + fn into_iter() -> Self::IntoIter {
LL + self.iter()
LL + }
LL + }
|
error: aborting due to 7 previous errors