Auto merge of #12054 - y21:issue12037, r=Jarcho

add external macro checks to `iter_without_into_iter` and `into_iter_without_iter` Fixes #12037 I think it's useful to still lint on local macros, since the user should still be able to add another impl with the `IntoIterator` or `iter` method. I think it's also fairly common to write a macro for generating many impls (e.g. for many similar types), so it'd be nice if we can continue linting in those cases. For that reason I went with `in_external_macro`. I also added a test for `#[allow]`ing the lint while I was at it. changelog: [`iter_without_into_iter`]: don't lint if the `iter` method is defined in an external macro changelog: [`into_iter_without_iter`]: don't lint if the `IntoIterator` impl is defined in an external macro
rust-lang · Dec 30, 2023 · c992247 · c992247
2 parents 174a0d7 + 0848e12
commit c992247
Show file tree

Hide file tree

Showing 5 changed files with 140 additions and 17 deletions.
diff --git a/clippy_lints/src/iter_without_into_iter.rs b/clippy_lints/src/iter_without_into_iter.rs
@@ -5,7 +5,8 @@ use clippy_utils::ty::{implements_trait, make_normalized_projection};
 use rustc_ast::Mutability;
 use rustc_errors::Applicability;
 use rustc_hir::{FnRetTy, ImplItemKind, ImplicitSelfKind, ItemKind, TyKind};
-use rustc_lint::{LateContext, LateLintPass};
+use rustc_lint::{LateContext, LateLintPass, LintContext};
+use rustc_middle::lint::in_external_macro;
 use rustc_middle::ty::{self, Ty};
 use rustc_session::declare_lint_pass;
 use rustc_span::{sym, Symbol};
