diff --git a/crates/ra_hir/src/semantics.rs b/crates/ra_hir/src/semantics.rs index 1c5dc3d5102..b4420d3785f 100644 --- a/crates/ra_hir/src/semantics.rs +++ b/crates/ra_hir/src/semantics.rs @@ -109,11 +109,14 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> { self.imp.parse(file_id) } - pub fn ast(&self, d: &T) -> ::AST { - let file_id = d.source().file_id; + pub fn diagnostic_fix_source( + &self, + d: &T, + ) -> ::AST { + let file_id = d.presentation().file_id; let root = self.db.parse_or_expand(file_id).unwrap(); self.imp.cache(root, file_id); - d.ast(self.db.upcast()) + d.fix_source(self.db.upcast()) } pub fn expand(&self, macro_call: &ast::MacroCall) -> Option { @@ -145,12 +148,8 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> { self.imp.original_range(node) } - pub fn diagnostics_fix_range(&self, diagnostics: &dyn Diagnostic) -> FileRange { - self.imp.diagnostics_fix_range(diagnostics) - } - - pub fn diagnostics_range(&self, diagnostics: &dyn Diagnostic) -> FileRange { - self.imp.diagnostics_range(diagnostics) + pub fn diagnostics_presentation_range(&self, diagnostics: &dyn Diagnostic) -> FileRange { + self.imp.diagnostics_presentation_range(diagnostics) } pub fn ancestors_with_macros(&self, node: SyntaxNode) -> impl Iterator + '_ { @@ -380,15 +379,8 @@ impl<'db> SemanticsImpl<'db> { original_range(self.db, node.as_ref()) } - fn diagnostics_fix_range(&self, diagnostics: &dyn Diagnostic) -> FileRange { - let src = diagnostics.fix_source(); - let root = self.db.parse_or_expand(src.file_id).unwrap(); - let node = src.value.to_node(&root); - original_range(self.db, src.with_value(&node)) - } - - fn diagnostics_range(&self, diagnostics: &dyn Diagnostic) -> FileRange { - let src = diagnostics.source(); + fn diagnostics_presentation_range(&self, diagnostics: &dyn Diagnostic) -> FileRange { + let src = diagnostics.presentation(); let root = self.db.parse_or_expand(src.file_id).unwrap(); let node = src.value.to_node(&root); original_range(self.db, src.with_value(&node)) diff --git a/crates/ra_hir_def/src/diagnostics.rs b/crates/ra_hir_def/src/diagnostics.rs index 30db48f8682..be961284655 100644 --- a/crates/ra_hir_def/src/diagnostics.rs +++ b/crates/ra_hir_def/src/diagnostics.rs @@ -2,7 +2,7 @@ use std::any::Any; -use hir_expand::diagnostics::Diagnostic; +use hir_expand::diagnostics::{AstDiagnostic, Diagnostic}; use ra_syntax::{ast, AstPtr, SyntaxNodePtr}; use hir_expand::{HirFileId, InFile}; @@ -18,10 +18,18 @@ impl Diagnostic for UnresolvedModule { fn message(&self) -> String { "unresolved module".to_string() } - fn source(&self) -> InFile { + fn presentation(&self) -> InFile { InFile::new(self.file, self.decl.clone().into()) } fn as_any(&self) -> &(dyn Any + Send + 'static) { self } } + +impl AstDiagnostic for UnresolvedModule { + type AST = ast::Module; + fn fix_source(&self, db: &dyn hir_expand::db::AstDatabase) -> Self::AST { + let root = db.parse_or_expand(self.file).unwrap(); + self.decl.to_node(&root) + } +} diff --git a/crates/ra_hir_expand/src/diagnostics.rs b/crates/ra_hir_expand/src/diagnostics.rs index 90a3b87f96a..2b74473cec9 100644 --- a/crates/ra_hir_expand/src/diagnostics.rs +++ b/crates/ra_hir_expand/src/diagnostics.rs @@ -16,18 +16,13 @@ use std::{any::Any, fmt}; -use ra_syntax::{SyntaxNode, SyntaxNodePtr}; +use ra_syntax::SyntaxNodePtr; use crate::{db::AstDatabase, InFile}; pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static { fn message(&self) -> String; - /// A source to be used in highlighting and other visual representations - fn source(&self) -> InFile; - /// A source to be used during the fix application - fn fix_source(&self) -> InFile { - self.source() - } + fn presentation(&self) -> InFile; fn as_any(&self) -> &(dyn Any + Send + 'static); fn is_experimental(&self) -> bool { false @@ -36,16 +31,10 @@ pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static { pub trait AstDiagnostic { type AST; - fn ast(&self, db: &dyn AstDatabase) -> Self::AST; + fn fix_source(&self, db: &dyn AstDatabase) -> Self::AST; } impl dyn Diagnostic { - pub fn syntax_node(&self, db: &impl AstDatabase) -> SyntaxNode { - let source = self.source(); - let node = db.parse_or_expand(source.file_id).unwrap(); - source.value.to_node(&node) - } - pub fn downcast_ref(&self) -> Option<&D> { self.as_any().downcast_ref() } diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs index efca0961995..1e3a446375b 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 98959ab684a..51adcecafae 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)), diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 0d2ff17e1f1..55593a8cb80 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -7,7 +7,7 @@ use std::cell::RefCell; use hir::{ - diagnostics::{AstDiagnostic, Diagnostic as _, DiagnosticSinkBuilder}, + diagnostics::{Diagnostic as _, DiagnosticSinkBuilder}, HasSource, HirDisplay, Semantics, VariantDef, }; use itertools::Itertools; @@ -63,10 +63,10 @@ pub(crate) fn diagnostics( .into(), ); res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_range(d).range, + range: sema.diagnostics_presentation_range(d).range, message: d.message(), severity: Severity::Error, - fix: Some((fix, sema.diagnostics_fix_range(d).range)), + fix: Some((fix, sema.diagnostic_fix_source(d).syntax().text_range())), }) }) .on::(|d| { @@ -78,56 +78,58 @@ pub(crate) fn diagnostics( let fix = if d.missed_fields.iter().any(|it| it.as_tuple_index().is_some()) { None } else { - let mut field_list = d.ast(db); - for f in d.missed_fields.iter() { - let field = make::record_expr_field( - make::name_ref(&f.to_string()), - Some(make::expr_unit()), - ); - field_list = field_list.append_field(&field); - } + let record_expr = sema.diagnostic_fix_source(d); + if let Some(old_field_list) = record_expr.record_expr_field_list() { + let mut new_field_list = old_field_list.clone(); + for f in d.missed_fields.iter() { + let field = make::record_expr_field( + make::name_ref(&f.to_string()), + Some(make::expr_unit()), + ); + new_field_list = new_field_list.append_field(&field); + } - let edit = { - let mut builder = TextEditBuilder::default(); - algo::diff(&d.ast(db).syntax(), &field_list.syntax()) - .into_text_edit(&mut builder); - builder.finish() - }; - Some(( - Fix::new("Fill struct fields", SourceFileEdit { file_id, edit }.into()), - sema.diagnostics_fix_range(d).range, - )) + let edit = { + let mut builder = TextEditBuilder::default(); + algo::diff(&old_field_list.syntax(), &new_field_list.syntax()) + .into_text_edit(&mut builder); + builder.finish() + }; + Some(( + Fix::new("Fill struct fields", SourceFileEdit { file_id, edit }.into()), + sema.original_range(&old_field_list.syntax()).range, + )) + } else { + None + } }; res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_range(d).range, + range: sema.diagnostics_presentation_range(d).range, message: d.message(), severity: Severity::Error, fix, }) }) .on::(|d| { - let node = d.ast(db); - let replacement = format!("Ok({})", node.syntax()); - let edit = TextEdit::replace(node.syntax().text_range(), replacement); + let tail_expr = sema.diagnostic_fix_source(d); + let tail_expr_range = tail_expr.syntax().text_range(); + let edit = TextEdit::replace(tail_expr_range, format!("Ok({})", tail_expr.syntax())); let source_change = SourceFileEdit { file_id, edit }.into(); res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_range(d).range, + range: sema.diagnostics_presentation_range(d).range, message: d.message(), severity: Severity::Error, - fix: Some(( - Fix::new("Wrap with ok", source_change), - sema.diagnostics_fix_range(d).range, - )), + fix: Some((Fix::new("Wrap with ok", source_change), tail_expr_range)), }) }) .on::(|d| { res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_range(d).range, + range: sema.diagnostics_presentation_range(d).range, message: d.message(), severity: Severity::Error, fix: missing_struct_field_fix(&sema, file_id, d) - .map(|fix| (fix, sema.diagnostics_fix_range(d).range)), + .map(|fix| (fix, sema.diagnostic_fix_source(d).syntax().text_range())), }) }) // Only collect experimental diagnostics when they're enabled. @@ -136,7 +138,7 @@ pub(crate) fn diagnostics( .build(|d| { res.borrow_mut().push(Diagnostic { message: d.message(), - range: sema.diagnostics_range(d).range, + range: sema.diagnostics_presentation_range(d).range, severity: Severity::Error, fix: None, }) @@ -154,9 +156,9 @@ fn missing_struct_field_fix( usage_file_id: FileId, d: &hir::diagnostics::NoSuchField, ) -> Option { - let record_expr = sema.ast(d); + let record_expr_field = sema.diagnostic_fix_source(d); - let record_lit = ast::RecordExpr::cast(record_expr.syntax().parent()?.parent()?)?; + let record_lit = ast::RecordExpr::cast(record_expr_field.syntax().parent()?.parent()?)?; let def_id = sema.resolve_variant(record_lit)?; let module; let def_file_id; @@ -184,12 +186,12 @@ fn missing_struct_field_fix( }; let def_file_id = def_file_id.original_file(sema.db); - let new_field_type = sema.type_of_expr(&record_expr.expr()?)?; + let new_field_type = sema.type_of_expr(&record_expr_field.expr()?)?; if new_field_type.is_unknown() { return None; } let new_field = make::record_field( - record_expr.field_name()?, + record_expr_field.field_name()?, make::ty(&new_field_type.display_source_code(sema.db, module.into()).ok()?), );