From 26e102a567aadcf86f2e5b575cb6b915991ba088 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 27 Jul 2020 15:53:57 +0300 Subject: [PATCH 01/14] Separate diagnostics and diagnostics fix ranges --- crates/ra_ide/src/diagnostics.rs | 98 ++++++++++++++++------------ crates/ra_ide/src/lib.rs | 2 +- crates/rust-analyzer/src/handlers.rs | 6 +- 3 files changed, 59 insertions(+), 47 deletions(-) diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 73c0b82754..5c8ea46abb 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -60,14 +60,16 @@ pub(crate) fn diagnostics( FileSystemEdit::CreateFile { anchor: original_file, dst: d.candidate.clone() } .into(), ); + let range = sema.diagnostics_range(d).range; res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_range(d).range, + range, message: d.message(), severity: Severity::Error, - fix: Some(fix), + fix: Some((fix, range)), }) }) .on::(|d| { + let range = sema.diagnostics_range(d).range; // Note that although we could add a diagnostics to // fill the missing tuple field, e.g : // `struct A(usize);` @@ -91,11 +93,15 @@ pub(crate) fn diagnostics( .into_text_edit(&mut builder); builder.finish() }; - Some(Fix::new("Fill struct fields", SourceFileEdit { file_id, edit }.into())) + Some(( + Fix::new("Fill struct fields", SourceFileEdit { file_id, edit }.into()), + range, + )) }; res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_range(d).range, + // TODO kb use a smaller range here + range, message: d.message(), severity: Severity::Error, fix, @@ -106,20 +112,21 @@ pub(crate) fn diagnostics( 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); + let range = sema.diagnostics_range(d).range; res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_range(d).range, + range, message: d.message(), severity: Severity::Error, - fix: Some(fix), + fix: Some((Fix::new("Wrap with ok", source_change), range)), }) }) .on::(|d| { + let range = sema.diagnostics_range(d).range; res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_range(d).range, + range, message: d.message(), severity: Severity::Error, - fix: missing_struct_field_fix(&sema, file_id, d), + fix: missing_struct_field_fix(&sema, file_id, d).map(|fix| (fix, range)), }) }) // Only collect experimental diagnostics when they're enabled. @@ -222,24 +229,24 @@ 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(), + fix: Some(( + Fix::new("Remove unnecessary braces", SourceFileEdit { file_id, edit }.into()), + use_range, )), }); } @@ -254,8 +261,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 +284,17 @@ 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(), + fix: Some(( + Fix::new( + "Use struct shorthand initialization", + SourceFileEdit { file_id, edit }.into(), + ), + field_range, )), }); } @@ -304,14 +314,14 @@ 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 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); let (analysis, file_position) = analysis_and_position(ra_fixture_before); let diagnostic = analysis.diagnostics(file_position.file_id, true).unwrap().pop().unwrap(); - let mut fix = diagnostic.fix.unwrap(); + let (mut fix, fix_range) = diagnostic.fix.unwrap(); let edit = fix.source_change.source_file_edits.pop().unwrap().edit; let target_file_contents = analysis.file_text(file_position.file_id).unwrap(); let actual = { @@ -322,10 +332,9 @@ 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_range.start() <= file_position.offset && fix_range.end() >= file_position.offset, + "diagnostic fix range {:?} does not touch cursor position {:?}", + fix_range, file_position.offset ); } @@ -337,7 +346,7 @@ mod tests { let (analysis, file_pos) = analysis_and_position(ra_fixture_before); let current_file_id = file_pos.file_id; let diagnostic = analysis.diagnostics(current_file_id, true).unwrap().pop().unwrap(); - let mut fix = diagnostic.fix.unwrap(); + let mut fix = diagnostic.fix.unwrap().0; let edit = fix.source_change.source_file_edits.pop().unwrap(); let changed_file_id = edit.file_id; let before = analysis.file_text(changed_file_id).unwrap(); @@ -628,21 +637,24 @@ fn test_fn() { range: 0..8, severity: Error, fix: Some( - Fix { - label: "Create module", - source_change: SourceChange { - source_file_edits: [], - file_system_edits: [ - CreateFile { - anchor: FileId( - 1, - ), - dst: "foo.rs", - }, - ], - is_snippet: false, + ( + Fix { + label: "Create module", + source_change: SourceChange { + source_file_edits: [], + file_system_edits: [ + CreateFile { + anchor: FileId( + 1, + ), + dst: "foo.rs", + }, + ], + is_snippet: false, + }, }, - }, + 0..8, + ), ), }, ] diff --git a/crates/ra_ide/src/lib.rs b/crates/ra_ide/src/lib.rs index 0fede0d879..45a4b2421e 100644 --- a/crates/ra_ide/src/lib.rs +++ b/crates/ra_ide/src/lib.rs @@ -105,7 +105,7 @@ pub struct Diagnostic { pub message: String, pub range: TextRange, pub severity: Severity, - pub fix: Option, + pub fix: Option<(Fix, TextRange)>, } #[derive(Debug)] diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index c2afcf192d..144c641b2a 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/crates/rust-analyzer/src/handlers.rs @@ -775,9 +775,9 @@ fn handle_fixes( let fixes_from_diagnostics = 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); + .filter_map(|d| d.fix) + .filter(|(_fix, fix_range)| fix_range.intersect(range).is_some()) + .map(|(fix, _range)| fix); for fix in fixes_from_diagnostics { let title = fix.label; let edit = to_proto::snippet_workspace_edit(&snap, fix.source_change)?; From 21e5224484b9214648826e1b15aa9150c79a407c Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 27 Jul 2020 18:45:08 +0300 Subject: [PATCH 02/14] Custom ranges for missing fields --- crates/ra_hir_expand/src/diagnostics.rs | 5 ++- crates/ra_hir_ty/src/diagnostics.rs | 52 ++++++++++++++++++++++-- crates/ra_hir_ty/src/diagnostics/expr.rs | 1 + crates/ra_ide/src/diagnostics.rs | 6 ++- 4 files changed, 56 insertions(+), 8 deletions(-) diff --git a/crates/ra_hir_expand/src/diagnostics.rs b/crates/ra_hir_expand/src/diagnostics.rs index 84ba97b14a..e889f070fa 100644 --- a/crates/ra_hir_expand/src/diagnostics.rs +++ b/crates/ra_hir_expand/src/diagnostics.rs @@ -36,8 +36,9 @@ pub trait AstDiagnostic { 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) + 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> { diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs index 977c0525b5..a5b00ed485 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, AstNode, AstPtr, SyntaxNode, SyntaxNodePtr}; use stdx::format_to; use crate::db::HirDatabase; @@ -61,6 +61,17 @@ pub struct MissingFields { pub file: HirFileId, pub field_list: AstPtr, pub missed_fields: Vec, + pub list_parent_path: Option>, +} + +impl MissingFields { + fn root(&self, db: &dyn AstDatabase) -> SyntaxNode { + db.parse_or_expand(self.file).unwrap() + } + + pub fn list_parent_ast(&self, db: &dyn AstDatabase) -> Option { + self.list_parent_path.as_ref().map(|path| path.to_node(&self.root(db))) + } } impl Diagnostic for MissingFields { @@ -83,9 +94,7 @@ 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() + self.field_list.to_node(&self.root(db)) } } @@ -318,6 +327,41 @@ mod tests { assert_eq!(annotations, actual); } + #[test] + fn structure_name_highlighted_for_missing_fields() { + check_diagnostics( + r#" +struct Beefy { + one: i32, + two: i32, + three: i32, + four: i32, + five: i32, + six: i32, + seven: i32, + eight: i32, + nine: i32, + ten: i32, +} +fn baz() { + let zz = Beefy { + //^^^^^... Missing structure fields: + // | - seven + one: (), + two: (), + three: (), + four: (), + five: (), + six: (), + eight: (), + nine: (), + ten: (), + }; +} +"#, + ); + } + #[test] fn no_such_field_diagnostics() { check_diagnostics( diff --git a/crates/ra_hir_ty/src/diagnostics/expr.rs b/crates/ra_hir_ty/src/diagnostics/expr.rs index 95bbf2d955..3c37fc58e9 100644 --- a/crates/ra_hir_ty/src/diagnostics/expr.rs +++ b/crates/ra_hir_ty/src/diagnostics/expr.rs @@ -111,6 +111,7 @@ impl<'a, 'b> ExprValidator<'a, 'b> { file: source_ptr.file_id, field_list: AstPtr::new(&field_list), missed_fields, + list_parent_path: record_lit.path().map(|path| AstPtr::new(&path)), }) } } diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 5c8ea46abb..7ae4bda0b8 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -100,8 +100,10 @@ pub(crate) fn diagnostics( }; res.borrow_mut().push(Diagnostic { - // TODO kb use a smaller range here - range, + range: d + .list_parent_ast(db) + .map(|path| path.syntax().text_range()) + .unwrap_or(range), message: d.message(), severity: Severity::Error, fix, From a61f2445cba2a48bb7ea6c8477e3198b297f3c67 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 27 Jul 2020 22:30:55 +0300 Subject: [PATCH 03/14] Less stubs --- .../ra_assists/src/handlers/fix_visibility.rs | 2 +- crates/ra_hir_expand/src/diagnostics.rs | 11 ++---- crates/ra_hir_ty/src/diagnostics.rs | 38 ++++++++++--------- crates/ra_ide/src/diagnostics.rs | 8 +--- 4 files changed, 28 insertions(+), 31 deletions(-) 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_expand/src/diagnostics.rs b/crates/ra_hir_expand/src/diagnostics.rs index e889f070fa..ffeca5e827 100644 --- a/crates/ra_hir_expand/src/diagnostics.rs +++ b/crates/ra_hir_expand/src/diagnostics.rs @@ -16,13 +16,16 @@ 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; fn source(&self) -> InFile; + fn highlighting_source(&self) -> InFile { + self.source() + } fn as_any(&self) -> &(dyn Any + Send + 'static); fn is_experimental(&self) -> bool { false @@ -35,12 +38,6 @@ pub trait AstDiagnostic { } 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 a5b00ed485..73d2414343 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, SyntaxNode, SyntaxNodePtr}; +use ra_syntax::{ast, AstNode, AstPtr, SyntaxNodePtr}; use stdx::format_to; use crate::db::HirDatabase; @@ -64,16 +64,6 @@ pub struct MissingFields { pub list_parent_path: Option>, } -impl MissingFields { - fn root(&self, db: &dyn AstDatabase) -> SyntaxNode { - db.parse_or_expand(self.file).unwrap() - } - - pub fn list_parent_ast(&self, db: &dyn AstDatabase) -> Option { - self.list_parent_path.as_ref().map(|path| path.to_node(&self.root(db))) - } -} - impl Diagnostic for MissingFields { fn message(&self) -> String { let mut buf = String::from("Missing structure fields:\n"); @@ -85,16 +75,25 @@ impl Diagnostic for MissingFields { fn source(&self) -> InFile { InFile { file_id: self.file, value: self.field_list.clone().into() } } + fn as_any(&self) -> &(dyn Any + Send + 'static) { self } + + fn highlighting_source(&self) -> InFile { + self.list_parent_path + .clone() + .map(|path| InFile { file_id: self.file, value: path.into() }) + .unwrap_or_else(|| self.source()) + } } impl AstDiagnostic for MissingFields { type AST = ast::RecordExprFieldList; fn ast(&self, db: &dyn AstDatabase) -> Self::AST { - self.field_list.to_node(&self.root(db)) + let root = db.parse_or_expand(self.file).unwrap(); + self.field_list.to_node(&root) } } @@ -260,7 +259,10 @@ impl AstDiagnostic for MismatchedArgCount { #[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; @@ -307,7 +309,9 @@ mod tests { db.diagnostics(|d| { // FXIME: macros... let file_id = d.source().file_id.original_file(&db); - let range = d.syntax_node(&db).text_range(); + let highlighting_source = d.highlighting_source(); + let node = db.parse_or_expand(highlighting_source.file_id).unwrap(); + let range = highlighting_source.value.to_node(&node).text_range(); let message = d.message().to_owned(); actual.entry(file_id).or_default().push((range, message)); }); @@ -345,7 +349,7 @@ struct Beefy { } fn baz() { let zz = Beefy { - //^^^^^... Missing structure fields: + //^^^^^ Missing structure fields: // | - seven one: (), two: (), @@ -370,8 +374,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 diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 7ae4bda0b8..e847df6ea9 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -69,7 +69,6 @@ pub(crate) fn diagnostics( }) }) .on::(|d| { - let range = sema.diagnostics_range(d).range; // Note that although we could add a diagnostics to // fill the missing tuple field, e.g : // `struct A(usize);` @@ -95,15 +94,12 @@ pub(crate) fn diagnostics( }; Some(( Fix::new("Fill struct fields", SourceFileEdit { file_id, edit }.into()), - range, + sema.diagnostics_range(d).range, )) }; res.borrow_mut().push(Diagnostic { - range: d - .list_parent_ast(db) - .map(|path| path.syntax().text_range()) - .unwrap_or(range), + range: d.highlighting_source().file_syntax(db).text_range(), message: d.message(), severity: Severity::Error, fix, From ee1586c1ed058ff0f090b552d52fe6bbe2dd7f7f Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 27 Jul 2020 22:46:25 +0300 Subject: [PATCH 04/14] Better naming --- crates/ra_hir/src/semantics.rs | 11 +++++ crates/ra_hir_def/src/diagnostics.rs | 2 +- crates/ra_hir_expand/src/diagnostics.rs | 6 +-- crates/ra_hir_ty/src/diagnostics.rs | 57 +++++++++++-------------- crates/ra_ide/src/diagnostics.rs | 31 ++++++++------ 5 files changed, 58 insertions(+), 49 deletions(-) diff --git a/crates/ra_hir/src/semantics.rs b/crates/ra_hir/src/semantics.rs index e392130ab6..1c5dc3d510 100644 --- a/crates/ra_hir/src/semantics.rs +++ b/crates/ra_hir/src/semantics.rs @@ -145,6 +145,10 @@ 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) } @@ -376,6 +380,13 @@ 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(); let root = self.db.parse_or_expand(src.file_id).unwrap(); diff --git a/crates/ra_hir_def/src/diagnostics.rs b/crates/ra_hir_def/src/diagnostics.rs index 30db48f868..e532695895 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 fix_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 ffeca5e827..074a8c45e8 100644 --- a/crates/ra_hir_expand/src/diagnostics.rs +++ b/crates/ra_hir_expand/src/diagnostics.rs @@ -22,9 +22,9 @@ use crate::{db::AstDatabase, InFile}; pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static { fn message(&self) -> String; - fn source(&self) -> InFile; - fn highlighting_source(&self) -> InFile { - self.source() + fn fix_source(&self) -> InFile; + fn source(&self) -> InFile { + self.fix_source() } fn as_any(&self) -> &(dyn Any + Send + 'static); fn is_experimental(&self) -> bool { diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs index 73d2414343..a4cede81dc 100644 --- a/crates/ra_hir_ty/src/diagnostics.rs +++ b/crates/ra_hir_ty/src/diagnostics.rs @@ -37,7 +37,7 @@ impl Diagnostic for NoSuchField { "no such field".to_string() } - fn source(&self) -> InFile { + fn fix_source(&self) -> InFile { InFile::new(self.file, self.field.clone().into()) } @@ -50,9 +50,8 @@ 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() + let root = db.parse_or_expand(self.file).unwrap(); + self.field.to_node(&root) } } @@ -72,20 +71,20 @@ impl Diagnostic for MissingFields { } buf } - fn source(&self) -> InFile { + fn fix_source(&self) -> InFile { InFile { file_id: self.file, value: self.field_list.clone().into() } } + fn source(&self) -> InFile { + self.list_parent_path + .clone() + .map(|path| InFile { file_id: self.file, value: path.into() }) + .unwrap_or_else(|| self.fix_source()) + } + fn as_any(&self) -> &(dyn Any + Send + 'static) { self } - - fn highlighting_source(&self) -> InFile { - self.list_parent_path - .clone() - .map(|path| InFile { file_id: self.file, value: path.into() }) - .unwrap_or_else(|| self.source()) - } } impl AstDiagnostic for MissingFields { @@ -112,7 +111,7 @@ impl Diagnostic for MissingPatFields { } buf } - fn source(&self) -> InFile { + fn fix_source(&self) -> InFile { InFile { file_id: self.file, value: self.field_list.clone().into() } } fn as_any(&self) -> &(dyn Any + Send + 'static) { @@ -131,7 +130,7 @@ impl Diagnostic for MissingMatchArms { fn message(&self) -> String { String::from("Missing match arm") } - fn source(&self) -> InFile { + fn fix_source(&self) -> InFile { InFile { file_id: self.file, value: self.match_expr.clone().into() } } fn as_any(&self) -> &(dyn Any + Send + 'static) { @@ -149,7 +148,7 @@ impl Diagnostic for MissingOkInTailExpr { fn message(&self) -> String { "wrap return expression in Ok".to_string() } - fn source(&self) -> InFile { + fn fix_source(&self) -> InFile { InFile { file_id: self.file, value: self.expr.clone().into() } } fn as_any(&self) -> &(dyn Any + Send + 'static) { @@ -162,8 +161,7 @@ impl AstDiagnostic for MissingOkInTailExpr { 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() + self.expr.to_node(&root) } } @@ -177,7 +175,7 @@ impl Diagnostic for BreakOutsideOfLoop { fn message(&self) -> String { "break outside of loop".to_string() } - fn source(&self) -> InFile { + fn fix_source(&self) -> InFile { InFile { file_id: self.file, value: self.expr.clone().into() } } fn as_any(&self) -> &(dyn Any + Send + 'static) { @@ -190,8 +188,7 @@ impl AstDiagnostic for BreakOutsideOfLoop { 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() + self.expr.to_node(&root) } } @@ -205,7 +202,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 fix_source(&self) -> InFile { InFile { file_id: self.file, value: self.expr.clone().into() } } fn as_any(&self) -> &(dyn Any + Send + 'static) { @@ -217,9 +214,8 @@ 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() + let root = db.parse_or_expand(self.file).unwrap(); + self.expr.to_node(&root) } } @@ -236,7 +232,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 fix_source(&self) -> InFile { InFile { file_id: self.file, value: self.call_expr.clone().into() } } fn as_any(&self) -> &(dyn Any + Send + 'static) { @@ -250,7 +246,7 @@ 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 root = db.parse_or_expand(self.file).unwrap(); let node = self.source().value.to_node(&root); ast::CallExpr::cast(node).unwrap() } @@ -308,12 +304,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 highlighting_source = d.highlighting_source(); - let node = db.parse_or_expand(highlighting_source.file_id).unwrap(); - let range = highlighting_source.value.to_node(&node).text_range(); + let source = d.source(); + let root = db.parse_or_expand(source.file_id).unwrap(); + let range = source.value.to_node(&root).text_range(); let message = d.message().to_owned(); - actual.entry(file_id).or_default().push((range, message)); + actual.entry(source.file_id.original_file(&db)).or_default().push((range, message)); }); for (file_id, diags) in actual.iter_mut() { diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index e847df6ea9..0d2ff17e1f 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -54,18 +54,19 @@ 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(), + FileSystemEdit::CreateFile { + anchor: d.file.original_file(db), + dst: d.candidate.clone(), + } + .into(), ); - let range = sema.diagnostics_range(d).range; res.borrow_mut().push(Diagnostic { - range, + range: sema.diagnostics_range(d).range, message: d.message(), severity: Severity::Error, - fix: Some((fix, range)), + fix: Some((fix, sema.diagnostics_fix_range(d).range)), }) }) .on::(|d| { @@ -94,12 +95,12 @@ pub(crate) fn diagnostics( }; Some(( Fix::new("Fill struct fields", SourceFileEdit { file_id, edit }.into()), - sema.diagnostics_range(d).range, + sema.diagnostics_fix_range(d).range, )) }; res.borrow_mut().push(Diagnostic { - range: d.highlighting_source().file_syntax(db).text_range(), + range: sema.diagnostics_range(d).range, message: d.message(), severity: Severity::Error, fix, @@ -110,21 +111,23 @@ pub(crate) fn diagnostics( let replacement = format!("Ok({})", node.syntax()); let edit = TextEdit::replace(node.syntax().text_range(), replacement); let source_change = SourceFileEdit { file_id, edit }.into(); - let range = sema.diagnostics_range(d).range; res.borrow_mut().push(Diagnostic { - range, + range: sema.diagnostics_range(d).range, message: d.message(), severity: Severity::Error, - fix: Some((Fix::new("Wrap with ok", source_change), range)), + fix: Some(( + Fix::new("Wrap with ok", source_change), + sema.diagnostics_fix_range(d).range, + )), }) }) .on::(|d| { - let range = sema.diagnostics_range(d).range; res.borrow_mut().push(Diagnostic { - range, + range: sema.diagnostics_range(d).range, message: d.message(), severity: Severity::Error, - fix: missing_struct_field_fix(&sema, file_id, d).map(|fix| (fix, range)), + fix: missing_struct_field_fix(&sema, file_id, d) + .map(|fix| (fix, sema.diagnostics_fix_range(d).range)), }) }) // Only collect experimental diagnostics when they're enabled. From cb0b13a583c0c20b57fd3529e2c01ab42bd8f04d Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 27 Jul 2020 23:32:16 +0300 Subject: [PATCH 05/14] Fix another missing fields diagnostics --- crates/ra_hir_ty/src/diagnostics.rs | 50 ++++--------------- crates/ra_hir_ty/src/diagnostics/expr.rs | 5 +- .../ra_hir_ty/src/diagnostics/match_check.rs | 8 +-- 3 files changed, 19 insertions(+), 44 deletions(-) diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs index a4cede81dc..48b578fb0e 100644 --- a/crates/ra_hir_ty/src/diagnostics.rs +++ b/crates/ra_hir_ty/src/diagnostics.rs @@ -59,8 +59,8 @@ impl AstDiagnostic for NoSuchField { pub struct MissingFields { pub file: HirFileId, pub field_list: AstPtr, + pub field_list_parent_path: Option>, pub missed_fields: Vec, - pub list_parent_path: Option>, } impl Diagnostic for MissingFields { @@ -76,7 +76,7 @@ impl Diagnostic for MissingFields { } fn source(&self) -> InFile { - self.list_parent_path + self.field_list_parent_path .clone() .map(|path| InFile { file_id: self.file, value: path.into() }) .unwrap_or_else(|| self.fix_source()) @@ -100,6 +100,7 @@ impl AstDiagnostic for MissingFields { pub struct MissingPatFields { pub file: HirFileId, pub field_list: AstPtr, + pub field_list_parent_path: Option>, pub missed_fields: Vec, } @@ -114,6 +115,12 @@ impl Diagnostic for MissingPatFields { 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 as_any(&self) -> &(dyn Any + Send + 'static) { self } @@ -326,41 +333,6 @@ mod tests { assert_eq!(annotations, actual); } - #[test] - fn structure_name_highlighted_for_missing_fields() { - check_diagnostics( - r#" -struct Beefy { - one: i32, - two: i32, - three: i32, - four: i32, - five: i32, - six: i32, - seven: i32, - eight: i32, - nine: i32, - ten: i32, -} -fn baz() { - let zz = Beefy { - //^^^^^ Missing structure fields: - // | - seven - one: (), - two: (), - three: (), - four: (), - five: (), - six: (), - eight: (), - nine: (), - ten: (), - }; -} -"#, - ); - } - #[test] fn no_such_field_diagnostics() { check_diagnostics( @@ -491,8 +463,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 3c37fc58e9..98959ab684 100644 --- a/crates/ra_hir_ty/src/diagnostics/expr.rs +++ b/crates/ra_hir_ty/src/diagnostics/expr.rs @@ -110,8 +110,8 @@ impl<'a, 'b> ExprValidator<'a, 'b> { 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)), missed_fields, - list_parent_path: record_lit.path().map(|path| AstPtr::new(&path)), }) } } @@ -141,6 +141,9 @@ impl<'a, 'b> ExprValidator<'a, 'b> { self.sink.push(MissingPatFields { file: source_ptr.file_id, field_list: AstPtr::new(&field_list), + 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 } => (), From 21184a1b2a4bea57a7666432749b171414136c60 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 27 Jul 2020 23:56:57 +0300 Subject: [PATCH 06/14] Restore accidentally removed public method --- crates/ra_hir_expand/src/diagnostics.rs | 7 ++++++- crates/ra_hir_ty/src/diagnostics.rs | 14 +++++--------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/crates/ra_hir_expand/src/diagnostics.rs b/crates/ra_hir_expand/src/diagnostics.rs index 074a8c45e8..23f28a7f71 100644 --- a/crates/ra_hir_expand/src/diagnostics.rs +++ b/crates/ra_hir_expand/src/diagnostics.rs @@ -16,7 +16,7 @@ use std::{any::Any, fmt}; -use ra_syntax::SyntaxNodePtr; +use ra_syntax::{SyntaxNode, SyntaxNodePtr}; use crate::{db::AstDatabase, InFile}; @@ -38,6 +38,11 @@ pub trait AstDiagnostic { } 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() } diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs index 48b578fb0e..9d29f30716 100644 --- a/crates/ra_hir_ty/src/diagnostics.rs +++ b/crates/ra_hir_ty/src/diagnostics.rs @@ -262,10 +262,7 @@ impl AstDiagnostic for MismatchedArgCount { #[cfg(test)] mod tests { use hir_def::{db::DefDatabase, AssocItemId, ModuleDefId}; - use hir_expand::{ - db::AstDatabase, - diagnostics::{Diagnostic, DiagnosticSinkBuilder}, - }; + use hir_expand::diagnostics::{Diagnostic, DiagnosticSinkBuilder}; use ra_db::{fixture::WithFixture, FileId, SourceDatabase, SourceDatabaseExt}; use ra_syntax::{TextRange, TextSize}; use rustc_hash::FxHashMap; @@ -310,12 +307,11 @@ mod tests { let mut actual: FxHashMap> = FxHashMap::default(); db.diagnostics(|d| { - // FXIME: macros... - let source = d.source(); - let root = db.parse_or_expand(source.file_id).unwrap(); - let range = source.value.to_node(&root).text_range(); + // FIXME: macros... + let file_id = d.source().file_id.original_file(&db); + let range = d.syntax_node(&db).text_range(); let message = d.message().to_owned(); - actual.entry(source.file_id.original_file(&db)).or_default().push((range, message)); + actual.entry(file_id).or_default().push((range, message)); }); for (file_id, diags) in actual.iter_mut() { From cfbbd91a886e2394e7411f9d7f4966dcbd454764 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 28 Jul 2020 10:24:59 +0300 Subject: [PATCH 07/14] Require source implementations for Diagnostic --- crates/ra_hir_def/src/diagnostics.rs | 2 +- crates/ra_hir_expand/src/diagnostics.rs | 13 ++++++++----- crates/ra_hir_ty/src/diagnostics.rs | 12 ++++++------ 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/crates/ra_hir_def/src/diagnostics.rs b/crates/ra_hir_def/src/diagnostics.rs index e532695895..30db48f868 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 fix_source(&self) -> InFile { + fn 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 23f28a7f71..90a3b87f96 100644 --- a/crates/ra_hir_expand/src/diagnostics.rs +++ b/crates/ra_hir_expand/src/diagnostics.rs @@ -22,9 +22,11 @@ use crate::{db::AstDatabase, InFile}; pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static { fn message(&self) -> String; - fn fix_source(&self) -> InFile; - fn source(&self) -> InFile { - self.fix_source() + /// 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 as_any(&self) -> &(dyn Any + Send + 'static); fn is_experimental(&self) -> bool { @@ -39,8 +41,9 @@ pub trait AstDiagnostic { 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) + 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> { diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs index 9d29f30716..efca096199 100644 --- a/crates/ra_hir_ty/src/diagnostics.rs +++ b/crates/ra_hir_ty/src/diagnostics.rs @@ -37,7 +37,7 @@ impl Diagnostic for NoSuchField { "no such field".to_string() } - fn fix_source(&self) -> InFile { + fn source(&self) -> InFile { InFile::new(self.file, self.field.clone().into()) } @@ -137,7 +137,7 @@ impl Diagnostic for MissingMatchArms { fn message(&self) -> String { String::from("Missing match arm") } - fn fix_source(&self) -> InFile { + fn source(&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 fix_source(&self) -> InFile { + fn source(&self) -> InFile { InFile { file_id: self.file, value: self.expr.clone().into() } } fn as_any(&self) -> &(dyn Any + Send + 'static) { @@ -182,7 +182,7 @@ impl Diagnostic for BreakOutsideOfLoop { fn message(&self) -> String { "break outside of loop".to_string() } - fn fix_source(&self) -> InFile { + fn source(&self) -> InFile { InFile { file_id: self.file, value: self.expr.clone().into() } } fn as_any(&self) -> &(dyn Any + Send + 'static) { @@ -209,7 +209,7 @@ impl Diagnostic for MissingUnsafe { fn message(&self) -> String { format!("This operation is unsafe and requires an unsafe function or block") } - fn fix_source(&self) -> InFile { + fn source(&self) -> InFile { InFile { file_id: self.file, value: self.expr.clone().into() } } fn as_any(&self) -> &(dyn Any + Send + 'static) { @@ -239,7 +239,7 @@ impl Diagnostic for MismatchedArgCount { let s = if self.expected == 1 { "" } else { "s" }; format!("Expected {} argument{}, found {}", self.expected, s, self.found) } - fn fix_source(&self) -> InFile { + fn source(&self) -> InFile { InFile { file_id: self.file, value: self.call_expr.clone().into() } } fn as_any(&self) -> &(dyn Any + Send + 'static) { From 9963f43d51071ea02f8f6d490b9c49882034b42c Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sun, 9 Aug 2020 01:59:26 +0300 Subject: [PATCH 08/14] Refactor the diagnostics --- crates/ra_hir/src/semantics.rs | 28 +++---- crates/ra_hir_def/src/diagnostics.rs | 12 ++- crates/ra_hir_expand/src/diagnostics.rs | 17 +---- crates/ra_hir_ty/src/diagnostics.rs | 97 ++++++++++-------------- crates/ra_hir_ty/src/diagnostics/expr.rs | 12 +-- crates/ra_ide/src/diagnostics.rs | 76 ++++++++++--------- 6 files changed, 106 insertions(+), 136 deletions(-) diff --git a/crates/ra_hir/src/semantics.rs b/crates/ra_hir/src/semantics.rs index 1c5dc3d510..b4420d3785 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 30db48f868..be96128465 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 90a3b87f96..2b74473cec 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 efca096199..1e3a446375 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 98959ab684..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,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 0d2ff17e1f..55593a8cb8 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()?), ); From 936861993935d5b2c78b953e2f4b719e1992bd73 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 10 Aug 2020 22:53:10 +0300 Subject: [PATCH 09/14] Make the fix AST source Optional --- crates/ra_hir/src/diagnostics.rs | 2 +- crates/ra_hir/src/semantics.rs | 8 +-- crates/ra_hir_def/src/diagnostics.rs | 10 +-- crates/ra_hir_expand/src/diagnostics.rs | 19 ++---- crates/ra_hir_ty/src/diagnostics.rs | 26 ++++---- crates/ra_ide/src/diagnostics.rs | 85 ++++++++++++++----------- 6 files changed, 77 insertions(+), 73 deletions(-) diff --git a/crates/ra_hir/src/diagnostics.rs b/crates/ra_hir/src/diagnostics.rs index 266b513dcf..564f6a5db2 100644 --- a/crates/ra_hir/src/diagnostics.rs +++ b/crates/ra_hir/src/diagnostics.rs @@ -1,7 +1,7 @@ //! FIXME: write short doc here pub use hir_def::diagnostics::UnresolvedModule; pub use hir_expand::diagnostics::{ - AstDiagnostic, Diagnostic, DiagnosticSink, DiagnosticSinkBuilder, + Diagnostic, DiagnosticSink, DiagnosticSinkBuilder, DiagnosticWithFix, }; 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 b4420d3785..c5bc2baffe 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::{diagnostics::DiagnosticWithFix, hygiene::Hygiene, name::AsName, ExpansionInfo}; use hir_ty::associated_type_shorthand_candidates; use itertools::Itertools; use ra_db::{FileId, FileRange}; @@ -109,12 +109,12 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> { self.imp.parse(file_id) } - pub fn diagnostic_fix_source( + pub fn diagnostic_fix_source( &self, d: &T, - ) -> ::AST { + ) -> Option<::AST> { let file_id = d.presentation().file_id; - let root = self.db.parse_or_expand(file_id).unwrap(); + let root = self.db.parse_or_expand(file_id)?; self.imp.cache(root, file_id); d.fix_source(self.db.upcast()) } diff --git a/crates/ra_hir_def/src/diagnostics.rs b/crates/ra_hir_def/src/diagnostics.rs index be96128465..033be683c4 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::{AstDiagnostic, Diagnostic}; +use hir_expand::diagnostics::{Diagnostic, DiagnosticWithFix}; use ra_syntax::{ast, AstPtr, SyntaxNodePtr}; use hir_expand::{HirFileId, InFile}; @@ -26,10 +26,10 @@ impl Diagnostic for UnresolvedModule { } } -impl AstDiagnostic for UnresolvedModule { +impl DiagnosticWithFix 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) + fn fix_source(&self, db: &dyn hir_expand::db::AstDatabase) -> Option { + let root = db.parse_or_expand(self.file)?; + Some(self.decl.to_node(&root)) } } diff --git a/crates/ra_hir_expand/src/diagnostics.rs b/crates/ra_hir_expand/src/diagnostics.rs index 2b74473cec..62a09a73ae 100644 --- a/crates/ra_hir_expand/src/diagnostics.rs +++ b/crates/ra_hir_expand/src/diagnostics.rs @@ -29,15 +29,9 @@ pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static { } } -pub trait AstDiagnostic { +pub trait DiagnosticWithFix { type AST; - fn fix_source(&self, db: &dyn AstDatabase) -> Self::AST; -} - -impl dyn Diagnostic { - pub fn downcast_ref(&self) -> Option<&D> { - self.as_any().downcast_ref() - } + fn fix_source(&self, db: &dyn AstDatabase) -> Option; } pub struct DiagnosticSink<'a> { @@ -83,12 +77,9 @@ impl<'a> DiagnosticSinkBuilder<'a> { self } - pub fn on(mut self, mut cb: F) -> Self { - let cb = move |diag: &dyn Diagnostic| match diag.downcast_ref::() { - Some(d) => { - cb(d); - Ok(()) - } + pub fn on Option<()> + 'a>(mut self, mut cb: F) -> Self { + let cb = move |diag: &dyn Diagnostic| match diag.as_any().downcast_ref::() { + Some(d) => cb(d).ok_or(()), None => Err(()), }; self.callbacks.push(Box::new(cb)); diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs index 1e3a446375..b34ba5bfc8 100644 --- a/crates/ra_hir_ty/src/diagnostics.rs +++ b/crates/ra_hir_ty/src/diagnostics.rs @@ -6,7 +6,7 @@ mod unsafe_check; use std::any::Any; use hir_def::DefWithBodyId; -use hir_expand::diagnostics::{AstDiagnostic, Diagnostic, DiagnosticSink}; +use hir_expand::diagnostics::{Diagnostic, DiagnosticSink, DiagnosticWithFix}; use hir_expand::{db::AstDatabase, name::Name, HirFileId, InFile}; use ra_prof::profile; use ra_syntax::{ast, AstPtr, SyntaxNodePtr}; @@ -46,12 +46,12 @@ impl Diagnostic for NoSuchField { } } -impl AstDiagnostic for NoSuchField { +impl DiagnosticWithFix for NoSuchField { type AST = ast::RecordExprField; - fn fix_source(&self, db: &dyn AstDatabase) -> Self::AST { - let root = db.parse_or_expand(self.file).unwrap(); - self.field.to_node(&root) + fn fix_source(&self, db: &dyn AstDatabase) -> Option { + let root = db.parse_or_expand(self.file)?; + Some(self.field.to_node(&root)) } } @@ -88,12 +88,12 @@ impl Diagnostic for MissingFields { } } -impl AstDiagnostic for MissingFields { +impl DiagnosticWithFix for MissingFields { type AST = ast::RecordExpr; - fn fix_source(&self, db: &dyn AstDatabase) -> Self::AST { - let root = db.parse_or_expand(self.file).unwrap(); - self.field_list_parent.to_node(&root) + fn fix_source(&self, db: &dyn AstDatabase) -> Option { + let root = db.parse_or_expand(self.file)?; + Some(self.field_list_parent.to_node(&root)) } } @@ -163,12 +163,12 @@ impl Diagnostic for MissingOkInTailExpr { } } -impl AstDiagnostic for MissingOkInTailExpr { +impl DiagnosticWithFix for MissingOkInTailExpr { type AST = ast::Expr; - fn fix_source(&self, db: &dyn AstDatabase) -> Self::AST { - let root = db.parse_or_expand(self.file).unwrap(); - self.expr.to_node(&root) + fn fix_source(&self, db: &dyn AstDatabase) -> Option { + let root = db.parse_or_expand(self.file)?; + Some(self.expr.to_node(&root)) } } diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 55593a8cb8..043ce357b9 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -62,12 +62,18 @@ pub(crate) fn diagnostics( } .into(), ); + let fix = sema + .diagnostic_fix_source(d) + .map(|unresolved_module| unresolved_module.syntax().text_range()) + .map(|fix_range| (fix, fix_range)); + res.borrow_mut().push(Diagnostic { range: sema.diagnostics_presentation_range(d).range, message: d.message(), severity: Severity::Error, - fix: Some((fix, sema.diagnostic_fix_source(d).syntax().text_range())), - }) + fix, + }); + Some(()) }) .on::(|d| { // Note that although we could add a diagnostics to @@ -78,30 +84,29 @@ pub(crate) fn diagnostics( let fix = if d.missed_fields.iter().any(|it| it.as_tuple_index().is_some()) { None } else { - 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); - } + sema.diagnostic_fix_source(d) + .and_then(|record_expr| record_expr.record_expr_field_list()) + .map(|old_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(&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 - } + let edit = { + let mut builder = TextEditBuilder::default(); + algo::diff(&old_field_list.syntax(), &new_field_list.syntax()) + .into_text_edit(&mut builder); + builder.finish() + }; + ( + Fix::new("Fill struct fields", SourceFileEdit { file_id, edit }.into()), + sema.original_range(&old_field_list.syntax()).range, + ) + }) }; res.borrow_mut().push(Diagnostic { @@ -109,28 +114,36 @@ pub(crate) fn diagnostics( message: d.message(), severity: Severity::Error, fix, - }) + }); + Some(()) }) .on::(|d| { - 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(); + let fix = sema.diagnostic_fix_source(d).map(|tail_expr| { + 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(); + (Fix::new("Wrap with ok", source_change), tail_expr_range) + }); + res.borrow_mut().push(Diagnostic { range: sema.diagnostics_presentation_range(d).range, message: d.message(), severity: Severity::Error, - fix: Some((Fix::new("Wrap with ok", source_change), tail_expr_range)), - }) + fix, + }); + Some(()) }) .on::(|d| { res.borrow_mut().push(Diagnostic { 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.diagnostic_fix_source(d).syntax().text_range())), - }) + fix: missing_struct_field_fix(&sema, file_id, d).and_then(|fix| { + Some((fix, sema.diagnostic_fix_source(d)?.syntax().text_range())) + }), + }); + Some(()) }) // Only collect experimental diagnostics when they're enabled. .filter(|diag| !diag.is_experimental() || enable_experimental) @@ -156,7 +169,7 @@ fn missing_struct_field_fix( usage_file_id: FileId, d: &hir::diagnostics::NoSuchField, ) -> Option { - let record_expr_field = sema.diagnostic_fix_source(d); + let record_expr_field = sema.diagnostic_fix_source(d)?; let record_lit = ast::RecordExpr::cast(record_expr_field.syntax().parent()?.parent()?)?; let def_id = sema.resolve_variant(record_lit)?; From 29fbc8e02180aac1f4d7819a9626206aa64028a0 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 11 Aug 2020 00:37:23 +0300 Subject: [PATCH 10/14] Move the DiagnosticsWithFix trait on the ide level --- crates/ra_hir/src/diagnostics.rs | 4 +- crates/ra_hir/src/semantics.rs | 12 ++--- crates/ra_hir_def/src/diagnostics.rs | 10 +--- crates/ra_hir_expand/src/diagnostics.rs | 7 +-- crates/ra_hir_ty/src/diagnostics.rs | 31 +------------ crates/ra_ide/src/diagnostics.rs | 28 ++++++++--- .../src/diagnostics/diagnostics_with_fix.rs | 46 +++++++++++++++++++ 7 files changed, 75 insertions(+), 63 deletions(-) create mode 100644 crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs diff --git a/crates/ra_hir/src/diagnostics.rs b/crates/ra_hir/src/diagnostics.rs index 564f6a5db2..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::{ - Diagnostic, DiagnosticSink, DiagnosticSinkBuilder, DiagnosticWithFix, -}; +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 c5bc2baffe..e9f7a033c5 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::DiagnosticWithFix, 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}; @@ -109,14 +109,8 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> { self.imp.parse(file_id) } - pub fn diagnostic_fix_source( - &self, - d: &T, - ) -> Option<::AST> { - let file_id = d.presentation().file_id; - let root = self.db.parse_or_expand(file_id)?; - self.imp.cache(root, file_id); - d.fix_source(self.db.upcast()) + pub fn cache(&self, root_node: SyntaxNode, file_id: HirFileId) { + self.imp.cache(root_node, file_id) } pub fn expand(&self, macro_call: &ast::MacroCall) -> Option { diff --git a/crates/ra_hir_def/src/diagnostics.rs b/crates/ra_hir_def/src/diagnostics.rs index 033be683c4..9435c72544 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, DiagnosticWithFix}; +use hir_expand::diagnostics::Diagnostic; use ra_syntax::{ast, AstPtr, SyntaxNodePtr}; use hir_expand::{HirFileId, InFile}; @@ -25,11 +25,3 @@ impl Diagnostic for UnresolvedModule { self } } - -impl DiagnosticWithFix for UnresolvedModule { - type AST = ast::Module; - fn fix_source(&self, db: &dyn hir_expand::db::AstDatabase) -> Option { - let root = db.parse_or_expand(self.file)?; - Some(self.decl.to_node(&root)) - } -} diff --git a/crates/ra_hir_expand/src/diagnostics.rs b/crates/ra_hir_expand/src/diagnostics.rs index 62a09a73ae..8358c488b8 100644 --- a/crates/ra_hir_expand/src/diagnostics.rs +++ b/crates/ra_hir_expand/src/diagnostics.rs @@ -18,7 +18,7 @@ use std::{any::Any, fmt}; 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; @@ -29,11 +29,6 @@ pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static { } } -pub trait DiagnosticWithFix { - type AST; - fn fix_source(&self, db: &dyn AstDatabase) -> Option; -} - pub struct DiagnosticSink<'a> { callbacks: Vec Result<(), ()> + 'a>>, filters: Vec bool + 'a>>, diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs index b34ba5bfc8..24435e8a71 100644 --- a/crates/ra_hir_ty/src/diagnostics.rs +++ b/crates/ra_hir_ty/src/diagnostics.rs @@ -6,8 +6,8 @@ mod unsafe_check; use std::any::Any; use hir_def::DefWithBodyId; -use hir_expand::diagnostics::{Diagnostic, DiagnosticSink, DiagnosticWithFix}; -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, AstPtr, SyntaxNodePtr}; use stdx::format_to; @@ -46,15 +46,6 @@ impl Diagnostic for NoSuchField { } } -impl DiagnosticWithFix for NoSuchField { - type AST = ast::RecordExprField; - - fn fix_source(&self, db: &dyn AstDatabase) -> Option { - let root = db.parse_or_expand(self.file)?; - Some(self.field.to_node(&root)) - } -} - #[derive(Debug)] pub struct MissingFields { pub file: HirFileId, @@ -88,15 +79,6 @@ impl Diagnostic for MissingFields { } } -impl DiagnosticWithFix for MissingFields { - type AST = ast::RecordExpr; - - fn fix_source(&self, db: &dyn AstDatabase) -> Option { - let root = db.parse_or_expand(self.file)?; - Some(self.field_list_parent.to_node(&root)) - } -} - #[derive(Debug)] pub struct MissingPatFields { pub file: HirFileId, @@ -163,15 +145,6 @@ impl Diagnostic for MissingOkInTailExpr { } } -impl DiagnosticWithFix for MissingOkInTailExpr { - type AST = ast::Expr; - - fn fix_source(&self, db: &dyn AstDatabase) -> Option { - let root = db.parse_or_expand(self.file)?; - Some(self.expr.to_node(&root)) - } -} - #[derive(Debug)] pub struct BreakOutsideOfLoop { pub file: HirFileId, diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 043ce357b9..ca1a7c1aae 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -7,11 +7,12 @@ use std::cell::RefCell; use hir::{ + db::AstDatabase, diagnostics::{Diagnostic as _, DiagnosticSinkBuilder}, HasSource, HirDisplay, Semantics, VariantDef, }; use itertools::Itertools; -use ra_db::SourceDatabase; +use ra_db::{SourceDatabase, Upcast}; use ra_ide_db::RootDatabase; use ra_prof::profile; use ra_syntax::{ @@ -23,6 +24,9 @@ use ra_text_edit::{TextEdit, TextEditBuilder}; use crate::{Diagnostic, FileId, FileSystemEdit, Fix, SourceFileEdit}; +mod diagnostics_with_fix; +use diagnostics_with_fix::DiagnosticWithFix; + #[derive(Debug, Copy, Clone)] pub enum Severity { Error, @@ -62,8 +66,7 @@ pub(crate) fn diagnostics( } .into(), ); - let fix = sema - .diagnostic_fix_source(d) + let fix = diagnostic_fix_source(&sema, d) .map(|unresolved_module| unresolved_module.syntax().text_range()) .map(|fix_range| (fix, fix_range)); @@ -84,7 +87,7 @@ pub(crate) fn diagnostics( let fix = if d.missed_fields.iter().any(|it| it.as_tuple_index().is_some()) { None } else { - sema.diagnostic_fix_source(d) + diagnostic_fix_source(&sema, d) .and_then(|record_expr| record_expr.record_expr_field_list()) .map(|old_field_list| { let mut new_field_list = old_field_list.clone(); @@ -105,6 +108,7 @@ pub(crate) fn diagnostics( ( Fix::new("Fill struct fields", SourceFileEdit { file_id, edit }.into()), sema.original_range(&old_field_list.syntax()).range, + // old_field_list.syntax().text_range(), ) }) }; @@ -118,7 +122,7 @@ pub(crate) fn diagnostics( Some(()) }) .on::(|d| { - let fix = sema.diagnostic_fix_source(d).map(|tail_expr| { + let fix = diagnostic_fix_source(&sema, d).map(|tail_expr| { let tail_expr_range = tail_expr.syntax().text_range(); let edit = TextEdit::replace(tail_expr_range, format!("Ok({})", tail_expr.syntax())); @@ -140,7 +144,7 @@ pub(crate) fn diagnostics( message: d.message(), severity: Severity::Error, fix: missing_struct_field_fix(&sema, file_id, d).and_then(|fix| { - Some((fix, sema.diagnostic_fix_source(d)?.syntax().text_range())) + Some((fix, diagnostic_fix_source(&sema, d)?.syntax().text_range())) }), }); Some(()) @@ -164,12 +168,22 @@ pub(crate) fn diagnostics( res.into_inner() } +fn diagnostic_fix_source( + sema: &Semantics, + d: &T, +) -> Option<::AST> { + let file_id = d.presentation().file_id; + let root = sema.db.parse_or_expand(file_id)?; + sema.cache(root, file_id); + d.fix_source(sema.db.upcast()) +} + fn missing_struct_field_fix( sema: &Semantics, usage_file_id: FileId, d: &hir::diagnostics::NoSuchField, ) -> Option { - let record_expr_field = sema.diagnostic_fix_source(d)?; + let record_expr_field = diagnostic_fix_source(&sema, d)?; let record_lit = ast::RecordExpr::cast(record_expr_field.syntax().parent()?.parent()?)?; let def_id = sema.resolve_variant(record_lit)?; 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..8578a90ec0 --- /dev/null +++ b/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs @@ -0,0 +1,46 @@ +use hir::{ + db::AstDatabase, + diagnostics::{MissingFields, MissingOkInTailExpr, NoSuchField, UnresolvedModule}, +}; +use ra_syntax::ast; + +// TODO kb +pub trait DiagnosticWithFix { + type AST; + fn fix_source(&self, db: &dyn AstDatabase) -> Option; +} + +impl DiagnosticWithFix for UnresolvedModule { + type AST = ast::Module; + fn fix_source(&self, db: &dyn AstDatabase) -> Option { + let root = db.parse_or_expand(self.file)?; + Some(self.decl.to_node(&root)) + } +} + +impl DiagnosticWithFix for NoSuchField { + type AST = ast::RecordExprField; + + fn fix_source(&self, db: &dyn AstDatabase) -> Option { + let root = db.parse_or_expand(self.file)?; + Some(self.field.to_node(&root)) + } +} + +impl DiagnosticWithFix for MissingFields { + type AST = ast::RecordExpr; + + fn fix_source(&self, db: &dyn AstDatabase) -> Option { + let root = db.parse_or_expand(self.file)?; + Some(self.field_list_parent.to_node(&root)) + } +} + +impl DiagnosticWithFix for MissingOkInTailExpr { + type AST = ast::Expr; + + fn fix_source(&self, db: &dyn AstDatabase) -> Option { + let root = db.parse_or_expand(self.file)?; + Some(self.expr.to_node(&root)) + } +} From c8cad76d25f7fab856c9646b70122e0f9f7d7218 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 11 Aug 2020 00:55:57 +0300 Subject: [PATCH 11/14] Improve the ide diagnostics trait API --- crates/ra_hir/src/semantics.rs | 5 +- crates/ra_hir_expand/src/diagnostics.rs | 7 +- crates/ra_ide/src/diagnostics.rs | 188 ++---------------- .../src/diagnostics/diagnostics_with_fix.rs | 165 +++++++++++++-- 4 files changed, 167 insertions(+), 198 deletions(-) diff --git a/crates/ra_hir/src/semantics.rs b/crates/ra_hir/src/semantics.rs index e9f7a033c5..2dfe69039f 100644 --- a/crates/ra_hir/src/semantics.rs +++ b/crates/ra_hir/src/semantics.rs @@ -109,10 +109,6 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> { self.imp.parse(file_id) } - pub fn cache(&self, root_node: SyntaxNode, file_id: HirFileId) { - self.imp.cache(root_node, file_id) - } - pub fn expand(&self, macro_call: &ast::MacroCall) -> Option { self.imp.expand(macro_call) } @@ -377,6 +373,7 @@ impl<'db> SemanticsImpl<'db> { let src = diagnostics.presentation(); 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_expand/src/diagnostics.rs b/crates/ra_hir_expand/src/diagnostics.rs index 8358c488b8..e58defa681 100644 --- a/crates/ra_hir_expand/src/diagnostics.rs +++ b/crates/ra_hir_expand/src/diagnostics.rs @@ -72,9 +72,12 @@ impl<'a> DiagnosticSinkBuilder<'a> { self } - pub fn on Option<()> + 'a>(mut self, mut cb: F) -> Self { + pub fn on(mut self, mut cb: F) -> Self { let cb = move |diag: &dyn Diagnostic| match diag.as_any().downcast_ref::() { - Some(d) => cb(d).ok_or(()), + Some(d) => { + cb(d); + Ok(()) + } None => Err(()), }; self.callbacks.push(Box::new(cb)); diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index ca1a7c1aae..165ff5249c 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -7,22 +7,20 @@ use std::cell::RefCell; use hir::{ - db::AstDatabase, - diagnostics::{Diagnostic as _, DiagnosticSinkBuilder}, - HasSource, HirDisplay, Semantics, VariantDef, + diagnostics::{Diagnostic as HirDiagnostics, DiagnosticSinkBuilder}, + Semantics, }; use itertools::Itertools; -use ra_db::{SourceDatabase, Upcast}; +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; @@ -58,96 +56,16 @@ pub(crate) fn diagnostics( let res = RefCell::new(res); let mut sink = DiagnosticSinkBuilder::new() .on::(|d| { - let fix = Fix::new( - "Create module", - FileSystemEdit::CreateFile { - anchor: d.file.original_file(db), - dst: d.candidate.clone(), - } - .into(), - ); - let fix = diagnostic_fix_source(&sema, d) - .map(|unresolved_module| unresolved_module.syntax().text_range()) - .map(|fix_range| (fix, fix_range)); - - res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_presentation_range(d).range, - message: d.message(), - severity: Severity::Error, - fix, - }); - Some(()) + 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 { - diagnostic_fix_source(&sema, d) - .and_then(|record_expr| record_expr.record_expr_field_list()) - .map(|old_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(&old_field_list.syntax(), &new_field_list.syntax()) - .into_text_edit(&mut builder); - builder.finish() - }; - ( - Fix::new("Fill struct fields", SourceFileEdit { file_id, edit }.into()), - sema.original_range(&old_field_list.syntax()).range, - // old_field_list.syntax().text_range(), - ) - }) - }; - - res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_presentation_range(d).range, - message: d.message(), - severity: Severity::Error, - fix, - }); - Some(()) + res.borrow_mut().push(diagnostic_with_fix(d, &sema)); }) .on::(|d| { - let fix = diagnostic_fix_source(&sema, d).map(|tail_expr| { - 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(); - (Fix::new("Wrap with ok", source_change), tail_expr_range) - }); - - res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_presentation_range(d).range, - message: d.message(), - severity: Severity::Error, - fix, - }); - Some(()) + res.borrow_mut().push(diagnostic_with_fix(d, &sema)); }) .on::(|d| { - res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_presentation_range(d).range, - message: d.message(), - severity: Severity::Error, - fix: missing_struct_field_fix(&sema, file_id, d).and_then(|fix| { - Some((fix, diagnostic_fix_source(&sema, d)?.syntax().text_range())) - }), - }); - Some(()) + res.borrow_mut().push(diagnostic_with_fix(d, &sema)); }) // Only collect experimental diagnostics when they're enabled. .filter(|diag| !diag.is_experimental() || enable_experimental) @@ -168,87 +86,15 @@ pub(crate) fn diagnostics( res.into_inner() } -fn diagnostic_fix_source( +fn diagnostic_with_fix( + d: &D, sema: &Semantics, - d: &T, -) -> Option<::AST> { - let file_id = d.presentation().file_id; - let root = sema.db.parse_or_expand(file_id)?; - sema.cache(root, file_id); - d.fix_source(sema.db.upcast()) -} - -fn missing_struct_field_fix( - sema: &Semantics, - usage_file_id: FileId, - d: &hir::diagnostics::NoSuchField, -) -> Option { - let record_expr_field = diagnostic_fix_source(&sema, d)?; - - 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), - }; - 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, - } +) -> Diagnostic { + Diagnostic { + range: sema.diagnostics_presentation_range(d).range, + message: d.message(), + severity: Severity::Error, + fix: d.fix(&sema), } } diff --git a/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs b/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs index 8578a90ec0..56d454ac61 100644 --- a/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs +++ b/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs @@ -1,46 +1,169 @@ +use crate::Fix; +use ast::{edit::IndentLevel, make}; use hir::{ db::AstDatabase, diagnostics::{MissingFields, MissingOkInTailExpr, NoSuchField, UnresolvedModule}, + HasSource, HirDisplay, Semantics, VariantDef, }; -use ra_syntax::ast; +use ra_db::FileId; +use ra_ide_db::{ + source_change::{FileSystemEdit, SourceFileEdit}, + RootDatabase, +}; +use ra_syntax::{algo, ast, AstNode, TextRange}; +use ra_text_edit::{TextEdit, TextEditBuilder}; // TODO kb pub trait DiagnosticWithFix { - type AST; - fn fix_source(&self, db: &dyn AstDatabase) -> Option; + fn fix(&self, sema: &Semantics) -> Option<(Fix, TextRange)>; } impl DiagnosticWithFix for UnresolvedModule { - type AST = ast::Module; - fn fix_source(&self, db: &dyn AstDatabase) -> Option { - let root = db.parse_or_expand(self.file)?; - Some(self.decl.to_node(&root)) + fn fix(&self, sema: &Semantics) -> Option<(Fix, TextRange)> { + let fix = Fix::new( + "Create module", + FileSystemEdit::CreateFile { + anchor: self.file.original_file(sema.db), + dst: self.candidate.clone(), + } + .into(), + ); + + let root = sema.db.parse_or_expand(self.file)?; + let unresolved_module = self.decl.to_node(&root); + Some((fix, unresolved_module.syntax().text_range())) } } impl DiagnosticWithFix for NoSuchField { - type AST = ast::RecordExprField; - - fn fix_source(&self, db: &dyn AstDatabase) -> Option { - let root = db.parse_or_expand(self.file)?; - Some(self.field.to_node(&root)) + fn fix(&self, sema: &Semantics) -> Option<(Fix, TextRange)> { + let root = sema.db.parse_or_expand(self.file)?; + let record_expr_field = self.field.to_node(&root); + let fix = + missing_struct_field_fix(&sema, self.file.original_file(sema.db), &record_expr_field)?; + Some((fix, record_expr_field.syntax().text_range())) } } impl DiagnosticWithFix for MissingFields { - type AST = ast::RecordExpr; + fn fix(&self, sema: &Semantics) -> Option<(Fix, TextRange)> { + // 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()) { + None + } else { + 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); + } - fn fix_source(&self, db: &dyn AstDatabase) -> Option { - let root = db.parse_or_expand(self.file)?; - Some(self.field_list_parent.to_node(&root)) + 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, + // old_field_list.syntax().text_range(), + )) + } } } impl DiagnosticWithFix for MissingOkInTailExpr { - type AST = ast::Expr; - - fn fix_source(&self, db: &dyn AstDatabase) -> Option { - let root = db.parse_or_expand(self.file)?; - Some(self.expr.to_node(&root)) + fn fix(&self, sema: &Semantics) -> Option<(Fix, TextRange)> { + 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_struct_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), + }; + 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, + } } } From 37aa68f050fae0079db7b6ebd81bacea4441fb7e Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 11 Aug 2020 15:08:55 +0300 Subject: [PATCH 12/14] Add rustdocs --- crates/ra_hir_expand/src/diagnostics.rs | 1 + crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/ra_hir_expand/src/diagnostics.rs b/crates/ra_hir_expand/src/diagnostics.rs index e58defa681..cc7dc3af22 100644 --- a/crates/ra_hir_expand/src/diagnostics.rs +++ b/crates/ra_hir_expand/src/diagnostics.rs @@ -22,6 +22,7 @@ use crate::InFile; pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static { fn message(&self) -> String; + /// A presentation source of the diagnostics, to use in highlighting and similar actions fn presentation(&self) -> InFile; fn as_any(&self) -> &(dyn Any + Send + 'static); fn is_experimental(&self) -> bool { diff --git a/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs b/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs index 56d454ac61..1955e15210 100644 --- a/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs +++ b/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs @@ -1,3 +1,4 @@ +//! Provides a way to derive fixes based on the diagnostic data. use crate::Fix; use ast::{edit::IndentLevel, make}; use hir::{ @@ -13,8 +14,9 @@ use ra_ide_db::{ use ra_syntax::{algo, ast, AstNode, TextRange}; use ra_text_edit::{TextEdit, TextEditBuilder}; -// TODO kb +/// A trait to implement fot the Diagnostic that has a fix available. pub trait DiagnosticWithFix { + /// Provides a fix with the fix range, if applicable in the current semantics. fn fix(&self, sema: &Semantics) -> Option<(Fix, TextRange)>; } From 188ec3459e795732ad097758f7bf6b6b95bdbf5e Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 11 Aug 2020 17:13:40 +0300 Subject: [PATCH 13/14] Simplify fix structure --- crates/ra_ide/src/diagnostics.rs | 68 +++++------- .../src/diagnostics/diagnostics_with_fix.rs | 103 +++++++++--------- crates/ra_ide/src/lib.rs | 12 +- crates/rust-analyzer/src/handlers.rs | 7 +- 4 files changed, 93 insertions(+), 97 deletions(-) diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 165ff5249c..757b76fd40 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -6,10 +6,7 @@ use std::cell::RefCell; -use hir::{ - diagnostics::{Diagnostic as HirDiagnostics, DiagnosticSinkBuilder}, - Semantics, -}; +use hir::{diagnostics::DiagnosticSinkBuilder, Semantics}; use itertools::Itertools; use ra_db::SourceDatabase; use ra_ide_db::RootDatabase; @@ -73,7 +70,7 @@ pub(crate) fn diagnostics( .build(|d| { res.borrow_mut().push(Diagnostic { message: d.message(), - range: sema.diagnostics_presentation_range(d).range, + range: sema.diagnostics_display_range(d).range, severity: Severity::Error, fix: None, }) @@ -86,12 +83,9 @@ pub(crate) fn diagnostics( res.into_inner() } -fn diagnostic_with_fix( - d: &D, - sema: &Semantics, -) -> Diagnostic { +fn diagnostic_with_fix(d: &D, sema: &Semantics) -> Diagnostic { Diagnostic { - range: sema.diagnostics_presentation_range(d).range, + range: sema.diagnostics_display_range(d).range, message: d.message(), severity: Severity::Error, fix: d.fix(&sema), @@ -120,8 +114,9 @@ fn check_unnecessary_braces_in_use_statement( 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()), + fix: Some(Fix::new( + "Remove unnecessary braces", + SourceFileEdit { file_id, edit }.into(), use_range, )), }); @@ -165,11 +160,9 @@ fn check_struct_shorthand_initialization( 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(), - ), + fix: Some(Fix::new( + "Use struct shorthand initialization", + SourceFileEdit { file_id, edit }.into(), field_range, )), }); @@ -197,7 +190,7 @@ mod tests { let (analysis, file_position) = analysis_and_position(ra_fixture_before); let diagnostic = analysis.diagnostics(file_position.file_id, true).unwrap().pop().unwrap(); - let (mut fix, fix_range) = diagnostic.fix.unwrap(); + let mut fix = diagnostic.fix.unwrap(); let edit = fix.source_change.source_file_edits.pop().unwrap().edit; let target_file_contents = analysis.file_text(file_position.file_id).unwrap(); let actual = { @@ -208,9 +201,10 @@ mod tests { assert_eq_text!(&after, &actual); assert!( - fix_range.start() <= file_position.offset && fix_range.end() >= file_position.offset, + 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_range, + fix.fix_trigger_range, file_position.offset ); } @@ -222,7 +216,7 @@ mod tests { let (analysis, file_pos) = analysis_and_position(ra_fixture_before); let current_file_id = file_pos.file_id; let diagnostic = analysis.diagnostics(current_file_id, true).unwrap().pop().unwrap(); - let mut fix = diagnostic.fix.unwrap().0; + let mut fix = diagnostic.fix.unwrap(); let edit = fix.source_change.source_file_edits.pop().unwrap(); let changed_file_id = edit.file_id; let before = analysis.file_text(changed_file_id).unwrap(); @@ -513,24 +507,22 @@ fn test_fn() { range: 0..8, severity: Error, fix: Some( - ( - Fix { - label: "Create module", - source_change: SourceChange { - source_file_edits: [], - file_system_edits: [ - CreateFile { - anchor: FileId( - 1, - ), - dst: "foo.rs", - }, - ], - is_snippet: false, - }, + Fix { + label: "Create module", + source_change: SourceChange { + source_file_edits: [], + file_system_edits: [ + CreateFile { + anchor: FileId( + 1, + ), + dst: "foo.rs", + }, + ], + is_snippet: false, }, - 0..8, - ), + 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 index 1955e15210..57b54a61ed 100644 --- a/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs +++ b/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs @@ -1,9 +1,9 @@ -//! Provides a way to derive fixes based on the diagnostic data. +//! Provides a way to attach fix actions to the use crate::Fix; use ast::{edit::IndentLevel, make}; use hir::{ db::AstDatabase, - diagnostics::{MissingFields, MissingOkInTailExpr, NoSuchField, UnresolvedModule}, + diagnostics::{Diagnostic, MissingFields, MissingOkInTailExpr, NoSuchField, UnresolvedModule}, HasSource, HirDisplay, Semantics, VariantDef, }; use ra_db::FileId; @@ -11,94 +11,90 @@ use ra_ide_db::{ source_change::{FileSystemEdit, SourceFileEdit}, RootDatabase, }; -use ra_syntax::{algo, ast, AstNode, TextRange}; +use ra_syntax::{algo, ast, AstNode}; use ra_text_edit::{TextEdit, TextEditBuilder}; -/// A trait to implement fot the Diagnostic that has a fix available. -pub trait DiagnosticWithFix { - /// Provides a fix with the fix range, if applicable in the current semantics. - fn fix(&self, sema: &Semantics) -> Option<(Fix, TextRange)>; +/// 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<(Fix, TextRange)> { - let fix = Fix::new( + 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(), - ); - - let root = sema.db.parse_or_expand(self.file)?; - let unresolved_module = self.decl.to_node(&root); - Some((fix, unresolved_module.syntax().text_range())) + unresolved_module.syntax().text_range(), + )) } } impl DiagnosticWithFix for NoSuchField { - fn fix(&self, sema: &Semantics) -> Option<(Fix, TextRange)> { + fn fix(&self, sema: &Semantics) -> Option { let root = sema.db.parse_or_expand(self.file)?; - let record_expr_field = self.field.to_node(&root); - let fix = - missing_struct_field_fix(&sema, self.file.original_file(sema.db), &record_expr_field)?; - Some((fix, record_expr_field.syntax().text_range())) + 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<(Fix, TextRange)> { + 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()) { - None - } else { - 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, - // old_field_list.syntax().text_range(), - )) + 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<(Fix, TextRange)> { + 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)) + Some(Fix::new("Wrap with ok", source_change, tail_expr_range)) } } -fn missing_struct_field_fix( +fn missing_record_expr_field_fix( sema: &Semantics, usage_file_id: FileId, record_expr_field: &ast::RecordExprField, @@ -159,8 +155,11 @@ fn missing_struct_field_fix( 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); + 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 { diff --git a/crates/ra_ide/src/lib.rs b/crates/ra_ide/src/lib.rs index 45a4b2421e..89fcb6f178 100644 --- a/crates/ra_ide/src/lib.rs +++ b/crates/ra_ide/src/lib.rs @@ -105,20 +105,26 @@ pub struct Diagnostic { pub message: String, pub range: TextRange, pub severity: Severity, - pub fix: Option<(Fix, TextRange)>, + pub fix: Option, } #[derive(Debug)] 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 144c641b2a..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| d.fix) - .filter(|(_fix, fix_range)| fix_range.intersect(range).is_some()) - .map(|(fix, _range)| fix); - for fix in fixes_from_diagnostics { + .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 { From db12ccee96bf37367b39ad99638d06da7123c088 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 11 Aug 2020 17:15:11 +0300 Subject: [PATCH 14/14] Better naming and docs --- crates/ra_hir/src/semantics.rs | 8 ++-- crates/ra_hir_def/src/diagnostics.rs | 2 +- crates/ra_hir_expand/src/diagnostics.rs | 4 +- crates/ra_hir_ty/src/diagnostics.rs | 39 ++++++++++--------- crates/ra_ide/src/diagnostics.rs | 2 +- .../src/diagnostics/diagnostics_with_fix.rs | 3 +- 6 files changed, 30 insertions(+), 28 deletions(-) diff --git a/crates/ra_hir/src/semantics.rs b/crates/ra_hir/src/semantics.rs index 2dfe69039f..e3c417b41b 100644 --- a/crates/ra_hir/src/semantics.rs +++ b/crates/ra_hir/src/semantics.rs @@ -138,8 +138,8 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> { self.imp.original_range(node) } - pub fn diagnostics_presentation_range(&self, diagnostics: &dyn Diagnostic) -> FileRange { - self.imp.diagnostics_presentation_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 + '_ { @@ -369,8 +369,8 @@ impl<'db> SemanticsImpl<'db> { original_range(self.db, node.as_ref()) } - fn diagnostics_presentation_range(&self, diagnostics: &dyn Diagnostic) -> FileRange { - let src = diagnostics.presentation(); + 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); diff --git a/crates/ra_hir_def/src/diagnostics.rs b/crates/ra_hir_def/src/diagnostics.rs index 9435c72544..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 presentation(&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 cc7dc3af22..b138500e73 100644 --- a/crates/ra_hir_expand/src/diagnostics.rs +++ b/crates/ra_hir_expand/src/diagnostics.rs @@ -22,8 +22,8 @@ use crate::InFile; pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static { fn message(&self) -> String; - /// A presentation source of the diagnostics, to use in highlighting and similar actions - fn presentation(&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 diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs index 24435e8a71..7ab7f79db6 100644 --- a/crates/ra_hir_ty/src/diagnostics.rs +++ b/crates/ra_hir_ty/src/diagnostics.rs @@ -37,7 +37,7 @@ impl Diagnostic for NoSuchField { "no such field".to_string() } - fn presentation(&self) -> InFile { + fn display_source(&self) -> InFile { InFile::new(self.file, self.field.clone().into()) } @@ -63,7 +63,7 @@ impl Diagnostic for MissingFields { buf } - fn presentation(&self) -> InFile { + fn display_source(&self) -> InFile { InFile { file_id: self.file, value: self @@ -95,13 +95,15 @@ impl Diagnostic for MissingPatFields { } buf } - fn presentation(&self) -> InFile { - let value = self - .field_list_parent_path - .clone() - .map(SyntaxNodePtr::from) - .unwrap_or_else(|| self.field_list_parent.clone().into()); - InFile { file_id: self.file, value } + 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 @@ -119,7 +121,7 @@ impl Diagnostic for MissingMatchArms { fn message(&self) -> String { String::from("Missing match arm") } - fn presentation(&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) { @@ -137,7 +139,7 @@ impl Diagnostic for MissingOkInTailExpr { fn message(&self) -> String { "wrap return expression in Ok".to_string() } - fn presentation(&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) { @@ -155,7 +157,7 @@ impl Diagnostic for BreakOutsideOfLoop { fn message(&self) -> String { "break outside of loop".to_string() } - fn presentation(&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) { @@ -173,7 +175,7 @@ impl Diagnostic for MissingUnsafe { fn message(&self) -> String { format!("This operation is unsafe and requires an unsafe function or block") } - fn presentation(&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) { @@ -194,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 presentation(&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) { @@ -256,12 +258,11 @@ mod tests { let mut actual: FxHashMap> = FxHashMap::default(); db.diagnostics(|d| { + let src = d.display_source(); + let root = db.parse_or_expand(src.file_id).unwrap(); // FIXME: macros... - 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 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)); }); diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 757b76fd40..1046d7ab37 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -183,7 +183,7 @@ mod tests { /// Takes a multi-file input fixture with annotated cursor positions, /// and checks that: /// * a diagnostic is produced - /// * this diagnostic fix 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); diff --git a/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs b/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs index 57b54a61ed..f7c73773f3 100644 --- a/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs +++ b/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs @@ -1,4 +1,5 @@ -//! Provides a way to attach fix actions to the +//! 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::{