Merge #1163

1163: fill struct fields diagnostic r=matklad a=pasa

implementation of #1095

Co-authored-by: Sergey Parilin <[email protected]>

11 files changed, 269 insertions, 246 deletions

diff --git a/crates/ra_assists/src/ast_editor.rs b/crates/ra_assists/src/ast_editor.rs
index 6854294ae..726e5c0a3 100644
--- a/crates/ra_assists/src/ast_editor.rs
+++ b/crates/ra_assists/src/ast_editor.rs
@@ -4,6 +4,7 @@ use arrayvec::ArrayVec;
 use ra_text_edit::TextEditBuilder;
 use ra_syntax::{AstNode, TreeArc, ast, SyntaxKind::*, SyntaxElement, SourceFile, InsertPosition, Direction};
 use ra_fmt::leading_indent;
+use hir::Name;
 
 pub struct AstEditor<N: AstNode> {
     original_ast: TreeArc<N>,
@@ -235,6 +236,10 @@ pub struct AstBuilder<N: AstNode> {
 }
 
 impl AstBuilder<ast::NamedField> {
+    pub fn from_name(name: &Name) -> TreeArc<ast::NamedField> {
+        ast_node_from_file_text(&format!("fn f() {{ S {{ {}: (), }} }}", name))
+    }
+
     fn from_text(text: &str) -> TreeArc<ast::NamedField> {
         ast_node_from_file_text(&format!("fn f() {{ S {{ {}, }} }}", text))
     }
diff --git a/crates/ra_assists/src/fill_struct_fields.rs b/crates/ra_assists/src/fill_struct_fields.rs
deleted file mode 100644
index 54b70e17d..000000000
--- a/crates/ra_assists/src/fill_struct_fields.rs
+++ /dev/null
@@ -1,226 +0,0 @@
-use hir::{AdtDef, db::HirDatabase};
-
-use ra_syntax::ast::{self, AstNode};
-
-use crate::{AssistCtx, Assist, AssistId, ast_editor::{AstEditor, AstBuilder}};
-
-pub(crate) fn fill_struct_fields(mut ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
-    let struct_lit = ctx.node_at_offset::<ast::StructLit>()?;
-    let named_field_list = struct_lit.named_field_list()?;
-
-    // Collect all fields from struct definition
-    let mut fields = {
-        let analyzer =
-            hir::SourceAnalyzer::new(ctx.db, ctx.frange.file_id, struct_lit.syntax(), None);
-        let struct_lit_ty = analyzer.type_of(ctx.db, struct_lit.into())?;
-        let struct_def = match struct_lit_ty.as_adt() {
-            Some((AdtDef::Struct(s), _)) => s,
-            _ => return None,
-        };
-        struct_def.fields(ctx.db)
-    };
-
-    // Filter out existing fields
-    for ast_field in named_field_list.fields() {
-        let name_from_ast = ast_field.name_ref()?.text().to_string();
-        fields.retain(|field| field.name(ctx.db).to_string() != name_from_ast);
-    }
-    if fields.is_empty() {
-        return None;
-    }
-
-    let db = ctx.db;
-    ctx.add_action(AssistId("fill_struct_fields"), "fill struct fields", |edit| {
-        let mut ast_editor = AstEditor::new(named_field_list);
-        if named_field_list.fields().count() == 0 && fields.len() > 2 {
-            ast_editor.make_multiline();
-        };
-
-        for field in fields {
-            let field = AstBuilder::<ast::NamedField>::from_pieces(
-                &AstBuilder::<ast::NameRef>::new(&field.name(db).to_string()),
-                Some(&AstBuilder::<ast::Expr>::unit()),
-            );
-            ast_editor.append_field(&field);
-        }
-
-        edit.target(struct_lit.syntax().range());
-        edit.set_cursor(struct_lit.syntax().range().start());
-
-        ast_editor.into_text_edit(edit.text_edit_builder());
-    });
-    ctx.build()
-}
-
-#[cfg(test)]
-mod tests {
-    use crate::helpers::{check_assist, check_assist_target};
-
-    use super::fill_struct_fields;
-
-    #[test]
-    fn fill_struct_fields_empty_body() {
-        check_assist(
-            fill_struct_fields,
-            r#"
-            struct S<'a, D> {
-                a: u32,
-                b: String,
-                c: (i32, i32),
-                d: D,
-                e: &'a str,
-            }
-
-            fn main() {
-                let s = S<|> {}
-            }
-            "#,
-            r#"
-            struct S<'a, D> {
-                a: u32,
-                b: String,
-                c: (i32, i32),
-                d: D,
-                e: &'a str,
-            }
-
-            fn main() {
-                let s = <|>S {
-                    a: (),
-                    b: (),
-                    c: (),
-                    d: (),
-                    e: (),
-                }
-            }
-            "#,
-        );
-    }
-
-    #[test]
-    fn fill_struct_fields_target() {
-        check_assist_target(
-            fill_struct_fields,
-            r#"
-            struct S<'a, D> {
-                a: u32,
-                b: String,
-                c: (i32, i32),
-                d: D,
-                e: &'a str,
-            }
-
-            fn main() {
-                let s = S<|> {}
-            }
-            "#,
-            "S {}",
-        );
-    }
-
-    #[test]
-    fn fill_struct_fields_preserve_self() {
-        check_assist(
-            fill_struct_fields,
-            r#"
-            struct Foo {
-                foo: u8,
-                bar: String,
-                baz: i128,
-            }
-
-            impl Foo {
-                pub fn new() -> Self {
-                    Self <|>{}
-                }
-            }
-            "#,
-            r#"
-            struct Foo {
-                foo: u8,
-                bar: String,
-                baz: i128,
-            }
-
-            impl Foo {
-                pub fn new() -> Self {
-                    <|>Self {
-                        foo: (),
-                        bar: (),
-                        baz: (),
-                    }
-                }
-            }
-            "#,
-        );
-    }
-
-    #[test]
-    fn fill_struct_fields_partial() {
-        check_assist(
-            fill_struct_fields,
-            r#"
-            struct S<'a, D> {
-                a: u32,
-                b: String,
-                c: (i32, i32),
-                d: D,
-                e: &'a str,
-            }
-
-            fn main() {
-                let s = S {
-                    c: (1, 2),
-                    e: "foo",<|>
-                }
-            }
-            "#,
-            r#"
-            struct S<'a, D> {
-                a: u32,
-                b: String,
-                c: (i32, i32),
-                d: D,
-                e: &'a str,
-            }
-
-            fn main() {
-                let s = <|>S {
-                    c: (1, 2),
-                    e: "foo",
-                    a: (),
-                    b: (),
-                    d: (),
-                }
-            }
-            "#,
-        );
-    }
-
-    #[test]
-    fn fill_struct_short() {
-        check_assist(
-            fill_struct_fields,
-            r#"
-            struct S {
-                foo: u32,
-                bar: String,
-            }
-
-            fn main() {
-                let s = S {<|> };
-            }
-            "#,
-            r#"
-            struct S {
-                foo: u32,
-                bar: String,
-            }
-
-            fn main() {
-                let s = <|>S { foo: (), bar: () };
-            }
-            "#,
-        );
-    }
-}
diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs
index a2998ae59..ae97a1ab5 100644
--- a/crates/ra_assists/src/lib.rs
+++ b/crates/ra_assists/src/lib.rs
@@ -93,7 +93,6 @@ mod flip_comma;
 mod flip_binexpr;
 mod change_visibility;
 mod fill_match_arms;
