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

needless_borrow stopped triggering on references to Option<&_> if they're returned as part of a tuple #11786

Closed
liketechnik opened this issue Nov 9, 2023 · 4 comments · Fixed by #11812
Labels
C-bug Category: Clippy is not doing the correct thing I-false-negative Issue: The lint should have been triggered on code, but wasn't

Comments

@liketechnik
Copy link

liketechnik commented Nov 9, 2023

Summary

I found out about this, because a project's CI pipeline failed on code for which a local invocation of cargo clippy reported no errors/warnings. I was able to pin this down to different versions of clippy being installed: clippy 0.1.72 reports the warning, while newer versions (see below) do not report it anymore.

The original code is from a private project, so I created the shown more minimal reproducer.

This issue might have something in common with #11326 which was reported against 1.73.0; which is the first version this issue started showing up, afaict.

Lint Name

needless_borrow

Reproducer

I tried this code:

struct Object;

impl Object {
    fn str_getter(&self) -> Option<&str> {
        None
    }
}

fn main() {
    let o = Object;
    broken(&o);
}

fn broken(o: &Object) -> (&str, usize) {
    (&o.str_getter().unwrap(), 1)
}

I expected to see this happen (as it happens with clippy 0.1.72 (b2b34bd 2023-06-06)):

user$ cargo clippy
    Checking repro-needless-borrow v0.1.0 (/tmp/bla)
warning: this expression creates a reference which is immediately dereferenced by the compiler
  --> src/main.rs:15:6
   |
15 |     (&o.str_getter().unwrap(), 1)
   |      ^^^^^^^^^^^^^^^^^^^^^^^^ help: change this to: `(o.str_getter().unwrap())`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
   = note: `#[warn(clippy::needless_borrow)]` on by default

warning: `repro-needless-borrow` (bin "repro-needless-borrow") generated 1 warning (run `cargo clippy --fix --bin "repro-needless-borrow"` to apply 1 suggestion)
    Finished dev [unoptimized + debuginfo] target(s) in 0.09s

Instead, this happened (with later clippy versions, starting at clippy 0.1.73 (cc66ad46 2023-10-03), see below):

    Checking repro-needless-borrow v0.1.0 (/tmp/bla)
    Finished dev [unoptimized + debuginfo] target(s) in 0.10s

Version

clippy version reporting the warning: 

- clippy 0.1.72 (b2b34bd 2023-06-06) (1.72 nightly) (with the project in ci and the minimal reproducer)
- clippy 0.1.72 (d5c2e9c 2023-09-13) (1.72.1 stable-2023-09-19) (reproduced locally)
- clippy 0.1.72 (5680fa1 2023-08-23) (1.72.0 stable-2023-08-24) (reproduced locally)


clippy version(s) not reporting the warning: 

- clippy 0.1.75 (fdaaaf9f 2023-11-08) (1.75 nigthly-2023-11-09) (with the local project and the minimal reproducer)
- clippy 0.1.74 (efc300e 2023-11-07) (1.74 beta-2023-11-08) (reproduced locally)
- clippy 0.1.73 (cc66ad46 2023-10-03) (1.73.0 stable-2023-10-05) (with the local project and the minimal reproducer),
@liketechnik liketechnik added C-bug Category: Clippy is not doing the correct thing I-false-negative Issue: The lint should have been triggered on code, but wasn't labels Nov 9, 2023
@m-rph
Copy link
Contributor

m-rph commented Nov 11, 2023

@rustbot claim

@m-rph
Copy link
Contributor

m-rph commented Nov 11, 2023

After a bisect, this regression seems to be introduced in #11166. This looks way outside what I can handle right now given my understanding, maybe @Jarcho can help?

@m-rph
Copy link
Contributor

m-rph commented Nov 11, 2023

@rustbot release-assignment

@Jarcho
Copy link
Contributor

Jarcho commented Nov 15, 2023

That code is definitely not simple. Thanks for bisecting it.

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-negative Issue: The lint should have been triggered on code, but wasn't
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants