9246: internal: unified missing fields diagnostic r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
This commit is contained in:
bors[bot] 2021-06-13 12:49:37 +00:00 committed by GitHub
commit cc6d761a99
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 343 additions and 402 deletions

View File

@ -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;
@ -16,7 +17,7 @@ pub use crate::diagnostics_sink::{
};
macro_rules! diagnostics {
($($diag:ident)*) => {
($($diag:ident),*) => {
pub enum AnyDiagnostic {$(
$diag(Box<$diag>),
)*}
@ -31,7 +32,7 @@ macro_rules! diagnostics {
};
}
diagnostics![UnresolvedModule];
diagnostics![UnresolvedModule, MissingFields];
#[derive(Debug)]
pub struct UnresolvedModule {
@ -321,102 +322,14 @@ impl Diagnostic for MissingUnsafe {
}
}
// Diagnostic: missing-structure-fields
//
// This diagnostic is triggered if record lacks some fields that exist in the corresponding structure.
//
// Example:
//
// ```rust
// struct A { a: u8, b: u8 }
//
// let a = A { a: 10 };
// ```
#[derive(Debug)]
pub struct MissingFields {
pub file: HirFileId,
pub field_list_parent: AstPtr<ast::RecordExpr>,
pub field_list_parent: Either<AstPtr<ast::RecordExpr>, AstPtr<ast::RecordPat>>,
pub field_list_parent_path: Option<AstPtr<ast::Path>>,
pub missed_fields: Vec<Name>,
}
impl Diagnostic for MissingFields {
fn code(&self) -> DiagnosticCode {
DiagnosticCode("missing-structure-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<SyntaxNodePtr> {
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: 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<ast::RecordPat>,
pub field_list_parent_path: Option<AstPtr<ast::Path>>,
pub missed_fields: Vec<Name>,
}
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<SyntaxNodePtr> {
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(..)`.

View File

@ -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},
@ -609,23 +609,21 @@ impl Module {
}
for decl in self.declarations(db) {
match decl {
crate::ModuleDef::Function(f) => f.diagnostics(db, sink, internal_diagnostics),
crate::ModuleDef::Module(m) => {
ModuleDef::Function(f) => acc.extend(f.diagnostics(db, sink, internal_diagnostics)),
ModuleDef::Module(m) => {
// Only add diagnostics from inline modules
if def_map[m.id.local_id].origin.is_inline() {
acc.extend(m.diagnostics(db, sink, internal_diagnostics))
}
}
_ => {
decl.diagnostics(db, sink);
}
_ => decl.diagnostics(db, sink),
}
}
for impl_def in self.impl_defs(db) {
for item in impl_def.items(db) {
if let AssocItem::Function(f) = item {
f.diagnostics(db, sink, internal_diagnostics);
acc.extend(f.diagnostics(db, sink, internal_diagnostics));
}
}
}
@ -1033,7 +1031,8 @@ impl Function {
db: &dyn HirDatabase,
sink: &mut DiagnosticSink,
internal_diagnostics: bool,
) {
) -> Vec<AnyDiagnostic> {
let mut acc: Vec<AnyDiagnostic> = Vec::new();
let krate = self.module(db).id.krate();
let source_map = db.body_with_source_map(self.id.into()).1;
@ -1099,64 +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();
sink.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,
})
}
}
}
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,
})
} => {
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(),
)
}
}
}
}
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 {
@ -1234,6 +1239,7 @@ impl Function {
for diag in hir_ty::diagnostics::validate_module_item(db, krate, self.id.into()) {
sink.push(diag)
}
acc
}
/// Whether this function declaration has a definition.

View File

@ -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<LocalFieldId>,
},
RecordPatMissingFields {
record_pat: PatId,
RecordMissingFields {
record: Either<ExprId, PatId>,
variant: VariantId,
missed_fields: Vec<LocalFieldId>,
},
@ -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,
});

View File

@ -5,6 +5,7 @@
//! original files. So we need to map the ranges.
mod unresolved_module;
mod missing_fields;
mod fixes;
mod field_shorthand;
@ -123,9 +124,6 @@ pub(crate) fn diagnostics(
}
let res = RefCell::new(res);
let sink_builder = DiagnosticSinkBuilder::new()
.on::<hir::diagnostics::MissingFields, _>(|d| {
res.borrow_mut().push(diagnostic_with_fix(d, &sema, resolve));
})
.on::<hir::diagnostics::MissingOkOrSomeInTailExpr, _>(|d| {
res.borrow_mut().push(diagnostic_with_fix(d, &sema, resolve));
})
@ -232,7 +230,8 @@ pub(crate) fn diagnostics(
let ctx = DiagnosticsContext { config, sema, resolve };
for diag in diags {
let d = match diag {
AnyDiagnostic::UnresolvedModule(d) => unresolved_module::render(&ctx, &d),
AnyDiagnostic::UnresolvedModule(d) => unresolved_module::unresolved_module(&ctx, &d),
AnyDiagnostic::MissingFields(d) => missing_fields::missing_fields(&ctx, &d),
};
if let Some(code) = d.code {
if ctx.config.disabled.contains(code.as_str()) {
@ -1056,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(

View File

@ -2,7 +2,6 @@
//! The same module also has all curret custom fixes for the diagnostics implemented.
mod change_case;
mod create_field;
mod fill_missing_fields;
mod remove_semicolon;
mod replace_with_find_map;
mod wrap_tail_expr;

View File

@ -1,217 +0,0 @@
use hir::{db::AstDatabase, diagnostics::MissingFields, Semantics};
use ide_assists::AssistResolveStrategy;
use ide_db::{source_change::SourceChange, RootDatabase};
use syntax::{algo, ast::make, AstNode};
use text_edit::TextEdit;
use crate::{
diagnostics::{fix, fixes::DiagnosticWithFixes},
Assist,
};
impl DiagnosticWithFixes for MissingFields {
fn fixes(
&self,
sema: &Semantics<RootDatabase>,
_resolve: &AssistResolveStrategy,
) -> Option<Vec<Assist>> {
// Note that although we could add a diagnostics to
// fill the missing tuple field, e.g :
// `struct A(usize);`
// `let a = A { 0: () }`
// but it is uncommon usage and it should not be encouraged.
if self.missed_fields.iter().any(|it| it.as_tuple_index().is_some()) {
return None;
}
let root = sema.db.parse_or_expand(self.file)?;
let field_list_parent = self.field_list_parent.to_node(&root);
let old_field_list = field_list_parent.record_expr_field_list()?;
let new_field_list = old_field_list.clone_for_update();
for f in self.missed_fields.iter() {
let field =
make::record_expr_field(make::name_ref(&f.to_string()), Some(make::expr_unit()))
.clone_for_update();
new_field_list.add_field(field);
}
let edit = {
let mut builder = TextEdit::builder();
algo::diff(old_field_list.syntax(), new_field_list.syntax())
.into_text_edit(&mut builder);
builder.finish()
};
Some(vec![fix(
"fill_missing_fields",
"Fill struct fields",
SourceChange::from_text_edit(self.file.original_file(sema.db), edit),
sema.original_range(field_list_parent.syntax()).range,
)])
}
}
#[cfg(test)]
mod tests {
use crate::diagnostics::tests::{check_fix, check_no_diagnostics};
#[test]
fn test_fill_struct_fields_empty() {
check_fix(
r#"
struct TestStruct { one: i32, two: i64 }
fn test_fn() {
let s = TestStruct {$0};
}
"#,
r#"
struct TestStruct { one: i32, two: i64 }
fn test_fn() {
let s = TestStruct { one: (), two: () };
}
"#,
);
}
#[test]
fn test_fill_struct_fields_self() {
check_fix(
r#"
struct TestStruct { one: i32 }
impl TestStruct {
fn test_fn() { let s = Self {$0}; }
}
"#,
r#"
struct TestStruct { one: i32 }
impl TestStruct {
fn test_fn() { let s = Self { one: () }; }
}
"#,
);
}
#[test]
fn test_fill_struct_fields_enum() {
check_fix(
r#"
enum Expr {
Bin { lhs: Box<Expr>, rhs: Box<Expr> }
}
impl Expr {
fn new_bin(lhs: Box<Expr>, rhs: Box<Expr>) -> Expr {
Expr::Bin {$0 }
}
}
"#,
r#"
enum Expr {
Bin { lhs: Box<Expr>, rhs: Box<Expr> }
}
impl Expr {
fn new_bin(lhs: Box<Expr>, rhs: Box<Expr>) -> Expr {
Expr::Bin { lhs: (), rhs: () }
}
}
"#,
);
}
#[test]
fn test_fill_struct_fields_partial() {
check_fix(
r#"
struct TestStruct { one: i32, two: i64 }
fn test_fn() {
let s = TestStruct{ two: 2$0 };
}
"#,
r"
struct TestStruct { one: i32, two: i64 }
fn test_fn() {
let s = TestStruct{ two: 2, one: () };
}
",
);
}
#[test]
fn test_fill_struct_fields_raw_ident() {
check_fix(
r#"
struct TestStruct { r#type: u8 }
fn test_fn() {
TestStruct { $0 };
}
"#,
r"
struct TestStruct { r#type: u8 }
fn test_fn() {
TestStruct { r#type: () };
}
",
);
}
#[test]
fn test_fill_struct_fields_no_diagnostic() {
check_no_diagnostics(
r#"
struct TestStruct { one: i32, two: i64 }
fn test_fn() {
let one = 1;
let s = TestStruct{ one, two: 2 };
}
"#,
);
}
#[test]
fn test_fill_struct_fields_no_diagnostic_on_spread() {
check_no_diagnostics(
r#"
struct TestStruct { one: i32, two: i64 }
fn test_fn() {
let one = 1;
let s = TestStruct{ ..a };
}
"#,
);
}
#[test]
fn test_fill_struct_fields_blank_line() {
check_fix(
r#"
struct S { a: (), b: () }
fn f() {
S {
$0
};
}
"#,
r#"
struct S { a: (), b: () }
fn f() {
S {
a: (),
b: (),
};
}
"#,
);
}
}

View File

@ -0,0 +1,256 @@
use either::Either;
use hir::{db::AstDatabase, InFile};
use ide_assists::Assist;
use ide_db::source_change::SourceChange;
use stdx::format_to;
use syntax::{algo, ast::make, AstNode, SyntaxNodePtr};
use text_edit::TextEdit;
use crate::diagnostics::{fix, Diagnostic, DiagnosticsContext};
// Diagnostic: missing-fields
//
// This diagnostic is triggered if record lacks some fields that exist in the corresponding structure.
//
// Example:
//
// ```rust
// struct A { a: u8, b: u8 }
//
// let a = A { a: 10 };
// ```
pub(super) fn missing_fields(ctx: &DiagnosticsContext<'_>, d: &hir::MissingFields) -> Diagnostic {
let mut message = String::from("Missing structure fields:\n");
for field in &d.missed_fields {
format_to!(message, "- {}\n", field);
}
let ptr = InFile::new(
d.file,
d.field_list_parent_path
.clone()
.map(SyntaxNodePtr::from)
.unwrap_or_else(|| d.field_list_parent.clone().either(|it| it.into(), |it| it.into())),
);
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<Vec<Assist>> {
// Note that although we could add a diagnostics to
// fill the missing tuple field, e.g :
// `struct A(usize);`
// `let a = A { 0: () }`
// but it is uncommon usage and it should not be encouraged.
if d.missed_fields.iter().any(|it| it.as_tuple_index().is_some()) {
return None;
}
let root = ctx.sema.db.parse_or_expand(d.file)?;
let field_list_parent = match &d.field_list_parent {
Either::Left(record_expr) => 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() {
let field =
make::record_expr_field(make::name_ref(&f.to_string()), Some(make::expr_unit()))
.clone_for_update();
new_field_list.add_field(field);
}
let edit = {
let mut builder = TextEdit::builder();
algo::diff(old_field_list.syntax(), new_field_list.syntax()).into_text_edit(&mut builder);
builder.finish()
};
Some(vec![fix(
"fill_missing_fields",
"Fill struct fields",
SourceChange::from_text_edit(d.file.original_file(ctx.sema.db), edit),
ctx.sema.original_range(field_list_parent.syntax()).range,
)])
}
#[cfg(test)]
mod tests {
use crate::diagnostics::tests::{check_diagnostics, check_fix, check_no_diagnostics};
#[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 test_fill_struct_fields_empty() {
check_fix(
r#"
struct TestStruct { one: i32, two: i64 }
fn test_fn() {
let s = TestStruct {$0};
}
"#,
r#"
struct TestStruct { one: i32, two: i64 }
fn test_fn() {
let s = TestStruct { one: (), two: () };
}
"#,
);
}
#[test]
fn test_fill_struct_fields_self() {
check_fix(
r#"
struct TestStruct { one: i32 }
impl TestStruct {
fn test_fn() { let s = Self {$0}; }
}
"#,
r#"
struct TestStruct { one: i32 }
impl TestStruct {
fn test_fn() { let s = Self { one: () }; }
}
"#,
);
}
#[test]
fn test_fill_struct_fields_enum() {
check_fix(
r#"
enum Expr {
Bin { lhs: Box<Expr>, rhs: Box<Expr> }
}
impl Expr {
fn new_bin(lhs: Box<Expr>, rhs: Box<Expr>) -> Expr {
Expr::Bin {$0 }
}
}
"#,
r#"
enum Expr {
Bin { lhs: Box<Expr>, rhs: Box<Expr> }
}
impl Expr {
fn new_bin(lhs: Box<Expr>, rhs: Box<Expr>) -> Expr {
Expr::Bin { lhs: (), rhs: () }
}
}
"#,
);
}
#[test]
fn test_fill_struct_fields_partial() {
check_fix(
r#"
struct TestStruct { one: i32, two: i64 }
fn test_fn() {
let s = TestStruct{ two: 2$0 };
}
"#,
r"
struct TestStruct { one: i32, two: i64 }
fn test_fn() {
let s = TestStruct{ two: 2, one: () };
}
",
);
}
#[test]
fn test_fill_struct_fields_raw_ident() {
check_fix(
r#"
struct TestStruct { r#type: u8 }
fn test_fn() {
TestStruct { $0 };
}
"#,
r"
struct TestStruct { r#type: u8 }
fn test_fn() {
TestStruct { r#type: () };
}
",
);
}
#[test]
fn test_fill_struct_fields_no_diagnostic() {
check_no_diagnostics(
r#"
struct TestStruct { one: i32, two: i64 }
fn test_fn() {
let one = 1;
let s = TestStruct{ one, two: 2 };
}
"#,
);
}
#[test]
fn test_fill_struct_fields_no_diagnostic_on_spread() {
check_no_diagnostics(
r#"
struct TestStruct { one: i32, two: i64 }
fn test_fn() {
let one = 1;
let s = TestStruct{ ..a };
}
"#,
);
}
#[test]
fn test_fill_struct_fields_blank_line() {
check_fix(
r#"
struct S { a: (), b: () }
fn f() {
S {
$0
};
}
"#,
r#"
struct S { a: (), b: () }
fn f() {
S {
a: (),
b: (),
};
}
"#,
);
}
}

View File

@ -8,7 +8,10 @@ use crate::diagnostics::{fix, Diagnostic, DiagnosticsContext};
// Diagnostic: unresolved-module
//
// This diagnostic is triggered if rust-analyzer is unable to discover referred module.
pub(super) fn render(ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedModule) -> Diagnostic {
pub(super) fn unresolved_module(
ctx: &DiagnosticsContext<'_>,
d: &hir::UnresolvedModule,
) -> Diagnostic {
Diagnostic::new(
"unresolved-module",
"unresolved module",