Skip to content

Commit 6383252

Browse files
matkladmatklad
committed
internal: unified missing fields diagnostic
1 parent c6509a4 commit 6383252

File tree

5 files changed

+94
-142
lines changed

5 files changed

+94
-142
lines changed

‎crates/hir/src/diagnostics.rs‎

Lines changed: 2 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use std::any::Any;
77

88
use cfg::{CfgExpr, CfgOptions, DnfExpr};
9+
use either::Either;
910
use hir_def::path::ModPath;
1011
use hir_expand::{name::Name, HirFileId, InFile};
1112
use stdx::format_to;
@@ -324,60 +325,11 @@ impl Diagnostic for MissingUnsafe {
324325
#[derive(Debug)]
325326
pub struct MissingFields {
326327
pub file: HirFileId,
327-
pub field_list_parent: AstPtr<ast::RecordExpr>,
328+
pub field_list_parent: Either<AstPtr<ast::RecordExpr>, AstPtr<ast::RecordPat>>,
328329
pub field_list_parent_path: Option<AstPtr<ast::Path>>,
329330
pub missed_fields: Vec<Name>,
330331
}
331332

332-
// Diagnostic: missing-pat-fields
333-
//
334-
// This diagnostic is triggered if pattern lacks some fields that exist in the corresponding structure.
335-
//
336-
// Example:
337-
//
338-
// ```rust
339-
// struct A { a: u8, b: u8 }
340-
//
341-
// let a = A { a: 10, b: 20 };
342-
//
343-
// if let A { a } = a {
344-
// // ...
345-
// }
346-
// ```
347-
#[derive(Debug)]
348-
pub struct MissingPatFields {
349-
pub file: HirFileId,
350-
pub field_list_parent: AstPtr<ast::RecordPat>,
351-
pub field_list_parent_path: Option<AstPtr<ast::Path>>,
352-
pub missed_fields: Vec<Name>,
353-
}
354-
355-
impl Diagnostic for MissingPatFields {
356-
fn code(&self) -> DiagnosticCode {
357-
DiagnosticCode("missing-pat-fields")
358-
}
359-
fn message(&self) -> String {
360-
let mut buf = String::from("Missing structure fields:\n");
361-
for field in &self.missed_fields {
362-
format_to!(buf, "- {}\n", field);
363-
}
364-
buf
365-
}
366-
fn display_source(&self) -> InFile<SyntaxNodePtr> {
367-
InFile {
368-
file_id: self.file,
369-
value: self
370-
.field_list_parent_path
371-
.clone()
372-
.map(SyntaxNodePtr::from)
373-
.unwrap_or_else(|| self.field_list_parent.clone().into()),
374-
}
375-
}
376-
fn as_any(&self) -> &(dyn Any + Send + 'static) {
377-
self
378-
}
379-
}
380-
381333
// Diagnostic: replace-filter-map-next-with-find-map
382334
//
383335
// This diagnostic is triggered when `.filter_map(..).next()` is used, rather than the more concise `.find_map(..)`.

‎crates/hir/src/lib.rs‎

Lines changed: 60 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,9 @@ pub use crate::{
8888
diagnostics::{
8989
AnyDiagnostic, BreakOutsideOfLoop, InactiveCode, InternalBailedOut, MacroError,
9090
MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkOrSomeInTailExpr,
91-
MissingPatFields, MissingUnsafe, NoSuchField, RemoveThisSemicolon,
92-
ReplaceFilterMapNextWithFindMap, UnimplementedBuiltinMacro, UnresolvedExternCrate,
93-
UnresolvedImport, UnresolvedMacroCall, UnresolvedModule, UnresolvedProcMacro,
91+
MissingUnsafe, NoSuchField, RemoveThisSemicolon, ReplaceFilterMapNextWithFindMap,
92+
UnimplementedBuiltinMacro, UnresolvedExternCrate, UnresolvedImport, UnresolvedMacroCall,
93+
UnresolvedModule, UnresolvedProcMacro,
9494
},
9595
has_source::HasSource,
9696
semantics::{PathResolution, Semantics, SemanticsScope},
@@ -1098,67 +1098,70 @@ impl Function {
10981098
BodyValidationDiagnostic::collect(db, self.id.into(), internal_diagnostics)
10991099
{
11001100
match diagnostic {
1101-
BodyValidationDiagnostic::RecordLiteralMissingFields {
1102-
record_expr,
1101+
BodyValidationDiagnostic::RecordMissingFields {
1102+
record,
11031103
variant,
11041104
missed_fields,
1105-
} => match source_map.expr_syntax(record_expr) {
1106-
Ok(source_ptr) => {
1107-
let root = source_ptr.file_syntax(db.upcast());
1108-
if let ast::Expr::RecordExpr(record_expr) = &source_ptr.value.to_node(&root)
1109-
{
1110-
if let Some(_) = record_expr.record_expr_field_list() {
1111-
let variant_data = variant.variant_data(db.upcast());
1112-
let missed_fields = missed_fields
1113-
.into_iter()
1114-
.map(|idx| variant_data.fields()[idx].name.clone())
1115-
.collect();
1116-
acc.push(
1117-
MissingFields {
1118-
file: source_ptr.file_id,
1119-
field_list_parent: AstPtr::new(record_expr),
1120-
field_list_parent_path: record_expr
1121-
.path()
1122-
.map(|path| AstPtr::new(&path)),
1123-
missed_fields,
1105+
} => {
1106+
let variant_data = variant.variant_data(db.upcast());
1107+
let missed_fields = missed_fields
1108+
.into_iter()
1109+
.map(|idx| variant_data.fields()[idx].name.clone())
1110+
.collect();
1111+
1112+
match record {
1113+
Either::Left(record_expr) => match source_map.expr_syntax(record_expr) {
1114+
Ok(source_ptr) => {
1115+
let root = source_ptr.file_syntax(db.upcast());
1116+
if let ast::Expr::RecordExpr(record_expr) =
1117+
&source_ptr.value.to_node(&root)
1118+
{
1119+
if let Some(_) = record_expr.record_expr_field_list() {
1120+
acc.push(
1121+
MissingFields {
1122+
file: source_ptr.file_id,
1123+
field_list_parent: Either::Left(AstPtr::new(
1124+
record_expr,
1125+
)),
1126+
field_list_parent_path: record_expr
1127+
.path()
1128+
.map(|path| AstPtr::new(&path)),
1129+
missed_fields,
1130+
}
1131+
.into(),
1132+
)
11241133
}
1125-
.into(),
1126-
)
1134+
}
11271135
}
1128-
}
1129-
}
1130-
Err(SyntheticSyntax) => (),
1131-
},
1132-
BodyValidationDiagnostic::RecordPatMissingFields {
1133-
record_pat,
1134-
variant,
1135-
missed_fields,
1136-
} => match source_map.pat_syntax(record_pat) {
1137-
Ok(source_ptr) => {
1138-
if let Some(expr) = source_ptr.value.as_ref().left() {
1139-
let root = source_ptr.file_syntax(db.upcast());
1140-
if let ast::Pat::RecordPat(record_pat) = expr.to_node(&root) {
1141-
if let Some(_) = record_pat.record_pat_field_list() {
1142-
let variant_data = variant.variant_data(db.upcast());
1143-
let missed_fields = missed_fields
1144-
.into_iter()
1145-
.map(|idx| variant_data.fields()[idx].name.clone())
1146-
.collect();
1147-
sink.push(MissingPatFields {
1148-
file: source_ptr.file_id,
1149-
field_list_parent: AstPtr::new(&record_pat),
1150-
field_list_parent_path: record_pat
1151-
.path()
1152-
.map(|path| AstPtr::new(&path)),
1153-
missed_fields,
1154-
})
1136+
Err(SyntheticSyntax) => (),
1137+
},
1138+
Either::Right(record_pat) => match source_map.pat_syntax(record_pat) {
1139+
Ok(source_ptr) => {
1140+
if let Some(expr) = source_ptr.value.as_ref().left() {
1141+
let root = source_ptr.file_syntax(db.upcast());
1142+
if let ast::Pat::RecordPat(record_pat) = expr.to_node(&root) {
1143+
if let Some(_) = record_pat.record_pat_field_list() {
1144+
acc.push(
1145+
MissingFields {
1146+
file: source_ptr.file_id,
1147+
field_list_parent: Either::Right(AstPtr::new(
1148+
&record_pat,
1149+
)),
1150+
field_list_parent_path: record_pat
1151+
.path()
1152+
.map(|path| AstPtr::new(&path)),
1153+
missed_fields,
1154+
}
1155+
.into(),
1156+
)
1157+
}
1158+
}
11551159
}
11561160
}
1157-
}
1161+
Err(SyntheticSyntax) => (),
1162+
},
11581163
}
1159-
Err(SyntheticSyntax) => (),
1160-
},
1161-
1164+
}
11621165
BodyValidationDiagnostic::ReplaceFilterMapNextWithFindMap { method_call_expr } => {
11631166
if let Ok(next_source_ptr) = source_map.expr_syntax(method_call_expr) {
11641167
sink.push(ReplaceFilterMapNextWithFindMap {

‎crates/hir_ty/src/diagnostics/expr.rs‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use hir_def::{
88
expr::Statement, path::path, resolver::HasResolver, AssocItemId, DefWithBodyId, HasModule,
99
};
1010
use hir_expand::name;
11+
use itertools::Either;
1112
use rustc_hash::FxHashSet;
1213

1314
use crate::{
@@ -26,13 +27,8 @@ pub(crate) use hir_def::{
2627
};
2728

2829
pub enum BodyValidationDiagnostic {
29-
RecordLiteralMissingFields {
30-
record_expr: ExprId,
31-
variant: VariantId,
32-
missed_fields: Vec<LocalFieldId>,
33-
},
34-
RecordPatMissingFields {
35-
record_pat: PatId,
30+
RecordMissingFields {
31+
record: Either<ExprId, PatId>,
3632
variant: VariantId,
3733
missed_fields: Vec<LocalFieldId>,
3834
},
@@ -95,8 +91,8 @@ impl ExprValidator {
9591
if let Some((variant, missed_fields, true)) =
9692
record_literal_missing_fields(db, &self.infer, id, expr)
9793
{
98-
self.diagnostics.push(BodyValidationDiagnostic::RecordLiteralMissingFields {
99-
record_expr: id,
94+
self.diagnostics.push(BodyValidationDiagnostic::RecordMissingFields {
95+
record: Either::Left(id),
10096
variant,
10197
missed_fields,
10298
});
@@ -116,8 +112,8 @@ impl ExprValidator {
116112
if let Some((variant, missed_fields, true)) =
117113
record_pattern_missing_fields(db, &self.infer, id, pat)
118114
{
119-
self.diagnostics.push(BodyValidationDiagnostic::RecordPatMissingFields {
120-
record_pat: id,
115+
self.diagnostics.push(BodyValidationDiagnostic::RecordMissingFields {
116+
record: Either::Right(id),
121117
variant,
122118
missed_fields,
123119
});

‎crates/ide/src/diagnostics.rs‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1055,20 +1055,6 @@ fn main() {
10551055
));
10561056
}
10571057

1058-
#[test]
1059-
fn missing_record_pat_field_diagnostic() {
1060-
check_diagnostics(
1061-
r#"
1062-
struct S { foo: i32, bar: () }
1063-
fn baz(s: S) {
1064-
let S { foo: _ } = s;
1065-
//^ Missing structure fields:
1066-
//| - bar
1067-
}
1068-
"#,
1069-
);
1070-
}
1071-
10721058
#[test]
10731059
fn missing_record_pat_field_no_diagnostic_if_not_exhaustive() {
10741060
check_diagnostics(

‎crates/ide/src/diagnostics/missing_fields.rs‎

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use either::Either;
12
use hir::{db::AstDatabase, InFile};
23
use ide_assists::Assist;
34
use ide_db::source_change::SourceChange;
@@ -7,7 +8,7 @@ use text_edit::TextEdit;
78

89
use crate::diagnostics::{fix, Diagnostic, DiagnosticsContext};
910

10-
// Diagnostic: missing-structure-fields
11+
// Diagnostic: missing-fields
1112
//
1213
// This diagnostic is triggered if record lacks some fields that exist in the corresponding structure.
1314
//
@@ -29,15 +30,11 @@ pub(super) fn missing_fields(ctx: &DiagnosticsContext<'_>, d: &hir::MissingField
2930
d.field_list_parent_path
3031
.clone()
3132
.map(SyntaxNodePtr::from)
32-
.unwrap_or_else(|| d.field_list_parent.clone().into()),
33+
.unwrap_or_else(|| d.field_list_parent.clone().either(|it| it.into(), |it| it.into())),
3334
);
3435

35-
Diagnostic::new(
36-
"missing-structure-fields",
37-
message,
38-
ctx.sema.diagnostics_display_range(ptr).range,
39-
)
40-
.with_fixes(fixes(ctx, d))
36+
Diagnostic::new("missing-fields", message, ctx.sema.diagnostics_display_range(ptr).range)
37+
.with_fixes(fixes(ctx, d))
4138
}
4239

4340
fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::MissingFields) -> Option<Vec<Assist>> {
@@ -51,7 +48,11 @@ fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::MissingFields) -> Option<Vec<Ass
5148
}
5249

5350
let root = ctx.sema.db.parse_or_expand(d.file)?;
54-
let field_list_parent = d.field_list_parent.to_node(&root);
51+
let field_list_parent = match &d.field_list_parent {
52+
Either::Left(record_expr) => record_expr.to_node(&root),
53+
// FIXE: patterns should be fixable as well.
54+
Either::Right(_) => return None,
55+
};
5556
let old_field_list = field_list_parent.record_expr_field_list()?;
5657
let new_field_list = old_field_list.clone_for_update();
5758
for f in d.missed_fields.iter() {
@@ -76,7 +77,21 @@ fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::MissingFields) -> Option<Vec<Ass
7677

7778
#[cfg(test)]
7879
mod tests {
79-
use crate::diagnostics::tests::{check_fix, check_no_diagnostics};
80+
use crate::diagnostics::tests::{check_diagnostics, check_fix, check_no_diagnostics};
81+
82+
#[test]
83+
fn missing_record_pat_field_diagnostic() {
84+
check_diagnostics(
85+
r#"
86+
struct S { foo: i32, bar: () }
87+
fn baz(s: S) {
88+
let S { foo: _ } = s;
89+
//^ Missing structure fields:
90+
//| - bar
91+
}
92+
"#,
93+
);
94+
}
8095

8196
#[test]
8297
fn test_fill_struct_fields_empty() {

0 commit comments

Comments
 (0)