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 (limited to 'crates') 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 From 12f8472d2800b2d7c05cb1fc466c80072ed8e283 Mon Sep 17 00:00:00 2001 From: Sergey Parilin Date: Mon, 6 May 2019 17:26:09 +0300 Subject: removed duplicating fill_struct_fields assist --- crates/ra_assists/src/fill_struct_fields.rs | 226 ---------------------------- crates/ra_assists/src/lib.rs | 2 - 2 files changed, 228 deletions(-) delete mode 100644 crates/ra_assists/src/fill_struct_fields.rs (limited to 'crates') 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) -> Option { - let struct_lit = ctx.node_at_offset::()?; - 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::::from_pieces( - &AstBuilder::::new(&field.name(db).to_string()), - Some(&AstBuilder::::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() -> &'static [fn(AssistCtx) -> Option