-mod fill_struct_fields;
 mod introduce_variable;
 mod inline_local_variable;
 mod replace_if_let_with_match;
@@ -110,7 +109,6 @@ fn all_assists<DB: HirDatabase>() -> &'static [fn(AssistCtx<DB>) -> Option<Assis
         add_impl::add_impl,
         change_visibility::change_visibility,
         fill_match_arms::fill_match_arms,
-        fill_struct_fields::fill_struct_fields,
         flip_comma::flip_comma,
         flip_binexpr::flip_binexpr,
         introduce_variable::introduce_variable,
diff --git a/crates/ra_hir/src/code_model_api.rs b/crates/ra_hir/src/code_model_api.rs
index 9dcae50a5..55e1793c5 100644
--- a/crates/ra_hir/src/code_model_api.rs
+++ b/crates/ra_hir/src/code_model_api.rs
@@ -8,7 +8,7 @@ use crate::{
     HirDatabase, DefDatabase,
     type_ref::TypeRef,
     nameres::{ModuleScope, Namespace, ImportId, CrateModuleId},
-    expr::{Body, BodySourceMap},
+    expr::{Body, BodySourceMap, validation::ExprValidator},
     ty::{ TraitRef, InferenceResult},
     adt::{EnumVariantId, StructFieldId, VariantDef},
     generics::HasGenericParams,
@@ -16,7 +16,7 @@ use crate::{
     ids::{FunctionId, StructId, EnumId, AstItemDef, ConstId, StaticId, TraitId, TypeAliasId},
     impl_block::ImplBlock,
     resolve::Resolver,
-    diagnostics::DiagnosticSink,
+    diagnostics::{DiagnosticSink},
     traits::{TraitItem, TraitData},
 };
 
@@ -431,8 +431,8 @@ impl Docs for EnumVariant {
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub enum DefWithBody {
     Function(Function),
-    Const(Const),
     Static(Static),
+    Const(Const),
 }
 
 impl_froms!(DefWithBody: Function, Const, Static);
@@ -562,7 +562,10 @@ impl Function {
     }
 
     pub fn diagnostics(&self, db: &impl HirDatabase, sink: &mut DiagnosticSink) {
-        self.infer(db).add_diagnostics(db, *self, sink);
+        let infer = self.infer(db);
+        infer.add_diagnostics(db, *self, sink);
+        let mut validator = ExprValidator::new(*self, infer, sink);
+        validator.validate_body(db);
     }
 }
 
diff --git a/crates/ra_hir/src/diagnostics.rs b/crates/ra_hir/src/diagnostics.rs
index d6a51b833..61cd9d6b1 100644
--- a/crates/ra_hir/src/diagnostics.rs
+++ b/crates/ra_hir/src/diagnostics.rs
@@ -3,7 +3,7 @@ use std::{fmt, any::Any};
 use ra_syntax::{SyntaxNodePtr, TreeArc, AstPtr, TextRange, ast, SyntaxNode};
 use relative_path::RelativePathBuf;
 
-use crate::{HirFileId, HirDatabase};
+use crate::{HirFileId, HirDatabase, Name};
 
 /// Diagnostic defines hir API for errors and warnings.
 ///
@@ -113,3 +113,25 @@ impl Diagnostic for UnresolvedModule {
         self
     }
 }
