Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

into_iter_without_iter false positive when there is a Deref impl #11635

Closed
dtolnay opened this issue Oct 7, 2023 · 2 comments · Fixed by #11639
Closed

into_iter_without_iter false positive when there is a Deref impl #11635

dtolnay opened this issue Oct 7, 2023 · 2 comments · Fixed by #11639
Assignees
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@dtolnay
Copy link
Member

dtolnay commented Oct 7, 2023

Summary

In https://rust-lang.github.io/rust-clippy/master/index.html#/into_iter_without_iter, into_iter_without_iter is motivated by letting users write the simpler val.iter() instead of <&Type>::into_iter(val) or (&val).into_iter().

However, there are cases where a fn iter is not needed in order for .iter() to be available. Namely for a type with a Deref impl, where val.deref().iter() returns the same thing as (&val).into_iter(), the simple val.iter() already works (and will be shown in the documentation by rustdoc). Handwriting a fn iter is needless boilerplate.

Lint Name

into_iter_without_iter

Reproducer

In the following code, clippy wants fn iter so that Thing(...).iter() works. But Thing(...).iter() already works.

#![warn(clippy::pedantic)]

use std::ops::Deref;

pub struct Thing(Vec<u8>);

impl Deref for Thing {
    type Target = [u8];

    fn deref(&self) -> &Self::Target {
        &self.0
    }
}

impl<'a> IntoIterator for &'a Thing {
    type Item = &'a u8;
    type IntoIter = <&'a [u8] as IntoIterator>::IntoIter;

    fn into_iter(self) -> Self::IntoIter {
        self.0.iter()
    }
}
warning: `IntoIterator` implemented for a reference type without an `iter` method
  --> src/lib.rs:15:1
   |
15 | / impl<'a> IntoIterator for &'a Thing {
16 | |     type Item = &'a u8;
17 | |     type IntoIter = <&'a [u8] as IntoIterator>::IntoIter;
18 | |
...  |
21 | |     }
22 | | }
   | |_^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#into_iter_without_iter
note: the lint level is defined here
  --> src/lib.rs:1:9
   |
1  | #![warn(clippy::pedantic)]
   |         ^^^^^^^^^^^^^^^^
   = note: `#[warn(clippy::into_iter_without_iter)]` implied by `#[warn(clippy::pedantic)]`
help: consider implementing `iter`
   |
15 + 
16 + impl Thing {
17 +     fn iter(&self) -> <&'a [u8] as IntoIterator>::IntoIter {
18 +         <&Self as IntoIterator>::into_iter(self)
19 +     }
20 + }
   |

Version

rustc 1.75.0-nightly (960754090 2023-10-06)
binary: rustc
commit-hash: 960754090acc9cdd2a5a57586f244c0fc712d26c
commit-date: 2023-10-06
host: aarch64-unknown-linux-gnu
release: 1.75.0-nightly
LLVM version: 17.0.2

Additional Labels

No response

@dtolnay dtolnay added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Oct 7, 2023
@dtolnay
Copy link
Member Author

dtolnay commented Oct 7, 2023

Mentioning @y21 @Jarcho since you worked on this lint recently in #11587.

dtolnay added a commit to dtolnay/miniserde that referenced this issue Oct 7, 2023
rust-lang/rust-clippy#11635

    warning: `IntoIterator` implemented for a reference type without an `iter` method
      --> src/json/array.rs:72:1
       |
    72 | / impl<'a> IntoIterator for &'a Array {
    73 | |     type Item = &'a Value;
    74 | |     type IntoIter = <&'a Vec<Value> as IntoIterator>::IntoIter;
    75 | |
    ...  |
    78 | |     }
    79 | | }
       | |_^
       |
       = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#into_iter_without_iter
       = note: `-W clippy::into-iter-without-iter` implied by `-W clippy::pedantic`
       = help: to override `-W clippy::pedantic` add `#[allow(clippy::into_iter_without_iter)]`
    help: consider implementing `iter`
       |
    72 +
    73 + impl Array {
    74 +     fn iter(&self) -> <&'a Vec<Value> as IntoIterator>::IntoIter {
    75 +         <&Self as IntoIterator>::into_iter(self)
    76 +     }
    77 + }
       |

    warning: `IntoIterator` implemented for a reference type without an `iter_mut` method
      --> src/json/array.rs:81:1
       |
    81 | / impl<'a> IntoIterator for &'a mut Array {
    82 | |     type Item = &'a mut Value;
    83 | |     type IntoIter = <&'a mut Vec<Value> as IntoIterator>::IntoIter;
    84 | |
    ...  |
    87 | |     }
    88 | | }
       | |_^
       |
       = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#into_iter_without_iter
    help: consider implementing `iter_mut`
       |
    81 +
    82 + impl Array {
    83 +     fn iter_mut(&mut self) -> <&'a mut Vec<Value> as IntoIterator>::IntoIter {
    84 +         <&mut Self as IntoIterator>::into_iter(self)
    85 +     }
    86 + }
       |

    warning: `IntoIterator` implemented for a reference type without an `iter` method
      --> src/json/object.rs:66:1
       |
    66 | / impl<'a> IntoIterator for &'a Object {
    67 | |     type Item = (&'a String, &'a Value);
    68 | |     type IntoIter = <&'a BTreeMap<String, Value> as IntoIterator>::IntoIter;
    69 | |
    ...  |
    72 | |     }
    73 | | }
       | |_^
       |
       = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#into_iter_without_iter
    help: consider implementing `iter`
       |
    66 +
    67 + impl Object {
    68 +     fn iter(&self) -> <&'a BTreeMap<String, Value> as IntoIterator>::IntoIter {
    69 +         <&Self as IntoIterator>::into_iter(self)
    70 +     }
    71 + }
       |

    warning: `IntoIterator` implemented for a reference type without an `iter_mut` method
      --> src/json/object.rs:75:1
       |
    75 | / impl<'a> IntoIterator for &'a mut Object {
    76 | |     type Item = (&'a String, &'a mut Value);
    77 | |     type IntoIter = <&'a mut BTreeMap<String, Value> as IntoIterator>::IntoIter;
    78 | |
    ...  |
    81 | |     }
    82 | | }
       | |_^
       |
       = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#into_iter_without_iter
    help: consider implementing `iter_mut`
       |
    75 +
    76 + impl Object {
    77 +     fn iter_mut(&mut self) -> <&'a mut BTreeMap<String, Value> as IntoIterator>::IntoIter {
    78 +         <&mut Self as IntoIterator>::into_iter(self)
    79 +     }
    80 + }
       |
@y21
Copy link
Member

y21 commented Oct 7, 2023

Yeah, I guess we can walk the deref impl chain and look for iter methods on each of the types and not lint if there's one
@rustbot claim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants