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

should_implement_trait should not warn async methods #4290

Closed
taiki-e opened this issue Jul 20, 2019 · 3 comments
Closed

should_implement_trait should not warn async methods #4290

taiki-e opened this issue Jul 20, 2019 · 3 comments
Labels
C-bug Category: Clippy is not doing the correct thing good-first-issue These issues are a good way to get started with Clippy T-async-await Type: Issues related to async/await

Comments

@taiki-e
Copy link
Member

taiki-e commented Jul 20, 2019

They are actually returning a different type than normal methods.

#![feature(async_await)]
#![warn(clippy::should_implement_trait)]

pub struct Foo;

impl Foo {
    // actual type: pub fn next(&mut self) -> impl Future<Output = Option<()>>
    pub async fn next(&mut self) -> Option<()> { //~ WARN: defining a method called `next` on this type; consider implementing the `std::iter::Iterator` trait or choosing a less ambiguous name
        unimplemented!()
    }
}

Playground

@flip1995
Copy link
Member

The warning is mostly about the method name next. Since this is a style lint, I think it is totally fine, that the lint triggers here. Maybe calling the function next_async would be better here?

@djc
Copy link
Contributor

djc commented Mar 27, 2020

I kind of disagree, since the Stream trait actually also calls its method next(). If Stream was in std, this lint could refer to Stream, but as long as that's not the case, clippy would be better off not warning about this IMO (I've been hit by this multiple times and always just end up silencing the lint).

@flip1995 flip1995 added good-first-issue These issues are a good way to get started with Clippy C-bug Category: Clippy is not doing the correct thing C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages T-async-await Type: Issues related to async/await and removed C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages labels Mar 27, 2020
rabisg0 added a commit to rabisg0/rust-clippy that referenced this issue Apr 8, 2020
When checking for functions that are potential candidates for trait
implementations check the function header to make sure modifiers like
asyncness, constness and safety match before triggering the lint.

Fixes rust-lang#5413, rust-lang#4290
rabisg0 added a commit to rabisg0/rust-clippy that referenced this issue Apr 8, 2020
When checking for functions that are potential candidates for trait
implementations check the function header to make sure modifiers like
asyncness, constness and safety match before triggering the lint.

Fixes rust-lang#5413, rust-lang#4290
rabisg0 added a commit to rabisg0/rust-clippy that referenced this issue Apr 8, 2020
When checking for functions that are potential candidates for trait
implementations check the function header to make sure modifiers like
asyncness, constness and safety match before triggering the lint.

Fixes rust-lang#5413, rust-lang#4290
bors added a commit that referenced this issue Apr 8, 2020
Check fn header along with decl when suggesting to implement trait

When checking for functions that are potential candidates for trait
implementations check the function header to make sure modifiers like
asyncness, constness and safety match before triggering the lint.

Fixes #5413, #4290

changelog: check fn header along with decl for should_implement_trait
rokob pushed a commit to rokob/rust-clippy that referenced this issue Apr 17, 2020
When checking for functions that are potential candidates for trait
implementations check the function header to make sure modifiers like
asyncness, constness and safety match before triggering the lint.

Fixes rust-lang#5413, rust-lang#4290
rokob pushed a commit to rokob/rust-clippy that referenced this issue Apr 17, 2020
When checking for functions that are potential candidates for trait
implementations check the function header to make sure modifiers like
asyncness, constness and safety match before triggering the lint.

Fixes rust-lang#5413, rust-lang#4290
rokob pushed a commit to rokob/rust-clippy that referenced this issue Apr 17, 2020
When checking for functions that are potential candidates for trait
implementations check the function header to make sure modifiers like
asyncness, constness and safety match before triggering the lint.

Fixes rust-lang#5413, rust-lang#4290
@phansch
Copy link
Member

phansch commented Apr 20, 2020

This was apparently fixed by #5437

@phansch phansch closed this as completed Apr 20, 2020
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 good-first-issue These issues are a good way to get started with Clippy T-async-await Type: Issues related to async/await
Projects
None yet
Development

No branches or pull requests

4 participants