From 6383252cc2770545505d40217732f14e93a396c4 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 13 Jun 2021 15:48:54 +0300 Subject: internal: unified missing fields diagnostic --- crates/hir/src/diagnostics.rs | 52 +----------- crates/hir/src/lib.rs | 117 ++++++++++++++------------- crates/hir_ty/src/diagnostics/expr.rs | 18 ++--- crates/ide/src/diagnostics.rs | 14 ---- crates/ide/src/diagnostics/missing_fields.rs | 35 +++++--- 5 files changed, 94 insertions(+), 142 deletions(-) diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index a5e982b75..158626dc0 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -6,6 +6,7 @@ use std::any::Any; use cfg::{CfgExpr, CfgOptions, DnfExpr}; +use either::Either; use hir_def::path::ModPath; use hir_expand::{name::Name, HirFileId, InFile}; use stdx::format_to; @@ -324,60 +325,11 @@ impl Diagnostic for MissingUnsafe { #[derive(Debug)] pub struct MissingFields { pub file: HirFileId, - pub field_list_parent: AstPtr, + pub field_list_parent: Either, AstPtr>, pub field_list_parent_path: Option>, pub missed_fields: Vec, } -// Diagnostic: missing-pat-fields -// -// This diagnostic is triggered if pattern lacks some fields that exist in the corresponding structure. -// -// Example: -// -// ```rust -// struct A { a: u8, b: u8 } -// -// let a = A { a: 10, b: 20 }; -// -// if let A { a } = a { -// // ... -// } -// ``` -#[derive(Debug)] -pub struct MissingPatFields { - pub file: HirFileId, - pub field_list_parent: AstPtr, - pub field_list_parent_path: Option>, - pub missed_fields: Vec, -} - -impl Diagnostic for MissingPatFields { - fn code(&self) -> DiagnosticCode { - DiagnosticCode("missing-pat-fields") - } - fn message(&self) -> String { - let mut buf = String::from("Missing structure fields:\n"); - for field in &self.missed_fields { - format_to!(buf, "- {}\n", field); - } - buf - } - fn display_source(&self) -> InFile { - InFile { - file_id: self.file, - value: self - .field_list_parent_path - .clone() - .map(SyntaxNodePtr::from) - .unwrap_or_else(|| self.field_list_parent.clone().into()), - } - } - fn as_any(&self) -> &(dyn Any + Send + 'static) { - self - } -} - // Diagnostic: replace-filter-map-next-with-find-map // // This diagnostic is triggered when `.filter_map(..).next()` is used, rather than the more concise `.find_map(..)`. diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 583d92f20..373134f62 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -88,9 +88,9 @@ pub use crate::{ diagnostics::{ AnyDiagnostic, BreakOutsideOfLoop, InactiveCode, InternalBailedOut, MacroError, MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkOrSomeInTailExpr, - MissingPatFields, MissingUnsafe, NoSuchField, RemoveThisSemicolon, - ReplaceFilterMapNextWithFindMap, UnimplementedBuiltinMacro, UnresolvedExternCrate, - UnresolvedImport, UnresolvedMacroCall, UnresolvedModule, UnresolvedProcMacro, + MissingUnsafe, NoSuchField, RemoveThisSemicolon, ReplaceFilterMapNextWithFindMap, + UnimplementedBuiltinMacro, UnresolvedExternCrate, UnresolvedImport, UnresolvedMacroCall, + UnresolvedModule, UnresolvedProcMacro, }, has_source::HasSource, semantics::{PathResolution, Semantics, SemanticsScope}, @@ -1098,67 +1098,70 @@ impl Function { BodyValidationDiagnostic::collect(db, self.id.into(), internal_diagnostics) { match diagnostic { - BodyValidationDiagnostic::RecordLiteralMissingFields { - record_expr, + BodyValidationDiagnostic::RecordMissingFields { + record, variant, missed_fields, - } => match source_map.expr_syntax(record_expr) { - Ok(source_ptr) => { - let root = source_ptr.file_syntax(db.upcast()); - if let ast::Expr::RecordExpr(record_expr) = &source_ptr.value.to_node(&root) - { - if let Some(_) = record_expr.record_expr_field_list() { - let variant_data = variant.variant_data(db.upcast()); - let missed_fields = missed_fields - .into_iter() - .map(|idx| variant_data.fields()[idx].name.clone()) - .collect(); - acc.push( - MissingFields { - file: source_ptr.file_id, - field_list_parent: AstPtr::new(record_expr), - field_list_parent_path: record_expr - .path() - .map(|path| AstPtr::new(&path)), - missed_fields, + } => { + let variant_data = variant.variant_data(db.upcast()); + let missed_fields = missed_fields + .into_iter() + .map(|idx| variant_data.fields()[idx].name.clone()) + .collect(); + + match record { + Either::Left(record_expr) => match source_map.expr_syntax(record_expr) { + Ok(source_ptr) => { + let root = source_ptr.file_syntax(db.upcast()); + if let ast::Expr::RecordExpr(record_expr) = + &source_ptr.value.to_node(&root) + { + if let Some(_) = record_expr.record_expr_field_list() { + acc.push( + MissingFields { + file: source_ptr.file_id, + field_list_parent: Either::Left(AstPtr::new( + record_expr, + )), + field_list_parent_path: record_expr + .path() + .map(|path| AstPtr::new(&path)), + missed_fields, + } + .into(), + ) } - .into(), - ) + } } - } - } - Err(SyntheticSyntax) => (), - }, - BodyValidationDiagnostic::RecordPatMissingFields { - record_pat, - variant, - missed_fields, - } => match source_map.pat_syntax(record_pat) { - Ok(source_ptr) => { - if let Some(expr) = source_ptr.value.as_ref().left() { - let root = source_ptr.file_syntax(db.upcast()); - if let ast::Pat::RecordPat(record_pat) = expr.to_node(&root) { - if let Some(_) = record_pat.record_pat_field_list() { - let variant_data = variant.variant_data(db.upcast()); - let missed_fields = missed_fields - .into_iter() - .map(|idx| variant_data.fields()[idx].name.clone()) - .collect(); - sink.push(MissingPatFields { - file: source_ptr.file_id, - field_list_parent: AstPtr::new(&record_pat), - field_list_parent_path: record_pat - .path() - .map(|path| AstPtr::new(&path)), - missed_fields, - }) + Err(SyntheticSyntax) => (), + }, + Either::Right(record_pat) => match source_map.pat_syntax(record_pat) { + Ok(source_ptr) => { + if let Some(expr) = source_ptr.value.as_ref().left() { + let root = source_ptr.file_syntax(db.upcast()); + if let ast::Pat::RecordPat(record_pat) = expr.to_node(&root) { + if let Some(_) = record_pat.record_pat_field_list() { + acc.push( + MissingFields { + file: source_ptr.file_id, + field_list_parent: Either::Right(AstPtr::new( + &record_pat, + )), + field_list_parent_path: record_pat + .path() + .map(|path| AstPtr::new(&path)), + missed_fields, + } + .into(), + ) + } + } } } - } + Err(SyntheticSyntax) => (), + }, } - Err(SyntheticSyntax) => (), - }, - + } BodyValidationDiagnostic::ReplaceFilterMapNextWithFindMap { method_call_expr } => { if let Ok(next_source_ptr) = source_map.expr_syntax(method_call_expr) { sink.push(ReplaceFilterMapNextWithFindMap { diff --git a/crates/hir_ty/src/diagnostics/expr.rs b/crates/hir_ty/src/diagnostics/expr.rs index c480ed352..2a211fd8e 100644 --- a/crates/hir_ty/src/diagnostics/expr.rs +++ b/crates/hir_ty/src/diagnostics/expr.rs @@ -8,6 +8,7 @@ use hir_def::{ expr::Statement, path::path, resolver::HasResolver, AssocItemId, DefWithBodyId, HasModule, }; use hir_expand::name; +use itertools::Either; use rustc_hash::FxHashSet; use crate::{ @@ -26,13 +27,8 @@ pub(crate) use hir_def::{ }; pub enum BodyValidationDiagnostic { - RecordLiteralMissingFields { - record_expr: ExprId, - variant: VariantId, - missed_fields: Vec, - }, - RecordPatMissingFields { - record_pat: PatId, + RecordMissingFields { + record: Either, variant: VariantId, missed_fields: Vec, }, @@ -95,8 +91,8 @@ impl ExprValidator { if let Some((variant, missed_fields, true)) = record_literal_missing_fields(db, &self.infer, id, expr) { - self.diagnostics.push(BodyValidationDiagnostic::RecordLiteralMissingFields { - record_expr: id, + self.diagnostics.push(BodyValidationDiagnostic::RecordMissingFields { + record: Either::Left(id), variant, missed_fields, }); @@ -116,8 +112,8 @@ impl ExprValidator { if let Some((variant, missed_fields, true)) = record_pattern_missing_fields(db, &self.infer, id, pat) { - self.diagnostics.push(BodyValidationDiagnostic::RecordPatMissingFields { - record_pat: id, + self.diagnostics.push(BodyValidationDiagnostic::RecordMissingFields { + record: Either::Right(id), variant, missed_fields, }); diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index ef8c8044c..3307e240b 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -1055,20 +1055,6 @@ fn main() { )); } - #[test] - fn missing_record_pat_field_diagnostic() { - check_diagnostics( - r#" -struct S { foo: i32, bar: () } -fn baz(s: S) { - let S { foo: _ } = s; - //^ Missing structure fields: - //| - bar -} -"#, - ); - } - #[test] fn missing_record_pat_field_no_diagnostic_if_not_exhaustive() { check_diagnostics( diff --git a/crates/ide/src/diagnostics/missing_fields.rs b/crates/ide/src/diagnostics/missing_fields.rs index aee780972..66575f713 100644 --- a/crates/ide/src/diagnostics/missing_fields.rs +++ b/crates/ide/src/diagnostics/missing_fields.rs @@ -1,3 +1,4 @@ +use either::Either; use hir::{db::AstDatabase, InFile}; use ide_assists::Assist; use ide_db::source_change::SourceChange; @@ -7,7 +8,7 @@ use text_edit::TextEdit; use crate::diagnostics::{fix, Diagnostic, DiagnosticsContext}; -// Diagnostic: missing-structure-fields +// Diagnostic: missing-fields // // This diagnostic is triggered if record lacks some fields that exist in the corresponding structure. // @@ -29,15 +30,11 @@ pub(super) fn missing_fields(ctx: &DiagnosticsContext<'_>, d: &hir::MissingField d.field_list_parent_path .clone() .map(SyntaxNodePtr::from) - .unwrap_or_else(|| d.field_list_parent.clone().into()), + .unwrap_or_else(|| d.field_list_parent.clone().either(|it| it.into(), |it| it.into())), ); - Diagnostic::new( - "missing-structure-fields", - message, - ctx.sema.diagnostics_display_range(ptr).range, - ) - .with_fixes(fixes(ctx, d)) + Diagnostic::new("missing-fields", message, ctx.sema.diagnostics_display_range(ptr).range) + .with_fixes(fixes(ctx, d)) } fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::MissingFields) -> Option> { @@ -51,7 +48,11 @@ fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::MissingFields) -> Option record_expr.to_node(&root), + // FIXE: patterns should be fixable as well. + Either::Right(_) => return None, + }; let old_field_list = field_list_parent.record_expr_field_list()?; let new_field_list = old_field_list.clone_for_update(); for f in d.missed_fields.iter() { @@ -76,7 +77,21 @@ fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::MissingFields) -> Option