diff --git a/crates/ra_assists/src/handlers/fix_visibility.rs b/crates/ra_assists/src/handlers/fix_visibility.rs index 1aefa79cc3..a19dbf33f6 100644 --- a/crates/ra_assists/src/handlers/fix_visibility.rs +++ b/crates/ra_assists/src/handlers/fix_visibility.rs @@ -121,7 +121,7 @@ fn add_vis_to_referenced_record_field(acc: &mut Assists, ctx: &AssistContext) -> Some(cap) => match current_visibility { Some(current_visibility) => builder.replace_snippet( cap, - dbg!(current_visibility.syntax()).text_range(), + current_visibility.syntax().text_range(), format!("$0{}", missing_visibility), ), None => builder.insert_snippet(cap, offset, format!("$0{} ", missing_visibility)), diff --git a/crates/ra_hir/src/diagnostics.rs b/crates/ra_hir/src/diagnostics.rs index 266b513dcf..363164b9b4 100644 --- a/crates/ra_hir/src/diagnostics.rs +++ b/crates/ra_hir/src/diagnostics.rs @@ -1,8 +1,6 @@ //! FIXME: write short doc here pub use hir_def::diagnostics::UnresolvedModule; -pub use hir_expand::diagnostics::{ - AstDiagnostic, Diagnostic, DiagnosticSink, DiagnosticSinkBuilder, -}; +pub use hir_expand::diagnostics::{Diagnostic, DiagnosticSink, DiagnosticSinkBuilder}; pub use hir_ty::diagnostics::{ MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkInTailExpr, NoSuchField, }; diff --git a/crates/ra_hir/src/semantics.rs b/crates/ra_hir/src/semantics.rs index 872f5fa4c7..36b688ccb6 100644 --- a/crates/ra_hir/src/semantics.rs +++ b/crates/ra_hir/src/semantics.rs @@ -8,7 +8,7 @@ use hir_def::{ resolver::{self, HasResolver, Resolver}, AsMacroCall, FunctionId, TraitId, VariantId, }; -use hir_expand::{diagnostics::AstDiagnostic, hygiene::Hygiene, name::AsName, ExpansionInfo}; +use hir_expand::{hygiene::Hygiene, name::AsName, ExpansionInfo}; use hir_ty::associated_type_shorthand_candidates; use itertools::Itertools; use ra_db::{FileId, FileRange}; @@ -110,13 +110,6 @@ 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; - let root = self.db.parse_or_expand(file_id).unwrap(); - self.imp.cache(root, file_id); - d.ast(self.db.upcast()) - } - pub fn expand(&self, macro_call: &ast::MacroCall) -> Option { self.imp.expand(macro_call) } @@ -146,8 +139,8 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> { self.imp.original_range(node) } - pub fn diagnostics_range(&self, diagnostics: &dyn Diagnostic) -> FileRange { - self.imp.diagnostics_range(diagnostics) + pub fn diagnostics_display_range(&self, diagnostics: &dyn Diagnostic) -> FileRange { + self.imp.diagnostics_display_range(diagnostics) } pub fn ancestors_with_macros(&self, node: SyntaxNode) -> impl Iterator + '_ { @@ -389,10 +382,11 @@ impl<'db> SemanticsImpl<'db> { original_range(self.db, node.as_ref()) } - fn diagnostics_range(&self, diagnostics: &dyn Diagnostic) -> FileRange { - let src = diagnostics.source(); + fn diagnostics_display_range(&self, diagnostics: &dyn Diagnostic) -> FileRange { + let src = diagnostics.display_source(); let root = self.db.parse_or_expand(src.file_id).unwrap(); let node = src.value.to_node(&root); + self.cache(root, src.file_id); 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 30db48f868..71d177070d 100644 --- a/crates/ra_hir_def/src/diagnostics.rs +++ b/crates/ra_hir_def/src/diagnostics.rs @@ -18,7 +18,7 @@ impl Diagnostic for UnresolvedModule { fn message(&self) -> String { "unresolved module".to_string() } - fn source(&self) -> InFile { + fn display_source(&self) -> InFile { InFile::new(self.file, self.decl.clone().into()) } fn as_any(&self) -> &(dyn Any + Send + 'static) { diff --git a/crates/ra_hir_expand/src/diagnostics.rs b/crates/ra_hir_expand/src/diagnostics.rs index 84ba97b14a..b138500e73 100644 --- a/crates/ra_hir_expand/src/diagnostics.rs +++ b/crates/ra_hir_expand/src/diagnostics.rs @@ -16,35 +16,20 @@ use std::{any::Any, fmt}; -use ra_syntax::{SyntaxNode, SyntaxNodePtr}; +use ra_syntax::SyntaxNodePtr; -use crate::{db::AstDatabase, InFile}; +use crate::InFile; pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static { fn message(&self) -> String; - fn source(&self) -> InFile; + /// Used in highlighting and related purposes + fn display_source(&self) -> InFile; fn as_any(&self) -> &(dyn Any + Send + 'static); fn is_experimental(&self) -> bool { false } } -pub trait AstDiagnostic { - type AST; - fn ast(&self, db: &dyn AstDatabase) -> Self::AST; -} - -impl dyn Diagnostic { - pub fn syntax_node(&self, db: &impl AstDatabase) -> SyntaxNode { - let node = db.parse_or_expand(self.source().file_id).unwrap(); - self.source().value.to_node(&node) - } - - pub fn downcast_ref(&self) -> Option<&D> { - self.as_any().downcast_ref() - } -} - pub struct DiagnosticSink<'a> { callbacks: Vec Result<(), ()> + 'a>>, filters: Vec bool + 'a>>, @@ -89,7 +74,7 @@ impl<'a> DiagnosticSinkBuilder<'a> { } pub fn on(mut self, mut cb: F) -> Self { - let cb = move |diag: &dyn Diagnostic| match diag.downcast_ref::() { + let cb = move |diag: &dyn Diagnostic| match diag.as_any().downcast_ref::() { Some(d) => { cb(d); Ok(()) diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs index 977c0525b5..7ab7f79db6 100644 --- a/crates/ra_hir_ty/src/diagnostics.rs +++ b/crates/ra_hir_ty/src/diagnostics.rs @@ -6,10 +6,10 @@ mod unsafe_check; use std::any::Any; use hir_def::DefWithBodyId; -use hir_expand::diagnostics::{AstDiagnostic, Diagnostic, DiagnosticSink}; -use hir_expand::{db::AstDatabase, name::Name, HirFileId, InFile}; +use hir_expand::diagnostics::{Diagnostic, DiagnosticSink}; +use hir_expand::{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 display_source(&self) -> InFile { InFile::new(self.file, self.field.clone().into()) } @@ -46,20 +46,11 @@ impl Diagnostic for NoSuchField { } } -impl AstDiagnostic for NoSuchField { - type AST = ast::RecordExprField; - - fn ast(&self, db: &dyn AstDatabase) -> Self::AST { - let root = db.parse_or_expand(self.source().file_id).unwrap(); - let node = self.source().value.to_node(&root); - ast::RecordExprField::cast(node).unwrap() - } -} - #[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,28 +62,28 @@ impl Diagnostic for MissingFields { } buf } - fn source(&self) -> InFile { - InFile { file_id: self.file, value: self.field_list.clone().into() } + + 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 } } -impl AstDiagnostic for MissingFields { - type AST = ast::RecordExprFieldList; - - fn ast(&self, db: &dyn AstDatabase) -> Self::AST { - let root = db.parse_or_expand(self.source().file_id).unwrap(); - let node = self.source().value.to_node(&root); - ast::RecordExprFieldList::cast(node).unwrap() - } -} - #[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, } @@ -104,8 +95,15 @@ impl Diagnostic for MissingPatFields { } buf } - fn source(&self) -> InFile { - InFile { file_id: self.file, value: self.field_list.clone().into() } + 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 @@ -123,7 +121,7 @@ impl Diagnostic for MissingMatchArms { fn message(&self) -> String { String::from("Missing match arm") } - fn source(&self) -> InFile { + fn display_source(&self) -> InFile { InFile { file_id: self.file, value: self.match_expr.clone().into() } } fn as_any(&self) -> &(dyn Any + Send + 'static) { @@ -141,7 +139,7 @@ impl Diagnostic for MissingOkInTailExpr { fn message(&self) -> String { "wrap return expression in Ok".to_string() } - fn source(&self) -> InFile { + fn display_source(&self) -> InFile { InFile { file_id: self.file, value: self.expr.clone().into() } } fn as_any(&self) -> &(dyn Any + Send + 'static) { @@ -149,16 +147,6 @@ impl Diagnostic for MissingOkInTailExpr { } } -impl AstDiagnostic for MissingOkInTailExpr { - type AST = ast::Expr; - - 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::Expr::cast(node).unwrap() - } -} - #[derive(Debug)] pub struct BreakOutsideOfLoop { pub file: HirFileId, @@ -169,7 +157,7 @@ impl Diagnostic for BreakOutsideOfLoop { fn message(&self) -> String { "break outside of loop".to_string() } - fn source(&self) -> InFile { + fn display_source(&self) -> InFile { InFile { file_id: self.file, value: self.expr.clone().into() } } fn as_any(&self) -> &(dyn Any + Send + 'static) { @@ -177,16 +165,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(); - let node = self.source().value.to_node(&root); - ast::Expr::cast(node).unwrap() - } -} - #[derive(Debug)] pub struct MissingUnsafe { pub file: HirFileId, @@ -197,7 +175,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 display_source(&self) -> InFile { InFile { file_id: self.file, value: self.expr.clone().into() } } fn as_any(&self) -> &(dyn Any + Send + 'static) { @@ -205,16 +183,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.source().file_id).unwrap(); - let node = self.source().value.to_node(&root); - ast::Expr::cast(node).unwrap() - } -} - #[derive(Debug)] pub struct MismatchedArgCount { pub file: HirFileId, @@ -228,7 +196,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 display_source(&self) -> InFile { InFile { file_id: self.file, value: self.call_expr.clone().into() } } fn as_any(&self) -> &(dyn Any + Send + 'static) { @@ -239,19 +207,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.source().file_id).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; @@ -296,9 +258,11 @@ mod tests { let mut actual: FxHashMap> = FxHashMap::default(); db.diagnostics(|d| { - // FXIME: macros... - let file_id = d.source().file_id.original_file(&db); - let range = d.syntax_node(&db).text_range(); + let src = d.display_source(); + let root = db.parse_or_expand(src.file_id).unwrap(); + // FIXME: macros... + let file_id = src.file_id.original_file(&db); + let range = src.value.to_node(&root).text_range(); let message = d.message().to_owned(); actual.entry(file_id).or_default().push((range, message)); }); @@ -326,8 +290,8 @@ struct S { foo: i32, bar: () } impl S { fn new() -> S { S { - //^... Missing structure fields: - //| - bar + //^ Missing structure fields: + //| - bar foo: 92, baz: 62, //^^^^^^^ no such field @@ -448,8 +412,8 @@ impl Foo { struct S { foo: i32, bar: () } fn baz(s: S) { let S { foo: _ } = s; - //^^^^^^^^^^ Missing structure fields: - // | - bar + //^ Missing structure fields: + //| - bar } "#, ); diff --git a/crates/ra_hir_ty/src/diagnostics/expr.rs b/crates/ra_hir_ty/src/diagnostics/expr.rs index 95bbf2d955..51adcecafa 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,7 +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: AstPtr::new(&record_expr), + field_list_parent_path: record_expr.path().map(|path| AstPtr::new(&path)), missed_fields, }) } @@ -131,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() @@ -139,7 +140,10 @@ 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)), missed_fields, }) } diff --git a/crates/ra_hir_ty/src/diagnostics/match_check.rs b/crates/ra_hir_ty/src/diagnostics/match_check.rs index 507edcb7de..deca244dbb 100644 --- a/crates/ra_hir_ty/src/diagnostics/match_check.rs +++ b/crates/ra_hir_ty/src/diagnostics/match_check.rs @@ -1161,15 +1161,15 @@ fn main() { //^ Missing match arm match a { Either::A { } => (), - //^^^ Missing structure fields: - // | - foo + //^^^^^^^^^ Missing structure fields: + // | - foo Either::B => (), } match a { //^ Missing match arm Either::A { } => (), - } //^^^ Missing structure fields: - // | - foo + } //^^^^^^^^^ Missing structure fields: + // | - foo match a { Either::A { foo: true } => (), diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 73c0b82754..1046d7ab37 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -6,22 +6,21 @@ use std::cell::RefCell; -use hir::{ - diagnostics::{AstDiagnostic, Diagnostic as _, DiagnosticSinkBuilder}, - HasSource, HirDisplay, Semantics, VariantDef, -}; +use hir::{diagnostics::DiagnosticSinkBuilder, Semantics}; use itertools::Itertools; use ra_db::SourceDatabase; use ra_ide_db::RootDatabase; use ra_prof::profile; use ra_syntax::{ - algo, - ast::{self, edit::IndentLevel, make, AstNode}, + ast::{self, AstNode}, SyntaxNode, TextRange, T, }; use ra_text_edit::{TextEdit, TextEditBuilder}; -use crate::{Diagnostic, FileId, FileSystemEdit, Fix, SourceFileEdit}; +use crate::{Diagnostic, FileId, Fix, SourceFileEdit}; + +mod diagnostics_with_fix; +use diagnostics_with_fix::DiagnosticWithFix; #[derive(Debug, Copy, Clone)] pub enum Severity { @@ -54,73 +53,16 @@ pub(crate) fn diagnostics( let res = RefCell::new(res); let mut sink = DiagnosticSinkBuilder::new() .on::(|d| { - let original_file = d.source().file_id.original_file(db); - let fix = Fix::new( - "Create module", - FileSystemEdit::CreateFile { anchor: original_file, dst: d.candidate.clone() } - .into(), - ); - res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_range(d).range, - message: d.message(), - severity: Severity::Error, - fix: Some(fix), - }) + res.borrow_mut().push(diagnostic_with_fix(d, &sema)); }) .on::(|d| { - // 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. - 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 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())) - }; - - res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_range(d).range, - message: d.message(), - severity: Severity::Error, - fix, - }) + res.borrow_mut().push(diagnostic_with_fix(d, &sema)); }) .on::(|d| { - let node = d.ast(db); - let replacement = format!("Ok({})", node.syntax()); - let edit = TextEdit::replace(node.syntax().text_range(), replacement); - let source_change = SourceFileEdit { file_id, edit }.into(); - let fix = Fix::new("Wrap with ok", source_change); - res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_range(d).range, - message: d.message(), - severity: Severity::Error, - fix: Some(fix), - }) + res.borrow_mut().push(diagnostic_with_fix(d, &sema)); }) .on::(|d| { - res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_range(d).range, - message: d.message(), - severity: Severity::Error, - fix: missing_struct_field_fix(&sema, file_id, d), - }) + res.borrow_mut().push(diagnostic_with_fix(d, &sema)); }) // Only collect experimental diagnostics when they're enabled. .filter(|diag| !diag.is_experimental() || enable_experimental) @@ -128,7 +70,7 @@ pub(crate) fn diagnostics( .build(|d| { res.borrow_mut().push(Diagnostic { message: d.message(), - range: sema.diagnostics_range(d).range, + range: sema.diagnostics_display_range(d).range, severity: Severity::Error, fix: None, }) @@ -141,77 +83,12 @@ pub(crate) fn diagnostics( res.into_inner() } -fn missing_struct_field_fix( - sema: &Semantics, - usage_file_id: FileId, - d: &hir::diagnostics::NoSuchField, -) -> Option { - let record_expr = sema.ast(d); - - let record_lit = ast::RecordExpr::cast(record_expr.syntax().parent()?.parent()?)?; - let def_id = sema.resolve_variant(record_lit)?; - let module; - let def_file_id; - let record_fields = match VariantDef::from(def_id) { - VariantDef::Struct(s) => { - module = s.module(sema.db); - let source = s.source(sema.db); - def_file_id = source.file_id; - let fields = source.value.field_list()?; - record_field_list(fields)? - } - VariantDef::Union(u) => { - module = u.module(sema.db); - let source = u.source(sema.db); - def_file_id = source.file_id; - source.value.record_field_list()? - } - VariantDef::EnumVariant(e) => { - module = e.module(sema.db); - let source = e.source(sema.db); - def_file_id = source.file_id; - let fields = source.value.field_list()?; - record_field_list(fields)? - } - }; - let def_file_id = def_file_id.original_file(sema.db); - - let new_field_type = sema.type_of_expr(&record_expr.expr()?)?; - if new_field_type.is_unknown() { - return None; - } - let new_field = make::record_field( - record_expr.field_name()?, - make::ty(&new_field_type.display_source_code(sema.db, module.into()).ok()?), - ); - - let last_field = record_fields.fields().last()?; - let last_field_syntax = last_field.syntax(); - let indent = IndentLevel::from_node(last_field_syntax); - - let mut new_field = new_field.to_string(); - if usage_file_id != def_file_id { - new_field = format!("pub(crate) {}", new_field); - } - new_field = format!("\n{}{}", indent, new_field); - - let needs_comma = !last_field_syntax.to_string().ends_with(','); - if needs_comma { - new_field = format!(",{}", new_field); - } - - let source_change = SourceFileEdit { - file_id: def_file_id, - edit: TextEdit::insert(last_field_syntax.text_range().end(), new_field), - }; - let fix = Fix::new("Create field", source_change.into()); - return Some(fix); - - fn record_field_list(field_def_list: ast::FieldList) -> Option { - match field_def_list { - ast::FieldList::RecordFieldList(it) => Some(it), - ast::FieldList::TupleFieldList(_) => None, - } +fn diagnostic_with_fix(d: &D, sema: &Semantics) -> Diagnostic { + Diagnostic { + range: sema.diagnostics_display_range(d).range, + message: d.message(), + severity: Severity::Error, + fix: d.fix(&sema), } } @@ -222,24 +99,25 @@ fn check_unnecessary_braces_in_use_statement( ) -> Option<()> { let use_tree_list = ast::UseTreeList::cast(node.clone())?; if let Some((single_use_tree,)) = use_tree_list.use_trees().collect_tuple() { - let range = use_tree_list.syntax().text_range(); + let use_range = use_tree_list.syntax().text_range(); let edit = text_edit_for_remove_unnecessary_braces_with_self_in_use_statement(&single_use_tree) .unwrap_or_else(|| { let to_replace = single_use_tree.syntax().text().to_string(); let mut edit_builder = TextEditBuilder::default(); - edit_builder.delete(range); - edit_builder.insert(range.start(), to_replace); + edit_builder.delete(use_range); + edit_builder.insert(use_range.start(), to_replace); edit_builder.finish() }); acc.push(Diagnostic { - range, + range: use_range, message: "Unnecessary braces in use statement".to_string(), severity: Severity::WeakWarning, fix: Some(Fix::new( "Remove unnecessary braces", SourceFileEdit { file_id, edit }.into(), + use_range, )), }); } @@ -254,8 +132,7 @@ fn text_edit_for_remove_unnecessary_braces_with_self_in_use_statement( if single_use_tree.path()?.segment()?.syntax().first_child_or_token()?.kind() == T![self] { let start = use_tree_list_node.prev_sibling_or_token()?.text_range().start(); let end = use_tree_list_node.text_range().end(); - let range = TextRange::new(start, end); - return Some(TextEdit::delete(range)); + return Some(TextEdit::delete(TextRange::new(start, end))); } None } @@ -278,13 +155,15 @@ fn check_struct_shorthand_initialization( edit_builder.insert(record_field.syntax().text_range().start(), field_name); let edit = edit_builder.finish(); + let field_range = record_field.syntax().text_range(); acc.push(Diagnostic { - range: record_field.syntax().text_range(), + range: field_range, message: "Shorthand struct initialization".to_string(), severity: Severity::WeakWarning, fix: Some(Fix::new( "Use struct shorthand initialization", SourceFileEdit { file_id, edit }.into(), + field_range, )), }); } @@ -304,7 +183,7 @@ mod tests { /// Takes a multi-file input fixture with annotated cursor positions, /// and checks that: /// * a diagnostic is produced - /// * this diagnostic touches the input cursor position + /// * this diagnostic fix trigger range touches the input cursor position /// * that the contents of the file containing the cursor match `after` after the diagnostic fix is applied fn check_fix(ra_fixture_before: &str, ra_fixture_after: &str) { let after = trim_indent(ra_fixture_after); @@ -322,10 +201,10 @@ mod tests { assert_eq_text!(&after, &actual); assert!( - diagnostic.range.start() <= file_position.offset - && diagnostic.range.end() >= file_position.offset, - "diagnostic range {:?} does not touch cursor position {:?}", - diagnostic.range, + fix.fix_trigger_range.start() <= file_position.offset + && fix.fix_trigger_range.end() >= file_position.offset, + "diagnostic fix range {:?} does not touch cursor position {:?}", + fix.fix_trigger_range, file_position.offset ); } @@ -642,6 +521,7 @@ fn test_fn() { ], is_snippet: false, }, + fix_trigger_range: 0..8, }, ), }, diff --git a/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs b/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs new file mode 100644 index 0000000000..f7c73773f3 --- /dev/null +++ b/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs @@ -0,0 +1,171 @@ +//! Provides a way to attach fixes to the diagnostics. +//! The same module also has all curret custom fixes for the diagnostics implemented. +use crate::Fix; +use ast::{edit::IndentLevel, make}; +use hir::{ + db::AstDatabase, + diagnostics::{Diagnostic, MissingFields, MissingOkInTailExpr, NoSuchField, UnresolvedModule}, + HasSource, HirDisplay, Semantics, VariantDef, +}; +use ra_db::FileId; +use ra_ide_db::{ + source_change::{FileSystemEdit, SourceFileEdit}, + RootDatabase, +}; +use ra_syntax::{algo, ast, AstNode}; +use ra_text_edit::{TextEdit, TextEditBuilder}; + +/// A [Diagnostic] that potentially has a fix available. +/// +/// [Diagnostic]: hir::diagnostics::Diagnostic +pub trait DiagnosticWithFix: Diagnostic { + fn fix(&self, sema: &Semantics) -> Option; +} + +impl DiagnosticWithFix for UnresolvedModule { + fn fix(&self, sema: &Semantics) -> Option { + let root = sema.db.parse_or_expand(self.file)?; + let unresolved_module = self.decl.to_node(&root); + Some(Fix::new( + "Create module", + FileSystemEdit::CreateFile { + anchor: self.file.original_file(sema.db), + dst: self.candidate.clone(), + } + .into(), + unresolved_module.syntax().text_range(), + )) + } +} + +impl DiagnosticWithFix for NoSuchField { + fn fix(&self, sema: &Semantics) -> Option { + let root = sema.db.parse_or_expand(self.file)?; + missing_record_expr_field_fix( + &sema, + self.file.original_file(sema.db), + &self.field.to_node(&root), + ) + } +} + +impl DiagnosticWithFix for MissingFields { + fn fix(&self, sema: &Semantics) -> Option { + // 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 old_field_list = self.field_list_parent.to_node(&root).record_expr_field_list()?; + let mut new_field_list = old_field_list.clone(); + for f in self.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(&old_field_list.syntax(), &new_field_list.syntax()) + .into_text_edit(&mut builder); + builder.finish() + }; + Some(Fix::new( + "Fill struct fields", + SourceFileEdit { file_id: self.file.original_file(sema.db), edit }.into(), + sema.original_range(&old_field_list.syntax()).range, + )) + } +} + +impl DiagnosticWithFix for MissingOkInTailExpr { + fn fix(&self, sema: &Semantics) -> Option { + let root = sema.db.parse_or_expand(self.file)?; + let tail_expr = self.expr.to_node(&root); + 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: self.file.original_file(sema.db), edit }.into(); + Some(Fix::new("Wrap with ok", source_change, tail_expr_range)) + } +} + +fn missing_record_expr_field_fix( + sema: &Semantics, + usage_file_id: FileId, + record_expr_field: &ast::RecordExprField, +) -> Option { + 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; + let record_fields = match VariantDef::from(def_id) { + VariantDef::Struct(s) => { + module = s.module(sema.db); + let source = s.source(sema.db); + def_file_id = source.file_id; + let fields = source.value.field_list()?; + record_field_list(fields)? + } + VariantDef::Union(u) => { + module = u.module(sema.db); + let source = u.source(sema.db); + def_file_id = source.file_id; + source.value.record_field_list()? + } + VariantDef::EnumVariant(e) => { + module = e.module(sema.db); + let source = e.source(sema.db); + def_file_id = source.file_id; + let fields = source.value.field_list()?; + record_field_list(fields)? + } + }; + let def_file_id = def_file_id.original_file(sema.db); + + 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.field_name()?, + make::ty(&new_field_type.display_source_code(sema.db, module.into()).ok()?), + ); + + let last_field = record_fields.fields().last()?; + let last_field_syntax = last_field.syntax(); + let indent = IndentLevel::from_node(last_field_syntax); + + let mut new_field = new_field.to_string(); + if usage_file_id != def_file_id { + new_field = format!("pub(crate) {}", new_field); + } + new_field = format!("\n{}{}", indent, new_field); + + let needs_comma = !last_field_syntax.to_string().ends_with(','); + if needs_comma { + new_field = format!(",{}", new_field); + } + + let source_change = SourceFileEdit { + file_id: def_file_id, + edit: TextEdit::insert(last_field_syntax.text_range().end(), new_field), + }; + return Some(Fix::new( + "Create field", + source_change.into(), + record_expr_field.syntax().text_range(), + )); + + fn record_field_list(field_def_list: ast::FieldList) -> Option { + match field_def_list { + ast::FieldList::RecordFieldList(it) => Some(it), + ast::FieldList::TupleFieldList(_) => None, + } + } +} diff --git a/crates/ra_ide/src/lib.rs b/crates/ra_ide/src/lib.rs index 0fede0d879..89fcb6f178 100644 --- a/crates/ra_ide/src/lib.rs +++ b/crates/ra_ide/src/lib.rs @@ -112,13 +112,19 @@ pub struct Diagnostic { pub struct Fix { pub label: String, pub source_change: SourceChange, + /// Allows to trigger the fix only when the caret is in the range given + pub fix_trigger_range: TextRange, } impl Fix { - pub fn new(label: impl Into, source_change: SourceChange) -> Self { + pub fn new( + label: impl Into, + source_change: SourceChange, + fix_trigger_range: TextRange, + ) -> Self { let label = label.into(); assert!(label.starts_with(char::is_uppercase) && !label.ends_with('.')); - Self { label, source_change } + Self { label, source_change, fix_trigger_range } } } diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index c2afcf192d..785dd2a267 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/crates/rust-analyzer/src/handlers.rs @@ -773,12 +773,11 @@ fn handle_fixes( let diagnostics = snap.analysis.diagnostics(file_id, snap.config.experimental_diagnostics)?; - let fixes_from_diagnostics = diagnostics + for fix in diagnostics .into_iter() - .filter_map(|d| Some((d.range, d.fix?))) - .filter(|(diag_range, _fix)| diag_range.intersect(range).is_some()) - .map(|(_range, fix)| fix); - for fix in fixes_from_diagnostics { + .filter_map(|d| d.fix) + .filter(|fix| fix.fix_trigger_range.intersect(range).is_some()) + { let title = fix.label; let edit = to_proto::snippet_workspace_edit(&snap, fix.source_change)?; let action = lsp_ext::CodeAction {