New lint: `manual_inspect`

[profetia]

What it does

For the following case:

if let Some(legacy_path) = legacy_bin_path(package_root, &bin.name(), has_lib) {
    warnings.push(format!(
        "path `{}` was erroneously implicitly accepted for binary `{}`,\n\
         please set bin.path in Cargo.toml",
        legacy_path.display(),
        bin.name()
    ));
    Some(legacy_path)
} else {
    None
}

It is better to use Option::inspect.

Advantage

• Better readability

Drawbacks

Cannot think of so far.

Example

if let Some(legacy_path) = legacy_bin_path(package_root, &bin.name(), has_lib) {
    warnings.push(format!(
        "path `{}` was erroneously implicitly accepted for binary `{}`,\n\
         please set bin.path in Cargo.toml",
        legacy_path.display(),
        bin.name()
    ));
    Some(legacy_path)
} else {
    None
}

Could be written as:

legacy_bin_path(package_root, &bin.name(), has_lib)
    .inspect(|legacy_path| {
        warnings.push(format!(
            "path `{}` was erroneously implicitly accepted for binary `{}`,\n\
             please set bin.path in Cargo.toml",
            legacy_path.display(),
            bin.name()
        ));
    })

Comparison with existing lints

The given example will be covered by clippy::manual_map if #16775 is merged, but using Option::inspect would be better.

Additional Context

One real world example: https://docs.rs/cargo/0.64.0/src/cargo/util/toml/targets.rs.html#325

[evolo22]

Hello! I would like to take a shot at implementing this.

[Jarcho]

manual_inspect already exists, but it only lints on calls to map.

[evolo22]

Thanks for the context! So the goal would be to extend the manual_inspect lint then? I am still interested to work on this if it is still available.

[profetia]

It would be great to implement it as a follow up to #16775, since they share a similar paradigm

[evolo22]

I see, I will keep a look out for when #16775 is merged.