new lint: `into_iter_without_iter`

This commit is contained in:
y21 2023-09-30 03:00:34 +02:00
parent 67a83ff057
commit 8eb586d154
5 changed files with 357 additions and 3 deletions

View File

@ -5011,6 +5011,7 @@ Released 2018-09-13
[`integer_division`]: https://rust-lang.github.io/rust-clippy/master/index.html#integer_division
[`into_iter_on_array`]: https://rust-lang.github.io/rust-clippy/master/index.html#into_iter_on_array
[`into_iter_on_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#into_iter_on_ref
[`into_iter_without_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#into_iter_without_iter
[`invalid_atomic_ordering`]: https://rust-lang.github.io/rust-clippy/master/index.html#invalid_atomic_ordering
[`invalid_null_ptr_usage`]: https://rust-lang.github.io/rust-clippy/master/index.html#invalid_null_ptr_usage
[`invalid_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#invalid_ref

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::INTO_ITER_WITHOUT_ITER_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,

View File

@ -2,11 +2,13 @@ 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_ast::Mutability;
use rustc_errors::Applicability;
use rustc_hir::{FnRetTy, ImplItemKind, ImplicitSelfKind, TyKind};
use rustc_hir::{FnRetTy, ImplItemKind, ImplicitSelfKind, ItemKind, TyKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::{self, Ty};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::sym;
use rustc_span::{sym, Symbol};
declare_clippy_lint! {
/// ### What it does
@ -46,7 +48,51 @@ declare_clippy_lint! {
pedantic,
"implementing `iter(_mut)` without an associated `IntoIterator for (&|&mut) Type` impl"
}
declare_lint_pass!(IterWithoutIntoIter => [ITER_WITHOUT_INTO_ITER]);
declare_clippy_lint! {
/// ### What it does
/// This is the opposite of the `iter_without_into_iter` lint.
/// It looks for `IntoIterator for (&|&mut) Type` implementations without an inherent `iter` or `iter_mut` method.
///
/// ### Why is this bad?
/// It's not bad, but having them is idiomatic and allows the type to be used in iterator chains
/// by just calling `.iter()`, instead of the more awkward `<&Type>::into_iter` or `(&val).iter()` syntax
/// in case of ambiguity with another `Intoiterator` impl.
///
/// ### Example
/// ```rust
/// struct MySlice<'a>(&'a [u8]);
/// impl<'a> IntoIterator for &MySlice<'a> {
/// type Item = &'a u8;
/// type IntoIter = std::slice::Iter<'a, u8>;
/// fn into_iter(self) -> Self::IntoIter {
/// 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.into_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.0.iter()
/// }
/// }
/// ```
#[clippy::version = "1.74.0"]
pub INTO_ITER_WITHOUT_ITER,
pedantic,
"implementing `IntoIterator for (&|&mut) Type` without an inherent `iter(_mut)` method"
}
declare_lint_pass!(IterWithoutIntoIter => [ITER_WITHOUT_INTO_ITER, INTO_ITER_WITHOUT_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
@ -56,7 +102,75 @@ fn is_nameable_in_impl_trait(ty: &rustc_hir::Ty<'_>) -> bool {
!matches!(ty.kind, TyKind::OpaqueDef(..))
}
fn type_has_inherent_method(cx: &LateContext<'_>, ty: Ty<'_>, method_name: Symbol) -> bool {
if let Some(ty_did) = ty.ty_adt_def().map(ty::AdtDef::did) {
cx.tcx.inherent_impls(ty_did).iter().any(|&did| {
cx.tcx
.associated_items(did)
.filter_by_name_unhygienic(method_name)
.next()
.is_some_and(|item| item.kind == ty::AssocKind::Fn)
})
} else {
false
}
}
impl LateLintPass<'_> for IterWithoutIntoIter {
fn check_item(&mut self, cx: &LateContext<'_>, item: &rustc_hir::Item<'_>) {
if let ItemKind::Impl(imp) = item.kind
&& let TyKind::Ref(_, self_ty_without_ref) = &imp.self_ty.kind
&& let Some(trait_ref) = imp.of_trait
&& trait_ref.trait_def_id().is_some_and(|did| cx.tcx.is_diagnostic_item(sym::IntoIterator, did))
&& let &ty::Ref(_, ty, mtbl) = cx.tcx.type_of(item.owner_id).instantiate_identity().kind()
&& let expected_method_name = match mtbl {
Mutability::Mut => sym::iter_mut,
Mutability::Not => sym::iter,
}
&& !type_has_inherent_method(cx, ty, expected_method_name)
&& let Some(iter_assoc_span) = imp.items.iter().find_map(|item| {
if item.ident.name == sym!(IntoIter) {
Some(cx.tcx.hir().impl_item(item.id).expect_type().span)
} else {
None
}
})
{
span_lint_and_then(
cx,
INTO_ITER_WITHOUT_ITER,
item.span,
&format!("`IntoIterator` implemented for a reference type without an `{expected_method_name}` method"),
|diag| {
// The suggestion forwards to the `IntoIterator` impl and uses a form of UFCS
// to avoid name ambiguities, as there might be an inherent into_iter method
// that we don't want to call.
let sugg = format!(
"
impl {self_ty_without_ref} {{
fn {expected_method_name}({ref_self}self) -> {iter_ty} {{
<{ref_self}Self as IntoIterator>::into_iter(self)
}}
}}
",
self_ty_without_ref = snippet(cx, self_ty_without_ref.ty.span, ".."),
ref_self = mtbl.ref_prefix_str(),
iter_ty = snippet(cx, iter_assoc_span, ".."),
);
diag.span_suggestion_verbose(
item.span.shrink_to_lo(),
format!("consider implementing `{expected_method_name}`"),
sugg,
// Just like iter_without_into_iter, this suggestion is on a best effort basis
// and requires potentially adding lifetimes or moving them around.
Applicability::Unspecified
);
}
);
}
}
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 {

View File

@ -0,0 +1,124 @@
//@no-rustfix
#![warn(clippy::into_iter_without_iter)]
use std::iter::IntoIterator;
fn main() {
{
struct S;
impl<'a> IntoIterator for &'a S {
//~^ ERROR: `IntoIterator` implemented for a reference type without an `iter` method
type IntoIter = std::slice::Iter<'a, u8>;
type Item = &'a u8;
fn into_iter(self) -> Self::IntoIter {
todo!()
}
}
impl<'a> IntoIterator for &'a mut S {
//~^ ERROR: `IntoIterator` implemented for a reference type without an `iter_mut` method
type IntoIter = std::slice::IterMut<'a, u8>;
type Item = &'a mut u8;
fn into_iter(self) -> Self::IntoIter {
todo!()
}
}
}
{
struct S<T>(T);
impl<'a, T> IntoIterator for &'a S<T> {
//~^ ERROR: `IntoIterator` implemented for a reference type without an `iter` method
type IntoIter = std::slice::Iter<'a, T>;
type Item = &'a T;
fn into_iter(self) -> Self::IntoIter {
todo!()
}
}
impl<'a, T> IntoIterator for &'a mut S<T> {
//~^ ERROR: `IntoIterator` implemented for a reference type without an `iter_mut` method
type IntoIter = std::slice::IterMut<'a, T>;
type Item = &'a mut T;
fn into_iter(self) -> Self::IntoIter {
todo!()
}
}
}
{
// Both iter and iter_mut methods exist, don't lint
struct S<'a, T>(&'a T);
impl<'a, T> S<'a, T> {
fn iter(&self) -> std::slice::Iter<'a, T> {
todo!()
}
fn iter_mut(&mut self) -> std::slice::IterMut<'a, T> {
todo!()
}
}
impl<'a, T> IntoIterator for &S<'a, T> {
type IntoIter = std::slice::Iter<'a, T>;
type Item = &'a T;
fn into_iter(self) -> Self::IntoIter {
todo!()
}
}
impl<'a, T> IntoIterator for &mut S<'a, T> {
type IntoIter = std::slice::IterMut<'a, T>;
type Item = &'a mut T;
fn into_iter(self) -> Self::IntoIter {
todo!()
}
}
}
{
// Only `iter` exists, no `iter_mut`
struct S<'a, T>(&'a T);
impl<'a, T> S<'a, T> {
fn iter(&self) -> std::slice::Iter<'a, T> {
todo!()
}
}
impl<'a, T> IntoIterator for &S<'a, T> {
type IntoIter = std::slice::Iter<'a, T>;
type Item = &'a T;
fn into_iter(self) -> Self::IntoIter {
todo!()
}
}
impl<'a, T> IntoIterator for &mut S<'a, T> {
//~^ ERROR: `IntoIterator` implemented for a reference type without an `iter_mut` method
type IntoIter = std::slice::IterMut<'a, T>;
type Item = &'a mut T;
fn into_iter(self) -> Self::IntoIter {
todo!()
}
}
}
{
// `iter` exists, but `IntoIterator` is implemented for an alias. inherent_impls doesn't "normalize"
// aliases so that `inherent_impls(Alias)` where `type Alias = S` returns nothing, so this can lead
// to fun FPs. Make sure it doesn't happen here (we're using type_of, which should skip the alias).
struct S;
impl S {
fn iter(&self) -> std::slice::Iter<'static, u8> {
todo!()
}
}
type Alias = S;
impl IntoIterator for &Alias {
type IntoIter = std::slice::Iter<'static, u8>;
type Item = &'static u8;
fn into_iter(self) -> Self::IntoIter {
todo!()
}
}
}
}

