Rust-clippy: Do not suggest replacing simple `if let Some(_) / None` with methods

Created on 14 Jan 2018  路  4Comments  路  Source: rust-lang/rust-clippy

This lint I don't quite agree with, and I realize opinions will differ on this.

Lint link: https://rust-lang-nursery.github.io/rust-clippy/v0.0.177/index.html#if_let_redundant_pattern_matching

Existing code in removed (red) and the proposed change by clippy in added (green) lines:

         // clear edges without touching the free list
         for node in &mut self.g.nodes {
-            if let Some(_) = node.weight {
+            if node.weight.is_some() {
                 node.next = [EdgeIndex::end(), EdgeIndex::end()];
             }
         }
-        if let None = node_weight {
+        if node_weight.is_none() {
             return None;
         }

etc with similar examples.

I think that using if let is fundamental to Rust, is general, works with any enum and is just as easy to understand as the alternative. I prefer general over Option-specific methods.

C-needs-discussion

Most helpful comment

Please suggest replacing o.is_some() with o.iter().count() == 1 and o.is_none() with o.iter().count() == 0.

I think that using collections is fundamental to Rust. This method is general, works with any collection and is just as easy to understand as the alternative. I prefer general over Option-specific methods.

All 4 comments

Advantages of Option methods:

  • can chain with other bool operations

    • thus trigger more clippy lints down the road

  • fewer characters
  • left to right reading order

Disadvantages of Option methods

  • methods calls can be anything
  • method calls are not fundamental to Rust (馃槅)
  • more code for the optimizer to handle

I don't have a strong opinion either way. I just do what clippy says

We could make it a restriction lint, but then I'd also like to see the opposite lint.


Side note:

-        if let None = node_weight {
+        if node_weight.is_none() {
             return None;
         }

shouldn't that simply be node_weight?;

shouldn't that simply be node_weight?;

I'd like to see a lint for that. :)

Please suggest replacing o.is_some() with o.iter().count() == 1 and o.is_none() with o.iter().count() == 0.

I think that using collections is fundamental to Rust. This method is general, works with any collection and is just as easy to understand as the alternative. I prefer general over Option-specific methods.

No trolling, please.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

BenjaminGill-Metaswitch picture BenjaminGill-Metaswitch  路  22Comments

Manishearth picture Manishearth  路  18Comments

mikerite picture mikerite  路  17Comments

matthiaskrgr picture matthiaskrgr  路  24Comments

kevincox picture kevincox  路  17Comments