From 7cae9ddeeb14aa81a911a0e69d9eec265cc364d3 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 21 Mar 2019 19:05:15 +0300 Subject: [PATCH 1/2] move diagnostics to ide_api --- crates/ra_ide_api/src/diagnostics.rs | 260 ++++++++++++++++++++- crates/ra_ide_api_light/src/diagnostics.rs | 246 ------------------- crates/ra_ide_api_light/src/lib.rs | 2 - 3 files changed, 250 insertions(+), 258 deletions(-) delete mode 100644 crates/ra_ide_api_light/src/diagnostics.rs diff --git a/crates/ra_ide_api/src/diagnostics.rs b/crates/ra_ide_api/src/diagnostics.rs index 53d95fb4c6..d487722258 100644 --- a/crates/ra_ide_api/src/diagnostics.rs +++ b/crates/ra_ide_api/src/diagnostics.rs @@ -1,21 +1,20 @@ +use itertools::Itertools; use hir::{Problem, source_binder}; use ra_ide_api_light::Severity; use ra_db::SourceDatabase; +use ra_syntax::{ + Location, SourceFile, SyntaxKind, TextRange, SyntaxNode, + ast::{self, AstNode}, -use crate::{Diagnostic, FileId, FileSystemEdit, SourceChange, db::RootDatabase}; +}; +use ra_text_edit::{TextEdit, TextEditBuilder}; + +use crate::{Diagnostic, FileId, FileSystemEdit, SourceChange, SourceFileEdit, db::RootDatabase}; pub(crate) fn diagnostics(db: &RootDatabase, file_id: FileId) -> Vec { let syntax = db.parse(file_id); - let mut res = ra_ide_api_light::diagnostics(&syntax) - .into_iter() - .map(|d| Diagnostic { - range: d.range, - message: d.msg, - severity: d.severity, - fix: d.fix.map(|fix| SourceChange::from_local_edit(file_id, fix)), - }) - .collect::>(); + let mut res = syntax_diagnostics(file_id, &syntax); if let Some(m) = source_binder::module_from_file_id(db, file_id) { for (name_node, problem) in m.problems(db) { let source_root = db.file_source_root(file_id); @@ -63,3 +62,244 @@ pub(crate) fn diagnostics(db: &RootDatabase, file_id: FileId) -> Vec }; res } + +fn syntax_diagnostics(file_id: FileId, file: &SourceFile) -> Vec { + fn location_to_range(location: Location) -> TextRange { + match location { + Location::Offset(offset) => TextRange::offset_len(offset, 1.into()), + Location::Range(range) => range, + } + } + + let mut errors: Vec = file + .errors() + .into_iter() + .map(|err| Diagnostic { + range: location_to_range(err.location()), + message: format!("Syntax Error: {}", err), + severity: Severity::Error, + fix: None, + }) + .collect(); + + for node in file.syntax().descendants() { + check_unnecessary_braces_in_use_statement(file_id, &mut errors, node); + check_struct_shorthand_initialization(file_id, &mut errors, node); + } + + errors +} + +fn check_unnecessary_braces_in_use_statement( + file_id: FileId, + acc: &mut Vec, + node: &SyntaxNode, +) -> Option<()> { + let use_tree_list = ast::UseTreeList::cast(node)?; + if let Some((single_use_tree,)) = use_tree_list.use_trees().collect_tuple() { + let range = use_tree_list.syntax().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.finish() + }); + + acc.push(Diagnostic { + range, + message: format!("Unnecessary braces in use statement"), + severity: Severity::WeakWarning, + fix: Some(SourceChange { + label: "Remove unnecessary braces".to_string(), + source_file_edits: vec![SourceFileEdit { file_id, edit }], + file_system_edits: Vec::new(), + cursor_position: None, + }), + }); + } + + Some(()) +} + +fn text_edit_for_remove_unnecessary_braces_with_self_in_use_statement( + single_use_tree: &ast::UseTree, +) -> Option { + let use_tree_list_node = single_use_tree.syntax().parent()?; + if single_use_tree.path()?.segment()?.syntax().first_child()?.kind() == SyntaxKind::SELF_KW { + let start = use_tree_list_node.prev_sibling()?.range().start(); + let end = use_tree_list_node.range().end(); + let range = TextRange::from_to(start, end); + let mut edit_builder = TextEditBuilder::default(); + edit_builder.delete(range); + return Some(edit_builder.finish()); + } + None +} + +fn check_struct_shorthand_initialization( + file_id: FileId, + acc: &mut Vec, + node: &SyntaxNode, +) -> Option<()> { + let struct_lit = ast::StructLit::cast(node)?; + let named_field_list = struct_lit.named_field_list()?; + for named_field in named_field_list.fields() { + if let (Some(name_ref), Some(expr)) = (named_field.name_ref(), named_field.expr()) { + let field_name = name_ref.syntax().text().to_string(); + let field_expr = expr.syntax().text().to_string(); + if field_name == field_expr { + let mut edit_builder = TextEditBuilder::default(); + edit_builder.delete(named_field.syntax().range()); + edit_builder.insert(named_field.syntax().range().start(), field_name); + let edit = edit_builder.finish(); + + acc.push(Diagnostic { + range: named_field.syntax().range(), + message: format!("Shorthand struct initialization"), + severity: Severity::WeakWarning, + fix: Some(SourceChange { + label: "use struct shorthand initialization".to_string(), + source_file_edits: vec![SourceFileEdit { file_id, edit }], + file_system_edits: Vec::new(), + cursor_position: None, + }), + }); + } + } + } + Some(()) +} + +#[cfg(test)] +mod tests { + use test_utils::assert_eq_text; + + use super::*; + + type DiagnosticChecker = fn(FileId, &mut Vec, &SyntaxNode) -> Option<()>; + + fn check_not_applicable(code: &str, func: DiagnosticChecker) { + let file = SourceFile::parse(code); + let mut diagnostics = Vec::new(); + for node in file.syntax().descendants() { + func(FileId(0), &mut diagnostics, node); + } + assert!(diagnostics.is_empty()); + } + + fn check_apply(before: &str, after: &str, func: DiagnosticChecker) { + let file = SourceFile::parse(before); + let mut diagnostics = Vec::new(); + for node in file.syntax().descendants() { + func(FileId(0), &mut diagnostics, node); + } + let diagnostic = + diagnostics.pop().unwrap_or_else(|| panic!("no diagnostics for:\n{}\n", before)); + let mut fix = diagnostic.fix.unwrap(); + let edit = fix.source_file_edits.pop().unwrap().edit; + let actual = edit.apply(&before); + assert_eq_text!(after, &actual); + } + + #[test] + fn test_check_unnecessary_braces_in_use_statement() { + check_not_applicable( + " + use a; + use a::{c, d::e}; + ", + check_unnecessary_braces_in_use_statement, + ); + check_apply("use {b};", "use b;", check_unnecessary_braces_in_use_statement); + check_apply("use a::{c};", "use a::c;", check_unnecessary_braces_in_use_statement); + check_apply("use a::{self};", "use a;", check_unnecessary_braces_in_use_statement); + check_apply( + "use a::{c, d::{e}};", + "use a::{c, d::e};", + check_unnecessary_braces_in_use_statement, + ); + } + + #[test] + fn test_check_struct_shorthand_initialization() { + check_not_applicable( + r#" + struct A { + a: &'static str + } + + fn main() { + A { + a: "hello" + } + } + "#, + check_struct_shorthand_initialization, + ); + + check_apply( + r#" +struct A { + a: &'static str +} + +fn main() { + let a = "haha"; + A { + a: a + } +} + "#, + r#" +struct A { + a: &'static str +} + +fn main() { + let a = "haha"; + A { + a + } +} + "#, + check_struct_shorthand_initialization, + ); + + check_apply( + r#" +struct A { + a: &'static str, + b: &'static str +} + +fn main() { + let a = "haha"; + let b = "bb"; + A { + a: a, + b + } +} + "#, + r#" +struct A { + a: &'static str, + b: &'static str +} + +fn main() { + let a = "haha"; + let b = "bb"; + A { + a, + b + } +} + "#, + check_struct_shorthand_initialization, + ); + } +} diff --git a/crates/ra_ide_api_light/src/diagnostics.rs b/crates/ra_ide_api_light/src/diagnostics.rs deleted file mode 100644 index 7c383ca2af..0000000000 --- a/crates/ra_ide_api_light/src/diagnostics.rs +++ /dev/null @@ -1,246 +0,0 @@ -use itertools::Itertools; - -use ra_syntax::{ - Location, SourceFile, SyntaxKind, TextRange, SyntaxNode, - ast::{self, AstNode}, - -}; -use ra_text_edit::{TextEdit, TextEditBuilder}; - -use crate::{Diagnostic, LocalEdit, Severity}; - -pub fn diagnostics(file: &SourceFile) -> Vec { - fn location_to_range(location: Location) -> TextRange { - match location { - Location::Offset(offset) => TextRange::offset_len(offset, 1.into()), - Location::Range(range) => range, - } - } - - let mut errors: Vec = file - .errors() - .into_iter() - .map(|err| Diagnostic { - range: location_to_range(err.location()), - msg: format!("Syntax Error: {}", err), - severity: Severity::Error, - fix: None, - }) - .collect(); - - for node in file.syntax().descendants() { - check_unnecessary_braces_in_use_statement(&mut errors, node); - check_struct_shorthand_initialization(&mut errors, node); - } - - errors -} - -fn check_unnecessary_braces_in_use_statement( - acc: &mut Vec, - node: &SyntaxNode, -) -> Option<()> { - let use_tree_list = ast::UseTreeList::cast(node)?; - if let Some((single_use_tree,)) = use_tree_list.use_trees().collect_tuple() { - let range = use_tree_list.syntax().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.finish() - }); - - acc.push(Diagnostic { - range, - msg: format!("Unnecessary braces in use statement"), - severity: Severity::WeakWarning, - fix: Some(LocalEdit { - label: "Remove unnecessary braces".to_string(), - edit, - cursor_position: None, - }), - }); - } - - Some(()) -} - -fn text_edit_for_remove_unnecessary_braces_with_self_in_use_statement( - single_use_tree: &ast::UseTree, -) -> Option { - let use_tree_list_node = single_use_tree.syntax().parent()?; - if single_use_tree.path()?.segment()?.syntax().first_child()?.kind() == SyntaxKind::SELF_KW { - let start = use_tree_list_node.prev_sibling()?.range().start(); - let end = use_tree_list_node.range().end(); - let range = TextRange::from_to(start, end); - let mut edit_builder = TextEditBuilder::default(); - edit_builder.delete(range); - return Some(edit_builder.finish()); - } - None -} - -fn check_struct_shorthand_initialization( - acc: &mut Vec, - node: &SyntaxNode, -) -> Option<()> { - let struct_lit = ast::StructLit::cast(node)?; - let named_field_list = struct_lit.named_field_list()?; - for named_field in named_field_list.fields() { - if let (Some(name_ref), Some(expr)) = (named_field.name_ref(), named_field.expr()) { - let field_name = name_ref.syntax().text().to_string(); - let field_expr = expr.syntax().text().to_string(); - if field_name == field_expr { - let mut edit_builder = TextEditBuilder::default(); - edit_builder.delete(named_field.syntax().range()); - edit_builder.insert(named_field.syntax().range().start(), field_name); - let edit = edit_builder.finish(); - - acc.push(Diagnostic { - range: named_field.syntax().range(), - msg: format!("Shorthand struct initialization"), - severity: Severity::WeakWarning, - fix: Some(LocalEdit { - label: "use struct shorthand initialization".to_string(), - edit, - cursor_position: None, - }), - }); - } - } - } - Some(()) -} - -#[cfg(test)] -mod tests { - use crate::test_utils::assert_eq_text; - - use super::*; - - type DiagnosticChecker = fn(&mut Vec, &SyntaxNode) -> Option<()>; - - fn check_not_applicable(code: &str, func: DiagnosticChecker) { - let file = SourceFile::parse(code); - let mut diagnostics = Vec::new(); - for node in file.syntax().descendants() { - func(&mut diagnostics, node); - } - assert!(diagnostics.is_empty()); - } - - fn check_apply(before: &str, after: &str, func: DiagnosticChecker) { - let file = SourceFile::parse(before); - let mut diagnostics = Vec::new(); - for node in file.syntax().descendants() { - func(&mut diagnostics, node); - } - let diagnostic = - diagnostics.pop().unwrap_or_else(|| panic!("no diagnostics for:\n{}\n", before)); - let fix = diagnostic.fix.unwrap(); - let actual = fix.edit.apply(&before); - assert_eq_text!(after, &actual); - } - - #[test] - fn test_check_unnecessary_braces_in_use_statement() { - check_not_applicable( - " - use a; - use a::{c, d::e}; - ", - check_unnecessary_braces_in_use_statement, - ); - check_apply("use {b};", "use b;", check_unnecessary_braces_in_use_statement); - check_apply("use a::{c};", "use a::c;", check_unnecessary_braces_in_use_statement); - check_apply("use a::{self};", "use a;", check_unnecessary_braces_in_use_statement); - check_apply( - "use a::{c, d::{e}};", - "use a::{c, d::e};", - check_unnecessary_braces_in_use_statement, - ); - } - - #[test] - fn test_check_struct_shorthand_initialization() { - check_not_applicable( - r#" - struct A { - a: &'static str - } - - fn main() { - A { - a: "hello" - } - } - "#, - check_struct_shorthand_initialization, - ); - - check_apply( - r#" -struct A { - a: &'static str -} - -fn main() { - let a = "haha"; - A { - a: a - } -} - "#, - r#" -struct A { - a: &'static str -} - -fn main() { - let a = "haha"; - A { - a - } -} - "#, - check_struct_shorthand_initialization, - ); - - check_apply( - r#" -struct A { - a: &'static str, - b: &'static str -} - -fn main() { - let a = "haha"; - let b = "bb"; - A { - a: a, - b - } -} - "#, - r#" -struct A { - a: &'static str, - b: &'static str -} - -fn main() { - let a = "haha"; - let b = "bb"; - A { - a, - b - } -} - "#, - check_struct_shorthand_initialization, - ); - } -} diff --git a/crates/ra_ide_api_light/src/lib.rs b/crates/ra_ide_api_light/src/lib.rs index ca13eb018c..47b30255b9 100644 --- a/crates/ra_ide_api_light/src/lib.rs +++ b/crates/ra_ide_api_light/src/lib.rs @@ -11,7 +11,6 @@ mod structure; mod test_utils; mod join_lines; mod typing; -mod diagnostics; use rustc_hash::FxHashSet; use ra_text_edit::TextEditBuilder; @@ -27,7 +26,6 @@ pub use crate::{ line_index::{LineCol, LineIndex}, line_index_utils::translate_offset_with_edit, structure::{file_structure, StructureNode}, - diagnostics::diagnostics, join_lines::join_lines, typing::{on_enter, on_dot_typed, on_eq_typed}, }; From 1b58e3e410187470b1993114f9557b2181efb75a Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 21 Mar 2019 19:21:00 +0300 Subject: [PATCH 2/2] cleanup --- crates/ra_ide_api/src/diagnostics.rs | 141 ++++++++++++++------------- 1 file changed, 73 insertions(+), 68 deletions(-) diff --git a/crates/ra_ide_api/src/diagnostics.rs b/crates/ra_ide_api/src/diagnostics.rs index d487722258..0690925284 100644 --- a/crates/ra_ide_api/src/diagnostics.rs +++ b/crates/ra_ide_api/src/diagnostics.rs @@ -12,58 +12,23 @@ use ra_text_edit::{TextEdit, TextEditBuilder}; use crate::{Diagnostic, FileId, FileSystemEdit, SourceChange, SourceFileEdit, db::RootDatabase}; pub(crate) fn diagnostics(db: &RootDatabase, file_id: FileId) -> Vec { - let syntax = db.parse(file_id); + let source_file = db.parse(file_id); + let mut res = Vec::new(); + + syntax_errors(&mut res, &source_file); + + for node in source_file.syntax().descendants() { + check_unnecessary_braces_in_use_statement(&mut res, file_id, node); + check_struct_shorthand_initialization(&mut res, file_id, node); + } - let mut res = syntax_diagnostics(file_id, &syntax); if let Some(m) = source_binder::module_from_file_id(db, file_id) { - for (name_node, problem) in m.problems(db) { - let source_root = db.file_source_root(file_id); - let diag = match problem { - Problem::UnresolvedModule { candidate } => { - let create_file = - FileSystemEdit::CreateFile { source_root, path: candidate.clone() }; - let fix = SourceChange { - label: "create module".to_string(), - source_file_edits: Vec::new(), - file_system_edits: vec![create_file], - cursor_position: None, - }; - Diagnostic { - range: name_node.range(), - message: "unresolved module".to_string(), - severity: Severity::Error, - fix: Some(fix), - } - } - Problem::NotDirOwner { move_to, candidate } => { - let move_file = FileSystemEdit::MoveFile { - src: file_id, - dst_source_root: source_root, - dst_path: move_to.clone(), - }; - let create_file = - FileSystemEdit::CreateFile { source_root, path: move_to.join(candidate) }; - let fix = SourceChange { - label: "move file and create module".to_string(), - source_file_edits: Vec::new(), - file_system_edits: vec![move_file, create_file], - cursor_position: None, - }; - Diagnostic { - range: name_node.range(), - message: "can't declare module at this location".to_string(), - severity: Severity::Error, - fix: Some(fix), - } - } - }; - res.push(diag) - } + check_module(&mut res, db, file_id, m); }; res } -fn syntax_diagnostics(file_id: FileId, file: &SourceFile) -> Vec { +fn syntax_errors(acc: &mut Vec, source_file: &SourceFile) { fn location_to_range(location: Location) -> TextRange { match location { Location::Offset(offset) => TextRange::offset_len(offset, 1.into()), @@ -71,28 +36,17 @@ fn syntax_diagnostics(file_id: FileId, file: &SourceFile) -> Vec { } } - let mut errors: Vec = file - .errors() - .into_iter() - .map(|err| Diagnostic { - range: location_to_range(err.location()), - message: format!("Syntax Error: {}", err), - severity: Severity::Error, - fix: None, - }) - .collect(); - - for node in file.syntax().descendants() { - check_unnecessary_braces_in_use_statement(file_id, &mut errors, node); - check_struct_shorthand_initialization(file_id, &mut errors, node); - } - - errors + acc.extend(source_file.errors().into_iter().map(|err| Diagnostic { + range: location_to_range(err.location()), + message: format!("Syntax Error: {}", err), + severity: Severity::Error, + fix: None, + })); } fn check_unnecessary_braces_in_use_statement( - file_id: FileId, acc: &mut Vec, + file_id: FileId, node: &SyntaxNode, ) -> Option<()> { let use_tree_list = ast::UseTreeList::cast(node)?; @@ -140,8 +94,8 @@ fn text_edit_for_remove_unnecessary_braces_with_self_in_use_statement( } fn check_struct_shorthand_initialization( - file_id: FileId, acc: &mut Vec, + file_id: FileId, node: &SyntaxNode, ) -> Option<()> { let struct_lit = ast::StructLit::cast(node)?; @@ -173,19 +127,70 @@ fn check_struct_shorthand_initialization( Some(()) } +fn check_module( + acc: &mut Vec, + db: &RootDatabase, + file_id: FileId, + module: hir::Module, +) { + let source_root = db.file_source_root(file_id); + for (name_node, problem) in module.problems(db) { + let diag = match problem { + Problem::UnresolvedModule { candidate } => { + let create_file = + FileSystemEdit::CreateFile { source_root, path: candidate.clone() }; + let fix = SourceChange { + label: "create module".to_string(), + source_file_edits: Vec::new(), + file_system_edits: vec![create_file], + cursor_position: None, + }; + Diagnostic { + range: name_node.range(), + message: "unresolved module".to_string(), + severity: Severity::Error, + fix: Some(fix), + } + } + Problem::NotDirOwner { move_to, candidate } => { + let move_file = FileSystemEdit::MoveFile { + src: file_id, + dst_source_root: source_root, + dst_path: move_to.clone(), + }; + let create_file = + FileSystemEdit::CreateFile { source_root, path: move_to.join(candidate) }; + let fix = SourceChange { + label: "move file and create module".to_string(), + source_file_edits: Vec::new(), + file_system_edits: vec![move_file, create_file], + cursor_position: None, + }; + Diagnostic { + range: name_node.range(), + message: "can't declare module at this location".to_string(), + severity: Severity::Error, + fix: Some(fix), + } + } + }; + acc.push(diag) + } +} + #[cfg(test)] mod tests { use test_utils::assert_eq_text; use super::*; - type DiagnosticChecker = fn(FileId, &mut Vec, &SyntaxNode) -> Option<()>; + type DiagnosticChecker = fn(&mut Vec, FileId, &SyntaxNode) -> Option<()>; fn check_not_applicable(code: &str, func: DiagnosticChecker) { let file = SourceFile::parse(code); let mut diagnostics = Vec::new(); for node in file.syntax().descendants() { - func(FileId(0), &mut diagnostics, node); + func(&mut diagnostics, FileId(0), node); } assert!(diagnostics.is_empty()); } @@ -194,7 +199,7 @@ mod tests { let file = SourceFile::parse(before); let mut diagnostics = Vec::new(); for node in file.syntax().descendants() { - func(FileId(0), &mut diagnostics, node); + func(&mut diagnostics, FileId(0), node); } let diagnostic = diagnostics.pop().unwrap_or_else(|| panic!("no diagnostics for:\n{}\n", before));