Skip to content

Commit

Permalink
Rollup merge of #131999 - jieyouxu:unit-bindings, r=WaffleLapkin
Browse files Browse the repository at this point in the history
Improve test coverage for `unit_bindings` lint

Follow-up to #112380, apparently at the time I didn't add much of any test coverage outside of just "generally works as intended on the test suites and in the crater run".

r? compiler
  • Loading branch information
matthiaskrgr authored Oct 21, 2024
2 parents d0c8d3e + 70ce711 commit 4aa07a7
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 0 deletions.
40 changes: 40 additions & 0 deletions tests/ui/lint/unit_bindings.deny_level.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
error: binding has unit type `()`
--> $DIR/unit_bindings.rs:50:5
|
LL | let _ = expr;
| ^^^^-^^^^^^^^
| |
| this pattern is inferred to be the unit type `()`
|
note: the lint level is defined here
--> $DIR/unit_bindings.rs:22:30
|
LL | #![cfg_attr(deny_level, deny(unit_bindings))]
| ^^^^^^^^^^^^^

error: binding has unit type `()`
--> $DIR/unit_bindings.rs:51:5
|
LL | let pat = expr;
| ^^^^---^^^^^^^^
| |
| this pattern is inferred to be the unit type `()`

error: binding has unit type `()`
--> $DIR/unit_bindings.rs:52:5
|
LL | let _pat = expr;
| ^^^^----^^^^^^^^
| |
| this pattern is inferred to be the unit type `()`

error: binding has unit type `()`
--> $DIR/unit_bindings.rs:55:5
|
LL | let list = v.sort();
| ^^^^----^^^^^^^^^^^^
| |
| this pattern is inferred to be the unit type `()`

error: aborting due to 4 previous errors

60 changes: 60 additions & 0 deletions tests/ui/lint/unit_bindings.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
//! Basic checks for `unit_bindings` lint.
//!
//! The `unit_bindings` lint tries to detect cases like `let list = list.sort()`. The lint will
//! trigger on bindings that have the unit `()` type **except** if:
//!
//! - The user wrote `()` on either side, i.e.
//! - `let () = <expr>;` or `let <expr> = ();`
//! - `let _ = ();`
//! - The binding occurs within macro expansions, e.g. `foo!();`.
//! - The user explicitly provided type annotations, e.g. `let x: () = <expr>`.
//!
//! Examples where the lint *should* fire on include:
//!
//! - `let _ = <expr>;`
//! - `let pat = <expr>;`
//! - `let _pat = <expr>;`
//@ revisions: default_level deny_level
//@[default_level] check-pass (`unit_bindings` is currently allow-by-default)

#![allow(unused)]
#![cfg_attr(deny_level, deny(unit_bindings))]

// The `list` binding below should trigger the lint if it's not contained in a macro expansion.
macro_rules! expands_to_sus {
() => {
let mut v = [1, 2, 3];
let list = v.sort();
}
}

// No warning for `y` and `z` because it is provided as type parameter.
fn ty_param_check<T: Copy>(x: T) {
let y = x;
let z: T = x;
}

fn main() {
// No warning if user explicitly wrote `()` on either side.
let expr = ();
let () = expr;
let _ = ();
// No warning if user explicitly annotates the unit type on the binding.
let pat: () = expr;
// No warning for let bindings with unit type in macro expansions.
expands_to_sus!();
// No warning for unit bindings in generic fns.
ty_param_check(());

let _ = expr; //[deny_level]~ ERROR binding has unit type
let pat = expr; //[deny_level]~ ERROR binding has unit type
let _pat = expr; //[deny_level]~ ERROR binding has unit type

let mut v = [1, 2, 3];
let list = v.sort(); //[deny_level]~ ERROR binding has unit type

// Limitation: the lint currently does not fire on nested unit LHS bindings, i.e.
// this will not currently trigger the lint.
let (nested, _) = (expr, 0i32);
}

0 comments on commit 4aa07a7

Please sign in to comment.