View File

@ -0,0 +1,114 @@
error: `IntoIterator` implemented for a reference type without an `iter` method
--> $DIR/into_iter_without_iter.rs:10:9
|
LL | / impl<'a> IntoIterator for &'a S {
LL | |
LL | | type IntoIter = std::slice::Iter<'a, u8>;
LL | | type Item = &'a u8;
... |
LL | | }
LL | | }
| |_________^
|
= note: `-D clippy::into-iter-without-iter` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::into_iter_without_iter)]`
help: consider implementing `iter`
|
LL ~
LL + impl S {
LL + fn iter(&self) -> std::slice::Iter<'a, u8> {
LL + <&Self as IntoIterator>::into_iter(self)
LL + }
LL + }
|
error: `IntoIterator` implemented for a reference type without an `iter_mut` method
--> $DIR/into_iter_without_iter.rs:18:9
|
LL | / impl<'a> IntoIterator for &'a mut S {
LL | |
LL | | type IntoIter = std::slice::IterMut<'a, u8>;
LL | | type Item = &'a mut u8;
... |
LL | | }
LL | | }
| |_________^
|
help: consider implementing `iter_mut`
|
LL ~
LL + impl S {
LL + fn iter_mut(&mut self) -> std::slice::IterMut<'a, u8> {
LL + <&mut Self as IntoIterator>::into_iter(self)
LL + }
LL + }
|
error: `IntoIterator` implemented for a reference type without an `iter` method
--> $DIR/into_iter_without_iter.rs:29:9
|
LL | / impl<'a, T> IntoIterator for &'a S<T> {
LL | |
LL | | type IntoIter = std::slice::Iter<'a, T>;
LL | | type Item = &'a T;
... |
LL | | }
LL | | }
| |_________^
|
help: consider implementing `iter`
|
LL ~
LL + impl S<T> {
LL + fn iter(&self) -> std::slice::Iter<'a, T> {
LL + <&Self as IntoIterator>::into_iter(self)
LL + }
LL + }
|
error: `IntoIterator` implemented for a reference type without an `iter_mut` method
--> $DIR/into_iter_without_iter.rs:37:9
|
LL | / impl<'a, T> IntoIterator for &'a mut S<T> {
LL | |
LL | | type IntoIter = std::slice::IterMut<'a, T>;
LL | | type Item = &'a mut T;
... |
LL | | }
LL | | }
| |_________^
|
help: consider implementing `iter_mut`
|
LL ~
LL + impl S<T> {
LL + fn iter_mut(&mut self) -> std::slice::IterMut<'a, T> {
LL + <&mut Self as IntoIterator>::into_iter(self)
LL + }
LL + }
|
error: `IntoIterator` implemented for a reference type without an `iter_mut` method
--> $DIR/into_iter_without_iter.rs:93:9
|
LL | / impl<'a, T> IntoIterator for &mut S<'a, T> {
LL | |
LL | | type IntoIter = std::slice::IterMut<'a, T>;
LL | | type Item = &'a mut T;
... |
LL | | }
LL | | }
| |_________^
|
help: consider implementing `iter_mut`
|
LL ~
LL + impl S<'a, T> {
LL + fn iter_mut(&mut self) -> std::slice::IterMut<'a, T> {
LL + <&mut Self as IntoIterator>::into_iter(self)
LL + }
LL + }
|
error: aborting due to 5 previous errors