Skip to content

Warn when intended Deref is not triggered correctly #7830

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

Closed
jamesmcm opened this issue Oct 17, 2021 · 2 comments
Closed

Warn when intended Deref is not triggered correctly #7830

jamesmcm opened this issue Oct 17, 2021 · 2 comments
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages

Comments

@jamesmcm
Copy link
Contributor

What it does

Aim would be to warn when intended Deref is not triggered correctly.

E.g. for https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=c78429c73f85949bd3799a9fdcaa4d74

use std::ops::Deref;

struct TestType;

impl Deref for TestType {
    type Target = str;

    fn deref(&self) -> &Self::Target {
        "test"
    }
}

impl std::fmt::Display for TestType {
    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
        write!(f, "{}", &**self)
    }
}

fn main() {
    let test = TestType {};
    println!("Hello, world!, {}", test);
}

If we write:

        write!(f, "{}", &self)

or

        write!(f, "{}", &*self)

Then the Deref to str is not triggered and we got a stack overflow due to recursively calling Display::fmt

At the moment there are no clippy or rustc warnings or errors, and we just get a stack overflow.

Categories (optional)

  • Kind: clippy::suspicious

What is the advantage of the recommended code over the original code

The recommended code correctly triggers the Deref and avoids the stack overflow.

@jamesmcm jamesmcm added the A-lint Area: New lints label Oct 17, 2021
@camsteffen
Copy link
Contributor

camsteffen commented Oct 18, 2021

We already have to_string_in_display. I think it should be enhanced and renamed to recursive_display_impl, meaning any usage of self as Display in impl Display.

@camsteffen camsteffen added C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages and removed A-lint Area: New lints labels Oct 18, 2021
@camsteffen
Copy link
Contributor

Duplicate of #2691

@camsteffen camsteffen marked this as a duplicate of #2691 Nov 2, 2021
bors added a commit that referenced this issue Feb 16, 2022
new lint: `recursive_format_impl`

The to_string_in_display lint is renamed to recursive_format_impl
A check is added for the use of self formatted with Display or Debug inside any format string in the same impl
The to_string_in_display check is kept as is - like in the format_in_format_args lint

This is my first contribution so please check it for better / more idiomatic checks + error messages. Note the format macro paths are shared with the `format_in_format_args` lint - maybe those could be moved to clippy utils too.

This relates to issues #2691 and #7830

------

changelog: Renamed `to_string_in_display` lint to [`recursive_format_impl`] with new check for any use of self as Display or Debug inside the same format trait impl.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages
Projects
None yet
Development

No branches or pull requests

2 participants