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

new_ret_no_self should allow Result<Self, E> #3620

Closed
Riateche opened this issue Jan 3, 2019 · 7 comments
Closed

new_ret_no_self should allow Result<Self, E> #3620

Riateche opened this issue Jan 3, 2019 · 7 comments

Comments

@Riateche
Copy link

Riateche commented Jan 3, 2019

new_ret_no_self currently triggers if new() returns Result<Self, E>. I think new() is still an idiomatic name for a constructor even if it can fail, so Result<Self, E> should be permitted.

@scottjmaddox
Copy link

And Option<Self>. This pattern is used in the standard library, for example in NonZeroU8::new.

@flip1995
Copy link
Member

flip1995 commented Jan 10, 2019

This was already fixed in #3338: Playground

struct A;

impl A {
    fn new() -> Result<Self, ()> {
        Err(())
    }
}

Feel free to reopen if I misunderstood the issue.

EDIT: Link fixed

@jplatte
Copy link
Contributor

jplatte commented Jan 11, 2019

This was already fixed in #3338: Playground

Your link just opens the Playground without any specific code. Seems like you just copied the URL instead of using the share button.

@scottjmaddox
Copy link

I'm not sure how to test it in the playground, but it's definitely not fixed in the latest release of clippy. It looks like the last release was Oct 5, though, so it's possible that pull request from Oct 19 fixes it.

When is the next release planned?

@mati865
Copy link
Contributor

mati865 commented Jan 11, 2019

@scottjmaddox Clippy doesn't have releases, you should install it with rustup: https://github.com/rust-lang/rust-clippy#step-1-install-rustup

@scottjmaddox
Copy link

@mati865 I was basing the "releases" on the versions posted at https://crates.io/crates/clippy. I did install through rustup, and it stills says up to date. Perhaps the question I should ask is, when will the version that includes the stated fix be available for install through rustup?

@flip1995
Copy link
Member

flip1995 commented Jan 11, 2019

Your link just opens the Playground without any specific code.

Oh sorry it was late. There you go: Playground You can test it via Tools -> Clippy in the top right corner

"releases" on the versions posted at https://crates.io/crates/clippy

This is deprecated and is only there to point people to rustup.

The Clippy version shipped with rustup depends on the toolchain you are using when running rustup component add clippy. If you're using the latest stable toolchain (1.31) you'll get

clippy 0.0.212 (2e26fdc 2018-11-22)

with beta

clippy 0.0.212 (b2601be 2018-11-27)

and nightly

clippy 0.0.212 (c63b634 2019-01-05)

So if you use rustc 1.31 and above this fix should be included.
Nvm. Somehow this fix isn't in the stable toolchain. It is in beta though. This means it will be in stable with 1.32 on Fri Jan 18 2019 (You can keep track of rust release dates on https://forge.rust-lang.org/).


For those interested: The reason the version date of the stable toolchain Clippy is 2018-11-22, is that it refers to https://github.com/rust-lang/rust-clippy/commits/rust-1.31.0 and not the master branch. The latest effective commit there is from the 19.10.2018 though (stable just barely missed this fix).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants