Auto merge of #15659 - HKalbasi:unused-var, r=HKalbasi

Add `unused_variables` native diagnostic

Author: bors

crates/hir-ty/src/layout/tests/closure.rs

@@ -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,
             }
         }

crates/hir-ty/src/mir.rs

@@ -280,7 +280,7 @@ impl ProjectionId {
     }
 }
 
-#[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 @@ impl MirBody {
                             }
                         }
                     }
-                    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 => (),

crates/hir-ty/src/mir/borrowck.rs

@@ -24,6 +24,7 @@ use super::{
 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 ever_initialized_map(
                         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 ever_initialized_map(
     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 => {
+                            record_usage(place.local, &mut result);
                         }
-                        ProjectionCase::Indirect => (),
+                    }
+                    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);
                     }

crates/hir-ty/src/mir/eval.rs

@@ -842,6 +842,7 @@ impl Evaluator<'_> {
                             }
                             StatementKind::Deinit(_) => not_supported!("de-init statement"),
                             StatementKind::StorageLive(_)
+                            | StatementKind::FakeRead(_)
                             | StatementKind::StorageDead(_)
                             | StatementKind::Nop => (),
                         }

crates/hir-ty/src/mir/lower.rs

@@ -529,6 +529,7 @@ impl<'ctx> MirLowerCtx<'ctx> {
                 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 @@ impl<'ctx> MirLowerCtx<'ctx> {
                 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 @@ impl<'ctx> MirLowerCtx<'ctx> {
             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 @@ impl<'ctx> MirLowerCtx<'ctx> {
         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 @@ impl<'ctx> MirLowerCtx<'ctx> {
                             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) {
@@ -1760,13 +1774,14 @@ impl<'ctx> MirLowerCtx<'ctx> {
                         }
                     }
                 }
-                hir_def::hir::Statement::Expr { expr, has_semi: _ } => {
+                &hir_def::hir::Statement::Expr { expr, has_semi: _ } => {
                     let scope2 = self.push_drop_scope();
-                    let Some((_, c)) = self.lower_expr_as_place(current, *expr, true)? else {
+                    let Some((p, c)) = self.lower_expr_as_place(current, expr, true)? else {
                         scope2.pop_assume_dropped(self);
                         scope.pop_assume_dropped(self);
                         return Ok(None);
                     };
+                    self.push_fake_read(c, p, expr.into());
                     current = scope2.pop_and_drop(self, c);
                 }
             }

crates/hir-ty/src/mir/monomorphization.rs

@@ -248,6 +248,7 @@ impl Filler<'_> {
                         | Rvalue::CopyForDeref(_) => (),
                     },
                     StatementKind::Deinit(_)
+                    | StatementKind::FakeRead(_)
                     | StatementKind::StorageLive(_)
                     | StatementKind::StorageDead(_)
                     | StatementKind::Nop => (),

crates/hir-ty/src/mir/pretty.rs

@@ -233,6 +233,11 @@ impl<'a> MirPrettyCtx<'a> {
                             this.place(p);
                             wln!(this, ");");
                         }
+                        StatementKind::FakeRead(p) => {
+                            w!(this, "FakeRead(");
+                            this.place(p);
+                            wln!(this, ");");
+                        }
                         StatementKind::Nop => wln!(this, "Nop;"),
                     }
                 }

crates/hir/src/diagnostics.rs

@@ -66,6 +66,7 @@ diagnostics![
     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,

crates/hir/src/lib.rs

@@ -98,7 +98,7 @@ pub use crate::{
         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 @@ impl DefWithBody {
                         // 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) => {

crates/ide-diagnostics/src/handlers/field_shorthand.rs

@@ -166,7 +166,7 @@ fn main() {
         check_diagnostics(
             r#"
 struct A { a: &'static str }
-fn f(a: A) { let A { a: hello } = a; }
+fn f(a: A) { let A { a: _hello } = a; }
 "#,
         );
         check_diagnostics(
@@ -181,12 +181,14 @@ fn f(a: A) { let A { 0: 0 } = a; }
 struct A { a: &'static str }
 fn f(a: A) {
     let A { a$0: a } = a;
+    _ = a;
 }
 "#,
             r#"
 struct A { a: &'static str }
 fn f(a: A) {
     let A { a } = a;
+    _ = a;
 }
 "#,
         );
@@ -196,12 +198,14 @@ fn f(a: A) {
 struct A { a: &'static str, b: &'static str }
 fn f(a: A) {
     let A { a$0: a, b } = a;
+    _ = (a, b);
 }
 "#,
             r#"
 struct A { a: &'static str, b: &'static str }
 fn f(a: A) {
     let A { a, b } = a;
+    _ = (a, b);
 }
 "#,
         );