Only suggest map_or for copy types

[detrumi]

Fixes #2686

[ghost]

Isn't this going to cause false negatives?

[oli-obk]

@mikerite can you create an MCVE of a false negative that you imagine could occur?

[ghost]

I was thinking of when the default isn't used in the closure.

#![warn(clippy::option_map_unwrap_or)]
pub fn main() {
    // Example
    {
        let default = "default".to_string();
        let option = Some("option");
        option.map(|value| format!("{}!", value)).unwrap_or(default);
    }

    // Example fixed
    {
        let default = "default".to_string();
        let option = Some("option");
        option.map_or(default, |value| format!("{}!", value));
    }
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=62f8e9b90002126394fc49fc7b9bab77

[oli-obk]

Oh right. That should be "easy" (as in simple but needs loads of code) to catch though: A visitor run on the map argument could check for any identifiers found by a visitor run on the unwrap_or argument.

[detrumi]

Oh right. That should be "easy" (as in simple but needs loads of code) to catch though: A visitor run on the map argument could check for any identifiers found by a visitor run on the unwrap_or argument.

So you first collect all identifiers, and then do a second run, right? How do I check for identifiers? My first thought was to do a visit_path and collect identifiers using a FxHashSet<Path>, but paths don't implement Hash.

[detrumi]

@oli-obk So I'm trying to check for identifiers using visit_path, but it's not finding the variable (which is a Path that you should find when visiting the expression, right?)

    fn visit_path(&mut self, path: &'tcx Path, _id: HirId) {
        if let Some(def_id) = path.def.opt_def_id() {
            self.identifiers.insert(def_id);
        }
        walk_path(self, path);
    }

Do I need to use visit_expr with pattern matching on ExprKind instead, or am I doing this wrong?

Edit: seems like getting the name of the last path segment did the trick.

[bors]

☔ The latest upstream changes (presumably #3795) made this pull request unmergeable. Please resolve the merge conflicts.

[flip1995]

ping from triage @detrumi. Sorry for the long wait time. This LGTM. Can you rebase this so it can be merged?

[detrumi]

@flip1995 Rebase done

[oli-obk]

@bors r=flip1995

[bors]

📌 Commit eb70a72 has been approved by flip1995

[bors]

⌛ Testing commit eb70a72 with merge ff27988...

[bors]

☀️ Test successful - checks-travis, status-appveyor
Approved by: flip1995
Pushing ff27988 to master...