From 7166e8549bad95b05f66acd508d07a6cd97d3dc0 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 13 Jun 2021 19:45:16 +0300 Subject: internal: refactor NoSuchField diagnostic --- crates/hir/src/diagnostics.rs | 25 +- crates/hir/src/lib.rs | 2 +- crates/ide/src/diagnostics.rs | 146 +----------- crates/ide/src/diagnostics/fixes/create_field.rs | 155 ------------ crates/ide/src/diagnostics/no_such_field.rs | 286 +++++++++++++++++++++++ 5 files changed, 300 insertions(+), 314 deletions(-) create mode 100644 crates/ide/src/diagnostics/no_such_field.rs diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index 3908e67a2..c7702e09f 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -35,6 +35,7 @@ diagnostics![ InactiveCode, MacroError, MissingFields, + NoSuchField, UnimplementedBuiltinMacro, UnresolvedExternCrate, UnresolvedImport, @@ -92,31 +93,9 @@ pub struct UnimplementedBuiltinMacro { pub node: InFile, } -// Diagnostic: no-such-field -// -// This diagnostic is triggered if created structure does not have field provided in record. #[derive(Debug)] pub struct NoSuchField { - pub file: HirFileId, - pub field: AstPtr, -} - -impl Diagnostic for NoSuchField { - fn code(&self) -> DiagnosticCode { - DiagnosticCode("no-such-field") - } - - fn message(&self) -> String { - "no such field".to_string() - } - - fn display_source(&self) -> InFile { - InFile::new(self.file, self.field.clone().into()) - } - - fn as_any(&self) -> &(dyn Any + Send + 'static) { - self - } + pub field: InFile>, } // Diagnostic: break-outside-of-loop diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index a361158e0..7faf6ec1f 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -1077,7 +1077,7 @@ impl Function { match d { hir_ty::InferenceDiagnostic::NoSuchField { expr } => { let field = source_map.field_syntax(*expr); - sink.push(NoSuchField { file: field.file_id, field: field.value }) + acc.push(NoSuchField { field }.into()) } hir_ty::InferenceDiagnostic::BreakOutsideOfLoop { expr } => { let ptr = source_map diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index 3fbd21c30..caaa89e0a 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -4,15 +4,16 @@ //! macro-expanded files, but we need to present them to the users in terms of //! original files. So we need to map the ranges. -mod unresolved_module; +mod inactive_code; +mod macro_error; +mod missing_fields; +mod no_such_field; +mod unimplemented_builtin_macro; mod unresolved_extern_crate; mod unresolved_import; mod unresolved_macro_call; +mod unresolved_module; mod unresolved_proc_macro; -mod unimplemented_builtin_macro; -mod macro_error; -mod inactive_code; -mod missing_fields; mod fixes; mod field_shorthand; @@ -161,9 +162,6 @@ pub(crate) fn diagnostics( .on::(|d| { res.borrow_mut().push(diagnostic_with_fix(d, &sema, resolve)); }) - .on::(|d| { - res.borrow_mut().push(diagnostic_with_fix(d, &sema, resolve)); - }) .on::(|d| { res.borrow_mut().push(diagnostic_with_fix(d, &sema, resolve)); }) @@ -220,14 +218,15 @@ pub(crate) fn diagnostics( for diag in diags { #[rustfmt::skip] let d = match diag { - AnyDiagnostic::UnresolvedModule(d) => unresolved_module::unresolved_module(&ctx, &d), + AnyDiagnostic::MacroError(d) => macro_error::macro_error(&ctx, &d), + AnyDiagnostic::MissingFields(d) => missing_fields::missing_fields(&ctx, &d), + AnyDiagnostic::NoSuchField(d) => no_such_field::no_such_field(&ctx, &d), + AnyDiagnostic::UnimplementedBuiltinMacro(d) => unimplemented_builtin_macro::unimplemented_builtin_macro(&ctx, &d), AnyDiagnostic::UnresolvedExternCrate(d) => unresolved_extern_crate::unresolved_extern_crate(&ctx, &d), AnyDiagnostic::UnresolvedImport(d) => unresolved_import::unresolved_import(&ctx, &d), AnyDiagnostic::UnresolvedMacroCall(d) => unresolved_macro_call::unresolved_macro_call(&ctx, &d), + AnyDiagnostic::UnresolvedModule(d) => unresolved_module::unresolved_module(&ctx, &d), AnyDiagnostic::UnresolvedProcMacro(d) => unresolved_proc_macro::unresolved_proc_macro(&ctx, &d), - AnyDiagnostic::UnimplementedBuiltinMacro(d) => unimplemented_builtin_macro::unimplemented_builtin_macro(&ctx, &d), - AnyDiagnostic::MissingFields(d) => missing_fields::missing_fields(&ctx, &d), - AnyDiagnostic::MacroError(d) => macro_error::macro_error(&ctx, &d), AnyDiagnostic::InactiveCode(d) => match inactive_code::inactive_code(&ctx, &d) { Some(it) => it, @@ -722,129 +721,6 @@ fn foo() { break; } ); } - #[test] - fn no_such_field_diagnostics() { - check_diagnostics( - r#" -struct S { foo: i32, bar: () } -impl S { - fn new() -> S { - S { - //^ Missing structure fields: - //| - bar - foo: 92, - baz: 62, - //^^^^^^^ no such field - } - } -} -"#, - ); - } - #[test] - fn no_such_field_with_feature_flag_diagnostics() { - check_diagnostics( - r#" -//- /lib.rs crate:foo cfg:feature=foo -struct MyStruct { - my_val: usize, - #[cfg(feature = "foo")] - bar: bool, -} - -impl MyStruct { - #[cfg(feature = "foo")] - pub(crate) fn new(my_val: usize, bar: bool) -> Self { - Self { my_val, bar } - } - #[cfg(not(feature = "foo"))] - pub(crate) fn new(my_val: usize, _bar: bool) -> Self { - Self { my_val } - } -} -"#, - ); - } - - #[test] - fn no_such_field_enum_with_feature_flag_diagnostics() { - check_diagnostics( - r#" -//- /lib.rs crate:foo cfg:feature=foo -enum Foo { - #[cfg(not(feature = "foo"))] - Buz, - #[cfg(feature = "foo")] - Bar, - Baz -} - -fn test_fn(f: Foo) { - match f { - Foo::Bar => {}, - Foo::Baz => {}, - } -} -"#, - ); - } - - #[test] - fn no_such_field_with_feature_flag_diagnostics_on_struct_lit() { - check_diagnostics( - r#" -//- /lib.rs crate:foo cfg:feature=foo -struct S { - #[cfg(feature = "foo")] - foo: u32, - #[cfg(not(feature = "foo"))] - bar: u32, -} - -impl S { - #[cfg(feature = "foo")] - fn new(foo: u32) -> Self { - Self { foo } - } - #[cfg(not(feature = "foo"))] - fn new(bar: u32) -> Self { - Self { bar } - } - fn new2(bar: u32) -> Self { - #[cfg(feature = "foo")] - { Self { foo: bar } } - #[cfg(not(feature = "foo"))] - { Self { bar } } - } - fn new2(val: u32) -> Self { - Self { - #[cfg(feature = "foo")] - foo: val, - #[cfg(not(feature = "foo"))] - bar: val, - } - } -} -"#, - ); - } - - #[test] - fn no_such_field_with_type_macro() { - check_diagnostics( - r#" -macro_rules! Type { () => { u32 }; } -struct Foo { bar: Type![] } - -impl Foo { - fn new() -> Self { - Foo { bar: 0 } - } -} -"#, - ); - } - #[test] fn missing_unsafe_diagnostic_with_raw_ptr() { check_diagnostics( diff --git a/crates/ide/src/diagnostics/fixes/create_field.rs b/crates/ide/src/diagnostics/fixes/create_field.rs index f6e45967a..8b1378917 100644 --- a/crates/ide/src/diagnostics/fixes/create_field.rs +++ b/crates/ide/src/diagnostics/fixes/create_field.rs @@ -1,156 +1 @@ -use hir::{db::AstDatabase, diagnostics::NoSuchField, HasSource, HirDisplay, Semantics}; -use ide_db::{base_db::FileId, source_change::SourceChange, RootDatabase}; -use syntax::{ - ast::{self, edit::IndentLevel, make}, - AstNode, -}; -use text_edit::TextEdit; -use crate::{ - diagnostics::{fix, DiagnosticWithFixes}, - Assist, AssistResolveStrategy, -}; -impl DiagnosticWithFixes for NoSuchField { - fn fixes( - &self, - sema: &Semantics, - _resolve: &AssistResolveStrategy, - ) -> Option> { - let root = sema.db.parse_or_expand(self.file)?; - missing_record_expr_field_fixes( - sema, - self.file.original_file(sema.db), - &self.field.to_node(&root), - ) - } -} - -fn missing_record_expr_field_fixes( - sema: &Semantics, - usage_file_id: FileId, - record_expr_field: &ast::RecordExprField, -) -> Option> { - let record_lit = ast::RecordExpr::cast(record_expr_field.syntax().parent()?.parent()?)?; - let def_id = sema.resolve_variant(record_lit)?; - let module; - let def_file_id; - let record_fields = match def_id { - hir::VariantDef::Struct(s) => { - module = s.module(sema.db); - let source = s.source(sema.db)?; - def_file_id = source.file_id; - let fields = source.value.field_list()?; - record_field_list(fields)? - } - hir::VariantDef::Union(u) => { - module = u.module(sema.db); - let source = u.source(sema.db)?; - def_file_id = source.file_id; - source.value.record_field_list()? - } - hir::VariantDef::Variant(e) => { - module = e.module(sema.db); - let source = e.source(sema.db)?; - def_file_id = source.file_id; - let fields = source.value.field_list()?; - record_field_list(fields)? - } - }; - let def_file_id = def_file_id.original_file(sema.db); - - let new_field_type = sema.type_of_expr(&record_expr_field.expr()?)?; - if new_field_type.is_unknown() { - return None; - } - let new_field = make::record_field( - None, - make::name(&record_expr_field.field_name()?.text()), - make::ty(&new_field_type.display_source_code(sema.db, module.into()).ok()?), - ); - - let last_field = record_fields.fields().last()?; - let last_field_syntax = last_field.syntax(); - let indent = IndentLevel::from_node(last_field_syntax); - - let mut new_field = new_field.to_string(); - if usage_file_id != def_file_id { - new_field = format!("pub(crate) {}", new_field); - } - new_field = format!("\n{}{}", indent, new_field); - - let needs_comma = !last_field_syntax.to_string().ends_with(','); - if needs_comma { - new_field = format!(",{}", new_field); - } - - let source_change = SourceChange::from_text_edit( - def_file_id, - TextEdit::insert(last_field_syntax.text_range().end(), new_field), - ); - - return Some(vec![fix( - "create_field", - "Create field", - source_change, - record_expr_field.syntax().text_range(), - )]); - - fn record_field_list(field_def_list: ast::FieldList) -> Option { - match field_def_list { - ast::FieldList::RecordFieldList(it) => Some(it), - ast::FieldList::TupleFieldList(_) => None, - } - } -} - -#[cfg(test)] -mod tests { - use crate::diagnostics::tests::check_fix; - - #[test] - fn test_add_field_from_usage() { - check_fix( - r" -fn main() { - Foo { bar: 3, baz$0: false}; -} -struct Foo { - bar: i32 -} -", - r" -fn main() { - Foo { bar: 3, baz: false}; -} -struct Foo { - bar: i32, - baz: bool -} -", - ) - } - - #[test] - fn test_add_field_in_other_file_from_usage() { - check_fix( - r#" -//- /main.rs -mod foo; - -fn main() { - foo::Foo { bar: 3, $0baz: false}; -} -//- /foo.rs -struct Foo { - bar: i32 -} -"#, - r#" -struct Foo { - bar: i32, - pub(crate) baz: bool -} -"#, - ) - } -} diff --git a/crates/ide/src/diagnostics/no_such_field.rs b/crates/ide/src/diagnostics/no_such_field.rs new file mode 100644 index 000000000..61962de28 --- /dev/null +++ b/crates/ide/src/diagnostics/no_such_field.rs @@ -0,0 +1,286 @@ +use hir::{db::AstDatabase, HasSource, HirDisplay, Semantics}; +use ide_db::{base_db::FileId, source_change::SourceChange, RootDatabase}; +use syntax::{ + ast::{self, edit::IndentLevel, make}, + AstNode, +}; +use text_edit::TextEdit; + +use crate::{ + diagnostics::{fix, Diagnostic, DiagnosticsContext}, + Assist, +}; + +// Diagnostic: no-such-field +// +// This diagnostic is triggered if created structure does not have field provided in record. +pub(super) fn no_such_field(ctx: &DiagnosticsContext<'_>, d: &hir::NoSuchField) -> Diagnostic { + Diagnostic::new( + "no-such-field", + "no such field".to_string(), + ctx.sema.diagnostics_display_range(d.field.clone().map(|it| it.into())).range, + ) + .with_fixes(fixes(ctx, d)) +} + +fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::NoSuchField) -> Option> { + let root = ctx.sema.db.parse_or_expand(d.field.file_id)?; + missing_record_expr_field_fixes( + &ctx.sema, + d.field.file_id.original_file(ctx.sema.db), + &d.field.value.to_node(&root), + ) +} + +fn missing_record_expr_field_fixes( + sema: &Semantics, + usage_file_id: FileId, + record_expr_field: &ast::RecordExprField, +) -> Option> { + let record_lit = ast::RecordExpr::cast(record_expr_field.syntax().parent()?.parent()?)?; + let def_id = sema.resolve_variant(record_lit)?; + let module; + let def_file_id; + let record_fields = match def_id { + hir::VariantDef::Struct(s) => { + module = s.module(sema.db); + let source = s.source(sema.db)?; + def_file_id = source.file_id; + let fields = source.value.field_list()?; + record_field_list(fields)? + } + hir::VariantDef::Union(u) => { + module = u.module(sema.db); + let source = u.source(sema.db)?; + def_file_id = source.file_id; + source.value.record_field_list()? + } + hir::VariantDef::Variant(e) => { + module = e.module(sema.db); + let source = e.source(sema.db)?; + def_file_id = source.file_id; + let fields = source.value.field_list()?; + record_field_list(fields)? + } + }; + let def_file_id = def_file_id.original_file(sema.db); + + let new_field_type = sema.type_of_expr(&record_expr_field.expr()?)?; + if new_field_type.is_unknown() { + return None; + } + let new_field = make::record_field( + None, + make::name(&record_expr_field.field_name()?.text()), + make::ty(&new_field_type.display_source_code(sema.db, module.into()).ok()?), + ); + + let last_field = record_fields.fields().last()?; + let last_field_syntax = last_field.syntax(); + let indent = IndentLevel::from_node(last_field_syntax); + + let mut new_field = new_field.to_string(); + if usage_file_id != def_file_id { + new_field = format!("pub(crate) {}", new_field); + } + new_field = format!("\n{}{}", indent, new_field); + + let needs_comma = !last_field_syntax.to_string().ends_with(','); + if needs_comma { + new_field = format!(",{}", new_field); + } + + let source_change = SourceChange::from_text_edit( + def_file_id, + TextEdit::insert(last_field_syntax.text_range().end(), new_field), + ); + + return Some(vec![fix( + "create_field", + "Create field", + source_change, + record_expr_field.syntax().text_range(), + )]); + + fn record_field_list(field_def_list: ast::FieldList) -> Option { + match field_def_list { + ast::FieldList::RecordFieldList(it) => Some(it), + ast::FieldList::TupleFieldList(_) => None, + } + } +} + +#[cfg(test)] +mod tests { + use crate::diagnostics::tests::{check_diagnostics, check_fix}; + + #[test] + fn no_such_field_diagnostics() { + check_diagnostics( + r#" +struct S { foo: i32, bar: () } +impl S { + fn new() -> S { + S { + //^ Missing structure fields: + //| - bar + foo: 92, + baz: 62, + //^^^^^^^ no such field + } + } +} +"#, + ); + } + #[test] + fn no_such_field_with_feature_flag_diagnostics() { + check_diagnostics( + r#" +//- /lib.rs crate:foo cfg:feature=foo +struct MyStruct { + my_val: usize, + #[cfg(feature = "foo")] + bar: bool, +} + +impl MyStruct { + #[cfg(feature = "foo")] + pub(crate) fn new(my_val: usize, bar: bool) -> Self { + Self { my_val, bar } + } + #[cfg(not(feature = "foo"))] + pub(crate) fn new(my_val: usize, _bar: bool) -> Self { + Self { my_val } + } +} +"#, + ); + } + + #[test] + fn no_such_field_enum_with_feature_flag_diagnostics() { + check_diagnostics( + r#" +//- /lib.rs crate:foo cfg:feature=foo +enum Foo { + #[cfg(not(feature = "foo"))] + Buz, + #[cfg(feature = "foo")] + Bar, + Baz +} + +fn test_fn(f: Foo) { + match f { + Foo::Bar => {}, + Foo::Baz => {}, + } +} +"#, + ); + } + + #[test] + fn no_such_field_with_feature_flag_diagnostics_on_struct_lit() { + check_diagnostics( + r#" +//- /lib.rs crate:foo cfg:feature=foo +struct S { + #[cfg(feature = "foo")] + foo: u32, + #[cfg(not(feature = "foo"))] + bar: u32, +} + +impl S { + #[cfg(feature = "foo")] + fn new(foo: u32) -> Self { + Self { foo } + } + #[cfg(not(feature = "foo"))] + fn new(bar: u32) -> Self { + Self { bar } + } + fn new2(bar: u32) -> Self { + #[cfg(feature = "foo")] + { Self { foo: bar } } + #[cfg(not(feature = "foo"))] + { Self { bar } } + } + fn new2(val: u32) -> Self { + Self { + #[cfg(feature = "foo")] + foo: val, + #[cfg(not(feature = "foo"))] + bar: val, + } + } +} +"#, + ); + } + + #[test] + fn no_such_field_with_type_macro() { + check_diagnostics( + r#" +macro_rules! Type { () => { u32 }; } +struct Foo { bar: Type![] } + +impl Foo { + fn new() -> Self { + Foo { bar: 0 } + } +} +"#, + ); + } + + #[test] + fn test_add_field_from_usage() { + check_fix( + r" +fn main() { + Foo { bar: 3, baz$0: false}; +} +struct Foo { + bar: i32 +} +", + r" +fn main() { + Foo { bar: 3, baz: false}; +} +struct Foo { + bar: i32, + baz: bool +} +", + ) + } + + #[test] + fn test_add_field_in_other_file_from_usage() { + check_fix( + r#" +//- /main.rs +mod foo; + +fn main() { + foo::Foo { bar: 3, $0baz: false}; +} +//- /foo.rs +struct Foo { + bar: i32 +} +"#, + r#" +struct Foo { + bar: i32, + pub(crate) baz: bool +} +"#, + ) + } +} -- cgit v1.2.3