7 files changed, 138 insertions, 104 deletions

diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs
index 977c0525b..7ab7f79db 100644
--- a/crates/ra_hir_ty/src/diagnostics.rs
+++ b/crates/ra_hir_ty/src/diagnostics.rs
@@ -6,10 +6,10 @@ mod unsafe_check;
 use std::any::Any;
 
 use hir_def::DefWithBodyId;
-use hir_expand::diagnostics::{AstDiagnostic, Diagnostic, DiagnosticSink};
-use hir_expand::{db::AstDatabase, name::Name, HirFileId, InFile};
+use hir_expand::diagnostics::{Diagnostic, DiagnosticSink};
+use hir_expand::{name::Name, HirFileId, InFile};
 use ra_prof::profile;
-use ra_syntax::{ast, AstNode, AstPtr, SyntaxNodePtr};
+use ra_syntax::{ast, AstPtr, SyntaxNodePtr};
 use stdx::format_to;
 
 use crate::db::HirDatabase;
@@ -37,7 +37,7 @@ impl Diagnostic for NoSuchField {
         "no such field".to_string()
     }
 
-    fn source(&self) -> InFile<SyntaxNodePtr> {
+    fn display_source(&self) -> InFile<SyntaxNodePtr> {
         InFile::new(self.file, self.field.clone().into())
     }
 
@@ -46,20 +46,11 @@ impl Diagnostic for NoSuchField {
     }
 }
 
-impl AstDiagnostic for NoSuchField {
-    type AST = ast::RecordExprField;
-
-    fn ast(&self, db: &dyn AstDatabase) -> Self::AST {
-        let root = db.parse_or_expand(self.source().file_id).unwrap();
-        let node = self.source().value.to_node(&root);
-        ast::RecordExprField::cast(node).unwrap()
-    }
-}
-
 #[derive(Debug)]
 pub struct MissingFields {
     pub file: HirFileId,
-    pub field_list: AstPtr<ast::RecordExprFieldList>,
+    pub field_list_parent: AstPtr<ast::RecordExpr>,
+    pub field_list_parent_path: Option<AstPtr<ast::Path>>,
     pub missed_fields: Vec<Name>,
 }
 
@@ -71,28 +62,28 @@ impl Diagnostic for MissingFields {
         }
         buf
     }
-    fn source(&self) -> InFile<SyntaxNodePtr> {
-        InFile { file_id: self.file, value: self.field_list.clone().into() }
+
+    fn display_source(&self) -> InFile<SyntaxNodePtr> {
+        InFile {
+            file_id: self.file,
+            value: self
+                .field_list_parent_path
+                .clone()
+                .map(SyntaxNodePtr::from)
+                .unwrap_or_else(|| self.field_list_parent.clone().into()),
+        }
     }
+
     fn as_any(&self) -> &(dyn Any + Send + 'static) {
         self
     }
 }
 
-impl AstDiagnostic for MissingFields {
-    type AST = ast::RecordExprFieldList;
-
-    fn ast(&self, db: &dyn AstDatabase) -> Self::AST {
-        let root = db.parse_or_expand(self.source().file_id).unwrap();
-        let node = self.source().value.to_node(&root);
-        ast::RecordExprFieldList::cast(node).unwrap()
-    }
-}
-
 #[derive(Debug)]
 pub struct MissingPatFields {
     pub file: HirFileId,
-    pub field_list: AstPtr<ast::RecordPatFieldList>,
+    pub field_list_parent: AstPtr<ast::RecordPat>,
+    pub field_list_parent_path: Option<AstPtr<ast::Path>>,
     pub missed_fields: Vec<Name>,
 }
 
@@ -104,8 +95,15 @@ impl Diagnostic for MissingPatFields {
         }
         buf
     }
