Diagnose private fields in record constructor

Author: Veykril

crates/hir-def/src/data/adt.rs

@@ -447,6 +447,7 @@ impl VariantData {
         }
     }
 
+    // FIXME: Linear lookup
     pub fn field(&self, name: &Name) -> Option<LocalFieldId> {
         self.fields().iter().find_map(|(id, data)| if &data.name == name { Some(id) } else { None })
     }

crates/hir-ty/src/infer.rs

@@ -195,6 +195,7 @@ pub(crate) type InferResult<T> = Result<InferOk<T>, TypeError>;
 pub enum InferenceDiagnostic {
     NoSuchField {
         expr: ExprId,
+        private: bool,
     },
     PrivateField {
         expr: ExprId,

crates/hir-ty/src/infer/expr.rs

@@ -514,9 +514,6 @@ impl InferenceContext<'_> {
             }
             Expr::RecordLit { path, fields, spread, .. } => {
                 let (ty, def_id) = self.resolve_variant(path.as_deref(), false);
-                if let Some(variant) = def_id {
-                    self.write_variant_resolution(tgt_expr.into(), variant);
-                }
 
                 if let Some(t) = expected.only_has_type(&mut self.table) {
                     self.unify(&ty, &t);
@@ -526,26 +523,56 @@ impl InferenceContext<'_> {
                     .as_adt()
                     .map(|(_, s)| s.clone())
                     .unwrap_or_else(|| Substitution::empty(Interner));
-                let field_types = def_id.map(|it| self.db.field_types(it)).unwrap_or_default();
-                let variant_data = def_id.map(|it| it.variant_data(self.db.upcast()));
-                for field in fields.iter() {
-                    let field_def =
-                        variant_data.as_ref().and_then(|it| match it.field(&field.name) {
-                            Some(local_id) => Some(FieldId { parent: def_id.unwrap(), local_id }),
-                            None => {
-                                self.push_diagnostic(InferenceDiagnostic::NoSuchField {
-                                    expr: field.expr,
-                                });
-                                None
-                            }
-                        });
-                    let field_ty = field_def.map_or(self.err_ty(), |it| {
-                        field_types[it.local_id].clone().substitute(Interner, &substs)
-                    });
-                    // Field type might have some unknown types
-                    // FIXME: we may want to emit a single type variable for all instance of type fields?
-                    let field_ty = self.insert_type_vars(field_ty);
-                    self.infer_expr_coerce(field.expr, &Expectation::has_type(field_ty));
+                if let Some(variant) = def_id {
+                    self.write_variant_resolution(tgt_expr.into(), variant);
+                }
+                match def_id {
+                    Some(_) if fields.is_empty() => {}
+                    Some(def) => {
+                        let field_types = self.db.field_types(def);
+                        let variant_data = def.variant_data(self.db.upcast());
+                        let visibilities = self.db.field_visibilities(def);
+                        for field in fields.iter() {
+                            let field_def = {
+                                match variant_data.field(&field.name) {
+                                    Some(local_id) => {
+                                        if !visibilities[local_id].is_visible_from(
+                                            self.db.upcast(),
+                                            self.resolver.module(),
+                                        ) {
+                                            self.push_diagnostic(
+                                                InferenceDiagnostic::NoSuchField {
+                                                    expr: field.expr,
+                                                    private: true,
+                                                },
+                                            );
+                                        }
+                                        Some(FieldId { parent: def, local_id })
+                                    }
+                                    None => {
+                                        self.push_diagnostic(InferenceDiagnostic::NoSuchField {
+                                            expr: field.expr,
+                                            private: false,
+                                        });
+                                        None
+                                    }
+                                }
+                            };
+                            let field_ty = field_def.map_or(self.err_ty(), |it| {
+                                field_types[it.local_id].clone().substitute(Interner, &substs)
+                            });
+
+                            // Field type might have some unknown types
+                            // FIXME: we may want to emit a single type variable for all instance of type fields?
+                            let field_ty = self.insert_type_vars(field_ty);
+                            self.infer_expr_coerce(field.expr, &Expectation::has_type(field_ty));
+                        }
+                    }
+                    None => {
+                        for field in fields.iter() {
+                            self.infer_expr_coerce(field.expr, &Expectation::None);
+                        }
+                    }
                 }
                 if let Some(expr) = spread {
                     self.infer_expr(*expr, &Expectation::has_type(ty.clone()));

crates/hir/src/diagnostics.rs

@@ -174,6 +174,7 @@ pub struct MalformedDerive {
 #[derive(Debug)]
 pub struct NoSuchField {
     pub field: InFile<AstPtr<ast::RecordExprField>>,
+    pub private: bool,
 }
 
 #[derive(Debug)]

crates/hir/src/lib.rs

@@ -1505,9 +1505,9 @@ impl DefWithBody {
         let expr_syntax = |expr| source_map.expr_syntax(expr).expect("unexpected synthetic");
         for d in &infer.diagnostics {
             match d {
-                &hir_ty::InferenceDiagnostic::NoSuchField { expr } => {
+                &hir_ty::InferenceDiagnostic::NoSuchField { expr, private } => {
                     let field = source_map.field_syntax(expr);
-                    acc.push(NoSuchField { field }.into())
+                    acc.push(NoSuchField { field, private }.into())
                 }
                 &hir_ty::InferenceDiagnostic::MismatchedArgCount { call_expr, expected, found } => {
                     acc.push(

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

@@ -14,14 +14,22 @@ use crate::{fix, Assist, Diagnostic, DiagnosticCode, DiagnosticsContext};
 pub(crate) fn no_such_field(ctx: &DiagnosticsContext<'_>, d: &hir::NoSuchField) -> Diagnostic {
     Diagnostic::new_with_syntax_node_ptr(
         ctx,
-        DiagnosticCode::RustcHardError("E0559"),
-        "no such field",
+        if d.private {
+            DiagnosticCode::RustcHardError("E0451")
+        } else {
+            DiagnosticCode::RustcHardError("E0559")
+        },
+        if d.private { "field is private" } else { "no such field" },
         d.field.clone().map(|it| it.into()),
     )
     .with_fixes(fixes(ctx, d))
 }
 
 fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::NoSuchField) -> Option<Vec<Assist>> {
+    if d.private {
+        // FIXME: quickfix to add required visibility
+        return None;
+    }
     let root = ctx.sema.db.parse_or_expand(d.field.file_id);
     missing_record_expr_field_fixes(
         &ctx.sema,
@@ -292,6 +300,25 @@ fn main() {
         0$0: 0
     }
 }
+"#,
+        )
+    }
+
+    #[test]
+    fn test_struct_field_private() {
+        check_diagnostics(
+            r#"
+mod m {
+    pub struct Struct {
+        field: u32
+    }
+}
+fn main() {
+    m::Struct {
+        field: 0
+      //^^^^^^^^ error: field is private
+    };
+}
 "#,
         )
     }