@@ -152,7 +153,8 @@ fn adt_has_inherent_method(cx: &LateContext<'_>, ty: Ty<'_>, method_name: Symbol
 
 impl LateLintPass<'_> for IterWithoutIntoIter {
     fn check_item(&mut self, cx: &LateContext<'_>, item: &rustc_hir::Item<'_>) {
-        if let ItemKind::Impl(imp) = item.kind
+        if !in_external_macro(cx.sess(), item.span)
+            && 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
@@ -219,7 +221,8 @@ impl {self_ty_without_ref} {{
             _ => return,
         };
 
-        if let ImplItemKind::Fn(sig, _) = item.kind
+        if !in_external_macro(cx.sess(), item.span)
+            && 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()

diff --git a/tests/ui/into_iter_without_iter.rs b/tests/ui/into_iter_without_iter.rs
@@ -1,5 +1,7 @@
 //@no-rustfix
+//@aux-build:proc_macros.rs
 #![warn(clippy::into_iter_without_iter)]
+extern crate proc_macros;
 
 use std::iter::IntoIterator;
 
@@ -111,6 +113,43 @@ impl IntoIterator for &Alias {
     }
 }
 
+// Fine to lint, the impls comes from a local macro.
+pub struct Issue12037;
+macro_rules! generate_impl {
+    () => {
+        impl<'a> IntoIterator for &'a Issue12037 {
+            type IntoIter = std::slice::Iter<'a, u8>;
+            type Item = &'a u8;
+            fn into_iter(self) -> Self::IntoIter {
+                todo!()
+            }
+        }
+    };
+}
+generate_impl!();
+
+// Impl comes from an external crate
+proc_macros::external! {
+    pub struct ImplWithForeignSpan;
+    impl<'a> IntoIterator for &'a ImplWithForeignSpan {
+        type IntoIter = std::slice::Iter<'a, u8>;
+        type Item = &'a u8;
+        fn into_iter(self) -> Self::IntoIter {
+            todo!()
+        }
+    }
+}
+
+pub struct Allowed;
+#[allow(clippy::into_iter_without_iter)]
+impl<'a> IntoIterator for &'a Allowed {
+    type IntoIter = std::slice::Iter<'a, u8>;
+    type Item = &'a u8;
+    fn into_iter(self) -> Self::IntoIter {
+        todo!()
+    }
+}
+
 fn main() {}
 
 pub mod issue11635 {

diff --git a/tests/ui/into_iter_without_iter.stderr b/tests/ui/into_iter_without_iter.stderr
@@ -1,5 +1,5 @@
 error: `IntoIterator` implemented for a reference type without an `iter` method
-  --> $DIR/into_iter_without_iter.rs:7:1
+  --> $DIR/into_iter_without_iter.rs:9:1
    |
 LL | / impl<'a> IntoIterator for &'a S1 {
 LL | |
@@ -23,7 +23,7 @@ LL + }
    |
 
 error: `IntoIterator` implemented for a reference type without an `iter_mut` method
-  --> $DIR/into_iter_without_iter.rs:15:1
+  --> $DIR/into_iter_without_iter.rs:17:1
    |
 LL | / impl<'a> IntoIterator for &'a mut S1 {
 LL | |
@@ -45,7 +45,7 @@ LL + }
    |
 
 error: `IntoIterator` implemented for a reference type without an `iter` method
-  --> $DIR/into_iter_without_iter.rs:25:1
+  --> $DIR/into_iter_without_iter.rs:27:1
    |
 LL | / impl<'a, T> IntoIterator for &'a S2<T> {
 LL | |
@@ -67,7 +67,7 @@ LL + }
    |
 
 error: `IntoIterator` implemented for a reference type without an `iter_mut` method
-  --> $DIR/into_iter_without_iter.rs:33:1
+  --> $DIR/into_iter_without_iter.rs:35:1
    |
 LL | / impl<'a, T> IntoIterator for &'a mut S2<T> {
 LL | |
@@ -89,7 +89,7 @@ LL + }
    |
 
 error: `IntoIterator` implemented for a reference type without an `iter_mut` method
-  --> $DIR/into_iter_without_iter.rs:84:1
+  --> $DIR/into_iter_without_iter.rs:86:1
    |
 LL | / impl<'a, T> IntoIterator for &mut S4<'a, T> {
 LL | |
@@ -110,5 +110,31 @@ LL +     }
 LL + }
    |
 
-error: aborting due to 5 previous errors
+error: `IntoIterator` implemented for a reference type without an `iter` method
+  --> $DIR/into_iter_without_iter.rs:120:9
+   |
+LL | /         impl<'a> IntoIterator for &'a Issue12037 {
+LL | |             type IntoIter = std::slice::Iter<'a, u8>;
+LL | |             type Item = &'a u8;
+LL | |             fn into_iter(self) -> Self::IntoIter {
+LL | |                 todo!()
+LL | |             }
+LL | |         }
+   | |_________^
+...
+LL |   generate_impl!();
+   |   ---------------- in this macro invocation
+   |
+   = note: this error originates in the macro `generate_impl` (in Nightly builds, run with -Z macro-backtrace for more info)
+help: consider implementing `iter`
+   |
+LL ~         
+LL + impl Issue12037 {
+LL +     fn iter(&self) -> std::slice::Iter<'a, u8> {
+LL +         <&Self as IntoIterator>::into_iter(self)
+LL +     }
+LL + }
+   |
+
+error: aborting due to 6 previous errors
 
diff --git a/tests/ui/iter_without_into_iter.rs b/tests/ui/iter_without_into_iter.rs
@@ -1,5 +1,7 @@
 //@no-rustfix
+//@aux-build:proc_macros.rs
 #![warn(clippy::iter_without_into_iter)]
+extern crate proc_macros;
 
 pub struct S1;
 impl S1 {
@@ -121,4 +123,33 @@ impl S12 {
     }
 }
 
+pub struct Issue12037;
+macro_rules! generate_impl {
+    () => {
+        impl Issue12037 {
+            fn iter(&self) -> std::slice::Iter<'_, u8> {
+                todo!()
+            }
+        }
+    };
+}
+generate_impl!();
+
+proc_macros::external! {
+    pub struct ImplWithForeignSpan;
+    impl ImplWithForeignSpan {
+        fn iter(&self) -> std::slice::Iter<'_, u8> {
+            todo!()
+        }
+    }
+}
+
+pub struct Allowed;
+impl Allowed {
+    #[allow(clippy::iter_without_into_iter)]
+    pub fn iter(&self) -> std::slice::Iter<'_, u8> {
+        todo!()
+    }
+}
+
 fn main() {}
