Skip to content

[Clippy made me do this] clippy failing to apply fixes #11571

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Astroide opened this issue Sep 26, 2023 · 1 comment
Open

[Clippy made me do this] clippy failing to apply fixes #11571

Astroide opened this issue Sep 26, 2023 · 1 comment

Comments

@Astroide
Copy link

Description

Full output of the command (cargo clippy --fix --lib -p eeeee):

    Checking eeeee v0.1.0 (/Users/astroide/Projets/eeeee)
warning: failed to automatically apply fixes suggested by rustc to crate `eeeee`

after fixes were automatically applied the compiler reported errors within these files:

  * src/compiler.rs
  * src/vm.rs

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see 
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust-clippy/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

The following errors were reported:
warning: this `if` statement can be collapsed
   --> src/compiler.rs:175:13
    |
175 | /             if matches!(instruction, vm::Instruction::PushNothing) {
176 | |                 if !self.instructions.is_empty() {
177 | |                     if matches!(self.instructions[self.instructions.len() - 1], vm::Instruction::Discard) {
178 | |                         self.instructions.pop();
...   |
181 | |                 }
182 | |             }
    | |_____________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_if
    = note: `#[warn(clippy::collapsible_if)]` on by default
help: collapse nested if block
    |
175 ~             if matches!(instruction, vm::Instruction::PushNothing) && !self.instructions.is_empty() {
176 +                 if matches!(self.instructions[self.instructions.len() - 1], vm::Instruction::Discard) {
177 +                     self.instructions.pop();
178 +                     continue
179 +                 }
180 +             }
    |

warning: this `if` statement can be collapsed
   --> src/compiler.rs:176:17
    |
176 | /                 if !self.instructions.is_empty() {
177 | |                     if matches!(self.instructions[self.instructions.len() - 1], vm::Instruction::Discard) {
178 | |                         self.instructions.pop();
179 | |                         continue
180 | |                     }
181 | |                 }
    | |_________________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_if
help: collapse nested if block
    |
176 ~                 if !self.instructions.is_empty() && matches!(self.instructions[self.instructions.len() - 1], vm::Instruction::Discard) {
177 +                     self.instructions.pop();
178 +                     continue
179 +                 }
    |

error[E0382]: use of moved value: `name`
   --> src/vm.rs:284:112
    |
281 |                     let name = self.program.names[index].clone();
    |                         ----   --------------------------------- this reinitialization might get skipped
    |                         |
    |                         move occurs because `name` has type `std::string::String`, which does not implement the `Copy` trait
...
284 |                         if let std::collections::hash_map::Entry::Occupied(mut e) = self.scopes[i].stuff.entry(name) {
    |                                                                                                                ^^^^ value moved here, in previous iteration of loop

error: aborting due to previous error; 2 warnings emitted

For more information about this error, try `rustc --explain E0382`.
Original diagnostics will follow.

warning: this `if` statement can be collapsed
   --> src/compiler.rs:175:13
    |
175 | /             if matches!(instruction, vm::Instruction::PushNothing) {
176 | |                 if self.instructions.len() > 0 {
177 | |                     if matches!(self.instructions[self.instructions.len() - 1], vm::Instruction::Discard) {
178 | |                         self.instructions.pop();
...   |
181 | |                 }
182 | |             }
    | |_____________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_if
    = note: `#[warn(clippy::collapsible_if)]` on by default
help: collapse nested if block
    |
175 ~             if matches!(instruction, vm::Instruction::PushNothing) && self.instructions.len() > 0 {
176 +                 if matches!(self.instructions[self.instructions.len() - 1], vm::Instruction::Discard) {
177 +                     self.instructions.pop();
178 +                     continue
179 +                 }
180 +             }
    |

warning: this `if` statement can be collapsed
   --> src/compiler.rs:176:17
    |
176 | /                 if self.instructions.len() > 0 {
177 | |                     if matches!(self.instructions[self.instructions.len() - 1], vm::Instruction::Discard) {
178 | |                         self.instructions.pop();
179 | |                         continue
180 | |                     }
181 | |                 }
    | |_________________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_if
help: collapse nested if block
    |
176 ~                 if self.instructions.len() > 0 && matches!(self.instructions[self.instructions.len() - 1], vm::Instruction::Discard) {
177 +                     self.instructions.pop();
178 +                     continue
179 +                 }
    |

warning: unused variable: `object`
   --> src/expressions.rs:201:26
    |
201 |         Expr::Property { object, name } => todo!(),
    |                          ^^^^^^ help: try ignoring the field: `object: _`
    |
    = note: `#[warn(unused_variables)]` on by default

warning: unused variable: `name`
   --> src/expressions.rs:201:34
    |
201 |         Expr::Property { object, name } => todo!(),
    |                                  ^^^^ help: try ignoring the field: `name: _`

warning: unused variable: `object`
   --> src/compiler.rs:289:26
    |
289 |         Expr::Property { object, name } => todo!(),
    |                          ^^^^^^ help: try ignoring the field: `object: _`

warning: unused variable: `name`
   --> src/compiler.rs:289:34
    |
289 |         Expr::Property { object, name } => todo!(),
    |                                  ^^^^ help: try ignoring the field: `name: _`

warning: unused variable: `with`
   --> src/compiler.rs:290:23
    |
290 |         Expr::Break { with } => todo!(),
    |                       ^^^^ help: try ignoring the field: `with: _`

warning: unused variable: `imports`
   --> src/compiler.rs:292:21
    |
292 |         Expr::Use { imports } => todo!(),
    |                     ^^^^^^^ help: try ignoring the field: `imports: _`

warning: this loop could be written as a `while let` loop
   --> src/parser/mod.rs:180:13
    |
180 | /             loop {
181 | |                 if let Some(Token { tt: TokenType::Ident(v), .. }) = peek!() {
182 | |                     next!();
183 | |                     arguments.push(v.clone());
...   |
191 | |                 }
192 | |             }
    | |_____________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#while_let_loop
    = note: `#[warn(clippy::while_let_loop)]` on by default
help: try
    |
180 ~             while let Some(Token { tt: TokenType::Ident(v), .. }) = if *pointer < input.len() {
181 +                 Some(&input[*pointer])
182 +             } else {
183 +                 None
184 +             } { .. }
    |

warning: usage of `contains_key` followed by `insert` on a `HashMap`
   --> src/vm.rs:284:25
    |
284 | /                         if self.scopes[i].stuff.contains_key(&name) {
285 | |                             self.scopes[i].stuff.insert(name, get!());
286 | |                             // found = true;
287 | |                             break
288 | |                         }
    | |_________________________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_entry
    = note: `#[warn(clippy::map_entry)]` on by default
help: try this
    |
284 ~                         if let std::collections::hash_map::Entry::Occupied(mut e) = self.scopes[i].stuff.entry(name) {
285 +                             e.insert(get!());
286 +                             // found = true;
287 +                             break
288 +                         }
    |

warning: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
   --> src/compiler.rs:106:17
    |
106 | /                 match instruction {
107 | |                     vm::Instruction::JumpTarget(n) => {
108 | |                         self.target_no += 1;
109 | |                         let new_n = self.target_no - 1;
...   |
112 | |                     _ => ()
113 | |                 }
    | |_________________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#single_match
    = note: `#[warn(clippy::single_match)]` on by default
help: try this
    |
106 ~                 if let vm::Instruction::JumpTarget(n) = instruction {
107 +                     self.target_no += 1;
108 +                     let new_n = self.target_no - 1;
109 +                     jump_map.insert(*n, new_n);
110 +                 }
    |

warning: length comparison to zero
   --> src/compiler.rs:173:15
    |
173 |         while self.instructions.len() > 0 {
    |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!self.instructions.is_empty()`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#len_zero
    = note: `#[warn(clippy::len_zero)]` on by default

warning: length comparison to zero
   --> src/compiler.rs:176:20
    |
176 |                 if self.instructions.len() > 0 {
    |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!self.instructions.is_empty()`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#len_zero

warning: this expression creates a reference which is immediately dereferenced by the compiler
   --> src/compiler.rs:274:19
    |
274 |             lower(&inside, builder);
    |                   ^^^^^^^ help: change this to: `inside`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
    = note: `#[warn(clippy::needless_borrow)]` on by default

warning: `eeeee` (lib) generated 14 warnings (run `cargo clippy --fix --lib -p eeeee` to apply 12 suggestions)
    Finished dev [unoptimized + debuginfo] target(s) in 9.92s
    ```

I'm not sure if there is extra information I should add...

### Version

```text
rustc 1.71.0 (8ede3aae2 2023-07-12)
binary: rustc
commit-hash: 8ede3aae28fe6e4d52b38157d7bfe0d3bceef225
commit-date: 2023-07-12
host: x86_64-apple-darwin
release: 1.71.0
LLVM version: 16.0.5

Additional Labels

No response

@y21
Copy link
Member

y21 commented Sep 26, 2023

The error here is caused by the map_entry lint. Reduced test case:

fn reduced(map: &mut HashMap<String, u8>, name: String) {
  if map.contains_key(&name) {
    map.insert(name, 42);
    return;
  }
  dbg!(name);
}
warning: usage of `contains_key` followed by `insert` on a `HashMap`
 --> src/main.rs:3:5
  |
3 | /     if map.contains_key(&name) {
4 | |         map.insert(name, 42);
5 | |         return;
6 | |     }
  | |_____^

help: try
  |
3 ~     if let std::collections::hash_map::Entry::Occupied(mut e) = map.entry(name) {
4 +         e.insert(42);
5 +         return;
6 +     }
  |

The suggestion doesn't compile because name is unconditionally moved into entry but also used later whereas in the original code it is only conditionally moved into insert and also returns in the same branch

Also related: #9925

Could probably fix this by only emitting a warning if the binding isn't used after the call to insert (edit: nevermind, this might not be enough because of loops) or if the key implements Copy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants