From e63315b8f189396cf556f21d4ca27ae4281d17d7 Mon Sep 17 00:00:00 2001 From: Josh Mcguigan Date: Wed, 8 Apr 2020 20:23:51 -0700 Subject: add record pat missing field diagnostic --- crates/ra_hir/src/source_analyzer.rs | 2 +- crates/ra_hir_def/src/body/lower.rs | 4 +- crates/ra_hir_def/src/expr.rs | 33 ++--------- crates/ra_hir_ty/src/diagnostics.rs | 23 +++++++ crates/ra_hir_ty/src/expr.rs | 112 ++++++++++++++++++++++++++--------- crates/ra_hir_ty/src/infer/pat.rs | 2 +- crates/ra_hir_ty/src/tests.rs | 40 +++++++++++++ 7 files changed, 159 insertions(+), 57 deletions(-) (limited to 'crates') diff --git a/crates/ra_hir/src/source_analyzer.rs b/crates/ra_hir/src/source_analyzer.rs index 45631f8fd..226fb4534 100644 --- a/crates/ra_hir/src/source_analyzer.rs +++ b/crates/ra_hir/src/source_analyzer.rs @@ -255,7 +255,7 @@ impl SourceAnalyzer { _ => return None, }; - let (variant, missing_fields) = + let (variant, missing_fields, _exhaustive) = record_pattern_missing_fields(db, infer, pat_id, &body[pat_id])?; let res = self.missing_fields(db, krate, substs, variant, missing_fields); Some(res) diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs index b0d71eb3d..0855c1d3a 100644 --- a/crates/ra_hir_def/src/body/lower.rs +++ b/crates/ra_hir_def/src/body/lower.rs @@ -668,7 +668,9 @@ impl ExprCollector<'_> { }); fields.extend(iter); - Pat::Record { path, args: fields } + let ellipsis = record_field_pat_list.dotdot_token().is_some(); + + Pat::Record { path, args: fields, ellipsis } } ast::Pat::SlicePat(p) => { let SlicePatComponents { prefix, slice, suffix } = p.components(); diff --git a/crates/ra_hir_def/src/expr.rs b/crates/ra_hir_def/src/expr.rs index 197bbe9bd..e11bdf3ec 100644 --- a/crates/ra_hir_def/src/expr.rs +++ b/crates/ra_hir_def/src/expr.rs @@ -376,35 +376,14 @@ pub enum Pat { Wild, Tuple(Vec), Or(Vec), - Record { - path: Option, - args: Vec, - // FIXME: 'ellipsis' option - }, - Range { - start: ExprId, - end: ExprId, - }, - Slice { - prefix: Vec, - slice: Option, - suffix: Vec, - }, + Record { path: Option, args: Vec, ellipsis: bool }, + Range { start: ExprId, end: ExprId }, + Slice { prefix: Vec, slice: Option, suffix: Vec }, Path(Path), Lit(ExprId), - Bind { - mode: BindingAnnotation, - name: Name, - subpat: Option, - }, - TupleStruct { - path: Option, - args: Vec, - }, - Ref { - pat: PatId, - mutability: Mutability, - }, + Bind { mode: BindingAnnotation, name: Name, subpat: Option }, + TupleStruct { path: Option, args: Vec }, + Ref { pat: PatId, mutability: Mutability }, } impl Pat { diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs index 8cbce1168..3f18acf1d 100644 --- a/crates/ra_hir_ty/src/diagnostics.rs +++ b/crates/ra_hir_ty/src/diagnostics.rs @@ -62,6 +62,29 @@ impl AstDiagnostic for MissingFields { } } +#[derive(Debug)] +pub struct MissingPatFields { + pub file: HirFileId, + pub field_list: AstPtr, + pub missed_fields: Vec, +} + +impl Diagnostic for MissingPatFields { + fn message(&self) -> String { + let mut buf = String::from("Missing structure fields:\n"); + for field in &self.missed_fields { + format_to!(buf, "- {}", field); + } + buf + } + fn source(&self) -> InFile { + InFile { file_id: self.file, value: self.field_list.into() } + } + fn as_any(&self) -> &(dyn Any + Send + 'static) { + self + } +} + #[derive(Debug)] pub struct MissingMatchArms { pub file: HirFileId, diff --git a/crates/ra_hir_ty/src/expr.rs b/crates/ra_hir_ty/src/expr.rs index e45e9ea14..a7c8d74ab 100644 --- a/crates/ra_hir_ty/src/expr.rs +++ b/crates/ra_hir_ty/src/expr.rs @@ -9,7 +9,7 @@ use rustc_hash::FxHashSet; use crate::{ db::HirDatabase, - diagnostics::{MissingFields, MissingMatchArms, MissingOkInTailExpr}, + diagnostics::{MissingFields, MissingMatchArms, MissingOkInTailExpr, MissingPatFields}, utils::variant_data, ApplicationTy, InferenceResult, Ty, TypeCtor, _match::{is_useful, MatchCheckCtx, Matrix, PatStack, Usefulness}, @@ -49,39 +49,97 @@ impl<'a, 'b> ExprValidator<'a, 'b> { if let Some((variant_def, missed_fields, true)) = record_literal_missing_fields(db, &self.infer, id, expr) { - // XXX: only look at source_map if we do have missing fields - let (_, source_map) = db.body_with_source_map(self.func.into()); - - if let Ok(source_ptr) = source_map.expr_syntax(id) { - if let Some(expr) = source_ptr.value.left() { - let root = source_ptr.file_syntax(db.upcast()); - if let ast::Expr::RecordLit(record_lit) = expr.to_node(&root) { - if let Some(field_list) = record_lit.record_field_list() { - let variant_data = variant_data(db.upcast(), variant_def); - let missed_fields = missed_fields - .into_iter() - .map(|idx| variant_data.fields()[idx].name.clone()) - .collect(); - self.sink.push(MissingFields { - file: source_ptr.file_id, - field_list: AstPtr::new(&field_list), - missed_fields, - }) - } - } - } - } + self.create_record_literal_missing_fields_diagnostic( + id, + db, + variant_def, + missed_fields, + ); } if let Expr::Match { expr, arms } = expr { self.validate_match(id, *expr, arms, db, self.infer.clone()); } } + for (id, pat) in body.pats.iter() { + if let Some((variant_def, missed_fields, true)) = + record_pattern_missing_fields(db, &self.infer, id, pat) + { + self.create_record_pattern_missing_fields_diagnostic( + id, + db, + variant_def, + missed_fields, + ); + } + } let body_expr = &body[body.body_expr]; if let Expr::Block { tail: Some(t), .. } = body_expr { self.validate_results_in_tail_expr(body.body_expr, *t, db); } } + fn create_record_literal_missing_fields_diagnostic( + &mut self, + id: ExprId, + db: &dyn HirDatabase, + variant_def: VariantId, + missed_fields: Vec, + ) { + // XXX: only look at source_map if we do have missing fields + let (_, source_map) = db.body_with_source_map(self.func.into()); + + if let Ok(source_ptr) = source_map.expr_syntax(id) { + if let Some(expr) = source_ptr.value.left() { + let root = source_ptr.file_syntax(db.upcast()); + if let ast::Expr::RecordLit(record_lit) = expr.to_node(&root) { + if let Some(field_list) = record_lit.record_field_list() { + let variant_data = variant_data(db.upcast(), variant_def); + let missed_fields = missed_fields + .into_iter() + .map(|idx| variant_data.fields()[idx].name.clone()) + .collect(); + self.sink.push(MissingFields { + file: source_ptr.file_id, + field_list: AstPtr::new(&field_list), + missed_fields, + }) + } + } + } + } + } + + fn create_record_pattern_missing_fields_diagnostic( + &mut self, + id: PatId, + db: &dyn HirDatabase, + variant_def: VariantId, + missed_fields: Vec, + ) { + // XXX: only look at source_map if we do have missing fields + let (_, source_map) = db.body_with_source_map(self.func.into()); + + if let Ok(source_ptr) = source_map.pat_syntax(id) { + if let Some(expr) = source_ptr.value.left() { + let root = source_ptr.file_syntax(db.upcast()); + if let ast::Pat::RecordPat(record_pat) = expr.to_node(&root) { + if let Some(field_list) = record_pat.record_field_pat_list() { + let variant_data = variant_data(db.upcast(), variant_def); + let missed_fields = missed_fields + .into_iter() + .map(|idx| variant_data.fields()[idx].name.clone()) + .collect(); + self.sink.push(MissingPatFields { + file: source_ptr.file_id, + field_list: AstPtr::new(&field_list), + missed_fields, + }) + } + } + } + } + } + fn validate_match( &mut self, id: ExprId, @@ -232,9 +290,9 @@ pub fn record_pattern_missing_fields( infer: &InferenceResult, id: PatId, pat: &Pat, -) -> Option<(VariantId, Vec)> { - let fields = match pat { - Pat::Record { path: _, args } => args, +) -> Option<(VariantId, Vec, /*exhaustive*/ bool)> { + let (fields, exhaustive) = match pat { + Pat::Record { path: _, args, ellipsis } => (args, !ellipsis), _ => return None, }; @@ -254,5 +312,5 @@ pub fn record_pattern_missing_fields( if missed_fields.is_empty() { return None; } - Some((variant_def, missed_fields)) + Some((variant_def, missed_fields, exhaustive)) } diff --git a/crates/ra_hir_ty/src/infer/pat.rs b/crates/ra_hir_ty/src/infer/pat.rs index 69bbb4307..078476f76 100644 --- a/crates/ra_hir_ty/src/infer/pat.rs +++ b/crates/ra_hir_ty/src/infer/pat.rs @@ -158,7 +158,7 @@ impl<'a> InferenceContext<'a> { Pat::TupleStruct { path: p, args: subpats } => { self.infer_tuple_struct_pat(p.as_ref(), subpats, expected, default_bm, pat) } - Pat::Record { path: p, args: fields } => { + Pat::Record { path: p, args: fields, ellipsis: _ } => { self.infer_record_pat(p.as_ref(), fields, expected, default_bm, pat) } Pat::Path(path) => { diff --git a/crates/ra_hir_ty/src/tests.rs b/crates/ra_hir_ty/src/tests.rs index 002cffba6..47a7b9ffd 100644 --- a/crates/ra_hir_ty/src/tests.rs +++ b/crates/ra_hir_ty/src/tests.rs @@ -409,3 +409,43 @@ fn no_such_field_with_feature_flag_diagnostics_on_struct_fields() { assert_snapshot!(diagnostics, @r###""###); } + +#[test] +fn missing_record_pat_field_diagnostic() { + let diagnostics = TestDB::with_files( + r" + //- /lib.rs + struct S { foo: i32, bar: () } + fn baz(s: S) { + let S { foo: _ } = s; + } + ", + ) + .diagnostics() + .0; + + assert_snapshot!(diagnostics, @r###" + "{ foo: _ }": Missing structure fields: + - bar + "### + ); +} + +#[test] +fn missing_record_pat_field_no_diagnostic_if_not_exhaustive() { + let diagnostics = TestDB::with_files( + r" + //- /lib.rs + struct S { foo: i32, bar: () } + fn baz(s: S) -> i32 { + match s { + S { foo, .. } => foo, + } + } + ", + ) + .diagnostics() + .0; + + assert_snapshot!(diagnostics, @""); +} -- cgit v1.2.3