diff --git a/tests/ui/iter_without_into_iter.stderr b/tests/ui/iter_without_into_iter.stderr
@@ -1,5 +1,5 @@
 error: `iter` method without an `IntoIterator` impl for `&S1`
-  --> $DIR/iter_without_into_iter.rs:6:5
+  --> $DIR/iter_without_into_iter.rs:8:5
    |
 LL | /     pub fn iter(&self) -> std::slice::Iter<'_, u8> {
 LL | |
@@ -22,7 +22,7 @@ LL + }
    |
 
 error: `iter_mut` method without an `IntoIterator` impl for `&mut S1`
-  --> $DIR/iter_without_into_iter.rs:10:5
+  --> $DIR/iter_without_into_iter.rs:12:5
    |
 LL | /     pub fn iter_mut(&mut self) -> std::slice::IterMut<'_, u8> {
 LL | |
@@ -43,7 +43,7 @@ LL + }
    |
 
 error: `iter` method without an `IntoIterator` impl for `&S3<'a>`
-  --> $DIR/iter_without_into_iter.rs:26:5
+  --> $DIR/iter_without_into_iter.rs:28:5
    |
 LL | /     pub fn iter(&self) -> std::slice::Iter<'_, u8> {
 LL | |
@@ -64,7 +64,7 @@ LL + }
    |
 
 error: `iter_mut` method without an `IntoIterator` impl for `&mut S3<'a>`
-  --> $DIR/iter_without_into_iter.rs:30:5
+  --> $DIR/iter_without_into_iter.rs:32:5
    |
 LL | /     pub fn iter_mut(&mut self) -> std::slice::IterMut<'_, u8> {
 LL | |
@@ -85,7 +85,7 @@ LL + }
    |
 
 error: `iter` method without an `IntoIterator` impl for `&S8<T>`
-  --> $DIR/iter_without_into_iter.rs:67:5
+  --> $DIR/iter_without_into_iter.rs:69:5
    |
 LL | /     pub fn iter(&self) -> std::slice::Iter<'static, T> {
 LL | |         todo!()
@@ -105,7 +105,7 @@ LL + }
    |
 
 error: `iter` method without an `IntoIterator` impl for `&S9<T>`
-  --> $DIR/iter_without_into_iter.rs:75:5
+  --> $DIR/iter_without_into_iter.rs:77:5
    |
 LL | /     pub fn iter(&self) -> std::slice::Iter<'_, T> {
 LL | |
@@ -126,7 +126,7 @@ LL + }
    |
 
 error: `iter_mut` method without an `IntoIterator` impl for `&mut S9<T>`
-  --> $DIR/iter_without_into_iter.rs:79:5
+  --> $DIR/iter_without_into_iter.rs:81:5
    |
 LL | /     pub fn iter_mut(&mut self) -> std::slice::IterMut<'_, T> {
 LL | |
@@ -146,5 +146,29 @@ LL +     }
 LL + }
    |
 
-error: aborting due to 7 previous errors
+error: `iter` method without an `IntoIterator` impl for `&Issue12037`
+  --> $DIR/iter_without_into_iter.rs:130:13
+   |
+LL | /             fn iter(&self) -> std::slice::Iter<'_, u8> {
+LL | |                 todo!()
+LL | |             }
+   | |_____________^
+...
+LL |   generate_impl!();
+   |   ---------------- in this macro invocation
+   |
+   = note: this error originates in the macro `generate_impl` (in Nightly builds, run with -Z macro-backtrace for more info)
+help: consider implementing `IntoIterator` for `&Issue12037`
+   |
+LL ~         
+LL + impl IntoIterator for &Issue12037 {
+LL +     type IntoIter = std::slice::Iter<'_, u8>;
+LL +     type Item = &u8;
+LL +     fn into_iter(self) -> Self::IntoIter {
+LL +         self.iter()
+LL +     }
+LL + }
+   |
+
+error: aborting due to 8 previous errors