From 26ed92568596ce45ad96c3e2ea5d54099702537f Mon Sep 17 00:00:00 2001 From: Sergey Parilin Date: Thu, 11 Apr 2019 00:00:56 +0300 Subject: fill struct fields diagnostic --- crates/ra_assists/src/ast_editor.rs | 5 ++ crates/ra_hir/src/code_model_api.rs | 11 +-- crates/ra_hir/src/diagnostics.rs | 24 ++++++- crates/ra_hir/src/expr.rs | 16 +++-- crates/ra_hir/src/expr/validation.rs | 92 +++++++++++++++++++++++++ crates/ra_hir/src/ty/tests.rs | 1 + crates/ra_ide_api/src/diagnostics.rs | 123 +++++++++++++++++++++++++++++++++- crates/ra_syntax/src/ast/generated.rs | 8 +-- crates/ra_syntax/src/grammar.ron | 7 +- 9 files changed, 269 insertions(+), 18 deletions(-) create mode 100644 crates/ra_hir/src/expr/validation.rs 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 { original_ast: TreeArc, @@ -235,6 +236,10 @@ pub struct AstBuilder { } impl AstBuilder { + pub fn from_name(name: &Name) -> TreeArc { + ast_node_from_file_text(&format!("fn f() {{ S {{ {}: (), }} }}", name)) + } + fn from_text(text: &str) -> TreeArc { ast_node_from_file_text(&format!("fn f() {{ S {{ {}, }} }}", text)) } 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, + pub missed_fields: Vec, +} + +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, + sink: &'a mut DiagnosticSink<'b>, +} + +impl<'a, 'b> ExprValidator<'a, 'b> { + pub(crate) fn new( + func: Function, + infer: Arc, + 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, + fields: &Vec, + spread: &Option, + 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 = 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 severity: Severity::Error, fix: Some(fix), }) + }) + .on::(|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::::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 { 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"], -- cgit v1.2.3