mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Add unused_variables native diagnostic

Browse Source
This commit is contained in:
hkalbasi 2023-09-24 21:29:15 +03:30
parent e5e937ae5e
commit 7834b8fadb
12 changed files with 282 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
crates/hir-ty/src/layout/tests/closure.rs
View File

@ -186,9 +186,9 @@ fn capture_specific_fields() {
fn match_pattern() {
size_and_align_expr! {
struct X(i64, i32, (u8, i128));
let y: X = X(2, 5, (7, 3));
let _y: X = X(2, 5, (7, 3));
move |x: i64| {
match y {
match _y {
_ => x,
}
}

8
crates/hir-ty/src/mir.rs
View File

@ -280,7 +280,7 @@ fn project(self, projection: PlaceElem, store: &mut ProjectionStore) -> Projecti
}
}
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub struct Place {
pub local: LocalId,
pub projection: ProjectionId,
@ -1007,7 +1007,7 @@ pub enum Rvalue {
#[derive(Debug, PartialEq, Eq, Clone)]
pub enum StatementKind {
Assign(Place, Rvalue),
//FakeRead(Box<(FakeReadCause, Place)>),
FakeRead(Place),
//SetDiscriminant {
// place: Box<Place>,
// variant_index: VariantIdx,
@ -1109,7 +1109,9 @@ fn for_operand(
}
}
}
StatementKind::Deinit(p) => f(p, &mut self.projection_store),
StatementKind::FakeRead(p) | StatementKind::Deinit(p) => {
f(p, &mut self.projection_store)
}
StatementKind::StorageLive(_)
| StatementKind::StorageDead(_)
| StatementKind::Nop => (),

88
crates/hir-ty/src/mir/borrowck.rs
View File

@ -24,6 +24,7 @@
pub enum MutabilityReason {
Mut { spans: Vec<MirSpan> },
Not,
Unused,
}
#[derive(Debug, Clone, PartialEq, Eq)]
@ -144,7 +145,8 @@ fn moved_out_of_ref(db: &dyn HirDatabase, body: &MirBody) -> Vec<MovedOutOfRef>
}
}
},
StatementKind::Deinit(_)
StatementKind::FakeRead(_)
| StatementKind::Deinit(_)
| StatementKind::StorageLive(_)
| StatementKind::StorageDead(_)
| StatementKind::Nop => (),
@ -264,7 +266,10 @@ fn dfs(
is_ever_initialized = false;
}
}
StatementKind::Deinit(_) | StatementKind::Nop | StatementKind::StorageLive(_) => (),
StatementKind::Deinit(_)
| StatementKind::FakeRead(_)
| StatementKind::Nop
| StatementKind::StorageLive(_) => (),
}
}
let Some(terminator) = &block.terminator else {
@ -331,16 +336,37 @@ fn dfs(
result
}
fn push_mut_span(local: LocalId, span: MirSpan, result: &mut ArenaMap<LocalId, MutabilityReason>) {
match &mut result[local] {
MutabilityReason::Mut { spans } => spans.push(span),
it @ (MutabilityReason::Not | MutabilityReason::Unused) => {
*it = MutabilityReason::Mut { spans: vec![span] }
}
};
}
fn record_usage(local: LocalId, result: &mut ArenaMap<LocalId, MutabilityReason>) {
match &mut result[local] {
it @ MutabilityReason::Unused => {
*it = MutabilityReason::Not;
}
_ => (),
};
}
fn record_usage_for_operand(arg: &Operand, result: &mut ArenaMap<LocalId, MutabilityReason>) {
if let Operand::Copy(p) | Operand::Move(p) = arg {
record_usage(p.local, result);
}
}
fn mutability_of_locals(
db: &dyn HirDatabase,
body: &MirBody,
) -> ArenaMap<LocalId, MutabilityReason> {
let mut result: ArenaMap<LocalId, MutabilityReason> =
body.locals.iter().map(|it| (it.0, MutabilityReason::Not)).collect();
let mut push_mut_span = |local, span| match &mut result[local] {
MutabilityReason::Mut { spans } => spans.push(span),
it @ MutabilityReason::Not => *it = MutabilityReason::Mut { spans: vec![span] },
};
body.locals.iter().map(|it| (it.0, MutabilityReason::Unused)).collect();
let ever_init_maps = ever_initialized_map(db, body);
for (block_id, mut ever_init_map) in ever_init_maps.into_iter() {
let block = &body.basic_blocks[block_id];
@ -350,23 +376,51 @@ fn mutability_of_locals(
match place_case(db, body, place) {
ProjectionCase::Direct => {
if ever_init_map.get(place.local).copied().unwrap_or_default() {
push_mut_span(place.local, statement.span);
push_mut_span(place.local, statement.span, &mut result);
} else {
ever_init_map.insert(place.local, true);
}
}
ProjectionCase::DirectPart => {
// Partial initialization is not supported, so it is definitely `mut`
push_mut_span(place.local, statement.span);
push_mut_span(place.local, statement.span, &mut result);
}
ProjectionCase::Indirect => (),
ProjectionCase::Indirect => {
record_usage(place.local, &mut result);
}
}
match value {
Rvalue::CopyForDeref(p)
| Rvalue::Discriminant(p)
| Rvalue::Len(p)
| Rvalue::Ref(_, p) => {
record_usage(p.local, &mut result);
}
Rvalue::Use(o)
| Rvalue::Repeat(o, _)
| Rvalue::Cast(_, o, _)
| Rvalue::UnaryOp(_, o) => record_usage_for_operand(o, &mut result),
Rvalue::CheckedBinaryOp(_, o1, o2) => {
for o in [o1, o2] {
record_usage_for_operand(o, &mut result);
}
}
Rvalue::Aggregate(_, args) => {
for arg in args.iter() {
record_usage_for_operand(arg, &mut result);
}
}
Rvalue::ShallowInitBox(_, _) | Rvalue::ShallowInitBoxWithAlloc(_) => (),
}
if let Rvalue::Ref(BorrowKind::Mut { .. }, p) = value {
if place_case(db, body, p) != ProjectionCase::Indirect {
push_mut_span(p.local, statement.span);
push_mut_span(p.local, statement.span, &mut result);
}
}
}
StatementKind::FakeRead(p) => {
record_usage(p.local, &mut result);
}
StatementKind::StorageDead(p) => {
ever_init_map.insert(*p, false);
}
@ -386,15 +440,21 @@ fn mutability_of_locals(
| TerminatorKind::FalseEdge { .. }
| TerminatorKind::FalseUnwind { .. }
| TerminatorKind::GeneratorDrop
| TerminatorKind::SwitchInt { .. }
| TerminatorKind::Drop { .. }
| TerminatorKind::DropAndReplace { .. }
| TerminatorKind::Assert { .. }
| TerminatorKind::Yield { .. } => (),
TerminatorKind::Call { destination, .. } => {
TerminatorKind::SwitchInt { discr, targets: _ } => {
record_usage_for_operand(discr, &mut result);
}
TerminatorKind::Call { destination, args, func, .. } => {
record_usage_for_operand(func, &mut result);
for arg in args.iter() {
record_usage_for_operand(arg, &mut result);
}
if destination.projection.lookup(&body.projection_store).len() == 0 {
if ever_init_map.get(destination.local).copied().unwrap_or_default() {
push_mut_span(destination.local, MirSpan::Unknown);
push_mut_span(destination.local, MirSpan::Unknown, &mut result);
} else {
ever_init_map.insert(destination.local, true);
}

1
crates/hir-ty/src/mir/eval.rs
View File

@ -842,6 +842,7 @@ fn interpret_mir(
}
StatementKind::Deinit(_) => not_supported!("de-init statement"),
StatementKind::StorageLive(_)
| StatementKind::FakeRead(_)
| StatementKind::StorageDead(_)
| StatementKind::Nop => (),
}

14
crates/hir-ty/src/mir/lower.rs
View File

@ -529,6 +529,7 @@ fn lower_expr_to_place_without_adjust(
else {
return Ok(None);
};
self.push_fake_read(current, cond_place, expr_id.into());
let (then_target, else_target) =
self.pattern_match(current, None, cond_place, *pat)?;
self.write_bytes_to_place(
@ -668,6 +669,7 @@ fn lower_expr_to_place_without_adjust(
else {
return Ok(None);
};
self.push_fake_read(current, cond_place, expr_id.into());
let mut end = None;
for MatchArm { pat, guard, expr } in arms.iter() {
let (then, mut otherwise) =
@ -1299,6 +1301,7 @@ fn lower_assignment(
return Ok(None);
};
if matches!(&self.body.exprs[lhs], Expr::Underscore) {
self.push_fake_read_for_operand(current, rhs_op, span);
return Ok(Some(current));
}
if matches!(
@ -1575,6 +1578,16 @@ fn push_statement(&mut self, block: BasicBlockId, statement: Statement) {
self.result.basic_blocks[block].statements.push(statement);
}
fn push_fake_read(&mut self, block: BasicBlockId, p: Place, span: MirSpan) {
self.push_statement(block, StatementKind::FakeRead(p).with_span(span));
}
fn push_fake_read_for_operand(&mut self, block: BasicBlockId, operand: Operand, span: MirSpan) {
if let Operand::Move(p) | Operand::Copy(p) = operand {
self.push_fake_read(block, p, span);
}
}
fn push_assignment(
&mut self,
block: BasicBlockId,
@ -1733,6 +1746,7 @@ fn lower_block_to_place(
return Ok(None);
};
current = c;
self.push_fake_read(current, init_place, span);
(current, else_block) =
self.pattern_match(current, None, init_place, *pat)?;
match (else_block, else_branch) {

1
crates/hir-ty/src/mir/monomorphization.rs
View File

@ -248,6 +248,7 @@ fn fill_body(&mut self, body: &mut MirBody) -> Result<(), MirLowerError> {
| Rvalue::CopyForDeref(_) => (),
},
StatementKind::Deinit(_)
| StatementKind::FakeRead(_)
| StatementKind::StorageLive(_)
| StatementKind::StorageDead(_)
| StatementKind::Nop => (),

5
crates/hir-ty/src/mir/pretty.rs
View File

@ -233,6 +233,11 @@ fn blocks(&mut self) {
this.place(p);
wln!(this, ");");
}
StatementKind::FakeRead(p) => {
w!(this, "FakeRead(");
this.place(p);
wln!(this, ");");
}
StatementKind::Nop => wln!(this, "Nop;"),
}
}

6
crates/hir/src/diagnostics.rs
View File

@ -66,6 +66,7 @@ fn from(d: $diag) -> AnyDiagnostic {
UnresolvedModule,
UnresolvedProcMacro,
UnusedMut,
UnusedVariable,
];
#[derive(Debug)]
@ -270,6 +271,11 @@ pub struct UnusedMut {
pub local: Local,
}
#[derive(Debug)]
pub struct UnusedVariable {
pub local: Local,
}
#[derive(Debug)]
pub struct MovedOutOfRef {
pub ty: Type,

15
crates/hir/src/lib.rs
View File

@ -98,7 +98,7 @@
ReplaceFilterMapNextWithFindMap, TypeMismatch, TypedHole, UndeclaredLabel,
UnimplementedBuiltinMacro, UnreachableLabel, UnresolvedExternCrate, UnresolvedField,
UnresolvedImport, UnresolvedMacroCall, UnresolvedMethodCall, UnresolvedModule,
UnresolvedProcMacro, UnusedMut,
UnresolvedProcMacro, UnusedMut, UnusedVariable,
},
has_source::HasSource,
semantics::{PathResolution, Semantics, SemanticsScope, TypeInfo, VisibleTraits},
@ -1697,9 +1697,20 @@ pub fn diagnostics(self, db: &dyn HirDatabase, acc: &mut Vec<AnyDiagnostic>) {
// Skip synthetic bindings
continue;
}
let need_mut = &mol[local];
let mut need_mut = &mol[local];
if body[binding_id].name.as_str() == Some("self")
&& need_mut == &mir::MutabilityReason::Unused
{
need_mut = &mir::MutabilityReason::Not;
}
let local = Local { parent: self.into(), binding_id };
match (need_mut, local.is_mut(db)) {
(mir::MutabilityReason::Unused, _) => {
let should_ignore = matches!(body[binding_id].name.as_str(), Some(it) if it.starts_with("_"));
if !should_ignore {
acc.push(UnusedVariable { local }.into())
}
}
(mir::MutabilityReason::Mut { .. }, true)
| (mir::MutabilityReason::Not, false) => (),
(mir::MutabilityReason::Mut { spans }, false) => {

66
crates/ide-diagnostics/src/handlers/mutability_errors.rs
View File

@ -324,6 +324,7 @@ fn main() {
let x_own = 2;
let ref mut x_ref = x_own;
//^^^^^^^^^^^^^ 💡 error: cannot mutate immutable variable `x_own`
_ = x_ref;
}
"#,
);
@ -331,7 +332,7 @@ fn main() {
r#"
struct Foo;
impl Foo {
fn method(&mut self, x: i32) {}
fn method(&mut self, _x: i32) {}
}
fn main() {
let x = Foo;
@ -391,6 +392,7 @@ fn main() {
//^^^^^ 💡 warn: variable does not need to be mutable
x = 7;
//^^^^^ 💡 error: cannot mutate immutable variable `x`
_ = y;
}
}
}
@ -404,12 +406,14 @@ fn mutation_in_dead_code() {
// there would be no mutability error for locals in dead code. Rustc tries to
// not emit `unused_mut` in this case, but since it works without `mut`, and
// special casing it is not trivial, we emit it.
// Update: now MIR based `unused-variable` is taking over `unused-mut` for the same reason.
check_diagnostics(
r#"
fn main() {
return;
let mut x = 2;
//^^^^^ 💡 warn: variable does not need to be mutable
//^^^^^ warn: unused variable
&mut x;
}
"#,
@ -419,7 +423,7 @@ fn main() {
fn main() {
loop {}
let mut x = 2;
//^^^^^ 💡 warn: variable does not need to be mutable
//^^^^^ warn: unused variable
&mut x;
}
"#,
@ -440,7 +444,7 @@ fn main(b: bool) {
g();
}
let mut x = 2;
//^^^^^ 💡 warn: variable does not need to be mutable
//^^^^^ warn: unused variable
&mut x;
}
"#,
@ -454,7 +458,7 @@ fn main(b: bool) {
return;
}
let mut x = 2;
//^^^^^ 💡 warn: variable does not need to be mutable
//^^^^^ warn: unused variable
&mut x;
}
"#,
@ -536,6 +540,7 @@ fn main() {
(k @ 5, ref mut t) if { continue; } => {
//^^^^^^^^^ 💡 error: cannot mutate immutable variable `z`
*t = 5;
_ = k;
}
_ => {
let y = (1, 2);
@ -588,6 +593,7 @@ fn main() {
b = 1;
c = (2, 3);
d = 3;
_ = (c, b, d);
}
}
"#,
@ -600,6 +606,7 @@ fn function_arguments_are_initialized() {
r#"
fn f(mut x: i32) {
//^^^^^ 💡 warn: variable does not need to be mutable
f(x + 2);
}
"#,
);
@ -615,8 +622,11 @@ fn f(x: i32) {
r#"
fn f((x, y): (i32, i32)) {
let t = [0; 2];
x = 5;
//^^^^^ 💡 error: cannot mutate immutable variable `x`
x = 5;
//^^^^^ 💡 error: cannot mutate immutable variable `x`
_ = x;
_ = y;
_ = t;
}
"#,
);
@ -645,6 +655,7 @@ fn for_loop() {
//^^^^^ 💡 warn: variable does not need to be mutable
a = 2;
//^^^^^ 💡 error: cannot mutate immutable variable `a`
_ = b;
}
}
"#,
@ -666,6 +677,7 @@ fn while_let() {
//^^^^^ 💡 error: cannot mutate immutable variable `a`
c = 2;
//^^^^^ 💡 error: cannot mutate immutable variable `c`
_ = (b, d);
}
}
}
@ -696,18 +708,18 @@ fn f() {
fn overloaded_index() {
check_diagnostics(
r#"
//- minicore: index
//- minicore: index, copy
use core::ops::{Index, IndexMut};
struct Foo;
impl Index<usize> for Foo {
type Output = (i32, u8);
fn index(&self, index: usize) -> &(i32, u8) {
fn index(&self, _index: usize) -> &(i32, u8) {
&(5, 2)
}
}
impl IndexMut<usize> for Foo {
fn index_mut(&mut self, index: usize) -> &mut (i32, u8) {
fn index_mut(&mut self, _index: usize) -> &mut (i32, u8) {
&mut (5, 2)
}
}
@ -715,26 +727,32 @@ fn f() {
let mut x = Foo;
//^^^^^ 💡 warn: variable does not need to be mutable
let y = &x[2];
_ = (x, y);
let x = Foo;
let y = &mut x[2];
//^💡 error: cannot mutate immutable variable `x`
_ = (x, y);
let mut x = &mut Foo;
//^^^^^ 💡 warn: variable does not need to be mutable
let y: &mut (i32, u8) = &mut x[2];
_ = (x, y);
let x = Foo;
let ref mut y = x[7];
//^ 💡 error: cannot mutate immutable variable `x`
_ = (x, y);
let (ref mut y, _) = x[3];
//^ 💡 error: cannot mutate immutable variable `x`
_ = y;
match x[10] {
//^ 💡 error: cannot mutate immutable variable `x`
(ref y, _) => (),
(_, ref mut y) => (),
(ref y, 5) => _ = y,
(_, ref mut y) => _ = y,
}
let mut x = Foo;
let mut i = 5;
//^^^^^ 💡 warn: variable does not need to be mutable
let y = &mut x[i];
_ = y;
}
"#,
);
@ -744,7 +762,7 @@ fn f() {
fn overloaded_deref() {
check_diagnostics(
r#"
//- minicore: deref_mut
//- minicore: deref_mut, copy
use core::ops::{Deref, DerefMut};
struct Foo;
@ -763,21 +781,27 @@ fn f() {
let mut x = Foo;
//^^^^^ 💡 warn: variable does not need to be mutable
let y = &*x;
_ = (x, y);
let x = Foo;
let y = &mut *x;
//^^ 💡 error: cannot mutate immutable variable `x`
_ = (x, y);
let x = Foo;
//^ warn: unused variable
let x = Foo;
let y: &mut (i32, u8) = &mut x;
//^^^^^^ 💡 error: cannot mutate immutable variable `x`
_ = (x, y);
let ref mut y = *x;
//^^ 💡 error: cannot mutate immutable variable `x`
_ = y;
let (ref mut y, _) = *x;
//^^ 💡 error: cannot mutate immutable variable `x`
_ = y;
match *x {
//^^ 💡 error: cannot mutate immutable variable `x`
(ref y, _) => (),
(_, ref mut y) => (),
(ref y, 5) => _ = y,
(_, ref mut y) => _ = y,
}
}
"#,
@ -866,6 +890,7 @@ pub fn test() {
data: 0
}
);
_ = tree;
}
"#,
);
@ -925,6 +950,7 @@ fn f() {
let x = X;
let closure4 = || { x.mutate(); };
//^ 💡 error: cannot mutate immutable variable `x`
_ = (closure2, closure3, closure4);
}
"#,
);
@ -941,7 +967,9 @@ fn f() {
z = 3;
let mut k = z;
//^^^^^ 💡 warn: variable does not need to be mutable
_ = k;
};
_ = (x, closure);
}
"#,
);
@ -958,6 +986,7 @@ fn f() {
}
}
};
_ = closure;
}
"#,
);
@ -972,7 +1001,8 @@ fn f() {
let mut x = X;
let c2 = || { x = X; x };
let mut x = X;
let c2 = move || { x = X; };
let c3 = move || { x = X; };
_ = (c1, c2, c3);
}
"#,
);
@ -1023,7 +1053,7 @@ fn x(t: &[u8]) {
a = 2;
//^^^^^ 💡 error: cannot mutate immutable variable `a`
_ = b;
}
_ => {}
}
@ -1079,6 +1109,7 @@ fn f() {
let x = Box::new(5);
let closure = || *x = 2;
//^ 💡 error: cannot mutate immutable variable `x`
_ = closure;
}
"#,
);
@ -1156,6 +1187,7 @@ macro_rules! mac {
fn main2() {
let mut x = mac![];
//^^^^^ 💡 warn: variable does not need to be mutable
_ = x;
}
"#,
);

110
crates/ide-diagnostics/src/handlers/unused_variables.rs Normal file
View File

@ -0,0 +1,110 @@
use crate::{Diagnostic, DiagnosticCode, DiagnosticsContext};
// Diagnostic: unused-variables
//
// This diagnostic is triggered when a local variable is not used.
pub(crate) fn unused_variables(
ctx: &DiagnosticsContext<'_>,
d: &hir::UnusedVariable,
) -> Diagnostic {
let ast = d.local.primary_source(ctx.sema.db).syntax_ptr();
Diagnostic::new_with_syntax_node_ptr(
ctx,
DiagnosticCode::RustcLint("unused_variables"),
"unused variable",
ast,
)
}
#[cfg(test)]
mod tests {
use crate::tests::check_diagnostics;
#[test]
fn unused_variables_simple() {
check_diagnostics(
r#"
//- minicore: fn
struct Foo { f1: i32, f2: i64 }
fn f(kkk: i32) {}
//^^^ warn: unused variable
fn main() {
let a = 2;
//^ warn: unused variable
let b = 5;
// note: `unused variable` implies `unused mut`, so we should not emit both at the same time.
let mut c = f(b);
//^^^^^ warn: unused variable
let (d, e) = (3, 5);
//^ warn: unused variable
let _ = e;
let f1 = 2;
let f2 = 5;
let f = Foo { f1, f2 };
match f {
Foo { f1, f2 } => {
//^^ warn: unused variable
_ = f2;
}
}
let g = false;
if g {}
let h: fn() -> i32 = || 2;
let i = h();
//^ warn: unused variable
}
"#,
);
}
#[test]
fn unused_self() {
check_diagnostics(
r#"
struct S {
}
impl S {
fn owned_self(self, u: i32) {}
//^ warn: unused variable
fn ref_self(&self, u: i32) {}
//^ warn: unused variable
fn ref_mut_self(&mut self, u: i32) {}
//^ warn: unused variable
fn owned_mut_self(mut self) {}
//^^^^^^^^ 💡 warn: variable does not need to be mutable
}
"#,
);
}
#[test]
fn allow_unused_variables_for_identifiers_starting_with_underline() {
check_diagnostics(
r#"
fn main() {
let _x = 2;
}
"#,
);
}
#[test]
fn respect_lint_attributes_for_unused_variables() {
check_diagnostics(
r#"
fn main() {
#[allow(unused_variables)]
let x = 2;
}
#[deny(unused)]
fn main2() {
let x = 2;
//^ error: unused variable
}
"#,
);
}
}

2
crates/ide-diagnostics/src/lib.rs
View File

@ -56,6 +56,7 @@ mod handlers {
pub(crate) mod unresolved_proc_macro;
pub(crate) mod undeclared_label;
pub(crate) mod unreachable_label;
pub(crate) mod unused_variables;
// The handlers below are unusual, the implement the diagnostics as well.
pub(crate) mod field_shorthand;
@ -368,6 +369,7 @@ pub fn diagnostics(
AnyDiagnostic::UnresolvedModule(d) => handlers::unresolved_module::unresolved_module(&ctx, &d),
AnyDiagnostic::UnresolvedProcMacro(d) => handlers::unresolved_proc_macro::unresolved_proc_macro(&ctx, &d, config.proc_macros_enabled, config.proc_attr_macros_enabled),
AnyDiagnostic::UnusedMut(d) => handlers::mutability_errors::unused_mut(&ctx, &d),
AnyDiagnostic::UnusedVariable(d) => handlers::unused_variables::unused_variables(&ctx, &d),
AnyDiagnostic::BreakOutsideOfLoop(d) => handlers::break_outside_of_loop::break_outside_of_loop(&ctx, &d),
AnyDiagnostic::MismatchedTupleStructPatArgCount(d) => handlers::mismatched_arg_count::mismatched_tuple_struct_pat_arg_count(&ctx, &d),
};