Skip to content

Commit

Permalink
new lint: iter_without_into_iter
Browse files Browse the repository at this point in the history
  • Loading branch information
y21 committed Sep 28, 2023
1 parent 91997a4 commit 330ebbb
Show file tree
Hide file tree
Showing 6 changed files with 409 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
135 changes: 135 additions & 0 deletions clippy_lints/src/iter_without_into_iter.rs
Original file line number Diff line number Diff line change
@@ -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,
);
});
}
}
}
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`
}

Expand Down
120 changes: 120 additions & 0 deletions tests/ui/iter_without_into_iter.rs
Original file line number Diff line number Diff line change
@@ -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!()
}
}
}
}
Loading

0 comments on commit 330ebbb

Please sign in to comment.