From 6940cfed1e24a67e816e69e1093e04c0eb73e070 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 12 Jun 2021 19:28:19 +0300 Subject: Move some hir_ty diagnostics to hir --- crates/hir/src/diagnostics.rs | 263 +++++++++++++++++++++++++++++++++++++++++- crates/hir/src/lib.rs | 175 ++++++++++++++++++++++++++-- 2 files changed, 421 insertions(+), 17 deletions(-) (limited to 'crates/hir') diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index e888fc23b..26dbcd86a 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -7,15 +7,12 @@ use std::any::Any; use cfg::{CfgExpr, CfgOptions, DnfExpr}; use hir_def::path::ModPath; -use hir_expand::{HirFileId, InFile}; +use hir_expand::{name::Name, HirFileId, InFile}; use stdx::format_to; use syntax::{ast, AstPtr, SyntaxNodePtr, TextRange}; pub use hir_ty::{ - diagnostics::{ - IncorrectCase, MismatchedArgCount, MissingFields, MissingMatchArms, - MissingOkOrSomeInTailExpr, RemoveThisSemicolon, ReplaceFilterMapNextWithFindMap, - }, + diagnostics::IncorrectCase, diagnostics_sink::{Diagnostic, DiagnosticCode, DiagnosticSink, DiagnosticSinkBuilder}, }; @@ -325,3 +322,259 @@ impl Diagnostic for MissingUnsafe { self } } + +// 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, + pub field_list_parent_path: Option>, + pub missed_fields: Vec, +} + +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 { + 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, + 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(..)`. +#[derive(Debug)] +pub struct ReplaceFilterMapNextWithFindMap { + pub file: HirFileId, + /// This expression is the whole method chain up to and including `.filter_map(..).next()`. + pub next_expr: AstPtr, +} + +impl Diagnostic for ReplaceFilterMapNextWithFindMap { + fn code(&self) -> DiagnosticCode { + DiagnosticCode("replace-filter-map-next-with-find-map") + } + fn message(&self) -> String { + "replace filter_map(..).next() with find_map(..)".to_string() + } + fn display_source(&self) -> InFile { + InFile { file_id: self.file, value: self.next_expr.clone().into() } + } + fn as_any(&self) -> &(dyn Any + Send + 'static) { + self + } +} + +// Diagnostic: mismatched-arg-count +// +// This diagnostic is triggered if a function is invoked with an incorrect amount of arguments. +#[derive(Debug)] +pub struct MismatchedArgCount { + pub file: HirFileId, + pub call_expr: AstPtr, + pub expected: usize, + pub found: usize, +} + +impl Diagnostic for MismatchedArgCount { + fn code(&self) -> DiagnosticCode { + DiagnosticCode("mismatched-arg-count") + } + fn message(&self) -> String { + let s = if self.expected == 1 { "" } else { "s" }; + format!("Expected {} argument{}, found {}", self.expected, s, self.found) + } + fn display_source(&self) -> InFile { + InFile { file_id: self.file, value: self.call_expr.clone().into() } + } + fn as_any(&self) -> &(dyn Any + Send + 'static) { + self + } + fn is_experimental(&self) -> bool { + true + } +} + +#[derive(Debug)] +pub struct RemoveThisSemicolon { + pub file: HirFileId, + pub expr: AstPtr, +} + +impl Diagnostic for RemoveThisSemicolon { + fn code(&self) -> DiagnosticCode { + DiagnosticCode("remove-this-semicolon") + } + + fn message(&self) -> String { + "Remove this semicolon".to_string() + } + + fn display_source(&self) -> InFile { + InFile { file_id: self.file, value: self.expr.clone().into() } + } + + fn as_any(&self) -> &(dyn Any + Send + 'static) { + self + } +} + +// Diagnostic: missing-ok-or-some-in-tail-expr +// +// This diagnostic is triggered if a block that should return `Result` returns a value not wrapped in `Ok`, +// or if a block that should return `Option` returns a value not wrapped in `Some`. +// +// Example: +// +// ```rust +// fn foo() -> Result { +// 10 +// } +// ``` +#[derive(Debug)] +pub struct MissingOkOrSomeInTailExpr { + pub file: HirFileId, + pub expr: AstPtr, + // `Some` or `Ok` depending on whether the return type is Result or Option + pub required: String, +} + +impl Diagnostic for MissingOkOrSomeInTailExpr { + fn code(&self) -> DiagnosticCode { + DiagnosticCode("missing-ok-or-some-in-tail-expr") + } + fn message(&self) -> String { + format!("wrap return expression in {}", self.required) + } + fn display_source(&self) -> InFile { + InFile { file_id: self.file, value: self.expr.clone().into() } + } + fn as_any(&self) -> &(dyn Any + Send + 'static) { + self + } +} + +// Diagnostic: missing-match-arm +// +// This diagnostic is triggered if `match` block is missing one or more match arms. +#[derive(Debug)] +pub struct MissingMatchArms { + pub file: HirFileId, + pub match_expr: AstPtr, + pub arms: AstPtr, +} + +impl Diagnostic for MissingMatchArms { + fn code(&self) -> DiagnosticCode { + DiagnosticCode("missing-match-arm") + } + fn message(&self) -> String { + String::from("Missing match arm") + } + fn display_source(&self) -> InFile { + InFile { file_id: self.file, value: self.match_expr.clone().into() } + } + fn as_any(&self) -> &(dyn Any + Send + 'static) { + self + } +} + +#[derive(Debug)] +pub struct InternalBailedOut { + pub file: HirFileId, + pub pat_syntax_ptr: SyntaxNodePtr, +} + +impl Diagnostic for InternalBailedOut { + fn code(&self) -> DiagnosticCode { + DiagnosticCode("internal:match-check-bailed-out") + } + fn message(&self) -> String { + format!("Internal: match check bailed out") + } + fn display_source(&self) -> InFile { + InFile { file_id: self.file, value: self.pat_syntax_ptr.clone() } + } + fn as_any(&self) -> &(dyn Any + Send + 'static) { + self + } +} diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 8804e63c5..dd5515c2b 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -36,9 +36,11 @@ use std::{iter, sync::Arc}; use arrayvec::ArrayVec; use base_db::{CrateDisplayName, CrateId, Edition, FileId}; use diagnostics::{ - BreakOutsideOfLoop, InactiveCode, MacroError, MissingUnsafe, NoSuchField, - UnimplementedBuiltinMacro, UnresolvedExternCrate, UnresolvedImport, UnresolvedMacroCall, - UnresolvedModule, UnresolvedProcMacro, + BreakOutsideOfLoop, InactiveCode, InternalBailedOut, MacroError, MismatchedArgCount, + MissingFields, MissingOkOrSomeInTailExpr, MissingPatFields, MissingUnsafe, NoSuchField, + RemoveThisSemicolon, ReplaceFilterMapNextWithFindMap, UnimplementedBuiltinMacro, + UnresolvedExternCrate, UnresolvedImport, UnresolvedMacroCall, UnresolvedModule, + UnresolvedProcMacro, }; use either::Either; use hir_def::{ @@ -61,6 +63,7 @@ use hir_ty::{ autoderef, consteval::ConstExt, could_unify, + diagnostics::BodyValidationDiagnostic, diagnostics_sink::DiagnosticSink, method_resolution::{self, def_crates, TyFingerprint}, primitive::UintTy, @@ -82,7 +85,10 @@ use syntax::{ }; use tt::{Ident, Leaf, Literal, TokenTree}; -use crate::db::{DefDatabase, HirDatabase}; +use crate::{ + db::{DefDatabase, HirDatabase}, + diagnostics::MissingMatchArms, +}; pub use crate::{ attrs::{HasAttrs, Namespace}, @@ -447,7 +453,12 @@ impl Module { self.id.def_map(db.upcast())[self.id.local_id].scope.visibility_of(def.clone().into()) } - pub fn diagnostics(self, db: &dyn HirDatabase, sink: &mut DiagnosticSink) { + pub fn diagnostics( + self, + db: &dyn HirDatabase, + sink: &mut DiagnosticSink, + internal_diagnostics: bool, + ) { let _p = profile::span("Module::diagnostics").detail(|| { format!("{:?}", self.name(db).map_or("".into(), |name| name.to_string())) }); @@ -593,11 +604,11 @@ impl Module { } for decl in self.declarations(db) { match decl { - crate::ModuleDef::Function(f) => f.diagnostics(db, sink), + crate::ModuleDef::Function(f) => f.diagnostics(db, sink, internal_diagnostics), crate::ModuleDef::Module(m) => { // Only add diagnostics from inline modules if def_map[m.id.local_id].origin.is_inline() { - m.diagnostics(db, sink) + m.diagnostics(db, sink, internal_diagnostics) } } _ => { @@ -609,7 +620,7 @@ impl Module { 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); + f.diagnostics(db, sink, internal_diagnostics); } } } @@ -1011,7 +1022,12 @@ impl Function { db.function_data(self.id).is_async() } - pub fn diagnostics(self, db: &dyn HirDatabase, sink: &mut DiagnosticSink) { + pub fn diagnostics( + self, + db: &dyn HirDatabase, + sink: &mut DiagnosticSink, + internal_diagnostics: bool, + ) { let krate = self.module(db).id.krate(); let source_map = db.body_with_source_map(self.id.into()).1; @@ -1067,14 +1083,149 @@ impl Function { sink.push(MissingUnsafe { file: in_file.file_id, expr: in_file.value }) } Err(SyntheticSyntax) => { - // FIXME: The `expr` was desugared, report or assert that - // this doesn't happen. + // FIXME: Here and eslwhere in this file, the `expr` was + // desugared, report or assert that this doesn't happen. + } + } + } + + for diagnostic in + BodyValidationDiagnostic::collect(db, self.id.into(), internal_diagnostics) + { + match diagnostic { + BodyValidationDiagnostic::RecordLiteralMissingFields { + record_expr, + 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, + }) + } + } + } + } + Err(SyntheticSyntax) => (), + }, + + BodyValidationDiagnostic::ReplaceFilterMapNextWithFindMap { method_call_expr } => { + if let Ok(next_source_ptr) = source_map.expr_syntax(method_call_expr) { + sink.push(ReplaceFilterMapNextWithFindMap { + file: next_source_ptr.file_id, + next_expr: next_source_ptr.value, + }); + } + } + BodyValidationDiagnostic::MismatchedArgCount { call_expr, expected, found } => { + match source_map.expr_syntax(call_expr) { + Ok(source_ptr) => sink.push(MismatchedArgCount { + file: source_ptr.file_id, + call_expr: source_ptr.value, + expected, + found, + }), + Err(SyntheticSyntax) => (), + } + } + BodyValidationDiagnostic::RemoveThisSemicolon { expr } => { + match source_map.expr_syntax(expr) { + Ok(source_ptr) => sink.push(RemoveThisSemicolon { + file: source_ptr.file_id, + expr: source_ptr.value, + }), + Err(SyntheticSyntax) => (), + } + } + BodyValidationDiagnostic::MissingOkOrSomeInTailExpr { expr, required } => { + match source_map.expr_syntax(expr) { + Ok(source_ptr) => sink.push(MissingOkOrSomeInTailExpr { + file: source_ptr.file_id, + expr: source_ptr.value, + required, + }), + Err(SyntheticSyntax) => (), + } + } + BodyValidationDiagnostic::MissingMatchArms { match_expr } => { + match source_map.expr_syntax(match_expr) { + Ok(source_ptr) => { + let root = source_ptr.file_syntax(db.upcast()); + if let ast::Expr::MatchExpr(match_expr) = + &source_ptr.value.to_node(&root) + { + if let (Some(match_expr), Some(arms)) = + (match_expr.expr(), match_expr.match_arm_list()) + { + sink.push(MissingMatchArms { + file: source_ptr.file_id, + match_expr: AstPtr::new(&match_expr), + arms: AstPtr::new(&arms), + }) + } + } + } + Err(SyntheticSyntax) => (), + } + } + BodyValidationDiagnostic::InternalBailedOut { pat } => { + match source_map.pat_syntax(pat) { + Ok(source_ptr) => { + let pat_syntax_ptr = source_ptr.value.either(Into::into, Into::into); + sink.push(InternalBailedOut { + file: source_ptr.file_id, + pat_syntax_ptr, + }); + } + Err(SyntheticSyntax) => (), + } } } } hir_ty::diagnostics::validate_module_item(db, krate, self.id.into(), sink); - hir_ty::diagnostics::validate_body(db, self.id.into(), sink); } /// Whether this function declaration has a definition. -- cgit v1.2.3