-    fn source(&self) -> InFile<SyntaxNodePtr> {
-        InFile { file_id: self.file, value: self.field_list.clone().into() }
+    fn display_source(&self) -> InFile<SyntaxNodePtr> {
+        InFile {
+            file_id: self.file,
+            value: self
+                .field_list_parent_path
+                .clone()
+                .map(SyntaxNodePtr::from)
+                .unwrap_or_else(|| self.field_list_parent.clone().into()),
+        }
     }
     fn as_any(&self) -> &(dyn Any + Send + 'static) {
         self
@@ -123,7 +121,7 @@ impl Diagnostic for MissingMatchArms {
     fn message(&self) -> String {
         String::from("Missing match arm")
     }
-    fn source(&self) -> InFile<SyntaxNodePtr> {
+    fn display_source(&self) -> InFile<SyntaxNodePtr> {
         InFile { file_id: self.file, value: self.match_expr.clone().into() }
     }
     fn as_any(&self) -> &(dyn Any + Send + 'static) {
@@ -141,7 +139,7 @@ impl Diagnostic for MissingOkInTailExpr {
     fn message(&self) -> String {
         "wrap return expression in Ok".to_string()
     }
-    fn source(&self) -> InFile<SyntaxNodePtr> {
+    fn display_source(&self) -> InFile<SyntaxNodePtr> {
         InFile { file_id: self.file, value: self.expr.clone().into() }
     }
     fn as_any(&self) -> &(dyn Any + Send + 'static) {
@@ -149,16 +147,6 @@ impl Diagnostic for MissingOkInTailExpr {
     }
 }
 
-impl AstDiagnostic for MissingOkInTailExpr {
-    type AST = ast::Expr;
-
-    fn ast(&self, db: &dyn AstDatabase) -> Self::AST {
-        let root = db.parse_or_expand(self.file).unwrap();
-        let node = self.source().value.to_node(&root);
-        ast::Expr::cast(node).unwrap()
-    }
-}
-
 #[derive(Debug)]
 pub struct BreakOutsideOfLoop {
     pub file: HirFileId,
@@ -169,7 +157,7 @@ impl Diagnostic for BreakOutsideOfLoop {
     fn message(&self) -> String {
         "break outside of loop".to_string()
     }
-    fn source(&self) -> InFile<SyntaxNodePtr> {
+    fn display_source(&self) -> InFile<SyntaxNodePtr> {
         InFile { file_id: self.file, value: self.expr.clone().into() }
     }
     fn as_any(&self) -> &(dyn Any + Send + 'static) {
@@ -177,16 +165,6 @@ impl Diagnostic for BreakOutsideOfLoop {
     }
 }
 
-impl AstDiagnostic for BreakOutsideOfLoop {
-    type AST = ast::Expr;
-
-    fn ast(&self, db: &dyn AstDatabase) -> Self::AST {
-        let root = db.parse_or_expand(self.file).unwrap();
-        let node = self.source().value.to_node(&root);
-        ast::Expr::cast(node).unwrap()
-    }
-}
-
 #[derive(Debug)]
 pub struct MissingUnsafe {
     pub file: HirFileId,
@@ -197,7 +175,7 @@ impl Diagnostic for MissingUnsafe {
     fn message(&self) -> String {
         format!("This operation is unsafe and requires an unsafe function or block")
     }
-    fn source(&self) -> InFile<SyntaxNodePtr> {
+    fn display_source(&self) -> InFile<SyntaxNodePtr> {
         InFile { file_id: self.file, value: self.expr.clone().into() }
     }
     fn as_any(&self) -> &(dyn Any + Send + 'static) {
@@ -205,16 +183,6 @@ impl Diagnostic for MissingUnsafe {
     }
 }
 
-impl AstDiagnostic for MissingUnsafe {
-    type AST = ast::Expr;
-
-    fn ast(&self, db: &dyn AstDatabase) -> Self::AST {
-        let root = db.parse_or_expand(self.source().file_id).unwrap();
-        let node = self.source().value.to_node(&root);
-        ast::Expr::cast(node).unwrap()
-    }
-}
-
 #[derive(Debug)]
 pub struct MismatchedArgCount {
     pub file: HirFileId,
@@ -228,7 +196,7 @@ impl Diagnostic for MismatchedArgCount {
         let s = if self.expected == 1 { "" } else { "s" };
         format!("Expected {} argument{}, found {}", self.expected, s, self.found)
     }
-    fn source(&self) -> InFile<SyntaxNodePtr> {
+    fn display_source(&self) -> InFile<SyntaxNodePtr> {
         InFile { file_id: self.file, value: self.call_expr.clone().into() }
     }
     fn as_any(&self) -> &(dyn Any + Send + 'static) {
@@ -239,19 +207,13 @@ impl Diagnostic for MismatchedArgCount {
     }
 }
 
-impl AstDiagnostic for MismatchedArgCount {
-    type AST = ast::CallExpr;
-    fn ast(&self, db: &dyn AstDatabase) -> Self::AST {
-        let root = db.parse_or_expand(self.source().file_id).unwrap();
-        let node = self.source().value.to_node(&root);
-        ast::CallExpr::cast(node).unwrap()
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use hir_def::{db::DefDatabase, AssocItemId, ModuleDefId};
-    use hir_expand::diagnostics::{Diagnostic, DiagnosticSinkBuilder};
+    use hir_expand::{
+        db::AstDatabase,
+        diagnostics::{Diagnostic, DiagnosticSinkBuilder},
+    };
     use ra_db::{fixture::WithFixture, FileId, SourceDatabase, SourceDatabaseExt};
     use ra_syntax::{TextRange, TextSize};
     use rustc_hash::FxHashMap;
@@ -296,9 +258,11 @@ mod tests {
 
         let mut actual: FxHashMap<FileId, Vec<(TextRange, String)>> = FxHashMap::default();
         db.diagnostics(|d| {
-            // FXIME: macros...
-            let file_id = d.source().file_id.original_file(&db);
-            let range = d.syntax_node(&db).text_range();
+            let src = d.display_source();
+            let root = db.parse_or_expand(src.file_id).unwrap();
+            // FIXME: macros...
+            let file_id = src.file_id.original_file(&db);
+            let range = src.value.to_node(&root).text_range();
             let message = d.message().to_owned();
             actual.entry(file_id).or_default().push((range, message));
         });
@@ -326,8 +290,8 @@ struct S { foo: i32, bar: () }
 impl S {
     fn new() -> S {
         S {
-        //^... Missing structure fields:
-        //|    - bar
+      //^ Missing structure fields:
+      //|    - bar
             foo: 92,
             baz: 62,
           //^^^^^^^ no such field
@@ -448,8 +412,8 @@ impl Foo {
 struct S { foo: i32, bar: () }
 fn baz(s: S) {
     let S { foo: _ } = s;
-        //^^^^^^^^^^ Missing structure fields:
-        //         | - bar
+      //^ Missing structure fields:
+      //| - bar
 }
 "#,
         );
diff --git a/crates/ra_hir_ty/src/diagnostics/expr.rs b/crates/ra_hir_ty/src/diagnostics/expr.rs
index 95bbf2d95..51adcecaf 100644
--- a/crates/ra_hir_ty/src/diagnostics/expr.rs
+++ b/crates/ra_hir_ty/src/diagnostics/expr.rs
@@ -100,8 +100,8 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
 
         if let Ok(source_ptr) = source_map.expr_syntax(id) {
             let root = source_ptr.file_syntax(db.upcast());
-            if let ast::Expr::RecordExpr(record_lit) = &source_ptr.value.to_node(&root) {
-                if let Some(field_list) = record_lit.record_expr_field_list() {
+            if let ast::Expr::RecordExpr(record_expr) = &source_ptr.value.to_node(&root) {
+                if let Some(_) = record_expr.record_expr_field_list() {
                     let variant_data = variant_data(db.upcast(), variant_def);
                     let missed_fields = missed_fields
                         .into_iter()
@@ -109,7 +109,8 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
                         .collect();
                     self.sink.push(MissingFields {
                         file: source_ptr.file_id,
-                        field_list: AstPtr::new(&field_list),
+                        field_list_parent: AstPtr::new(&record_expr),
+                        field_list_parent_path: record_expr.path().map(|path| AstPtr::new(&path)),
                         missed_fields,
                     })
                 }
@@ -131,7 +132,7 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
             if let Some(expr) = source_ptr.value.as_ref().left() {
                 let root = source_ptr.file_syntax(db.upcast());
                 if let ast::Pat::RecordPat(record_pat) = expr.to_node(&root) {
-                    if let Some(field_list) = record_pat.record_pat_field_list() {
+                    if let Some(_) = record_pat.record_pat_field_list() {
                         let variant_data = variant_data(db.upcast(), variant_def);
                         let missed_fields = missed_fields
                             .into_iter()
@@ -139,7 +140,10 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
                             .collect();
                         self.sink.push(MissingPatFields {
                             file: source_ptr.file_id,
-                            field_list: AstPtr::new(&field_list),
+                            field_list_parent: AstPtr::new(&record_pat),
+                            field_list_parent_path: record_pat
+                                .path()
+                                .map(|path| AstPtr::new(&path)),
                             missed_fields,
                         })
                     }
diff --git a/crates/ra_hir_ty/src/diagnostics/match_check.rs b/crates/ra_hir_ty/src/diagnostics/match_check.rs
index 507edcb7d..deca244db 100644
--- a/crates/ra_hir_ty/src/diagnostics/match_check.rs
+++ b/crates/ra_hir_ty/src/diagnostics/match_check.rs
@@ -1161,15 +1161,15 @@ fn main() {
         //^ Missing match arm
     match a {
         Either::A { } => (),
-                //^^^ Missing structure fields:
-                //  | - foo
+      //^^^^^^^^^ Missing structure fields:
+      //        | - foo
         Either::B => (),
     }
     match a {
         //^ Missing match arm
         Either::A { } => (),
-    }           //^^^ Missing structure fields:
-                //  | - foo
+    } //^^^^^^^^^ Missing structure fields:
+      //        | - foo
 
     match a {
         Either::A { foo: true } => (),
diff --git a/crates/ra_hir_ty/src/diagnostics/unsafe_check.rs b/crates/ra_hir_ty/src/diagnostics/unsafe_check.rs
index 5cc76bdce..61ffbf5d1 100644
--- a/crates/ra_hir_ty/src/diagnostics/unsafe_check.rs
+++ b/crates/ra_hir_ty/src/diagnostics/unsafe_check.rs
@@ -6,6 +6,7 @@ use std::sync::Arc;
 use hir_def::{
     body::Body,
     expr::{Expr, ExprId, UnaryOp},
+    resolver::{resolver_for_expr, ResolveValueResult, ValueNs},
     DefWithBodyId,
 };
 use hir_expand::diagnostics::DiagnosticSink;
@@ -70,7 +71,7 @@ pub fn unsafe_expressions(
 ) -> Vec<UnsafeExpr> {
     let mut unsafe_exprs = vec![];
     let body = db.body(def);
-    walk_unsafe(&mut unsafe_exprs, db, infer, &body, body.body_expr, false);
+    walk_unsafe(&mut unsafe_exprs, db, infer, def, &body, body.body_expr, false);
 
     unsafe_exprs
 }
@@ -79,6 +80,7 @@ fn walk_unsafe(
     unsafe_exprs: &mut Vec<UnsafeExpr>,
     db: &dyn HirDatabase,
     infer: &InferenceResult,
+    def: DefWithBodyId,
     body: &Body,
     current: ExprId,
     inside_unsafe_block: bool,
@@ -97,6 +99,15 @@ fn walk_unsafe(
                 }
             }
         }
+        Expr::Path(path) => {
+            let resolver = resolver_for_expr(db.upcast(), def, current);
+            let value_or_partial = resolver.resolve_path_in_value_ns(db.upcast(), path.mod_path());
+            if let Some(ResolveValueResult::ValueNs(ValueNs::StaticId(id))) = value_or_partial {
+                if db.static_data(id).mutable {
+                    unsafe_exprs.push(UnsafeExpr { expr: current, inside_unsafe_block });
+                }
+            }
+        }
         Expr::MethodCall { .. } => {
             if infer
                 .method_resolution(current)
@@ -112,13 +123,13 @@ fn walk_unsafe(
             }
         }
         Expr::Unsafe { body: child } => {
-            return walk_unsafe(unsafe_exprs, db, infer, body, *child, true);
+            return walk_unsafe(unsafe_exprs, db, infer, def, body, *child, true);
         }
         _ => {}
     }
 
     expr.walk_child_exprs(|child| {
-        walk_unsafe(unsafe_exprs, db, infer, body, child, inside_unsafe_block);
+        walk_unsafe(unsafe_exprs, db, infer, def, body, child, inside_unsafe_block);
     });
 }
 
@@ -170,4 +181,25 @@ fn main() {
 "#,
         );
     }
+
+    #[test]
+    fn missing_unsafe_diagnostic_with_static_mut() {
+        check_diagnostics(
+            r#"
+struct Ty {
+    a: u8,
+}
+
+static mut static_mut: Ty = Ty { a: 0 };
+
+fn main() {
+    let x = static_mut.a;
+          //^^^^^^^^^^ This operation is unsafe and requires an unsafe function or block
+    unsafe {
+        let x = static_mut.a;
+    }
+}
+"#,
+        );
+    }
 }
diff --git a/crates/ra_hir_ty/src/infer.rs b/crates/ra_hir_ty/src/infer.rs
index 28f32a0a4..3d12039a6 100644
--- a/crates/ra_hir_ty/src/infer.rs
+++ b/crates/ra_hir_ty/src/infer.rs
@@ -440,6 +440,12 @@ impl<'a> InferenceContext<'a> {
                 let ty = self.insert_type_vars(ty.subst(&substs));
                 forbid_unresolved_segments((ty, Some(strukt.into())), unresolved)
             }
+            TypeNs::AdtId(AdtId::UnionId(u)) => {
+                let substs = Ty::substs_from_path(&ctx, path, u.into(), true);
+                let ty = self.db.ty(u.into());
+                let ty = self.insert_type_vars(ty.subst(&substs));
+                forbid_unresolved_segments((ty, Some(u.into())), unresolved)
+            }
             TypeNs::EnumVariantId(var) => {
                 let substs = Ty::substs_from_path(&ctx, path, var.into(), true);
                 let ty = self.db.ty(var.parent.into());
@@ -490,10 +496,7 @@ impl<'a> InferenceContext<'a> {
                 // FIXME potentially resolve assoc type
                 (Ty::Unknown, None)
             }
-            TypeNs::AdtId(AdtId::EnumId(_))
-            | TypeNs::AdtId(AdtId::UnionId(_))
-            | TypeNs::BuiltinType(_)
-            | TypeNs::TraitId(_) => {
+            TypeNs::AdtId(AdtId::EnumId(_)) | TypeNs::BuiltinType(_) | TypeNs::TraitId(_) => {
                 // FIXME diagnostic
                 (Ty::Unknown, None)
             }
diff --git a/crates/ra_hir_ty/src/lower.rs b/crates/ra_hir_ty/src/lower.rs
index 1eacc6f95..7638f167b 100644
--- a/crates/ra_hir_ty/src/lower.rs
+++ b/crates/ra_hir_ty/src/lower.rs
@@ -518,6 +518,7 @@ impl Ty {
         let (segment, generic_def) = match resolved {
             ValueTyDefId::FunctionId(it) => (last, Some(it.into())),
             ValueTyDefId::StructId(it) => (last, Some(it.into())),
+            ValueTyDefId::UnionId(it) => (last, Some(it.into())),
             ValueTyDefId::ConstId(it) => (last, Some(it.into())),
             ValueTyDefId::StaticId(_) => (last, None),
             ValueTyDefId::EnumVariantId(var) => {
@@ -1148,11 +1149,12 @@ impl_from!(BuiltinType, AdtId(StructId, EnumId, UnionId), TypeAliasId for TyDefI
 pub enum ValueTyDefId {
     FunctionId(FunctionId),
     StructId(StructId),
+    UnionId(UnionId),
     EnumVariantId(EnumVariantId),
     ConstId(ConstId),
     StaticId(StaticId),
 }
-impl_from!(FunctionId, StructId, EnumVariantId, ConstId, StaticId for ValueTyDefId);
+impl_from!(FunctionId, StructId, UnionId, EnumVariantId, ConstId, StaticId for ValueTyDefId);
 
 /// Build the declared type of an item. This depends on the namespace; e.g. for
 /// `struct Foo(usize)`, we have two types: The type of the struct itself, and
@@ -1179,6 +1181,7 @@ pub(crate) fn value_ty_query(db: &dyn HirDatabase, def: ValueTyDefId) -> Binders
     match def {
         ValueTyDefId::FunctionId(it) => type_for_fn(db, it),
         ValueTyDefId::StructId(it) => type_for_struct_constructor(db, it),
+        ValueTyDefId::UnionId(it) => type_for_adt(db, it.into()),
         ValueTyDefId::EnumVariantId(it) => type_for_enum_variant_constructor(db, it),
         ValueTyDefId::ConstId(it) => type_for_const(db, it),
         ValueTyDefId::StaticId(it) => type_for_static(db, it),
diff --git a/crates/ra_hir_ty/src/tests/simple.rs b/crates/ra_hir_ty/src/tests/simple.rs
index 3fd7d5cd4..5a7cf9455 100644
--- a/crates/ra_hir_ty/src/tests/simple.rs
+++ b/crates/ra_hir_ty/src/tests/simple.rs
@@ -334,16 +334,44 @@ fn infer_union() {
             bar: f32,
         }
 
+        fn test() {
+            let u = MyUnion { foo: 0 };
+            unsafe { baz(u); }
+            let u = MyUnion { bar: 0.0 };
+            unsafe { baz(u); }
+        }
+
         unsafe fn baz(u: MyUnion) {
             let inner = u.foo;
+            let inner = u.bar;
         }
         "#,
         expect![[r#"
-            61..62 'u': MyUnion
-            73..99 '{     ...foo; }': ()
-            83..88 'inner': u32
-            91..92 'u': MyUnion
-            91..96 'u.foo': u32
+            57..172 '{     ...); } }': ()
+            67..68 'u': MyUnion
+            71..89 'MyUnio...o: 0 }': MyUnion
+            86..87 '0': u32
+            95..113 'unsafe...(u); }': ()
+            102..113 '{ baz(u); }': ()
+            104..107 'baz': fn baz(MyUnion)
+            104..110 'baz(u)': ()
+            108..109 'u': MyUnion
+            122..123 'u': MyUnion
+            126..146 'MyUnio... 0.0 }': MyUnion
+            141..144 '0.0': f32
+            152..170 'unsafe...(u); }': ()
+            159..170 '{ baz(u); }': ()
+            161..164 'baz': fn baz(MyUnion)
+            161..167 'baz(u)': ()
+            165..166 'u': MyUnion
+            188..189 'u': MyUnion
+            200..249 '{     ...bar; }': ()
+            210..215 'inner': u32
+            218..219 'u': MyUnion
+            218..223 'u.foo': u32
+            233..238 'inner': f32
+            241..242 'u': MyUnion
+            241..246 'u.bar': f32
         "#]],
     );
 }