map_clone suggests removing clone altogether

[jyn514]

I tried this code (https://github.com/rust-lang/rust/blob/a6ff925f8b5598a1f6d84964525baa1d4a08fd63/compiler/rustc_codegen_ssa/src/back/write.rs#L1026):

    let cgcx = CodegenContext::<B> {
        incr_comp_session_dir: sess.incr_comp_session_dir_opt().map(|r| r.clone()),
    };

I expected to see this happen: Not sure this lint should have fired in the first place? This is a Ref, not an iterator.

Instead, this happened: Clippy suggests removing the map altogether, giving a type error.

error[E0308]: mismatched types
    --> compiler/rustc_codegen_ssa/src/back/write.rs:1014:32
     |
1014 |         incr_comp_session_dir: sess.incr_comp_session_dir_opt(),
     |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `std::path::PathBuf`, found struct `std::cell::Ref`
     |
     = note: expected enum `std::option::Option<std::path::PathBuf>`
                found enum `std::option::Option<std::cell::Ref<'_, std::path::PathBuf, >>`

Original diagnostics will follow.

warning: you are needlessly cloning iterator elements
    --> compiler/rustc_codegen_ssa/src/back/write.rs:1014:64
     |
1014 |         incr_comp_session_dir: sess.incr_comp_session_dir_opt().map(|r| r.clone()),
     |                                                                ^^^^^^^^^^^^^^^^^^^ help: remove the `map` call
     |
     = note: `#[warn(clippy::map_clone)]` on by default
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_clone

Meta

• cargo clippy -V: clippy 0.0.212 (ffa2e7a 2020-10-24)

[giraffate]

map_clone works with not only Iterator but Option although not written in doc.

[jyn514]

Gotcha, thanks. It should still suggest cloned() instead of removing the clone altogether, though.

[jyn514]

@rustbot modify labels: +A-suggestion +L-suggestion-causes-error

[jyn514]

Actually I think this is still wrong - r.clone() clones the path, not the Ref. So .cloned() wouldn't do anything here - it would leave you with an Option<Ref<PathBuf>> still. I think clippy isn't taking deref coercion into account.

[camsteffen]

I can fix this.