False positive for vec_init_then_push with control flow in push arg

[digama0]

Lint name: vec_init_then_push

Here's a simple example:

#![warn(clippy::vec_init_then_push)]

pub fn foo<T>(mut it: impl Iterator<Item=T>) -> Vec<T> {
  let mut vec = Vec::new();
  loop {
    vec.push(if let Some(i) = it.next() { i } else { return vec });
  }
}

The result:

warning: calls to `push` immediately after creation
 --> src/main.rs:4:3
  |
4 | /   let mut vec = Vec::new();
5 | |   loop {
6 | |     vec.push(if let Some(i) = it.next() { i } else { return vec });
  | |___________________________________________________________________^ help: consider using the `vec![]` macro: `let mut vec = vec![..];`
  |
note: the lint level is defined here
 --> src/main.rs:1:9
  |
1 | #![warn(clippy::vec_init_then_push)]
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#vec_init_then_push

The fact that the span goes over a loop { is surprising, although perhaps this is the result of some compiler refactoring. The lint seems to trigger when there is complex control flow inside the argument to push; in my real example it was a vec.push(loop { ... break val }). If not for the outer loop this could still be refactored to a vec![...] as suggested, but with it this is definitely an incorrect suggestion. If the loop ended with a break I would still be inclined to consider it a false positive, because loops can hide other kinds of control flow like internal break/continue, so rather than doing a full control flow analysis it should just not trigger if the push in question is syntactically in a loop or if.

Meta

• cargo clippy -V: clippy 0.1.51 (a4cbb44 2021-01-20)
• rustc -Vv:

  rustc 1.51.0-nightly (a4cbb44ae 2021-01-20)
  binary: rustc
  commit-hash: a4cbb44ae2c80545db957763b502dc7f6ea22085
  commit-date: 2021-01-20
  host: x86_64-unknown-linux-gnu
  release: 1.51.0-nightly
  LLVM version: 11.0.1
  

[digama0]

Here's an even more surprising example:

#![warn(clippy::vec_init_then_push)]

pub enum Tree {
  Nil,
  Node(Box<Tree>, Box<Tree>)
}

impl Tree {
  pub fn left_spine(&self) -> Vec<&Tree> {
    let mut p = self;
    let mut vec = Vec::new();
    loop {
      match p {
        Tree::Nil => return vec,
        Tree::Node(left, right) => {
          vec.push(right);
          p = left;
        }
      }
    }
  }
}

warning: calls to `push` immediately after creation
  --> src/main.rs:11:5
   |
11 | /     let mut vec = Vec::new();
12 | |     loop {
13 | |       match p {
14 | |         Tree::Nil => return vec,
15 | |         Tree::Node(left, right) => {
16 | |           vec.push(right);
   | |__________________________^ help: consider using the `vec![]` macro: `let mut vec = vec![..];`
   |
note: the lint level is defined here
  --> src/main.rs:1:9
   |
1  | #![warn(clippy::vec_init_then_push)]
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#vec_init_then_push