Refactor the diagnostics

This commit is contained in:
Kirill Bulatov 2020-08-09 01:59:26 +03:00
parent cfbbd91a88
commit 9963f43d51
6 changed files with 106 additions and 136 deletions

View file

@ -109,11 +109,14 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> {
self.imp.parse(file_id)
}
pub fn ast<T: AstDiagnostic + Diagnostic>(&self, d: &T) -> <T as AstDiagnostic>::AST {
let file_id = d.source().file_id;
pub fn diagnostic_fix_source<T: AstDiagnostic + Diagnostic>(
&self,
d: &T,
) -> <T as AstDiagnostic>::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<SyntaxNode> {
@ -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<Item = SyntaxNode> + '_ {
@ -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))

View file

@ -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<SyntaxNodePtr> {
fn presentation(&self) -> InFile<SyntaxNodePtr> {
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)
}
}

View file

@ -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<SyntaxNodePtr>;
/// A source to be used during the fix application
fn fix_source(&self) -> InFile<SyntaxNodePtr> {
self.source()
}
fn presentation(&self) -> InFile<SyntaxNodePtr>;
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<D: Diagnostic>(&self) -> Option<&D> {
self.as_any().downcast_ref()
}

View file

@ -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<SyntaxNodePtr> {
fn presentation(&self) -> InFile<SyntaxNodePtr> {
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<ast::RecordExprFieldList>,
pub field_list_parent: AstPtr<ast::RecordExpr>,
pub field_list_parent_path: Option<AstPtr<ast::Path>>,
pub missed_fields: Vec<Name>,
}
@ -71,15 +71,16 @@ impl Diagnostic for MissingFields {
}
buf
}
fn fix_source(&self) -> InFile<SyntaxNodePtr> {
InFile { file_id: self.file, value: self.field_list.clone().into() }
}
fn source(&self) -> InFile<SyntaxNodePtr> {
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<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) {
@ -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<ast::RecordPatFieldList>,
pub field_list_parent: AstPtr<ast::RecordPat>,
pub field_list_parent_path: Option<AstPtr<ast::Path>>,
pub missed_fields: Vec<Name>,
}
@ -112,14 +113,13 @@ impl Diagnostic for MissingPatFields {
}
buf
}
fn fix_source(&self) -> InFile<SyntaxNodePtr> {
InFile { file_id: self.file, value: self.field_list.clone().into() }
}
fn source(&self) -> InFile<SyntaxNodePtr> {
self.field_list_parent_path
fn presentation(&self) -> InFile<SyntaxNodePtr> {
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<SyntaxNodePtr> {
fn presentation(&self) -> InFile<SyntaxNodePtr> {
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<SyntaxNodePtr> {
fn presentation(&self) -> InFile<SyntaxNodePtr> {
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<SyntaxNodePtr> {
fn presentation(&self) -> InFile<SyntaxNodePtr> {
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<SyntaxNodePtr> {
fn presentation(&self) -> InFile<SyntaxNodePtr> {
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<SyntaxNodePtr> {
fn presentation(&self) -> InFile<SyntaxNodePtr> {
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<FileId, Vec<(TextRange, String)>> = 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));
});

View file

@ -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)),

View file

@ -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::<hir::diagnostics::MissingFields, _>(|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::<hir::diagnostics::MissingOkInTailExpr, _>(|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::<hir::diagnostics::NoSuchField, _>(|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<Fix> {
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()?),
);