+
+#[derive(Debug)]
+pub struct MissingFields {
+    pub file: HirFileId,
+    pub field_list: AstPtr<ast::NamedFieldList>,
+    pub missed_fields: Vec<Name>,
+}
+
+impl Diagnostic for MissingFields {
+    fn message(&self) -> String {
+        "fill structure fields".to_string()
+    }
+    fn file(&self) -> HirFileId {
+        self.file
+    }
+    fn syntax_node_ptr(&self) -> SyntaxNodePtr {
+        self.field_list.into()
+    }
+    fn as_any(&self) -> &(dyn Any + Send + 'static) {
+        self
+    }
+}
diff --git a/crates/ra_hir/src/expr.rs b/crates/ra_hir/src/expr.rs
index 692da2895..480eaf171 100644
--- a/crates/ra_hir/src/expr.rs
+++ b/crates/ra_hir/src/expr.rs
@@ -19,6 +19,7 @@ use crate::{path::GenericArgs, ty::primitive::{IntTy, UncertainIntTy, FloatTy, U
 pub use self::scope::ExprScopes;
 
 pub(crate) mod scope;
+pub(crate) mod validation;
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct ExprId(RawId);
@@ -670,8 +671,9 @@ where
             ast::ExprKind::StructLit(e) => {
                 let path = e.path().and_then(Path::from_ast);
                 let mut field_ptrs = Vec::new();
-                let fields = if let Some(nfl) = e.named_field_list() {
-                    nfl.fields()
+                let struct_lit = if let Some(nfl) = e.named_field_list() {
+                    let fields = nfl
+                        .fields()
                         .inspect(|field| field_ptrs.push(AstPtr::new(*field)))
                         .map(|field| StructLitField {
                             name: field
@@ -694,12 +696,14 @@ where
                                 self.exprs.alloc(Expr::Missing)
                             },
                         })
-                        .collect()
+                        .collect();
+                    let spread = nfl.spread().map(|s| self.collect_expr(s));
+                    Expr::StructLit { path, fields, spread }
                 } else {
-                    Vec::new()
+                    Expr::StructLit { path, fields: Vec::new(), spread: None }
                 };
-                let spread = e.spread().map(|s| self.collect_expr(s));
-                let res = self.alloc_expr(Expr::StructLit { path, fields, spread }, syntax_ptr);
+
+                let res = self.alloc_expr(struct_lit, syntax_ptr);
                 for (i, ptr) in field_ptrs.into_iter().enumerate() {
                     self.source_map.field_map.insert((res, i), ptr);
                 }
diff --git a/crates/ra_hir/src/expr/validation.rs b/crates/ra_hir/src/expr/validation.rs
new file mode 100644
index 000000000..fd4907313
--- /dev/null
+++ b/crates/ra_hir/src/expr/validation.rs
@@ -0,0 +1,92 @@
+use std::sync::Arc;
+use rustc_hash::FxHashSet;
+
+use ra_syntax::ast::{AstNode, StructLit};
+
+use crate::{
+    expr::AstPtr,
+    HirDatabase,
+    Function,
+    Name,
+    diagnostics::{DiagnosticSink, MissingFields},
+    adt::AdtDef,
+    Path,
+    ty::InferenceResult
+};
+use super::{Expr, StructLitField, ExprId};
+
+pub(crate) struct ExprValidator<'a, 'b: 'a> {
+    func: Function,
+    infer: Arc<InferenceResult>,
+    sink: &'a mut DiagnosticSink<'b>,
+}
+
+impl<'a, 'b> ExprValidator<'a, 'b> {
+    pub(crate) fn new(
+        func: Function,
+        infer: Arc<InferenceResult>,
+        sink: &'a mut DiagnosticSink<'b>,
+    ) -> ExprValidator<'a, 'b> {
+        ExprValidator { func, infer, sink }
+    }
+
+    pub(crate) fn validate_body(&mut self, db: &impl HirDatabase) {
+        let body = self.func.body(db);
+        for e in body.exprs() {
+            match e {
+                (id, Expr::StructLit { path, fields, spread }) => {
+                    self.validate_struct_literal(id, path, fields, spread, db)
+                }
+                _ => (),
+            }
+        }
+    }
+
+    fn validate_struct_literal(
+        &mut self,
+        id: ExprId,
+        _path: &Option<Path>,
+        fields: &Vec<StructLitField>,
+        spread: &Option<ExprId>,
+        db: &impl HirDatabase,
+    ) {
+        if let Some(_) = spread {
+            return;
+        }
+        let lit_fields: FxHashSet<_> = fields.into_iter().map(|f| &f.name).collect();
+        let struct_ty = &self.infer[id];
+        if let Some((AdtDef::Struct(s), _)) = struct_ty.as_adt() {
+            let missed_fields: Vec<Name> = s
+                .fields(db)
+                .iter()
+                .filter_map(|f| {
+                    let name = f.name(db);
+                    if lit_fields.contains(&name) {
+                        None
+                    } else {
+                        Some(name)
+                    }
+                })
+                .collect();
+            if missed_fields.is_empty() {
+                return;
+            }
+            let source_map = self.func.body_source_map(db);
+            let file_id = self.func.source(db).0;
+            let source_file = db.parse(file_id.original_file(db));
+            if let Some(field_list_node) = source_map
+                .expr_syntax(id)
+                .map(|ptr| ptr.to_node(&source_file))
+                .and_then(StructLit::cast)
+                .and_then(|lit| lit.named_field_list())
+            {
+                let field_list_ptr = AstPtr::new(field_list_node);
+                self.sink.push(MissingFields {
+                    file: file_id,
+                    field_list: field_list_ptr,
+                    missed_fields,
+                })
+            }
+        }
+    }
+}
diff --git a/crates/ra_hir/src/ty/tests.rs b/crates/ra_hir/src/ty/tests.rs
index 0aecde37c..a38fe35c7 100644
--- a/crates/ra_hir/src/ty/tests.rs
+++ b/crates/ra_hir/src/ty/tests.rs
@@ -2662,6 +2662,7 @@ fn no_such_field_diagnostics() {
 
     assert_snapshot_matches!(diagnostics, @r###"
 "baz: 62": no such field
+"{\n            foo: 92,\n            baz: 62,\n        }": fill structure fields
 "###
     );
 }
diff --git a/crates/ra_ide_api/src/diagnostics.rs b/crates/ra_ide_api/src/diagnostics.rs
index b27cb690a..855a3ff0f 100644
--- a/crates/ra_ide_api/src/diagnostics.rs
+++ b/crates/ra_ide_api/src/diagnostics.rs
@@ -5,8 +5,9 @@ use hir::{source_binder, diagnostics::{Diagnostic as _, DiagnosticSink}};
 use ra_db::SourceDatabase;
 use ra_syntax::{
     Location, SourceFile, SyntaxKind, TextRange, SyntaxNode,
-    ast::{self, AstNode},
+    ast::{self, AstNode, NamedFieldList, NamedField},
 };
+use ra_assists::ast_editor::{AstEditor, AstBuilder};
 use ra_text_edit::{TextEdit, TextEditBuilder};
 use ra_prof::profile;
 
@@ -48,6 +49,27 @@ pub(crate) fn diagnostics(db: &RootDatabase, file_id: FileId) -> Vec<Diagnostic>
             severity: Severity::Error,
             fix: Some(fix),
         })
