New lint: option_manual_map

[Jarcho]

fixes: #6
changelog: Added lint: match_map

[rust-highfive]

r? @ebroto

(rust-highfive has picked a reviewer for you, use r? to override)

[bors]

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

[ebroto]

r? @llogiq (I'm leaving the team, so I'm reassigning my PRs to other active members)

[bors]

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

[llogiq]

r=me after a rebase.

[camsteffen]

The lint could also work on Result. That could be deferred for a later enhancement, but either way, I'd suggest renaming the lint to manual_map.

[llogiq]

I think you should be able to rebase ignoring clippy_lints/src/lib.rs, then running cargo dev update_lints, which should regenerate the lint list. That will likely get rid of the conflicts.

[llogiq]

@Jarcho do you intend to resume work on this? Can we help you with something?

[Jarcho]

Basically ended up rewriting this.

• The lint name has been changed, but still only lints for Option.
• Some can now contain any pattern so long as there are no binding annotations, and the other pattern is None.
• as_ref and as_mut will be added when needed. Matching on &Option will now work.
• No longer checks for identity

[Jarcho]

I'm not sure why that's failing. That lint should be allowed there.

[llogiq]

The reason appears to be that the test is declared to work with rustfix, but the fixed code fails to compile.

[bors]

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

[llogiq]

Having looked into it a bit more from my PC, I can see that line 42 of the fixed test falls afoul of clippy::option_map_unit_fn. Now we can either a) silence that lint in the test or b) add a check of the map closure's return type and skip the lint if it's unit.

[Jarcho]

There's an #[allow(clippy::option_map_unit_fn)] right before that line.

[giraffate]

I think an #[allow(clippy::option_map_unit_fn)] before expression doesn't work. If it is before statements, it works.

[Jarcho]

It is on a statement. From tests/ui/manual_map_option.fixed (line 41 & 42):

    #[allow(clippy::option_map_unit_fn)]
    Some(String::new()).as_mut().map(|x| x.push_str(""));

Is that not supposed to work?

There is also a check to make sure option_map_unit_fn is allowed before making the suggestion, so it was fine when it was:

    #[allow(clippy::option_map_unit_fn)]
    match &mut Some(String::new()) {
        Some(x) => Some(x.push_str("")),
        None => None,
    };

[giraffate]

Is that not supposed to work?

Yes, I think so. I think it works before statements, not a statement. I don't know the detail, but it seems to work like this when that lint (option_map_unit_fn in this case ) is called in check_stmt.

[Jarcho]

Looks like wrapping it in a block worked.

[llogiq]

Great! Thank you for pushing this to completion! @bors r+

[bors]

📌 Commit 23aa2f8 has been approved by llogiq

[bors]

⌛ Testing commit 23aa2f8 with merge a2c25fa...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing a2c25fa to master...