From 9963f43d51071ea02f8f6d490b9c49882034b42c Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sun, 9 Aug 2020 01:59:26 +0300 Subject: Refactor the diagnostics --- crates/ra_hir_ty/src/diagnostics.rs | 97 +++++++++++++------------------- crates/ra_hir_ty/src/diagnostics/expr.rs | 12 ++-- 2 files changed, 44 insertions(+), 65 deletions(-) (limited to 'crates/ra_hir_ty') diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs index efca09619..1e3a44637 100644 --- a/crates/ra_hir_ty/src/diagnostics.rs +++ b/crates/ra_hir_ty/src/diagnostics.rs @@ -9,7 +9,7 @@ use hir_def::DefWithBodyId; use hir_expand::diagnostics::{AstDiagnostic, Diagnostic, DiagnosticSink}; use hir_expand::{db::AstDatabase, name::Name, HirFileId, InFile}; use ra_prof::profile; -use ra_syntax::{ast, AstNode, AstPtr, SyntaxNodePtr}; +use ra_syntax::{ast, AstPtr, SyntaxNodePtr}; use stdx::format_to; use crate::db::HirDatabase; @@ -37,7 +37,7 @@ impl Diagnostic for NoSuchField { "no such field".to_string() } - fn source(&self) -> InFile { + fn presentation(&self) -> InFile { InFile::new(self.file, self.field.clone().into()) } @@ -49,7 +49,7 @@ impl Diagnostic for NoSuchField { impl AstDiagnostic for NoSuchField { type AST = ast::RecordExprField; - fn ast(&self, db: &dyn AstDatabase) -> Self::AST { + fn fix_source(&self, db: &dyn AstDatabase) -> Self::AST { let root = db.parse_or_expand(self.file).unwrap(); self.field.to_node(&root) } @@ -58,7 +58,7 @@ impl AstDiagnostic for NoSuchField { #[derive(Debug)] pub struct MissingFields { pub file: HirFileId, - pub field_list: AstPtr, + pub field_list_parent: AstPtr, pub field_list_parent_path: Option>, pub missed_fields: Vec, } @@ -71,15 +71,16 @@ impl Diagnostic for MissingFields { } buf } - fn fix_source(&self) -> InFile { - InFile { file_id: self.file, value: self.field_list.clone().into() } - } - fn source(&self) -> InFile { - self.field_list_parent_path - .clone() - .map(|path| InFile { file_id: self.file, value: path.into() }) - .unwrap_or_else(|| self.fix_source()) + fn presentation(&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) { @@ -88,18 +89,18 @@ impl Diagnostic for MissingFields { } impl AstDiagnostic for MissingFields { - type AST = ast::RecordExprFieldList; + type AST = ast::RecordExpr; - fn ast(&self, db: &dyn AstDatabase) -> Self::AST { + fn fix_source(&self, db: &dyn AstDatabase) -> Self::AST { let root = db.parse_or_expand(self.file).unwrap(); - self.field_list.to_node(&root) + self.field_list_parent.to_node(&root) } } #[derive(Debug)] pub struct MissingPatFields { pub file: HirFileId, - pub field_list: AstPtr, + pub field_list_parent: AstPtr, pub field_list_parent_path: Option>, pub missed_fields: Vec, } @@ -112,14 +113,13 @@ impl Diagnostic for MissingPatFields { } buf } - fn fix_source(&self) -> InFile { - InFile { file_id: self.file, value: self.field_list.clone().into() } - } - fn source(&self) -> InFile { - self.field_list_parent_path + fn presentation(&self) -> InFile { + let value = self + .field_list_parent_path .clone() - .map(|path| InFile { file_id: self.file, value: path.into() }) - .unwrap_or_else(|| self.fix_source()) + .map(SyntaxNodePtr::from) + .unwrap_or_else(|| self.field_list_parent.clone().into()); + InFile { file_id: self.file, value } } fn as_any(&self) -> &(dyn Any + Send + 'static) { self @@ -137,7 +137,7 @@ impl Diagnostic for MissingMatchArms { fn message(&self) -> String { String::from("Missing match arm") } - fn source(&self) -> InFile { + fn presentation(&self) -> InFile { InFile { file_id: self.file, value: self.match_expr.clone().into() } } fn as_any(&self) -> &(dyn Any + Send + 'static) { @@ -155,7 +155,7 @@ impl Diagnostic for MissingOkInTailExpr { fn message(&self) -> String { "wrap return expression in Ok".to_string() } - fn source(&self) -> InFile { + fn presentation(&self) -> InFile { InFile { file_id: self.file, value: self.expr.clone().into() } } fn as_any(&self) -> &(dyn Any + Send + 'static) { @@ -166,7 +166,7 @@ impl Diagnostic for MissingOkInTailExpr { impl AstDiagnostic for MissingOkInTailExpr { type AST = ast::Expr; - fn ast(&self, db: &dyn AstDatabase) -> Self::AST { + fn fix_source(&self, db: &dyn AstDatabase) -> Self::AST { let root = db.parse_or_expand(self.file).unwrap(); self.expr.to_node(&root) } @@ -182,7 +182,7 @@ impl Diagnostic for BreakOutsideOfLoop { fn message(&self) -> String { "break outside of loop".to_string() } - fn source(&self) -> InFile { + fn presentation(&self) -> InFile { InFile { file_id: self.file, value: self.expr.clone().into() } } fn as_any(&self) -> &(dyn Any + Send + 'static) { @@ -190,15 +190,6 @@ impl Diagnostic for BreakOutsideOfLoop { } } -impl AstDiagnostic for BreakOutsideOfLoop { - type AST = ast::Expr; - - fn ast(&self, db: &dyn AstDatabase) -> Self::AST { - let root = db.parse_or_expand(self.file).unwrap(); - self.expr.to_node(&root) - } -} - #[derive(Debug)] pub struct MissingUnsafe { pub file: HirFileId, @@ -209,7 +200,7 @@ impl Diagnostic for MissingUnsafe { fn message(&self) -> String { format!("This operation is unsafe and requires an unsafe function or block") } - fn source(&self) -> InFile { + fn presentation(&self) -> InFile { InFile { file_id: self.file, value: self.expr.clone().into() } } fn as_any(&self) -> &(dyn Any + Send + 'static) { @@ -217,15 +208,6 @@ impl Diagnostic for MissingUnsafe { } } -impl AstDiagnostic for MissingUnsafe { - type AST = ast::Expr; - - fn ast(&self, db: &dyn AstDatabase) -> Self::AST { - let root = db.parse_or_expand(self.file).unwrap(); - self.expr.to_node(&root) - } -} - #[derive(Debug)] pub struct MismatchedArgCount { pub file: HirFileId, @@ -239,7 +221,7 @@ impl Diagnostic for MismatchedArgCount { let s = if self.expected == 1 { "" } else { "s" }; format!("Expected {} argument{}, found {}", self.expected, s, self.found) } - fn source(&self) -> InFile { + fn presentation(&self) -> InFile { InFile { file_id: self.file, value: self.call_expr.clone().into() } } fn as_any(&self) -> &(dyn Any + Send + 'static) { @@ -250,19 +232,13 @@ impl Diagnostic for MismatchedArgCount { } } -impl AstDiagnostic for MismatchedArgCount { - type AST = ast::CallExpr; - fn ast(&self, db: &dyn AstDatabase) -> Self::AST { - let root = db.parse_or_expand(self.file).unwrap(); - let node = self.source().value.to_node(&root); - ast::CallExpr::cast(node).unwrap() - } -} - #[cfg(test)] mod tests { use hir_def::{db::DefDatabase, AssocItemId, ModuleDefId}; - use hir_expand::diagnostics::{Diagnostic, DiagnosticSinkBuilder}; + use hir_expand::{ + db::AstDatabase, + diagnostics::{Diagnostic, DiagnosticSinkBuilder}, + }; use ra_db::{fixture::WithFixture, FileId, SourceDatabase, SourceDatabaseExt}; use ra_syntax::{TextRange, TextSize}; use rustc_hash::FxHashMap; @@ -308,8 +284,11 @@ mod tests { let mut actual: FxHashMap> = FxHashMap::default(); db.diagnostics(|d| { // FIXME: macros... - let file_id = d.source().file_id.original_file(&db); - let range = d.syntax_node(&db).text_range(); + let diagnostics_presentation = d.presentation(); + let root = db.parse_or_expand(diagnostics_presentation.file_id).unwrap(); + + let file_id = diagnostics_presentation.file_id.original_file(&db); + let range = diagnostics_presentation.value.to_node(&root).text_range(); let message = d.message().to_owned(); actual.entry(file_id).or_default().push((range, message)); }); diff --git a/crates/ra_hir_ty/src/diagnostics/expr.rs b/crates/ra_hir_ty/src/diagnostics/expr.rs index 98959ab68..51adcecaf 100644 --- a/crates/ra_hir_ty/src/diagnostics/expr.rs +++ b/crates/ra_hir_ty/src/diagnostics/expr.rs @@ -100,8 +100,8 @@ impl<'a, 'b> ExprValidator<'a, 'b> { if let Ok(source_ptr) = source_map.expr_syntax(id) { let root = source_ptr.file_syntax(db.upcast()); - if let ast::Expr::RecordExpr(record_lit) = &source_ptr.value.to_node(&root) { - if let Some(field_list) = record_lit.record_expr_field_list() { + 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_data(db.upcast(), variant_def); let missed_fields = missed_fields .into_iter() @@ -109,8 +109,8 @@ impl<'a, 'b> ExprValidator<'a, 'b> { .collect(); self.sink.push(MissingFields { file: source_ptr.file_id, - field_list: AstPtr::new(&field_list), - field_list_parent_path: record_lit.path().map(|path| AstPtr::new(&path)), + field_list_parent: AstPtr::new(&record_expr), + field_list_parent_path: record_expr.path().map(|path| AstPtr::new(&path)), missed_fields, }) } @@ -132,7 +132,7 @@ impl<'a, 'b> ExprValidator<'a, 'b> { 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(field_list) = record_pat.record_pat_field_list() { + if let Some(_) = record_pat.record_pat_field_list() { let variant_data = variant_data(db.upcast(), variant_def); let missed_fields = missed_fields .into_iter() @@ -140,7 +140,7 @@ impl<'a, 'b> ExprValidator<'a, 'b> { .collect(); self.sink.push(MissingPatFields { file: source_ptr.file_id, - field_list: AstPtr::new(&field_list), + field_list_parent: AstPtr::new(&record_pat), field_list_parent_path: record_pat .path() .map(|path| AstPtr::new(&path)), -- cgit v1.2.3