needless_borrow suggests removing a necessary borrow in a loop

[martinvonz]

Summary

With a very recent nightly build, the needless_borrow lint suggests removing a necessary borrow when the borrow occurs in a loop (though I haven't spent any time trying to figure out exactly what the condition is).

Lint Name

needless_borrow

Reproducer

I tried this code:

fn main() {
    let x = std::path::PathBuf::from("foo");
    for _ in 0..3 {
        std::fs::read(&x).ok();
    }
}

I saw this happen:

    Checking borrow v0.0.1 (/tmp/tmp.GV7LLZAxFz)
warning: failed to automatically apply fixes suggested by rustc to crate `borrow`

after fixes were automatically applied the compiler reported errors within these files:

  * src/main.rs

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

The following errors were reported:
error[E0382]: use of moved value: `x`
 --> src/main.rs:4:23
  |
2 |     let x = std::path::PathBuf::from("foo");
  |         - move occurs because `x` has type `std::path::PathBuf`, which does not implement the `Copy` trait
3 |     for _ in 0..3 {
4 |         std::fs::read(x).ok();
  |                       ^ value moved here, in previous iteration of loop

error: aborting due to previous error

For more information about this error, try `rustc --explain E0382`.
Original diagnostics will follow.

warning: the borrowed expression implements the required traits
 --> src/main.rs:4:23
  |
4 |         std::fs::read(&x).ok();
  |                       ^^ help: change this to: `x`
  |
  = 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: `borrow` (bin "borrow") generated 1 warning
warning: `borrow` (bin "borrow" test) generated 1 warning (1 duplicate)
    Finished dev [unoptimized + debuginfo] target(s) in 0.53s

I expected to see this happen:
No warning

Version

rustc 1.67.0-nightly (11ebe6512 2022-11-01)
binary: rustc
commit-hash: 11ebe6512b4c77633c59f8dcdd421df3b79d1a9f
commit-date: 2022-11-01
host: x86_64-unknown-linux-gnu
release: 1.67.0-nightly
LLVM version: 15.0.4

Additional Labels

No response

[smoelius]

@martinvonz Would you mind testing your code again with a more recent version of Clippy?

There was a bug like you described, but I think it has been fixed.

While it's possible I am doing something wrong, I don't seem to get a warning from your code with 6b4e7dd, for example.

[martinvonz]

I tried just now with rustc 1.67.0-nightly (edf0182 2022-11-02) but the bug is still there. I suppose your fix just hasn't reached the nightly build yet. I don't feel like building clippy from source just to test, but I'm happy to test again once it's reached nightly. Do you know when that will be?

[smoelius]

I'm not sure. I think an "Update Clippy" PR is opened about every two weeks, and it looks like the last one was opened on October 24. So November 7-ish, maybe(?).

[martinvonz]

Feel free to close this. It does seem to be the same problem and same fix. Or remind me to check again in a week or so :)

[smoelius]

I'll try to remember to remind you. :)

[seritools]

Just stumbled upon this myself, still getting this with 1.67.0-nightly (2022-11-13 e631891f7ad40eac3ef5) (playground)

use std::path::PathBuf;

pub fn foo() {
    let path = PathBuf::new();
    let directory_path: PathBuf = PathBuf::new();

    loop {
        let _ = path.join(&directory_path);
    }
}

[smoelius]

I just ran Clippy master (a599527) on this example and got no warnings.

So I think the fix just hasn't made it into the Rust tree yet.

[smoelius]

@martinvonz I think this is fixed in nightly, if you want to give it another shot.

[martinvonz]

Yes, looks good now. Thanks!

[eric-seppanen]

Looks like beta 1.66 also has this problem. I tried the original example and get the lint; with --fix I get the "failed to automatically apply fixes... This likely indicates a bug" message.

I'm running

$ cargo +beta clippy -V
clippy 0.1.66 (0040709 2022-11-20)
$ cargo +beta -V
cargo 1.66.0-beta.2 (d65d197ad 2022-11-15)

[eric-seppanen]

And this problem is in stable 1.66 as well. This is a regression from 1.65.