feat: diagnostics for uninitialized variable access

crates/mun_hir/src/diagnostics.rs

@@ -432,3 +432,27 @@ impl Diagnostic for NoSuchField {
         self
     }
 }
+
+#[derive(Debug)]
+pub struct PossiblyUninitializedVariable {
+    pub file: FileId,
+    pub pat: SyntaxNodePtr,
+}
+
+impl Diagnostic for PossiblyUninitializedVariable {
+    fn message(&self) -> String {
+        "use of possibly-uninitialized variable".to_string()
+    }
+
+    fn file(&self) -> FileId {
+        self.file
+    }
+
+    fn syntax_node_ptr(&self) -> SyntaxNodePtr {
+        self.pat
+    }
+
+    fn as_any(&self) -> &(dyn Any + Send + 'static) {
+        self
+    }
+}

crates/mun_hir/src/expr/validator.rs

@@ -3,6 +3,9 @@ use std::sync::Arc;
 
 mod uninitialized_access;
 
+#[cfg(test)]
+mod tests;
+
 pub struct ExprValidator<'a, 'b: 'a, 'd, DB: HirDatabase> {
     func: Function,
     infer: Arc<InferenceResult>,

crates/mun_hir/src/expr/validator/snapshots/tests__uninitialized_access.snap

@@ -0,0 +1,6 @@
+---
+source: crates/mun_hir/src/expr/validator/tests.rs
+expression: "fn foo() {\n    let a:int;\n    let b = a + 3;\n}"
+---
+[38; 39): use of possibly-uninitialized variable
+

crates/mun_hir/src/expr/validator/snapshots/tests__uninitialized_access_if.snap

@@ -0,0 +1,6 @@
+---
+source: crates/mun_hir/src/expr/validator/tests.rs
+expression: "fn foo() {\n    let a:int;\n    if true { a = 3; } else { a = 4; }\n    let b = a + 4;  // correct, `a` is initialized either way\n}\n\nfn bar() {\n    let a:int;\n    if true { a = 3; }\n    let b = a + 4;  // `a` is possibly-unitialized\n}\n\nfn baz() {\n    let a:int;\n    if true { return } else { a = 4 };\n    let b = a + 4;  // correct, `a` is initialized either way\n}\n\nfn foz() {\n    let a:int;\n    if true { a = 4 } else { return };\n    let b = a + 4;  // correct, `a` is initialized either way\n}\n\nfn boz() {\n    let a:int;\n    return;\n    let b = a + 4;  // `a` is not initialized but this is dead code anyway\n}"
+---
+[191; 192): use of possibly-uninitialized variable
+

crates/mun_hir/src/expr/validator/snapshots/tests__uninitialized_access_while.snap

@@ -0,0 +1,6 @@
+---
+source: crates/mun_hir/src/expr/validator/tests.rs
+expression: "fn foo(b:int) {\n    let a:int;\n    while b < 4 { b += 1; a = b; a += 1; }\n    let c = a + 4;  // `a` is possibly-unitialized\n}"
+---
+[86; 87): use of possibly-uninitialized variable
+

crates/mun_hir/src/expr/validator/tests.rs

@@ -0,0 +1,95 @@
+use crate::db::SourceDatabase;
+use crate::expr::validator::ExprValidator;
+use crate::{diagnostics::DiagnosticSink, ids::LocationCtx, mock::MockDatabase, Function};
+use mun_syntax::{ast, AstNode};
+use std::fmt::Write;
+
+#[test]
+fn test_uninitialized_access() {
+    diagnostics_snapshot(
+        r#"
+    fn foo() {
+        let a:int;
+        let b = a + 3;
+    }
+    "#,
+    )
+}
+
+#[test]
+fn test_uninitialized_access_if() {
+    diagnostics_snapshot(
+        r#"
+    fn foo() {
+        let a:int;
+        if true { a = 3; } else { a = 4; }
+        let b = a + 4;  // correct, `a` is initialized either way
+    }
+
+    fn bar() {
+        let a:int;
+        if true { a = 3; }
+        let b = a + 4;  // `a` is possibly-unitialized
+    }
+
+    fn baz() {
+        let a:int;
+        if true { return } else { a = 4 };
+        let b = a + 4;  // correct, `a` is initialized either way
+    }
+
+    fn foz() {
+        let a:int;
+        if true { a = 4 } else { return };
+        let b = a + 4;  // correct, `a` is initialized either way
+    }
+
+    fn boz() {
+        let a:int;
+        return;
+        let b = a + 4;  // `a` is not initialized but this is dead code anyway
+    }
+    "#,
+    )
+}
+
+#[test]
+fn test_uninitialized_access_while() {
+    diagnostics_snapshot(
+        r#"
+    fn foo(b:int) {
+        let a:int;
+        while b < 4 { b += 1; a = b; a += 1; }
+        let c = a + 4;  // `a` is possibly-unitialized
+    }
+    "#,
+    )
+}
+
+fn diagnostics(content: &str) -> String {
+    let (db, file_id) = MockDatabase::with_single_file(content);
+    let source_file = db.parse(file_id).ok().unwrap();
+
+    let mut diags = String::new();
+
+    let mut diag_sink = DiagnosticSink::new(|diag| {
+        write!(diags, "{}: {}\n", diag.highlight_range(), diag.message()).unwrap();
+    });
+
+    let ctx = LocationCtx::new(&db, file_id);
+    for node in source_file.syntax().descendants() {
+        if let Some(def) = ast::FunctionDef::cast(node.clone()) {
+            let fun = Function {
+                id: ctx.to_def(&def),
+            };
+            ExprValidator::new(fun, &db, &mut diag_sink).validate_body();
+        }
+    }
+    drop(diag_sink);
+    diags
+}
+
+fn diagnostics_snapshot(text: &str) {
+    let text = text.trim().replace("\n    ", "\n");
+    insta::assert_snapshot!(insta::_macro_support::AutoName, diagnostics(&text), &text);
+}

crates/mun_hir/src/expr/validator/uninitialized_access.rs

@@ -1,57 +1,212 @@
 use super::ExprValidator;
-use crate::HirDatabase;
-//use crate::{HirDatabase, ExprId, PatId, Expr, Path};
-//use std::collections::HashSet;
+use crate::diagnostics::PossiblyUninitializedVariable;
+use crate::{BinaryOp, Expr, ExprId, HirDatabase, PatId, Path, Resolution, Resolver, Statement};
+use std::collections::HashSet;
 
-//enum ExprSide {
-//    left,
-//    Expr
-//}
+#[derive(Copy, Clone, PartialEq, Eq)]
+enum ExprKind {
+    Normal,
+    Place,
+    Both,
+}
 
 impl<'a, 'b, 'd, D: HirDatabase> ExprValidator<'a, 'b, 'd, D> {
     /// Validates that all binding access has previously been initialized.
     pub(super) fn validate_uninitialized_access(&mut self) {
-        //        let mut initialized_patterns = HashSet::new();
-        //
-        //        // Add all parameter patterns to the set of initialized patterns (they must have been
-        //        // initialized)
-        //        for (pat, _) in self.body.params.iter() {
-        //            initialized_patterns.insert(*pat)
-        //        }
-        //
-        //        self.validate_expr_access(&mut initialized_patterns, self.body.body_expr);
+        let mut initialized_patterns = HashSet::new();
+
+        // Add all parameter patterns to the set of initialized patterns (they must have been
+        // initialized)
+        for (pat, _) in self.body.params.iter() {
+            initialized_patterns.insert(*pat);
+        }
+
+        self.validate_expr_access(
+            &mut initialized_patterns,
+            self.body.body_expr,
+            ExprKind::Normal,
+        );
+    }
+
+    /// Validates that the specified expr does not access unitialized bindings
+    fn validate_expr_access(
+        &mut self,
+        initialized_patterns: &mut HashSet<PatId>,
+        expr: ExprId,
+        expr_side: ExprKind,
+    ) {
+        let body = self.body.clone();
+        match &body[expr] {
+            Expr::Call { callee, args } => {
+                self.validate_expr_access(initialized_patterns, *callee, expr_side);
+                for arg in args.iter() {
+                    self.validate_expr_access(initialized_patterns, *arg, expr_side);
+                }
+            }
+            Expr::Path(p) => {
+                let resolver = crate::expr::resolver_for_expr(self.body.clone(), self.db, expr);
+                self.validate_path_access(initialized_patterns, &resolver, p, expr, expr_side);
+            }
+            Expr::If {
+                condition,
+                then_branch,
+                else_branch,
+            } => {
+                self.validate_expr_access(initialized_patterns, *condition, ExprKind::Normal);
+                let mut then_branch_initialized_patterns = initialized_patterns.clone();
+                self.validate_expr_access(
+                    &mut then_branch_initialized_patterns,
+                    *then_branch,
+                    ExprKind::Normal,
+                );
+                if let Some(else_branch) = else_branch {
+                    let mut else_branch_initialized_patterns = initialized_patterns.clone();
+                    self.validate_expr_access(
+                        &mut else_branch_initialized_patterns,
+                        *else_branch,
+                        ExprKind::Normal,
+                    );
+                    let then_is_never = self.infer[*then_branch].is_never();
+                    let else_is_never = self.infer[*else_branch].is_never();
+                    match (then_is_never, else_is_never) {
+                        (false, false) => {
+                            initialized_patterns.extend(
+                                then_branch_initialized_patterns
+                                    .intersection(&else_branch_initialized_patterns),
+                            );
+                        }
+                        (true, false) => {
+                            initialized_patterns.extend(else_branch_initialized_patterns);
+                        }
+                        (false, true) => {
+                            initialized_patterns.extend(then_branch_initialized_patterns);
+                        }
+                        (true, true) => {}
+                    };
+                }
+            }
+            Expr::UnaryOp { expr, .. } => {
+                self.validate_expr_access(initialized_patterns, *expr, ExprKind::Normal);
+            }
+            Expr::BinaryOp { lhs, rhs, op } => {
+                let lhs_expr_kind = match op {
+                    Some(BinaryOp::Assignment { op: Some(_) }) => ExprKind::Both,
+                    Some(BinaryOp::Assignment { op: None }) => ExprKind::Place,
+                    _ => ExprKind::Normal,
+                };
+                self.validate_expr_access(initialized_patterns, *lhs, lhs_expr_kind);
+                self.validate_expr_access(initialized_patterns, *rhs, ExprKind::Normal)
+            }
+            Expr::Block { statements, tail } => {
+                for statement in statements.iter() {
+                    match statement {
+                        Statement::Let {
+                            pat, initializer, ..
+                        } => {
+                            if let Some(initializer) = initializer {
+                                self.validate_expr_access(
+                                    initialized_patterns,
+                                    *initializer,
+                                    ExprKind::Normal,
+                                );
+                                initialized_patterns.insert(*pat);
+                            }
+                        }
+                        Statement::Expr(expr) => {
+                            self.validate_expr_access(
+                                initialized_patterns,
+                                *expr,
+                                ExprKind::Normal,
+                            );
+                            if self.infer[*expr].is_never() {
+                                return;
+                            }
+                        }
+                    }
+                }
+                if let Some(tail) = tail {
+                    self.validate_expr_access(initialized_patterns, *tail, ExprKind::Normal)
+                }
+            }
+            Expr::Return { expr } => {
+                if let Some(expr) = expr {
+                    self.validate_expr_access(initialized_patterns, *expr, ExprKind::Normal)
+                }
+            }
+            Expr::Break { expr } => {
+                if let Some(expr) = expr {
+                    self.validate_expr_access(initialized_patterns, *expr, ExprKind::Normal)
+                }
+            }
+            Expr::Loop { body } => {
+                self.validate_expr_access(initialized_patterns, *body, ExprKind::Normal)
+            }
+            Expr::While { condition, body } => {
+                self.validate_expr_access(initialized_patterns, *condition, ExprKind::Normal);
+                self.validate_expr_access(
+                    &mut initialized_patterns.clone(),
+                    *body,
+                    ExprKind::Normal,
+                );
+            }
+            Expr::RecordLit { fields, spread, .. } => {
+                for field in fields.iter() {
+                    self.validate_expr_access(initialized_patterns, field.expr, ExprKind::Normal);
+                }
+                if let Some(expr) = spread {
+                    self.validate_expr_access(initialized_patterns, *expr, ExprKind::Normal);
+                }
+            }
+            Expr::Field { expr, .. } => {
+                self.validate_expr_access(initialized_patterns, *expr, ExprKind::Normal);
+            }
+            Expr::Literal(_) => {}
+            Expr::Missing => {}
+        }
     }
 
-    //    /// Validates that the specified expr does not access unitialized bindings
-    //    fn validate_expr_access(&mut self, initialized_patterns: &mut HashSet<PatId>, expr: ExprId, exprSide:ExprSide) {
-    //        let body = self.body.clone();
-    //        match &body[expr] {
-    //            Expr::Call { callee, args } => {
-    //                self.validate_expr_access(initialized_patterns, *callee);
-    //                for arg in args.iter() {
-    //                    self.validate_expr_access(initialized_patterns, *callee);
-    //                }
-    //            },
-    //            Expr::Path(p) => {
-    //                let resolver = expr::resolver_for_expr(self.body.clone(), self.db, tgt_expr);
-    //                self.validate_path_access(initialized_patterns, &resolver, p);
-    //            }
-    //            Expr::If { .. } => {},
-    //            Expr::UnaryOp { .. } => {},
-    //            Expr::BinaryOp { .. } => {},
-    //            Expr::Block { .. } => {},
-    //            Expr::Return { .. } => {},
-    //            Expr::Break { .. } => {},
-    //            Expr::Loop { .. } => {},
-    //            Expr::While { .. } => {},
-    //            Expr::RecordLit { .. } => {},
-    //            Expr::Field { .. } => {},
-    //            Expr::Literal(_) => {},
-    //            Expr::Missing => {},
-    //        }
-    //    }
-    //
-    //    fn validate_expr_access(&mut self, initialized_patterns: &mut HashSet<PatId>, p: &Path) {
-    //
-    //    }
+    fn validate_path_access(
+        &mut self,
+        initialized_patterns: &mut HashSet<PatId>,
+        resolver: &Resolver,
+        path: &Path,
+        expr: ExprId,
+        expr_side: ExprKind,
+    ) {
+        let resolution = match resolver
+            .resolve_path_without_assoc_items(self.db, path)
+            .take_values()
+        {
+            Some(resolution) => resolution,
+            None => {
+                // This has already been caught by the inferencing step
+                return;
+            }
+        };
+
+        let pat = match resolution {
+            Resolution::LocalBinding(pat) => pat,
+            Resolution::Def(_def) => return,
+        };
+
+        if expr_side == ExprKind::Normal || expr_side == ExprKind::Both {
+            // Check if the binding has already been initialized
+            if initialized_patterns.get(&pat).is_none() {
+                let (_, body_source_map) = self.db.body_with_source_map(self.func.into());
+                self.sink.push(PossiblyUninitializedVariable {
+                    file: self.func.module(self.db).file_id(),
+                    pat: body_source_map
+                        .expr_syntax(expr)
+                        .unwrap()
+                        .ast
+                        .either(|it| it.syntax_node_ptr(), |it| it.syntax_node_ptr()),
+                })
+            }
+        }
+
+        if expr_side == ExprKind::Place {
+            // The binding should be initialized
+            initialized_patterns.insert(pat);
+        }
+    }
 }