+    })
+    .on::<hir::diagnostics::MissingFields, _>(|d| {
+        let file_id = d.file().original_file(db);
+        let source_file = db.parse(file_id);
+        let syntax_node = d.syntax_node_ptr();
+        let node = NamedFieldList::cast(syntax_node.to_node(&source_file)).unwrap();
+        let mut ast_editor = AstEditor::new(node);
+        for f in d.missed_fields.iter() {
+            ast_editor.append_field(&AstBuilder::<NamedField>::from_name(f));
+        }
+
+        let mut builder = TextEditBuilder::default();
+        ast_editor.into_text_edit(&mut builder);
+        let fix =
+            SourceChange::source_file_edit_from("fill struct fields", file_id, builder.finish());
+        res.borrow_mut().push(Diagnostic {
+            range: d.highlight_range(),
+            message: d.message(),
+            severity: Severity::Error,
+            fix: Some(fix),
+        })
     });
     if let Some(m) = source_binder::module_from_file_id(db, file_id) {
         m.diagnostics(db, &mut sink);
@@ -187,6 +209,105 @@ mod tests {
         assert_eq_text!(after, &actual);
     }
 
+    fn check_apply_diagnostic_fix(before: &str, after: &str) {
+        let (analysis, file_id) = single_file(before);
+        let diagnostic = analysis.diagnostics(file_id).unwrap().pop().unwrap();
+        let mut fix = diagnostic.fix.unwrap();
+        let edit = fix.source_file_edits.pop().unwrap().edit;
+        let actual = edit.apply(&before);
+        assert_eq_text!(after, &actual);
+    }
+
+    fn check_no_diagnostic(content: &str) {
+        let (analysis, file_id) = single_file(content);
+        let diagnostics = analysis.diagnostics(file_id).unwrap();
+        assert_eq!(diagnostics.len(), 0);
+    }
+
+    #[test]
+    fn test_fill_struct_fields_empty() {
+        let before = r"
+            struct TestStruct {
+                one: i32,
+                two: i64,
+            }
+
+            fn test_fn() {
+                let s = TestStruct{};
+            }
+        ";
+        let after = r"
+            struct TestStruct {
+                one: i32,
+                two: i64,
+            }
+
+            fn test_fn() {
+                let s = TestStruct{ one: (), two: ()};
+            }
+        ";
+        check_apply_diagnostic_fix(before, after);
+    }
+
+    #[test]
+    fn test_fill_struct_fields_partial() {
+        let before = r"
+            struct TestStruct {
+                one: i32,
+                two: i64,
+            }
+
+            fn test_fn() {
+                let s = TestStruct{ two: 2 };
+            }
+        ";
+        let after = r"
+            struct TestStruct {
+                one: i32,
+                two: i64,
+            }
+
+            fn test_fn() {
+                let s = TestStruct{ two: 2, one: () };
+            }
+        ";
+        check_apply_diagnostic_fix(before, after);
+    }
+
+    #[test]
+    fn test_fill_struct_fields_no_diagnostic() {
+        let content = r"
+            struct TestStruct {
+                one: i32,
+                two: i64,
+            }
+            
+            fn test_fn() {
+                let one = 1;
+                let s = TestStruct{ one, two: 2 };
+            }
+        ";
+
+        check_no_diagnostic(content);
+    }
+
+    #[test]
+    fn test_fill_struct_fields_no_diagnostic_on_spread() {
+        let content = r"
+            struct TestStruct {
+                one: i32,
+                two: i64,
+            }
+            
+            fn test_fn() {
+                let one = 1;
+                let s = TestStruct{ ..a };
+            }
+        ";
+
+        check_no_diagnostic(content);
+    }
+
     #[test]
     fn test_unresolved_module_diagnostic() {
         let (analysis, file_id) = single_file("mod foo;");
diff --git a/crates/ra_syntax/src/ast/generated.rs b/crates/ra_syntax/src/ast/generated.rs
index 89d3a35c5..e73fe22e9 100644
--- a/crates/ra_syntax/src/ast/generated.rs
+++ b/crates/ra_syntax/src/ast/generated.rs
@@ -2371,6 +2371,10 @@ impl NamedFieldList {
     pub fn fields(&self) -> impl Iterator<Item = &NamedField> {
         super::children(self)
     }
+
+    pub fn spread(&self) -> Option<&Expr> {
+        super::child_opt(self)
+    }
 }
 
 // NeverType
@@ -3564,10 +3568,6 @@ impl StructLit {
     pub fn named_field_list(&self) -> Option<&NamedFieldList> {
         super::child_opt(self)
     }
-
-    pub fn spread(&self) -> Option<&Expr> {
-        super::child_opt(self)
-    }
 }
 
 // StructPat
diff --git a/crates/ra_syntax/src/grammar.ron b/crates/ra_syntax/src/grammar.ron
index c7abdd6dc..b8665bbc8 100644
--- a/crates/ra_syntax/src/grammar.ron
+++ b/crates/ra_syntax/src/grammar.ron
@@ -451,8 +451,11 @@ Grammar(
             traits: [ "AttrsOwner" ]
         ),
         "MatchGuard": (options: ["Expr"]),
-        "StructLit": (options: ["Path", "NamedFieldList", ["spread", "Expr"]]),
-        "NamedFieldList": (collections: [ ["fields", "NamedField"] ]),
+        "StructLit": (options: ["Path", "NamedFieldList"]),
+        "NamedFieldList": (
+            collections: [ ["fields", "NamedField"] ],
+            options: [["spread", "Expr"]]
+        ),
         "NamedField": (options: ["NameRef", "Expr"]),
         "CallExpr": (
             traits: ["ArgListOwner"],