From 3d2f0400a26ef6b07d61a06e1b543072b627570e Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 14 Jun 2021 12:24:58 +0300 Subject: [PATCH 1/9] internal: start ide diagnostics crate --- Cargo.lock | 4 ++++ crates/ide_diagnostics/Cargo.toml | 12 ++++++++++++ crates/ide_diagnostics/src/lib.rs | 0 3 files changed, 16 insertions(+) create mode 100644 crates/ide_diagnostics/Cargo.toml create mode 100644 crates/ide_diagnostics/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index e47b879647..04c2353416 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -668,6 +668,10 @@ dependencies = [ "text_edit", ] +[[package]] +name = "ide_diagnostics" +version = "0.0.0" + [[package]] name = "ide_ssr" version = "0.0.0" diff --git a/crates/ide_diagnostics/Cargo.toml b/crates/ide_diagnostics/Cargo.toml new file mode 100644 index 0000000000..11cd8d5701 --- /dev/null +++ b/crates/ide_diagnostics/Cargo.toml @@ -0,0 +1,12 @@ +[package] +name = "ide_diagnostics" +version = "0.0.0" +description = "TBD" +license = "MIT OR Apache-2.0" +authors = ["rust-analyzer developers"] +edition = "2018" + +[lib] +doctest = false + +[dependencies] diff --git a/crates/ide_diagnostics/src/lib.rs b/crates/ide_diagnostics/src/lib.rs new file mode 100644 index 0000000000..e69de29bb2 From 1d2772c2c7dc0a42d8a9429d24ea41412add61b3 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 14 Jun 2021 13:15:05 +0300 Subject: [PATCH 2/9] internal: move diagnostics to a new crate --- Cargo.lock | 17 + crates/base_db/src/fixture.rs | 8 + crates/ide/Cargo.toml | 1 + crates/ide/src/diagnostics.rs | 498 ----------------- crates/ide/src/fixture.rs | 8 - crates/ide/src/lib.rs | 7 +- crates/ide_diagnostics/Cargo.toml | 18 + .../src}/break_outside_of_loop.rs | 4 +- .../src}/field_shorthand.rs | 4 +- .../src}/inactive_code.rs | 7 +- .../src}/incorrect_case.rs | 26 +- crates/ide_diagnostics/src/lib.rs | 510 ++++++++++++++++++ .../src}/macro_error.rs | 4 +- .../src}/mismatched_arg_count.rs | 4 +- .../src}/missing_fields.rs | 4 +- .../src}/missing_match_arms.rs | 6 +- .../src}/missing_ok_or_some_in_tail_expr.rs | 4 +- .../src}/missing_unsafe.rs | 4 +- .../src}/no_such_field.rs | 7 +- .../src}/remove_this_semicolon.rs | 7 +- .../replace_filter_map_next_with_find_map.rs | 9 +- .../src}/unimplemented_builtin_macro.rs | 5 +- .../src}/unlinked_file.rs | 7 +- .../src}/unresolved_extern_crate.rs | 4 +- .../src}/unresolved_import.rs | 4 +- .../src}/unresolved_macro_call.rs | 4 +- .../src}/unresolved_module.rs | 4 +- .../src}/unresolved_proc_macro.rs | 5 +- 28 files changed, 612 insertions(+), 578 deletions(-) delete mode 100644 crates/ide/src/diagnostics.rs rename crates/{ide/src/diagnostics => ide_diagnostics/src}/break_outside_of_loop.rs (84%) rename crates/{ide/src/diagnostics => ide_diagnostics/src}/field_shorthand.rs (97%) rename crates/{ide/src/diagnostics => ide_diagnostics/src}/inactive_code.rs (95%) rename crates/{ide/src/diagnostics => ide_diagnostics/src}/incorrect_case.rs (94%) rename crates/{ide/src/diagnostics => ide_diagnostics/src}/macro_error.rs (96%) rename crates/{ide/src/diagnostics => ide_diagnostics/src}/mismatched_arg_count.rs (97%) rename crates/{ide/src/diagnostics => ide_diagnostics/src}/missing_fields.rs (98%) rename crates/{ide/src/diagnostics => ide_diagnostics/src}/missing_match_arms.rs (99%) rename crates/{ide/src/diagnostics => ide_diagnostics/src}/missing_ok_or_some_in_tail_expr.rs (97%) rename crates/{ide/src/diagnostics => ide_diagnostics/src}/missing_unsafe.rs (95%) rename crates/{ide/src/diagnostics => ide_diagnostics/src}/no_such_field.rs (97%) rename crates/{ide/src/diagnostics => ide_diagnostics/src}/remove_this_semicolon.rs (91%) rename crates/{ide/src/diagnostics => ide_diagnostics/src}/replace_filter_map_next_with_find_map.rs (95%) rename crates/{ide/src/diagnostics => ide_diagnostics/src}/unimplemented_builtin_macro.rs (86%) rename crates/{ide/src/diagnostics => ide_diagnostics/src}/unlinked_file.rs (97%) rename crates/{ide/src/diagnostics => ide_diagnostics/src}/unresolved_extern_crate.rs (90%) rename crates/{ide/src/diagnostics => ide_diagnostics/src}/unresolved_import.rs (94%) rename crates/{ide/src/diagnostics => ide_diagnostics/src}/unresolved_macro_call.rs (94%) rename crates/{ide/src/diagnostics => ide_diagnostics/src}/unresolved_module.rs (96%) rename crates/{ide/src/diagnostics => ide_diagnostics/src}/unresolved_proc_macro.rs (93%) diff --git a/Cargo.lock b/Cargo.lock index 04c2353416..55016ccf7f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -591,6 +591,7 @@ dependencies = [ "ide_assists", "ide_completion", "ide_db", + "ide_diagnostics", "ide_ssr", "indexmap", "itertools", @@ -671,6 +672,22 @@ dependencies = [ [[package]] name = "ide_diagnostics" version = "0.0.0" +dependencies = [ + "cfg", + "cov-mark", + "either", + "expect-test", + "hir", + "ide_assists", + "ide_db", + "itertools", + "profile", + "rustc-hash", + "stdx", + "syntax", + "test_utils", + "text_edit", +] [[package]] name = "ide_ssr" diff --git a/crates/base_db/src/fixture.rs b/crates/base_db/src/fixture.rs index da4afb5ebc..1b17db102c 100644 --- a/crates/base_db/src/fixture.rs +++ b/crates/base_db/src/fixture.rs @@ -24,6 +24,14 @@ pub trait WithFixture: Default + SourceDatabaseExt + 'static { (db, fixture.files[0]) } + fn with_many_files(ra_fixture: &str) -> (Self, Vec) { + let fixture = ChangeFixture::parse(ra_fixture); + let mut db = Self::default(); + fixture.change.apply(&mut db); + assert!(fixture.file_position.is_none()); + (db, fixture.files) + } + fn with_files(ra_fixture: &str) -> Self { let fixture = ChangeFixture::parse(ra_fixture); let mut db = Self::default(); diff --git a/crates/ide/Cargo.toml b/crates/ide/Cargo.toml index f12928225f..0e84473940 100644 --- a/crates/ide/Cargo.toml +++ b/crates/ide/Cargo.toml @@ -29,6 +29,7 @@ ide_db = { path = "../ide_db", version = "0.0.0" } cfg = { path = "../cfg", version = "0.0.0" } profile = { path = "../profile", version = "0.0.0" } ide_assists = { path = "../ide_assists", version = "0.0.0" } +ide_diagnostics = { path = "../ide_diagnostics", version = "0.0.0" } ide_ssr = { path = "../ide_ssr", version = "0.0.0" } ide_completion = { path = "../ide_completion", version = "0.0.0" } diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs deleted file mode 100644 index 815a633e5e..0000000000 --- a/crates/ide/src/diagnostics.rs +++ /dev/null @@ -1,498 +0,0 @@ -//! Collects diagnostics & fixits for a single file. -//! -//! The tricky bit here is that diagnostics are produced by hir in terms of -//! macro-expanded files, but we need to present them to the users in terms of -//! original files. So we need to map the ranges. - -mod break_outside_of_loop; -mod inactive_code; -mod incorrect_case; -mod macro_error; -mod mismatched_arg_count; -mod missing_fields; -mod missing_match_arms; -mod missing_ok_or_some_in_tail_expr; -mod missing_unsafe; -mod no_such_field; -mod remove_this_semicolon; -mod replace_filter_map_next_with_find_map; -mod unimplemented_builtin_macro; -mod unlinked_file; -mod unresolved_extern_crate; -mod unresolved_import; -mod unresolved_macro_call; -mod unresolved_module; -mod unresolved_proc_macro; - -mod field_shorthand; - -use hir::{diagnostics::AnyDiagnostic, Semantics}; -use ide_assists::AssistResolveStrategy; -use ide_db::{base_db::SourceDatabase, RootDatabase}; -use itertools::Itertools; -use rustc_hash::FxHashSet; -use syntax::{ - ast::{self, AstNode}, - SyntaxNode, TextRange, -}; -use text_edit::TextEdit; -use unlinked_file::UnlinkedFile; - -use crate::{Assist, AssistId, AssistKind, FileId, Label, SourceChange}; - -#[derive(Copy, Clone, Debug, PartialEq)] -pub struct DiagnosticCode(pub &'static str); - -impl DiagnosticCode { - pub fn as_str(&self) -> &str { - self.0 - } -} - -#[derive(Debug)] -pub struct Diagnostic { - pub code: DiagnosticCode, - pub message: String, - pub range: TextRange, - pub severity: Severity, - pub unused: bool, - pub experimental: bool, - pub fixes: Option>, -} - -impl Diagnostic { - fn new(code: &'static str, message: impl Into, range: TextRange) -> Diagnostic { - let message = message.into(); - Diagnostic { - code: DiagnosticCode(code), - message, - range, - severity: Severity::Error, - unused: false, - experimental: false, - fixes: None, - } - } - - fn experimental(mut self) -> Diagnostic { - self.experimental = true; - self - } - - fn severity(mut self, severity: Severity) -> Diagnostic { - self.severity = severity; - self - } - - fn with_fixes(mut self, fixes: Option>) -> Diagnostic { - self.fixes = fixes; - self - } - - fn with_unused(mut self, unused: bool) -> Diagnostic { - self.unused = unused; - self - } -} - -#[derive(Debug, Copy, Clone)] -pub enum Severity { - Error, - WeakWarning, -} - -#[derive(Default, Debug, Clone)] -pub struct DiagnosticsConfig { - pub disable_experimental: bool, - pub disabled: FxHashSet, -} - -struct DiagnosticsContext<'a> { - config: &'a DiagnosticsConfig, - sema: Semantics<'a, RootDatabase>, - resolve: &'a AssistResolveStrategy, -} - -pub(crate) fn diagnostics( - db: &RootDatabase, - config: &DiagnosticsConfig, - resolve: &AssistResolveStrategy, - file_id: FileId, -) -> Vec { - let _p = profile::span("diagnostics"); - let sema = Semantics::new(db); - let parse = db.parse(file_id); - let mut res = Vec::new(); - - // [#34344] Only take first 128 errors to prevent slowing down editor/ide, the number 128 is chosen arbitrarily. - res.extend( - parse.errors().iter().take(128).map(|err| { - Diagnostic::new("syntax-error", format!("Syntax Error: {}", err), err.range()) - }), - ); - - for node in parse.tree().syntax().descendants() { - check_unnecessary_braces_in_use_statement(&mut res, file_id, &node); - field_shorthand::check(&mut res, file_id, &node); - } - - let mut diags = Vec::new(); - let module = sema.to_module_def(file_id); - if let Some(m) = module { - m.diagnostics(db, &mut diags) - } - - let ctx = DiagnosticsContext { config, sema, resolve }; - if module.is_none() { - let d = UnlinkedFile { file: file_id }; - let d = unlinked_file::unlinked_file(&ctx, &d); - res.push(d) - } - - for diag in diags { - #[rustfmt::skip] - let d = match diag { - AnyDiagnostic::BreakOutsideOfLoop(d) => break_outside_of_loop::break_outside_of_loop(&ctx, &d), - AnyDiagnostic::IncorrectCase(d) => incorrect_case::incorrect_case(&ctx, &d), - AnyDiagnostic::MacroError(d) => macro_error::macro_error(&ctx, &d), - AnyDiagnostic::MismatchedArgCount(d) => mismatched_arg_count::mismatched_arg_count(&ctx, &d), - AnyDiagnostic::MissingFields(d) => missing_fields::missing_fields(&ctx, &d), - AnyDiagnostic::MissingMatchArms(d) => missing_match_arms::missing_match_arms(&ctx, &d), - AnyDiagnostic::MissingOkOrSomeInTailExpr(d) => missing_ok_or_some_in_tail_expr::missing_ok_or_some_in_tail_expr(&ctx, &d), - AnyDiagnostic::MissingUnsafe(d) => missing_unsafe::missing_unsafe(&ctx, &d), - AnyDiagnostic::NoSuchField(d) => no_such_field::no_such_field(&ctx, &d), - AnyDiagnostic::RemoveThisSemicolon(d) => remove_this_semicolon::remove_this_semicolon(&ctx, &d), - AnyDiagnostic::ReplaceFilterMapNextWithFindMap(d) => replace_filter_map_next_with_find_map::replace_filter_map_next_with_find_map(&ctx, &d), - AnyDiagnostic::UnimplementedBuiltinMacro(d) => unimplemented_builtin_macro::unimplemented_builtin_macro(&ctx, &d), - AnyDiagnostic::UnresolvedExternCrate(d) => unresolved_extern_crate::unresolved_extern_crate(&ctx, &d), - AnyDiagnostic::UnresolvedImport(d) => unresolved_import::unresolved_import(&ctx, &d), - AnyDiagnostic::UnresolvedMacroCall(d) => unresolved_macro_call::unresolved_macro_call(&ctx, &d), - AnyDiagnostic::UnresolvedModule(d) => unresolved_module::unresolved_module(&ctx, &d), - AnyDiagnostic::UnresolvedProcMacro(d) => unresolved_proc_macro::unresolved_proc_macro(&ctx, &d), - - AnyDiagnostic::InactiveCode(d) => match inactive_code::inactive_code(&ctx, &d) { - Some(it) => it, - None => continue, - } - }; - res.push(d) - } - - res.retain(|d| { - !ctx.config.disabled.contains(d.code.as_str()) - && !(ctx.config.disable_experimental && d.experimental) - }); - - res -} - -fn check_unnecessary_braces_in_use_statement( - acc: &mut Vec, - file_id: FileId, - node: &SyntaxNode, -) -> Option<()> { - let use_tree_list = ast::UseTreeList::cast(node.clone())?; - if let Some((single_use_tree,)) = use_tree_list.use_trees().collect_tuple() { - // If there is a comment inside the bracketed `use`, - // assume it is a commented out module path and don't show diagnostic. - if use_tree_list.has_inner_comment() { - return Some(()); - } - - 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 = TextEdit::builder(); - edit_builder.delete(use_range); - edit_builder.insert(use_range.start(), to_replace); - edit_builder.finish() - }); - - acc.push( - Diagnostic::new( - "unnecessary-braces", - "Unnecessary braces in use statement".to_string(), - use_range, - ) - .severity(Severity::WeakWarning) - .with_fixes(Some(vec![fix( - "remove_braces", - "Remove unnecessary braces", - SourceChange::from_text_edit(file_id, edit), - use_range, - )])), - ); - } - - 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()?.self_token().is_some() { - let start = use_tree_list_node.prev_sibling_or_token()?.text_range().start(); - let end = use_tree_list_node.text_range().end(); - return Some(TextEdit::delete(TextRange::new(start, end))); - } - None -} - -fn fix(id: &'static str, label: &str, source_change: SourceChange, target: TextRange) -> Assist { - let mut res = unresolved_fix(id, label, target); - res.source_change = Some(source_change); - res -} - -fn unresolved_fix(id: &'static str, label: &str, target: TextRange) -> Assist { - assert!(!id.contains(' ')); - Assist { - id: AssistId(id, AssistKind::QuickFix), - label: Label::new(label), - group: None, - target, - source_change: None, - } -} - -#[cfg(test)] -mod tests { - use expect_test::Expect; - use ide_assists::AssistResolveStrategy; - use stdx::trim_indent; - use test_utils::{assert_eq_text, extract_annotations}; - - use crate::{fixture, DiagnosticsConfig}; - - /// Takes a multi-file input fixture with annotated cursor positions, - /// and checks that: - /// * a diagnostic is produced - /// * the first 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 - #[track_caller] - pub(crate) fn check_fix(ra_fixture_before: &str, ra_fixture_after: &str) { - check_nth_fix(0, ra_fixture_before, ra_fixture_after); - } - /// Takes a multi-file input fixture with annotated cursor positions, - /// and checks that: - /// * a diagnostic is produced - /// * every diagnostic fixes trigger range touches the input cursor position - /// * that the contents of the file containing the cursor match `after` after each diagnostic fix is applied - pub(crate) fn check_fixes(ra_fixture_before: &str, ra_fixtures_after: Vec<&str>) { - for (i, ra_fixture_after) in ra_fixtures_after.iter().enumerate() { - check_nth_fix(i, ra_fixture_before, ra_fixture_after) - } - } - - #[track_caller] - fn check_nth_fix(nth: usize, ra_fixture_before: &str, ra_fixture_after: &str) { - let after = trim_indent(ra_fixture_after); - - let (analysis, file_position) = fixture::position(ra_fixture_before); - let diagnostic = analysis - .diagnostics( - &DiagnosticsConfig::default(), - AssistResolveStrategy::All, - file_position.file_id, - ) - .unwrap() - .pop() - .expect("no diagnostics"); - let fix = &diagnostic.fixes.expect("diagnostic misses fixes")[nth]; - let actual = { - let source_change = fix.source_change.as_ref().unwrap(); - let file_id = *source_change.source_file_edits.keys().next().unwrap(); - let mut actual = analysis.file_text(file_id).unwrap().to_string(); - - for edit in source_change.source_file_edits.values() { - edit.apply(&mut actual); - } - actual - }; - - assert_eq_text!(&after, &actual); - assert!( - fix.target.contains_inclusive(file_position.offset), - "diagnostic fix range {:?} does not touch cursor position {:?}", - fix.target, - file_position.offset - ); - } - - /// Checks that there's a diagnostic *without* fix at `$0`. - pub(crate) fn check_no_fix(ra_fixture: &str) { - let (analysis, file_position) = fixture::position(ra_fixture); - let diagnostic = analysis - .diagnostics( - &DiagnosticsConfig::default(), - AssistResolveStrategy::All, - file_position.file_id, - ) - .unwrap() - .pop() - .unwrap(); - assert!(diagnostic.fixes.is_none(), "got a fix when none was expected: {:?}", diagnostic); - } - - pub(crate) fn check_expect(ra_fixture: &str, expect: Expect) { - let (analysis, file_id) = fixture::file(ra_fixture); - let diagnostics = analysis - .diagnostics(&DiagnosticsConfig::default(), AssistResolveStrategy::All, file_id) - .unwrap(); - expect.assert_debug_eq(&diagnostics) - } - - #[track_caller] - pub(crate) fn check_diagnostics(ra_fixture: &str) { - let mut config = DiagnosticsConfig::default(); - config.disabled.insert("inactive-code".to_string()); - check_diagnostics_with_config(config, ra_fixture) - } - - #[track_caller] - pub(crate) fn check_diagnostics_with_config(config: DiagnosticsConfig, ra_fixture: &str) { - let (analysis, files) = fixture::files(ra_fixture); - for file_id in files { - let diagnostics = - analysis.diagnostics(&config, AssistResolveStrategy::All, file_id).unwrap(); - - let expected = extract_annotations(&*analysis.file_text(file_id).unwrap()); - let mut actual = - diagnostics.into_iter().map(|d| (d.range, d.message)).collect::>(); - actual.sort_by_key(|(range, _)| range.start()); - assert_eq!(expected, actual); - } - } - - #[test] - fn test_check_unnecessary_braces_in_use_statement() { - check_diagnostics( - r#" -use a; -use a::{c, d::e}; - -mod a { - mod c {} - mod d { - mod e {} - } -} -"#, - ); - check_diagnostics( - r#" -use a; -use a::{ - c, - // d::e -}; - -mod a { - mod c {} - mod d { - mod e {} - } -} -"#, - ); - check_fix( - r" - mod b {} - use {$0b}; - ", - r" - mod b {} - use b; - ", - ); - check_fix( - r" - mod b {} - use {b$0}; - ", - r" - mod b {} - use b; - ", - ); - check_fix( - r" - mod a { mod c {} } - use a::{c$0}; - ", - r" - mod a { mod c {} } - use a::c; - ", - ); - check_fix( - r" - mod a {} - use a::{self$0}; - ", - r" - mod a {} - use a; - ", - ); - check_fix( - r" - mod a { mod c {} mod d { mod e {} } } - use a::{c, d::{e$0}}; - ", - r" - mod a { mod c {} mod d { mod e {} } } - use a::{c, d::e}; - ", - ); - } - - #[test] - fn test_disabled_diagnostics() { - let mut config = DiagnosticsConfig::default(); - config.disabled.insert("unresolved-module".into()); - - let (analysis, file_id) = fixture::file(r#"mod foo;"#); - - let diagnostics = - analysis.diagnostics(&config, AssistResolveStrategy::All, file_id).unwrap(); - assert!(diagnostics.is_empty()); - - let diagnostics = analysis - .diagnostics(&DiagnosticsConfig::default(), AssistResolveStrategy::All, file_id) - .unwrap(); - assert!(!diagnostics.is_empty()); - } - - #[test] - fn import_extern_crate_clash_with_inner_item() { - // This is more of a resolver test, but doesn't really work with the hir_def testsuite. - - check_diagnostics( - r#" -//- /lib.rs crate:lib deps:jwt -mod permissions; - -use permissions::jwt; - -fn f() { - fn inner() {} - jwt::Claims {}; // should resolve to the local one with 0 fields, and not get a diagnostic -} - -//- /permissions.rs -pub mod jwt { - pub struct Claims {} -} - -//- /jwt/lib.rs crate:jwt -pub struct Claims { - field: u8, -} - "#, - ); - } -} diff --git a/crates/ide/src/fixture.rs b/crates/ide/src/fixture.rs index 38e2e866be..cf679edd3b 100644 --- a/crates/ide/src/fixture.rs +++ b/crates/ide/src/fixture.rs @@ -12,14 +12,6 @@ pub(crate) fn file(ra_fixture: &str) -> (Analysis, FileId) { (host.analysis(), change_fixture.files[0]) } -/// Creates analysis for many files. -pub(crate) fn files(ra_fixture: &str) -> (Analysis, Vec) { - let mut host = AnalysisHost::default(); - let change_fixture = ChangeFixture::parse(ra_fixture); - host.db.apply_change(change_fixture.change); - (host.analysis(), change_fixture.files) -} - /// Creates analysis from a multi-file fixture, returns positions marked with $0. pub(crate) fn position(ra_fixture: &str) -> (Analysis, FilePosition) { let mut host = AnalysisHost::default(); diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index 0511efae38..0019b7ba57 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -24,7 +24,6 @@ mod display; mod annotations; mod call_hierarchy; -mod diagnostics; mod expand_macro; mod extend_selection; mod file_structure; @@ -71,7 +70,6 @@ use crate::display::ToNav; pub use crate::{ annotations::{Annotation, AnnotationConfig, AnnotationKind}, call_hierarchy::CallItem, - diagnostics::{Diagnostic, DiagnosticsConfig, Severity}, display::navigation_target::NavigationTarget, expand_macro::ExpandedMacro, file_structure::{StructureNode, StructureNodeKind}, @@ -109,6 +107,7 @@ pub use ide_db::{ symbol_index::Query, RootDatabase, SymbolKind, }; +pub use ide_diagnostics::{Diagnostic, DiagnosticsConfig, Severity}; pub use ide_ssr::SsrError; pub use syntax::{TextRange, TextSize}; pub use text_edit::{Indel, TextEdit}; @@ -549,7 +548,7 @@ impl Analysis { resolve: AssistResolveStrategy, file_id: FileId, ) -> Cancellable> { - self.with_db(|db| diagnostics::diagnostics(db, config, &resolve, file_id)) + self.with_db(|db| ide_diagnostics::diagnostics(db, config, &resolve, file_id)) } /// Convenience function to return assists + quick fixes for diagnostics @@ -568,7 +567,7 @@ impl Analysis { self.with_db(|db| { let ssr_assists = ssr::ssr_assists(db, &resolve, frange); let diagnostic_assists = if include_fixes { - diagnostics::diagnostics(db, diagnostics_config, &resolve, frange.file_id) + ide_diagnostics::diagnostics(db, diagnostics_config, &resolve, frange.file_id) .into_iter() .flat_map(|it| it.fixes.unwrap_or_default()) .filter(|it| it.target.intersect(frange.range).is_some()) diff --git a/crates/ide_diagnostics/Cargo.toml b/crates/ide_diagnostics/Cargo.toml index 11cd8d5701..738fca14ed 100644 --- a/crates/ide_diagnostics/Cargo.toml +++ b/crates/ide_diagnostics/Cargo.toml @@ -10,3 +10,21 @@ edition = "2018" doctest = false [dependencies] +cov-mark = "2.0.0-pre.1" +itertools = "0.10.0" +rustc-hash = "1.1.0" +either = "1.5.3" + +profile = { path = "../profile", version = "0.0.0" } +stdx = { path = "../stdx", version = "0.0.0" } +syntax = { path = "../syntax", version = "0.0.0" } +text_edit = { path = "../text_edit", version = "0.0.0" } +cfg = { path = "../cfg", version = "0.0.0" } +hir = { path = "../hir", version = "0.0.0" } +ide_db = { path = "../ide_db", version = "0.0.0" } +ide_assists = { path = "../ide_assists", version = "0.0.0" } + +[dev-dependencies] +expect-test = "1.1" + +test_utils = { path = "../test_utils" } diff --git a/crates/ide/src/diagnostics/break_outside_of_loop.rs b/crates/ide_diagnostics/src/break_outside_of_loop.rs similarity index 84% rename from crates/ide/src/diagnostics/break_outside_of_loop.rs rename to crates/ide_diagnostics/src/break_outside_of_loop.rs index 80e68f3cc0..79e8cea373 100644 --- a/crates/ide/src/diagnostics/break_outside_of_loop.rs +++ b/crates/ide_diagnostics/src/break_outside_of_loop.rs @@ -1,4 +1,4 @@ -use crate::diagnostics::{Diagnostic, DiagnosticsContext}; +use crate::{Diagnostic, DiagnosticsContext}; // Diagnostic: break-outside-of-loop // @@ -16,7 +16,7 @@ pub(super) fn break_outside_of_loop( #[cfg(test)] mod tests { - use crate::diagnostics::tests::check_diagnostics; + use crate::tests::check_diagnostics; #[test] fn break_outside_of_loop() { diff --git a/crates/ide/src/diagnostics/field_shorthand.rs b/crates/ide_diagnostics/src/field_shorthand.rs similarity index 97% rename from crates/ide/src/diagnostics/field_shorthand.rs rename to crates/ide_diagnostics/src/field_shorthand.rs index c7f4dab8ea..0b6af99654 100644 --- a/crates/ide/src/diagnostics/field_shorthand.rs +++ b/crates/ide_diagnostics/src/field_shorthand.rs @@ -5,7 +5,7 @@ use ide_db::{base_db::FileId, source_change::SourceChange}; use syntax::{ast, match_ast, AstNode, SyntaxNode}; use text_edit::TextEdit; -use crate::{diagnostics::fix, Diagnostic, Severity}; +use crate::{fix, Diagnostic, Severity}; pub(super) fn check(acc: &mut Vec, file_id: FileId, node: &SyntaxNode) { match_ast! { @@ -101,7 +101,7 @@ fn check_pat_field_shorthand( #[cfg(test)] mod tests { - use crate::diagnostics::tests::{check_diagnostics, check_fix}; + use crate::tests::{check_diagnostics, check_fix}; #[test] fn test_check_expr_field_shorthand() { diff --git a/crates/ide/src/diagnostics/inactive_code.rs b/crates/ide_diagnostics/src/inactive_code.rs similarity index 95% rename from crates/ide/src/diagnostics/inactive_code.rs rename to crates/ide_diagnostics/src/inactive_code.rs index d9d3e88c1f..34837cc0d0 100644 --- a/crates/ide/src/diagnostics/inactive_code.rs +++ b/crates/ide_diagnostics/src/inactive_code.rs @@ -1,10 +1,7 @@ use cfg::DnfExpr; use stdx::format_to; -use crate::{ - diagnostics::{Diagnostic, DiagnosticsContext}, - Severity, -}; +use crate::{Diagnostic, DiagnosticsContext, Severity}; // Diagnostic: inactive-code // @@ -37,7 +34,7 @@ pub(super) fn inactive_code( #[cfg(test)] mod tests { - use crate::{diagnostics::tests::check_diagnostics_with_config, DiagnosticsConfig}; + use crate::{tests::check_diagnostics_with_config, DiagnosticsConfig}; pub(crate) fn check(ra_fixture: &str) { let config = DiagnosticsConfig::default(); diff --git a/crates/ide/src/diagnostics/incorrect_case.rs b/crates/ide_diagnostics/src/incorrect_case.rs similarity index 94% rename from crates/ide/src/diagnostics/incorrect_case.rs rename to crates/ide_diagnostics/src/incorrect_case.rs index 8323944004..04fc779ce4 100644 --- a/crates/ide/src/diagnostics/incorrect_case.rs +++ b/crates/ide_diagnostics/src/incorrect_case.rs @@ -4,8 +4,10 @@ use ide_db::base_db::FilePosition; use syntax::AstNode; use crate::{ - diagnostics::{unresolved_fix, Diagnostic, DiagnosticsContext}, - references::rename::rename_with_semantics, + // references::rename::rename_with_semantics, + unresolved_fix, + Diagnostic, + DiagnosticsContext, Severity, }; @@ -26,28 +28,34 @@ pub(super) fn incorrect_case(ctx: &DiagnosticsContext<'_>, d: &hir::IncorrectCas } fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::IncorrectCase) -> Option> { + if true { + return None; + } + let root = ctx.sema.db.parse_or_expand(d.file)?; let name_node = d.ident.to_node(&root); let name_node = InFile::new(d.file, name_node.syntax()); let frange = name_node.original_file_range(ctx.sema.db); - let file_position = FilePosition { file_id: frange.file_id, offset: frange.range.start() }; + let _file_position = FilePosition { file_id: frange.file_id, offset: frange.range.start() }; let label = format!("Rename to {}", d.suggested_text); - let mut res = unresolved_fix("change_case", &label, frange.range); + let res = unresolved_fix("change_case", &label, frange.range); if ctx.resolve.should_resolve(&res.id) { - let source_change = rename_with_semantics(&ctx.sema, file_position, &d.suggested_text); - res.source_change = Some(source_change.ok().unwrap_or_default()); + //let source_change = rename_with_semantics(&ctx.sema, file_position, &d.suggested_text); + //res.source_change = Some(source_change.ok().unwrap_or_default()); + todo!() } Some(vec![res]) } -#[cfg(test)] +#[cfg(TODO)] mod change_case { use crate::{ - diagnostics::tests::{check_diagnostics, check_fix}, - fixture, AssistResolveStrategy, DiagnosticsConfig, + fixture, + tests::{check_diagnostics, check_fix}, + AssistResolveStrategy, DiagnosticsConfig, }; #[test] diff --git a/crates/ide_diagnostics/src/lib.rs b/crates/ide_diagnostics/src/lib.rs index e69de29bb2..a104a702d6 100644 --- a/crates/ide_diagnostics/src/lib.rs +++ b/crates/ide_diagnostics/src/lib.rs @@ -0,0 +1,510 @@ +//! Collects diagnostics & fixits for a single file. +//! +//! The tricky bit here is that diagnostics are produced by hir in terms of +//! macro-expanded files, but we need to present them to the users in terms of +//! original files. So we need to map the ranges. + +mod break_outside_of_loop; +mod inactive_code; +mod incorrect_case; +mod macro_error; +mod mismatched_arg_count; +mod missing_fields; +mod missing_match_arms; +mod missing_ok_or_some_in_tail_expr; +mod missing_unsafe; +mod no_such_field; +mod remove_this_semicolon; +mod replace_filter_map_next_with_find_map; +mod unimplemented_builtin_macro; +mod unlinked_file; +mod unresolved_extern_crate; +mod unresolved_import; +mod unresolved_macro_call; +mod unresolved_module; +mod unresolved_proc_macro; + +mod field_shorthand; + +use hir::{diagnostics::AnyDiagnostic, Semantics}; +use ide_assists::AssistResolveStrategy; +use ide_db::{ + base_db::{FileId, SourceDatabase}, + label::Label, + source_change::SourceChange, + RootDatabase, +}; +use itertools::Itertools; +use rustc_hash::FxHashSet; +use syntax::{ + ast::{self, AstNode}, + SyntaxNode, TextRange, +}; +use text_edit::TextEdit; +use unlinked_file::UnlinkedFile; + +use ide_assists::{Assist, AssistId, AssistKind}; + +#[derive(Copy, Clone, Debug, PartialEq)] +pub struct DiagnosticCode(pub &'static str); + +impl DiagnosticCode { + pub fn as_str(&self) -> &str { + self.0 + } +} + +#[derive(Debug)] +pub struct Diagnostic { + pub code: DiagnosticCode, + pub message: String, + pub range: TextRange, + pub severity: Severity, + pub unused: bool, + pub experimental: bool, + pub fixes: Option>, +} + +impl Diagnostic { + fn new(code: &'static str, message: impl Into, range: TextRange) -> Diagnostic { + let message = message.into(); + Diagnostic { + code: DiagnosticCode(code), + message, + range, + severity: Severity::Error, + unused: false, + experimental: false, + fixes: None, + } + } + + fn experimental(mut self) -> Diagnostic { + self.experimental = true; + self + } + + fn severity(mut self, severity: Severity) -> Diagnostic { + self.severity = severity; + self + } + + fn with_fixes(mut self, fixes: Option>) -> Diagnostic { + self.fixes = fixes; + self + } + + fn with_unused(mut self, unused: bool) -> Diagnostic { + self.unused = unused; + self + } +} + +#[derive(Debug, Copy, Clone)] +pub enum Severity { + Error, + WeakWarning, +} + +#[derive(Default, Debug, Clone)] +pub struct DiagnosticsConfig { + pub disable_experimental: bool, + pub disabled: FxHashSet, +} + +struct DiagnosticsContext<'a> { + config: &'a DiagnosticsConfig, + sema: Semantics<'a, RootDatabase>, + resolve: &'a AssistResolveStrategy, +} + +pub fn diagnostics( + db: &RootDatabase, + config: &DiagnosticsConfig, + resolve: &AssistResolveStrategy, + file_id: FileId, +) -> Vec { + let _p = profile::span("diagnostics"); + let sema = Semantics::new(db); + let parse = db.parse(file_id); + let mut res = Vec::new(); + + // [#34344] Only take first 128 errors to prevent slowing down editor/ide, the number 128 is chosen arbitrarily. + res.extend( + parse.errors().iter().take(128).map(|err| { + Diagnostic::new("syntax-error", format!("Syntax Error: {}", err), err.range()) + }), + ); + + for node in parse.tree().syntax().descendants() { + check_unnecessary_braces_in_use_statement(&mut res, file_id, &node); + field_shorthand::check(&mut res, file_id, &node); + } + + let mut diags = Vec::new(); + let module = sema.to_module_def(file_id); + if let Some(m) = module { + m.diagnostics(db, &mut diags) + } + + let ctx = DiagnosticsContext { config, sema, resolve }; + if module.is_none() { + let d = UnlinkedFile { file: file_id }; + let d = unlinked_file::unlinked_file(&ctx, &d); + res.push(d) + } + + for diag in diags { + #[rustfmt::skip] + let d = match diag { + AnyDiagnostic::BreakOutsideOfLoop(d) => break_outside_of_loop::break_outside_of_loop(&ctx, &d), + AnyDiagnostic::IncorrectCase(d) => incorrect_case::incorrect_case(&ctx, &d), + AnyDiagnostic::MacroError(d) => macro_error::macro_error(&ctx, &d), + AnyDiagnostic::MismatchedArgCount(d) => mismatched_arg_count::mismatched_arg_count(&ctx, &d), + AnyDiagnostic::MissingFields(d) => missing_fields::missing_fields(&ctx, &d), + AnyDiagnostic::MissingMatchArms(d) => missing_match_arms::missing_match_arms(&ctx, &d), + AnyDiagnostic::MissingOkOrSomeInTailExpr(d) => missing_ok_or_some_in_tail_expr::missing_ok_or_some_in_tail_expr(&ctx, &d), + AnyDiagnostic::MissingUnsafe(d) => missing_unsafe::missing_unsafe(&ctx, &d), + AnyDiagnostic::NoSuchField(d) => no_such_field::no_such_field(&ctx, &d), + AnyDiagnostic::RemoveThisSemicolon(d) => remove_this_semicolon::remove_this_semicolon(&ctx, &d), + AnyDiagnostic::ReplaceFilterMapNextWithFindMap(d) => replace_filter_map_next_with_find_map::replace_filter_map_next_with_find_map(&ctx, &d), + AnyDiagnostic::UnimplementedBuiltinMacro(d) => unimplemented_builtin_macro::unimplemented_builtin_macro(&ctx, &d), + AnyDiagnostic::UnresolvedExternCrate(d) => unresolved_extern_crate::unresolved_extern_crate(&ctx, &d), + AnyDiagnostic::UnresolvedImport(d) => unresolved_import::unresolved_import(&ctx, &d), + AnyDiagnostic::UnresolvedMacroCall(d) => unresolved_macro_call::unresolved_macro_call(&ctx, &d), + AnyDiagnostic::UnresolvedModule(d) => unresolved_module::unresolved_module(&ctx, &d), + AnyDiagnostic::UnresolvedProcMacro(d) => unresolved_proc_macro::unresolved_proc_macro(&ctx, &d), + + AnyDiagnostic::InactiveCode(d) => match inactive_code::inactive_code(&ctx, &d) { + Some(it) => it, + None => continue, + } + }; + res.push(d) + } + + res.retain(|d| { + !ctx.config.disabled.contains(d.code.as_str()) + && !(ctx.config.disable_experimental && d.experimental) + }); + + res +} + +fn check_unnecessary_braces_in_use_statement( + acc: &mut Vec, + file_id: FileId, + node: &SyntaxNode, +) -> Option<()> { + let use_tree_list = ast::UseTreeList::cast(node.clone())?; + if let Some((single_use_tree,)) = use_tree_list.use_trees().collect_tuple() { + // If there is a comment inside the bracketed `use`, + // assume it is a commented out module path and don't show diagnostic. + if use_tree_list.has_inner_comment() { + return Some(()); + } + + 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 = TextEdit::builder(); + edit_builder.delete(use_range); + edit_builder.insert(use_range.start(), to_replace); + edit_builder.finish() + }); + + acc.push( + Diagnostic::new( + "unnecessary-braces", + "Unnecessary braces in use statement".to_string(), + use_range, + ) + .severity(Severity::WeakWarning) + .with_fixes(Some(vec![fix( + "remove_braces", + "Remove unnecessary braces", + SourceChange::from_text_edit(file_id, edit), + use_range, + )])), + ); + } + + 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()?.self_token().is_some() { + let start = use_tree_list_node.prev_sibling_or_token()?.text_range().start(); + let end = use_tree_list_node.text_range().end(); + return Some(TextEdit::delete(TextRange::new(start, end))); + } + None +} + +fn fix(id: &'static str, label: &str, source_change: SourceChange, target: TextRange) -> Assist { + let mut res = unresolved_fix(id, label, target); + res.source_change = Some(source_change); + res +} + +fn unresolved_fix(id: &'static str, label: &str, target: TextRange) -> Assist { + assert!(!id.contains(' ')); + Assist { + id: AssistId(id, AssistKind::QuickFix), + label: Label::new(label), + group: None, + target, + source_change: None, + } +} + +#[cfg(test)] +mod tests { + use expect_test::Expect; + use ide_assists::AssistResolveStrategy; + use ide_db::{ + base_db::{fixture::WithFixture, SourceDatabaseExt}, + RootDatabase, + }; + use stdx::trim_indent; + use test_utils::{assert_eq_text, extract_annotations}; + + use crate::DiagnosticsConfig; + + /// Takes a multi-file input fixture with annotated cursor positions, + /// and checks that: + /// * a diagnostic is produced + /// * the first 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 + #[track_caller] + pub(crate) fn check_fix(ra_fixture_before: &str, ra_fixture_after: &str) { + check_nth_fix(0, ra_fixture_before, ra_fixture_after); + } + /// Takes a multi-file input fixture with annotated cursor positions, + /// and checks that: + /// * a diagnostic is produced + /// * every diagnostic fixes trigger range touches the input cursor position + /// * that the contents of the file containing the cursor match `after` after each diagnostic fix is applied + pub(crate) fn check_fixes(ra_fixture_before: &str, ra_fixtures_after: Vec<&str>) { + for (i, ra_fixture_after) in ra_fixtures_after.iter().enumerate() { + check_nth_fix(i, ra_fixture_before, ra_fixture_after) + } + } + + #[track_caller] + fn check_nth_fix(nth: usize, ra_fixture_before: &str, ra_fixture_after: &str) { + let after = trim_indent(ra_fixture_after); + + let (db, file_position) = RootDatabase::with_position(ra_fixture_before); + let diagnostic = super::diagnostics( + &db, + &DiagnosticsConfig::default(), + &AssistResolveStrategy::All, + file_position.file_id, + ) + .pop() + .expect("no diagnostics"); + let fix = &diagnostic.fixes.expect("diagnostic misses fixes")[nth]; + let actual = { + let source_change = fix.source_change.as_ref().unwrap(); + let file_id = *source_change.source_file_edits.keys().next().unwrap(); + let mut actual = db.file_text(file_id).to_string(); + + for edit in source_change.source_file_edits.values() { + edit.apply(&mut actual); + } + actual + }; + + assert_eq_text!(&after, &actual); + assert!( + fix.target.contains_inclusive(file_position.offset), + "diagnostic fix range {:?} does not touch cursor position {:?}", + fix.target, + file_position.offset + ); + } + + /// Checks that there's a diagnostic *without* fix at `$0`. + pub(crate) fn check_no_fix(ra_fixture: &str) { + let (db, file_position) = RootDatabase::with_position(ra_fixture); + let diagnostic = super::diagnostics( + &db, + &DiagnosticsConfig::default(), + &AssistResolveStrategy::All, + file_position.file_id, + ) + .pop() + .unwrap(); + assert!(diagnostic.fixes.is_none(), "got a fix when none was expected: {:?}", diagnostic); + } + + pub(crate) fn check_expect(ra_fixture: &str, expect: Expect) { + let (db, file_id) = RootDatabase::with_single_file(ra_fixture); + let diagnostics = super::diagnostics( + &db, + &DiagnosticsConfig::default(), + &AssistResolveStrategy::All, + file_id, + ); + expect.assert_debug_eq(&diagnostics) + } + + #[track_caller] + pub(crate) fn check_diagnostics(ra_fixture: &str) { + let mut config = DiagnosticsConfig::default(); + config.disabled.insert("inactive-code".to_string()); + check_diagnostics_with_config(config, ra_fixture) + } + + #[track_caller] + pub(crate) fn check_diagnostics_with_config(config: DiagnosticsConfig, ra_fixture: &str) { + let (db, files) = RootDatabase::with_many_files(ra_fixture); + for file_id in files { + let diagnostics = + super::diagnostics(&db, &config, &AssistResolveStrategy::All, file_id); + + let expected = extract_annotations(&*db.file_text(file_id)); + let mut actual = + diagnostics.into_iter().map(|d| (d.range, d.message)).collect::>(); + actual.sort_by_key(|(range, _)| range.start()); + assert_eq!(expected, actual); + } + } + + #[test] + fn test_check_unnecessary_braces_in_use_statement() { + check_diagnostics( + r#" +use a; +use a::{c, d::e}; + +mod a { + mod c {} + mod d { + mod e {} + } +} +"#, + ); + check_diagnostics( + r#" +use a; +use a::{ + c, + // d::e +}; + +mod a { + mod c {} + mod d { + mod e {} + } +} +"#, + ); + check_fix( + r" + mod b {} + use {$0b}; + ", + r" + mod b {} + use b; + ", + ); + check_fix( + r" + mod b {} + use {b$0}; + ", + r" + mod b {} + use b; + ", + ); + check_fix( + r" + mod a { mod c {} } + use a::{c$0}; + ", + r" + mod a { mod c {} } + use a::c; + ", + ); + check_fix( + r" + mod a {} + use a::{self$0}; + ", + r" + mod a {} + use a; + ", + ); + check_fix( + r" + mod a { mod c {} mod d { mod e {} } } + use a::{c, d::{e$0}}; + ", + r" + mod a { mod c {} mod d { mod e {} } } + use a::{c, d::e}; + ", + ); + } + + #[test] + fn test_disabled_diagnostics() { + let mut config = DiagnosticsConfig::default(); + config.disabled.insert("unresolved-module".into()); + + let (db, file_id) = RootDatabase::with_single_file(r#"mod foo;"#); + + let diagnostics = super::diagnostics(&db, &config, &AssistResolveStrategy::All, file_id); + assert!(diagnostics.is_empty()); + + let diagnostics = super::diagnostics( + &db, + &DiagnosticsConfig::default(), + &AssistResolveStrategy::All, + file_id, + ); + assert!(!diagnostics.is_empty()); + } + + #[test] + fn import_extern_crate_clash_with_inner_item() { + // This is more of a resolver test, but doesn't really work with the hir_def testsuite. + + check_diagnostics( + r#" +//- /lib.rs crate:lib deps:jwt +mod permissions; + +use permissions::jwt; + +fn f() { + fn inner() {} + jwt::Claims {}; // should resolve to the local one with 0 fields, and not get a diagnostic +} + +//- /permissions.rs +pub mod jwt { + pub struct Claims {} +} + +//- /jwt/lib.rs crate:jwt +pub struct Claims { + field: u8, +} + "#, + ); + } +} diff --git a/crates/ide/src/diagnostics/macro_error.rs b/crates/ide_diagnostics/src/macro_error.rs similarity index 96% rename from crates/ide/src/diagnostics/macro_error.rs rename to crates/ide_diagnostics/src/macro_error.rs index 5f97f190d1..180f297ebf 100644 --- a/crates/ide/src/diagnostics/macro_error.rs +++ b/crates/ide_diagnostics/src/macro_error.rs @@ -1,4 +1,4 @@ -use crate::diagnostics::{Diagnostic, DiagnosticsContext}; +use crate::{Diagnostic, DiagnosticsContext}; // Diagnostic: macro-error // @@ -15,7 +15,7 @@ pub(super) fn macro_error(ctx: &DiagnosticsContext<'_>, d: &hir::MacroError) -> #[cfg(test)] mod tests { use crate::{ - diagnostics::tests::{check_diagnostics, check_diagnostics_with_config}, + tests::{check_diagnostics, check_diagnostics_with_config}, DiagnosticsConfig, }; diff --git a/crates/ide/src/diagnostics/mismatched_arg_count.rs b/crates/ide_diagnostics/src/mismatched_arg_count.rs similarity index 97% rename from crates/ide/src/diagnostics/mismatched_arg_count.rs rename to crates/ide_diagnostics/src/mismatched_arg_count.rs index 08e1cfa5fc..c5749c8a6b 100644 --- a/crates/ide/src/diagnostics/mismatched_arg_count.rs +++ b/crates/ide_diagnostics/src/mismatched_arg_count.rs @@ -1,4 +1,4 @@ -use crate::diagnostics::{Diagnostic, DiagnosticsContext}; +use crate::{Diagnostic, DiagnosticsContext}; // Diagnostic: mismatched-arg-count // @@ -18,7 +18,7 @@ pub(super) fn mismatched_arg_count( #[cfg(test)] mod tests { - use crate::diagnostics::tests::check_diagnostics; + use crate::tests::check_diagnostics; #[test] fn simple_free_fn_zero() { diff --git a/crates/ide/src/diagnostics/missing_fields.rs b/crates/ide_diagnostics/src/missing_fields.rs similarity index 98% rename from crates/ide/src/diagnostics/missing_fields.rs rename to crates/ide_diagnostics/src/missing_fields.rs index d01f050414..f242ee4813 100644 --- a/crates/ide/src/diagnostics/missing_fields.rs +++ b/crates/ide_diagnostics/src/missing_fields.rs @@ -6,7 +6,7 @@ use stdx::format_to; use syntax::{algo, ast::make, AstNode, SyntaxNodePtr}; use text_edit::TextEdit; -use crate::diagnostics::{fix, Diagnostic, DiagnosticsContext}; +use crate::{fix, Diagnostic, DiagnosticsContext}; // Diagnostic: missing-fields // @@ -77,7 +77,7 @@ fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::MissingFields) -> Option, d: &hir::MissingOkOrSomeInTailExpr) -> Op #[cfg(test)] mod tests { - use crate::diagnostics::tests::{check_diagnostics, check_fix}; + use crate::tests::{check_diagnostics, check_fix}; #[test] fn test_wrap_return_type_option() { diff --git a/crates/ide/src/diagnostics/missing_unsafe.rs b/crates/ide_diagnostics/src/missing_unsafe.rs similarity index 95% rename from crates/ide/src/diagnostics/missing_unsafe.rs rename to crates/ide_diagnostics/src/missing_unsafe.rs index 5c47e8d0af..f5f38a0d35 100644 --- a/crates/ide/src/diagnostics/missing_unsafe.rs +++ b/crates/ide_diagnostics/src/missing_unsafe.rs @@ -1,4 +1,4 @@ -use crate::diagnostics::{Diagnostic, DiagnosticsContext}; +use crate::{Diagnostic, DiagnosticsContext}; // Diagnostic: missing-unsafe // @@ -13,7 +13,7 @@ pub(super) fn missing_unsafe(ctx: &DiagnosticsContext<'_>, d: &hir::MissingUnsaf #[cfg(test)] mod tests { - use crate::diagnostics::tests::check_diagnostics; + use crate::tests::check_diagnostics; #[test] fn missing_unsafe_diagnostic_with_raw_ptr() { diff --git a/crates/ide/src/diagnostics/no_such_field.rs b/crates/ide_diagnostics/src/no_such_field.rs similarity index 97% rename from crates/ide/src/diagnostics/no_such_field.rs rename to crates/ide_diagnostics/src/no_such_field.rs index edc63c2468..c4fa387ca2 100644 --- a/crates/ide/src/diagnostics/no_such_field.rs +++ b/crates/ide_diagnostics/src/no_such_field.rs @@ -6,10 +6,7 @@ use syntax::{ }; use text_edit::TextEdit; -use crate::{ - diagnostics::{fix, Diagnostic, DiagnosticsContext}, - Assist, -}; +use crate::{fix, Assist, Diagnostic, DiagnosticsContext}; // Diagnostic: no-such-field // @@ -112,7 +109,7 @@ fn missing_record_expr_field_fixes( #[cfg(test)] mod tests { - use crate::diagnostics::tests::{check_diagnostics, check_fix}; + use crate::tests::{check_diagnostics, check_fix}; #[test] fn no_such_field_diagnostics() { diff --git a/crates/ide/src/diagnostics/remove_this_semicolon.rs b/crates/ide_diagnostics/src/remove_this_semicolon.rs similarity index 91% rename from crates/ide/src/diagnostics/remove_this_semicolon.rs rename to crates/ide_diagnostics/src/remove_this_semicolon.rs index 814cb0f8c2..dc6c9c0834 100644 --- a/crates/ide/src/diagnostics/remove_this_semicolon.rs +++ b/crates/ide_diagnostics/src/remove_this_semicolon.rs @@ -3,10 +3,7 @@ use ide_db::source_change::SourceChange; use syntax::{ast, AstNode}; use text_edit::TextEdit; -use crate::{ - diagnostics::{fix, Diagnostic, DiagnosticsContext}, - Assist, -}; +use crate::{fix, Assist, Diagnostic, DiagnosticsContext}; // Diagnostic: remove-this-semicolon // @@ -45,7 +42,7 @@ fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::RemoveThisSemicolon) -> Option, d: &hir::UnresolvedModule) -> Option Date: Mon, 14 Jun 2021 13:18:03 +0300 Subject: [PATCH 3/9] internal: prepare to move assist definitions --- crates/ide/src/lib.rs | 4 ++-- crates/ide_assists/src/lib.rs | 30 ++++++++++++++---------------- crates/ide_assists/src/tests.rs | 26 +++++++++++++------------- crates/ide_diagnostics/src/lib.rs | 3 +-- 4 files changed, 30 insertions(+), 33 deletions(-) diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index 0019b7ba57..98d01f0ced 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -535,7 +535,7 @@ impl Analysis { ) -> Cancellable> { self.with_db(|db| { let ssr_assists = ssr::ssr_assists(db, &resolve, frange); - let mut acc = Assist::get(db, config, resolve, frange); + let mut acc = ide_assists::assists(db, config, resolve, frange); acc.extend(ssr_assists.into_iter()); acc }) @@ -576,7 +576,7 @@ impl Analysis { Vec::new() }; - let mut res = Assist::get(db, assist_config, resolve, frange); + let mut res = ide_assists::assists(db, assist_config, resolve, frange); res.extend(ssr_assists.into_iter()); res.extend(diagnostic_assists.into_iter()); diff --git a/crates/ide_assists/src/lib.rs b/crates/ide_assists/src/lib.rs index 331a6df2b9..8049182843 100644 --- a/crates/ide_assists/src/lib.rs +++ b/crates/ide_assists/src/lib.rs @@ -151,22 +151,20 @@ pub struct Assist { pub source_change: Option, } -impl Assist { - /// Return all the assists applicable at the given position. - pub fn get( - db: &RootDatabase, - config: &AssistConfig, - resolve: AssistResolveStrategy, - range: FileRange, - ) -> Vec { - let sema = Semantics::new(db); - let ctx = AssistContext::new(sema, config, range); - let mut acc = Assists::new(&ctx, resolve); - handlers::all().iter().for_each(|handler| { - handler(&mut acc, &ctx); - }); - acc.finish() - } +/// Return all the assists applicable at the given position. +pub fn assists( + db: &RootDatabase, + config: &AssistConfig, + resolve: AssistResolveStrategy, + range: FileRange, +) -> Vec { + let sema = Semantics::new(db); + let ctx = AssistContext::new(sema, config, range); + let mut acc = Assists::new(&ctx, resolve); + handlers::all().iter().for_each(|handler| { + handler(&mut acc, &ctx); + }); + acc.finish() } mod handlers { diff --git a/crates/ide_assists/src/tests.rs b/crates/ide_assists/src/tests.rs index bdf9cb71c5..60cecd94c8 100644 --- a/crates/ide_assists/src/tests.rs +++ b/crates/ide_assists/src/tests.rs @@ -16,8 +16,8 @@ use syntax::TextRange; use test_utils::{assert_eq_text, extract_offset}; use crate::{ - handlers::Handler, Assist, AssistConfig, AssistContext, AssistKind, AssistResolveStrategy, - Assists, SingleResolve, + assists, handlers::Handler, Assist, AssistConfig, AssistContext, AssistKind, + AssistResolveStrategy, Assists, SingleResolve, }; pub(crate) const TEST_CONFIG: AssistConfig = AssistConfig { @@ -78,14 +78,14 @@ fn check_doc_test(assist_id: &str, before: &str, after: &str) { let before = db.file_text(file_id).to_string(); let frange = FileRange { file_id, range: selection.into() }; - let assist = Assist::get(&db, &TEST_CONFIG, AssistResolveStrategy::All, frange) + let assist = assists(&db, &TEST_CONFIG, AssistResolveStrategy::All, frange) .into_iter() .find(|assist| assist.id.0 == assist_id) .unwrap_or_else(|| { panic!( "\n\nAssist is not applicable: {}\nAvailable assists: {}", assist_id, - Assist::get(&db, &TEST_CONFIG, AssistResolveStrategy::None, frange) + assists(&db, &TEST_CONFIG, AssistResolveStrategy::None, frange) .into_iter() .map(|assist| assist.id.0) .collect::>() @@ -210,7 +210,7 @@ fn assist_order_field_struct() { let (before_cursor_pos, before) = extract_offset(before); let (db, file_id) = with_single_file(&before); let frange = FileRange { file_id, range: TextRange::empty(before_cursor_pos) }; - let assists = Assist::get(&db, &TEST_CONFIG, AssistResolveStrategy::None, frange); + let assists = assists(&db, &TEST_CONFIG, AssistResolveStrategy::None, frange); let mut assists = assists.iter(); assert_eq!(assists.next().expect("expected assist").label, "Change visibility to pub(crate)"); @@ -235,7 +235,7 @@ pub fn test_some_range(a: int) -> bool { "#, ); - let assists = Assist::get(&db, &TEST_CONFIG, AssistResolveStrategy::None, frange); + let assists = assists(&db, &TEST_CONFIG, AssistResolveStrategy::None, frange); let expected = labels(&assists); expect![[r#" @@ -264,7 +264,7 @@ pub fn test_some_range(a: int) -> bool { let mut cfg = TEST_CONFIG; cfg.allowed = Some(vec![AssistKind::Refactor]); - let assists = Assist::get(&db, &cfg, AssistResolveStrategy::None, frange); + let assists = assists(&db, &cfg, AssistResolveStrategy::None, frange); let expected = labels(&assists); expect![[r#" @@ -279,7 +279,7 @@ pub fn test_some_range(a: int) -> bool { { let mut cfg = TEST_CONFIG; cfg.allowed = Some(vec![AssistKind::RefactorExtract]); - let assists = Assist::get(&db, &cfg, AssistResolveStrategy::None, frange); + let assists = assists(&db, &cfg, AssistResolveStrategy::None, frange); let expected = labels(&assists); expect![[r#" @@ -292,7 +292,7 @@ pub fn test_some_range(a: int) -> bool { { let mut cfg = TEST_CONFIG; cfg.allowed = Some(vec![AssistKind::QuickFix]); - let assists = Assist::get(&db, &cfg, AssistResolveStrategy::None, frange); + let assists = assists(&db, &cfg, AssistResolveStrategy::None, frange); let expected = labels(&assists); expect![[r#""#]].assert_eq(&expected); @@ -317,7 +317,7 @@ pub fn test_some_range(a: int) -> bool { cfg.allowed = Some(vec![AssistKind::RefactorExtract]); { - let assists = Assist::get(&db, &cfg, AssistResolveStrategy::None, frange); + let assists = assists(&db, &cfg, AssistResolveStrategy::None, frange); assert_eq!(2, assists.len()); let mut assists = assists.into_iter(); @@ -353,7 +353,7 @@ pub fn test_some_range(a: int) -> bool { } { - let assists = Assist::get( + let assists = assists( &db, &cfg, AssistResolveStrategy::Single(SingleResolve { @@ -397,7 +397,7 @@ pub fn test_some_range(a: int) -> bool { } { - let assists = Assist::get( + let assists = assists( &db, &cfg, AssistResolveStrategy::Single(SingleResolve { @@ -462,7 +462,7 @@ pub fn test_some_range(a: int) -> bool { } { - let assists = Assist::get(&db, &cfg, AssistResolveStrategy::All, frange); + let assists = assists(&db, &cfg, AssistResolveStrategy::All, frange); assert_eq!(2, assists.len()); let mut assists = assists.into_iter(); diff --git a/crates/ide_diagnostics/src/lib.rs b/crates/ide_diagnostics/src/lib.rs index a104a702d6..0d98307a26 100644 --- a/crates/ide_diagnostics/src/lib.rs +++ b/crates/ide_diagnostics/src/lib.rs @@ -27,7 +27,6 @@ mod unresolved_proc_macro; mod field_shorthand; use hir::{diagnostics::AnyDiagnostic, Semantics}; -use ide_assists::AssistResolveStrategy; use ide_db::{ base_db::{FileId, SourceDatabase}, label::Label, @@ -43,7 +42,7 @@ use syntax::{ use text_edit::TextEdit; use unlinked_file::UnlinkedFile; -use ide_assists::{Assist, AssistId, AssistKind}; +use ide_assists::{Assist, AssistId, AssistKind, AssistResolveStrategy}; #[derive(Copy, Clone, Debug, PartialEq)] pub struct DiagnosticCode(pub &'static str); From a91071b57be6e64ad2fd277998ada0ae6206457b Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 14 Jun 2021 13:27:11 +0300 Subject: [PATCH 4/9] internal: cut deps between assists and diagnostics --- Cargo.lock | 1 - crates/ide_assists/src/lib.rs | 131 +---------------- crates/ide_db/src/assists.rs | 136 ++++++++++++++++++ crates/ide_db/src/lib.rs | 1 + crates/ide_diagnostics/Cargo.toml | 1 - crates/ide_diagnostics/src/incorrect_case.rs | 3 +- crates/ide_diagnostics/src/lib.rs | 5 +- crates/ide_diagnostics/src/missing_fields.rs | 3 +- .../src/missing_ok_or_some_in_tail_expr.rs | 3 +- .../ide_diagnostics/src/unresolved_module.rs | 3 +- 10 files changed, 147 insertions(+), 140 deletions(-) create mode 100644 crates/ide_db/src/assists.rs diff --git a/Cargo.lock b/Cargo.lock index 55016ccf7f..8472771183 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -678,7 +678,6 @@ dependencies = [ "either", "expect-test", "hir", - "ide_assists", "ide_db", "itertools", "profile", diff --git a/crates/ide_assists/src/lib.rs b/crates/ide_assists/src/lib.rs index 8049182843..fa378a622d 100644 --- a/crates/ide_assists/src/lib.rs +++ b/crates/ide_assists/src/lib.rs @@ -17,139 +17,16 @@ mod tests; pub mod utils; pub mod path_transform; -use std::str::FromStr; - use hir::Semantics; -use ide_db::{base_db::FileRange, label::Label, source_change::SourceChange, RootDatabase}; +use ide_db::{base_db::FileRange, RootDatabase}; use syntax::TextRange; pub(crate) use crate::assist_context::{AssistContext, Assists}; pub use assist_config::AssistConfig; - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum AssistKind { - // FIXME: does the None variant make sense? Probably not. - None, - - QuickFix, - Generate, - Refactor, - RefactorExtract, - RefactorInline, - RefactorRewrite, -} - -impl AssistKind { - pub fn contains(self, other: AssistKind) -> bool { - if self == other { - return true; - } - - match self { - AssistKind::None | AssistKind::Generate => true, - AssistKind::Refactor => match other { - AssistKind::RefactorExtract - | AssistKind::RefactorInline - | AssistKind::RefactorRewrite => true, - _ => false, - }, - _ => false, - } - } - - pub fn name(&self) -> &str { - match self { - AssistKind::None => "None", - AssistKind::QuickFix => "QuickFix", - AssistKind::Generate => "Generate", - AssistKind::Refactor => "Refactor", - AssistKind::RefactorExtract => "RefactorExtract", - AssistKind::RefactorInline => "RefactorInline", - AssistKind::RefactorRewrite => "RefactorRewrite", - } - } -} - -impl FromStr for AssistKind { - type Err = String; - - fn from_str(s: &str) -> Result { - match s { - "None" => Ok(AssistKind::None), - "QuickFix" => Ok(AssistKind::QuickFix), - "Generate" => Ok(AssistKind::Generate), - "Refactor" => Ok(AssistKind::Refactor), - "RefactorExtract" => Ok(AssistKind::RefactorExtract), - "RefactorInline" => Ok(AssistKind::RefactorInline), - "RefactorRewrite" => Ok(AssistKind::RefactorRewrite), - unknown => Err(format!("Unknown AssistKind: '{}'", unknown)), - } - } -} - -/// Unique identifier of the assist, should not be shown to the user -/// directly. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct AssistId(pub &'static str, pub AssistKind); - -/// A way to control how many asssist to resolve during the assist resolution. -/// When an assist is resolved, its edits are calculated that might be costly to always do by default. -#[derive(Debug)] -pub enum AssistResolveStrategy { - /// No assists should be resolved. - None, - /// All assists should be resolved. - All, - /// Only a certain assist should be resolved. - Single(SingleResolve), -} - -/// Hold the [`AssistId`] data of a certain assist to resolve. -/// The original id object cannot be used due to a `'static` lifetime -/// and the requirement to construct this struct dynamically during the resolve handling. -#[derive(Debug)] -pub struct SingleResolve { - /// The id of the assist. - pub assist_id: String, - // The kind of the assist. - pub assist_kind: AssistKind, -} - -impl AssistResolveStrategy { - pub fn should_resolve(&self, id: &AssistId) -> bool { - match self { - AssistResolveStrategy::None => false, - AssistResolveStrategy::All => true, - AssistResolveStrategy::Single(single_resolve) => { - single_resolve.assist_id == id.0 && single_resolve.assist_kind == id.1 - } - } - } -} - -#[derive(Clone, Debug)] -pub struct GroupLabel(pub String); - -#[derive(Debug, Clone)] -pub struct Assist { - pub id: AssistId, - /// Short description of the assist, as shown in the UI. - pub label: Label, - pub group: Option, - /// Target ranges are used to sort assists: the smaller the target range, - /// the more specific assist is, and so it should be sorted first. - pub target: TextRange, - /// Computing source change sometimes is much more costly then computing the - /// other fields. Additionally, the actual change is not required to show - /// the lightbulb UI, it only is needed when the user tries to apply an - /// assist. So, we compute it lazily: the API allow requesting assists with - /// or without source change. We could (and in fact, used to) distinguish - /// between resolved and unresolved assists at the type level, but this is - /// cumbersome, especially if you want to embed an assist into another data - /// structure, such as a diagnostic. - pub source_change: Option, -} +pub use ide_db::assists::{ + Assist, AssistId, AssistKind, AssistResolveStrategy, GroupLabel, SingleResolve, +}; /// Return all the assists applicable at the given position. pub fn assists( diff --git a/crates/ide_db/src/assists.rs b/crates/ide_db/src/assists.rs new file mode 100644 index 0000000000..7881d83691 --- /dev/null +++ b/crates/ide_db/src/assists.rs @@ -0,0 +1,136 @@ +//! This module defines the `Assist` data structure. The actual assist live in +//! the `ide_assists` downstream crate. We want to define the data structures in +//! this low-level crate though, because `ide_diagnostics` also need them +//! (fixits for diagnostics and assists are the same thing under the hood). We +//! want to compile `ide_assists` and `ide_diagnostics` in parallel though, so +//! we pull the common definitions upstream, to this crate. + +use std::str::FromStr; + +use syntax::TextRange; + +use crate::{label::Label, source_change::SourceChange}; + +#[derive(Debug, Clone)] +pub struct Assist { + pub id: AssistId, + /// Short description of the assist, as shown in the UI. + pub label: Label, + pub group: Option, + /// Target ranges are used to sort assists: the smaller the target range, + /// the more specific assist is, and so it should be sorted first. + pub target: TextRange, + /// Computing source change sometimes is much more costly then computing the + /// other fields. Additionally, the actual change is not required to show + /// the lightbulb UI, it only is needed when the user tries to apply an + /// assist. So, we compute it lazily: the API allow requesting assists with + /// or without source change. We could (and in fact, used to) distinguish + /// between resolved and unresolved assists at the type level, but this is + /// cumbersome, especially if you want to embed an assist into another data + /// structure, such as a diagnostic. + pub source_change: Option, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum AssistKind { + // FIXME: does the None variant make sense? Probably not. + None, + + QuickFix, + Generate, + Refactor, + RefactorExtract, + RefactorInline, + RefactorRewrite, +} + +impl AssistKind { + pub fn contains(self, other: AssistKind) -> bool { + if self == other { + return true; + } + + match self { + AssistKind::None | AssistKind::Generate => true, + AssistKind::Refactor => match other { + AssistKind::RefactorExtract + | AssistKind::RefactorInline + | AssistKind::RefactorRewrite => true, + _ => false, + }, + _ => false, + } + } + + pub fn name(&self) -> &str { + match self { + AssistKind::None => "None", + AssistKind::QuickFix => "QuickFix", + AssistKind::Generate => "Generate", + AssistKind::Refactor => "Refactor", + AssistKind::RefactorExtract => "RefactorExtract", + AssistKind::RefactorInline => "RefactorInline", + AssistKind::RefactorRewrite => "RefactorRewrite", + } + } +} + +impl FromStr for AssistKind { + type Err = String; + + fn from_str(s: &str) -> Result { + match s { + "None" => Ok(AssistKind::None), + "QuickFix" => Ok(AssistKind::QuickFix), + "Generate" => Ok(AssistKind::Generate), + "Refactor" => Ok(AssistKind::Refactor), + "RefactorExtract" => Ok(AssistKind::RefactorExtract), + "RefactorInline" => Ok(AssistKind::RefactorInline), + "RefactorRewrite" => Ok(AssistKind::RefactorRewrite), + unknown => Err(format!("Unknown AssistKind: '{}'", unknown)), + } + } +} + +/// Unique identifier of the assist, should not be shown to the user +/// directly. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct AssistId(pub &'static str, pub AssistKind); + +/// A way to control how many asssist to resolve during the assist resolution. +/// When an assist is resolved, its edits are calculated that might be costly to always do by default. +#[derive(Debug)] +pub enum AssistResolveStrategy { + /// No assists should be resolved. + None, + /// All assists should be resolved. + All, + /// Only a certain assist should be resolved. + Single(SingleResolve), +} + +/// Hold the [`AssistId`] data of a certain assist to resolve. +/// The original id object cannot be used due to a `'static` lifetime +/// and the requirement to construct this struct dynamically during the resolve handling. +#[derive(Debug)] +pub struct SingleResolve { + /// The id of the assist. + pub assist_id: String, + // The kind of the assist. + pub assist_kind: AssistKind, +} + +impl AssistResolveStrategy { + pub fn should_resolve(&self, id: &AssistId) -> bool { + match self { + AssistResolveStrategy::None => false, + AssistResolveStrategy::All => true, + AssistResolveStrategy::Single(single_resolve) => { + single_resolve.assist_id == id.0 && single_resolve.assist_kind == id.1 + } + } + } +} + +#[derive(Clone, Debug)] +pub struct GroupLabel(pub String); diff --git a/crates/ide_db/src/lib.rs b/crates/ide_db/src/lib.rs index 105607dca4..2ac215c066 100644 --- a/crates/ide_db/src/lib.rs +++ b/crates/ide_db/src/lib.rs @@ -3,6 +3,7 @@ //! It is mainly a `HirDatabase` for semantic analysis, plus a `SymbolsDatabase`, for fuzzy search. mod apply_change; +pub mod assists; pub mod label; pub mod line_index; pub mod symbol_index; diff --git a/crates/ide_diagnostics/Cargo.toml b/crates/ide_diagnostics/Cargo.toml index 738fca14ed..fa2adf2122 100644 --- a/crates/ide_diagnostics/Cargo.toml +++ b/crates/ide_diagnostics/Cargo.toml @@ -22,7 +22,6 @@ text_edit = { path = "../text_edit", version = "0.0.0" } cfg = { path = "../cfg", version = "0.0.0" } hir = { path = "../hir", version = "0.0.0" } ide_db = { path = "../ide_db", version = "0.0.0" } -ide_assists = { path = "../ide_assists", version = "0.0.0" } [dev-dependencies] expect-test = "1.1" diff --git a/crates/ide_diagnostics/src/incorrect_case.rs b/crates/ide_diagnostics/src/incorrect_case.rs index 04fc779ce4..8e1a93aa71 100644 --- a/crates/ide_diagnostics/src/incorrect_case.rs +++ b/crates/ide_diagnostics/src/incorrect_case.rs @@ -1,6 +1,5 @@ use hir::{db::AstDatabase, InFile}; -use ide_assists::Assist; -use ide_db::base_db::FilePosition; +use ide_db::{assists::Assist, base_db::FilePosition}; use syntax::AstNode; use crate::{ diff --git a/crates/ide_diagnostics/src/lib.rs b/crates/ide_diagnostics/src/lib.rs index 0d98307a26..2a16c73a8b 100644 --- a/crates/ide_diagnostics/src/lib.rs +++ b/crates/ide_diagnostics/src/lib.rs @@ -28,6 +28,7 @@ mod field_shorthand; use hir::{diagnostics::AnyDiagnostic, Semantics}; use ide_db::{ + assists::{Assist, AssistId, AssistKind, AssistResolveStrategy}, base_db::{FileId, SourceDatabase}, label::Label, source_change::SourceChange, @@ -42,8 +43,6 @@ use syntax::{ use text_edit::TextEdit; use unlinked_file::UnlinkedFile; -use ide_assists::{Assist, AssistId, AssistKind, AssistResolveStrategy}; - #[derive(Copy, Clone, Debug, PartialEq)] pub struct DiagnosticCode(pub &'static str); @@ -265,8 +264,8 @@ fn unresolved_fix(id: &'static str, label: &str, target: TextRange) -> Assist { #[cfg(test)] mod tests { use expect_test::Expect; - use ide_assists::AssistResolveStrategy; use ide_db::{ + assists::AssistResolveStrategy, base_db::{fixture::WithFixture, SourceDatabaseExt}, RootDatabase, }; diff --git a/crates/ide_diagnostics/src/missing_fields.rs b/crates/ide_diagnostics/src/missing_fields.rs index f242ee4813..5af67f461c 100644 --- a/crates/ide_diagnostics/src/missing_fields.rs +++ b/crates/ide_diagnostics/src/missing_fields.rs @@ -1,7 +1,6 @@ use either::Either; use hir::{db::AstDatabase, InFile}; -use ide_assists::Assist; -use ide_db::source_change::SourceChange; +use ide_db::{assists::Assist, source_change::SourceChange}; use stdx::format_to; use syntax::{algo, ast::make, AstNode, SyntaxNodePtr}; use text_edit::TextEdit; diff --git a/crates/ide_diagnostics/src/missing_ok_or_some_in_tail_expr.rs b/crates/ide_diagnostics/src/missing_ok_or_some_in_tail_expr.rs index 9e36ca296f..01c79b6f50 100644 --- a/crates/ide_diagnostics/src/missing_ok_or_some_in_tail_expr.rs +++ b/crates/ide_diagnostics/src/missing_ok_or_some_in_tail_expr.rs @@ -1,6 +1,5 @@ use hir::db::AstDatabase; -use ide_assists::Assist; -use ide_db::source_change::SourceChange; +use ide_db::{assists::Assist, source_change::SourceChange}; use syntax::AstNode; use text_edit::TextEdit; diff --git a/crates/ide_diagnostics/src/unresolved_module.rs b/crates/ide_diagnostics/src/unresolved_module.rs index b11e71b3e7..5aa9dae179 100644 --- a/crates/ide_diagnostics/src/unresolved_module.rs +++ b/crates/ide_diagnostics/src/unresolved_module.rs @@ -1,6 +1,5 @@ use hir::db::AstDatabase; -use ide_assists::Assist; -use ide_db::{base_db::AnchoredPathBuf, source_change::FileSystemEdit}; +use ide_db::{assists::Assist, base_db::AnchoredPathBuf, source_change::FileSystemEdit}; use syntax::AstNode; use crate::{fix, Diagnostic, DiagnosticsContext}; From 26c978f258ed2af45a6979eefea9860c1eaeacda Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 14 Jun 2021 18:46:27 +0300 Subject: [PATCH 5/9] internal: adapt diagnostics to the new rename API --- crates/ide/src/references/rename.rs | 435 +----------------- crates/ide_db/src/lib.rs | 4 +- crates/ide_db/src/rename.rs | 444 +++++++++++++++++++ crates/ide_diagnostics/src/incorrect_case.rs | 46 +- 4 files changed, 474 insertions(+), 455 deletions(-) create mode 100644 crates/ide_db/src/rename.rs diff --git a/crates/ide/src/references/rename.rs b/crates/ide/src/references/rename.rs index 6b3d02bf43..88b6b12601 100644 --- a/crates/ide/src/references/rename.rs +++ b/crates/ide/src/references/rename.rs @@ -2,44 +2,23 @@ //! //! All reference and file rename requests go through here where the corresponding [`SourceChange`]s //! will be calculated. -use std::fmt::{self, Display}; - -use either::Either; -use hir::{AsAssocItem, FieldSource, HasSource, InFile, ModuleSource, Semantics}; +use hir::{AsAssocItem, InFile, Semantics}; use ide_db::{ - base_db::{AnchoredPathBuf, FileId, FileRange}, + base_db::FileId, defs::{Definition, NameClass, NameRefClass}, - search::FileReference, + rename::{bail, format_err, source_edit_from_references, IdentifierKind}, RootDatabase, }; use stdx::never; -use syntax::{ - ast::{self, NameOwner}, - lex_single_syntax_kind, AstNode, SyntaxKind, SyntaxNode, T, -}; +use syntax::{ast, AstNode, SyntaxNode}; use text_edit::TextEdit; -use crate::{FilePosition, FileSystemEdit, RangeInfo, SourceChange, TextRange}; +use crate::{FilePosition, RangeInfo, SourceChange}; + +pub use ide_db::rename::RenameError; type RenameResult = Result; -#[derive(Debug)] -pub struct RenameError(String); - -impl fmt::Display for RenameError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - Display::fmt(&self.0, f) - } -} - -macro_rules! format_err { - ($fmt:expr) => {RenameError(format!($fmt))}; - ($fmt:expr, $($arg:tt)+) => {RenameError(format!($fmt, $($arg)+))} -} - -macro_rules! bail { - ($($tokens:tt)*) => {return Err(format_err!($($tokens)*))} -} /// Prepares a rename. The sole job of this function is to return the TextRange of the thing that is /// being targeted for a rename. @@ -52,8 +31,8 @@ pub(crate) fn prepare_rename( let syntax = source_file.syntax(); let def = find_definition(&sema, syntax, position)?; - let frange = def_name_range(&&sema, def) - .ok_or_else(|| format_err!("No references found at position"))?; + let frange = + def.rename_range(&sema).ok_or_else(|| format_err!("No references found at position"))?; Ok(RangeInfo::new(frange.range, ())) } @@ -98,14 +77,7 @@ pub(crate) fn rename_with_semantics( } } - match def { - Definition::ModuleDef(hir::ModuleDef::Module(module)) => rename_mod(sema, module, new_name), - Definition::SelfType(_) => bail!("Cannot rename `Self`"), - Definition::ModuleDef(hir::ModuleDef::BuiltinType(_)) => { - bail!("Cannot rename builtin type") - } - def => rename_reference(sema, def, new_name), - } + def.rename(sema, new_name) } /// Called by the client when it is about to rename a file. @@ -116,38 +88,12 @@ pub(crate) fn will_rename_file( ) -> Option { let sema = Semantics::new(db); let module = sema.to_module_def(file_id)?; - let mut change = rename_mod(&sema, module, new_name_stem).ok()?; + let def = Definition::ModuleDef(module.into()); + let mut change = def.rename(&sema, new_name_stem).ok()?; change.file_system_edits.clear(); Some(change) } -#[derive(Copy, Clone, Debug, PartialEq)] -enum IdentifierKind { - Ident, - Lifetime, - Underscore, -} - -impl IdentifierKind { - fn classify(new_name: &str) -> RenameResult { - match lex_single_syntax_kind(new_name) { - Some(res) => match res { - (SyntaxKind::IDENT, _) => Ok(IdentifierKind::Ident), - (T![_], _) => Ok(IdentifierKind::Underscore), - (SyntaxKind::LIFETIME_IDENT, _) if new_name != "'static" && new_name != "'_" => { - Ok(IdentifierKind::Lifetime) - } - (SyntaxKind::LIFETIME_IDENT, _) => { - bail!("Invalid name `{}`: not a lifetime identifier", new_name) - } - (_, Some(syntax_error)) => bail!("Invalid name `{}`: {}", new_name, syntax_error), - (_, None) => bail!("Invalid name `{}`: not an identifier", new_name), - }, - None => bail!("Invalid name `{}`: not an identifier", new_name), - } - } -} - fn find_definition( sema: &Semantics, syntax: &SyntaxNode, @@ -189,126 +135,6 @@ fn find_definition( .ok_or_else(|| format_err!("No references found at position")) } -fn rename_mod( - sema: &Semantics, - module: hir::Module, - new_name: &str, -) -> RenameResult { - if IdentifierKind::classify(new_name)? != IdentifierKind::Ident { - bail!("Invalid name `{0}`: cannot rename module to {0}", new_name); - } - - let mut source_change = SourceChange::default(); - - let InFile { file_id, value: def_source } = module.definition_source(sema.db); - let file_id = file_id.original_file(sema.db); - if let ModuleSource::SourceFile(..) = def_source { - // mod is defined in path/to/dir/mod.rs - let path = if module.is_mod_rs(sema.db) { - format!("../{}/mod.rs", new_name) - } else { - format!("{}.rs", new_name) - }; - let dst = AnchoredPathBuf { anchor: file_id, path }; - let move_file = FileSystemEdit::MoveFile { src: file_id, dst }; - source_change.push_file_system_edit(move_file); - } - - if let Some(InFile { file_id, value: decl_source }) = module.declaration_source(sema.db) { - let file_id = file_id.original_file(sema.db); - match decl_source.name() { - Some(name) => source_change.insert_source_edit( - file_id, - TextEdit::replace(name.syntax().text_range(), new_name.to_string()), - ), - _ => never!("Module source node is missing a name"), - } - } - let def = Definition::ModuleDef(hir::ModuleDef::Module(module)); - let usages = def.usages(sema).all(); - let ref_edits = usages.iter().map(|(&file_id, references)| { - (file_id, source_edit_from_references(references, def, new_name)) - }); - source_change.extend(ref_edits); - - Ok(source_change) -} - -fn rename_reference( - sema: &Semantics, - mut def: Definition, - new_name: &str, -) -> RenameResult { - let ident_kind = IdentifierKind::classify(new_name)?; - - if matches!( - def, // is target a lifetime? - Definition::GenericParam(hir::GenericParam::LifetimeParam(_)) | Definition::Label(_) - ) { - match ident_kind { - IdentifierKind::Ident | IdentifierKind::Underscore => { - cov_mark::hit!(rename_not_a_lifetime_ident_ref); - bail!("Invalid name `{}`: not a lifetime identifier", new_name); - } - IdentifierKind::Lifetime => cov_mark::hit!(rename_lifetime), - } - } else { - match (ident_kind, def) { - (IdentifierKind::Lifetime, _) => { - cov_mark::hit!(rename_not_an_ident_ref); - bail!("Invalid name `{}`: not an identifier", new_name); - } - (IdentifierKind::Ident, _) => cov_mark::hit!(rename_non_local), - (IdentifierKind::Underscore, _) => (), - } - } - - def = match def { - // HACK: resolve trait impl items to the item def of the trait definition - // so that we properly resolve all trait item references - Definition::ModuleDef(mod_def) => mod_def - .as_assoc_item(sema.db) - .and_then(|it| it.containing_trait_impl(sema.db)) - .and_then(|it| { - it.items(sema.db).into_iter().find_map(|it| match (it, mod_def) { - (hir::AssocItem::Function(trait_func), hir::ModuleDef::Function(func)) - if trait_func.name(sema.db) == func.name(sema.db) => - { - Some(Definition::ModuleDef(hir::ModuleDef::Function(trait_func))) - } - (hir::AssocItem::Const(trait_konst), hir::ModuleDef::Const(konst)) - if trait_konst.name(sema.db) == konst.name(sema.db) => - { - Some(Definition::ModuleDef(hir::ModuleDef::Const(trait_konst))) - } - ( - hir::AssocItem::TypeAlias(trait_type_alias), - hir::ModuleDef::TypeAlias(type_alias), - ) if trait_type_alias.name(sema.db) == type_alias.name(sema.db) => { - Some(Definition::ModuleDef(hir::ModuleDef::TypeAlias(trait_type_alias))) - } - _ => None, - }) - }) - .unwrap_or(def), - _ => def, - }; - let usages = def.usages(sema).all(); - - if !usages.is_empty() && ident_kind == IdentifierKind::Underscore { - cov_mark::hit!(rename_underscore_multiple); - bail!("Cannot rename reference to `_` as it is being referenced multiple times"); - } - let mut source_change = SourceChange::default(); - source_change.extend(usages.iter().map(|(&file_id, references)| { - (file_id, source_edit_from_references(references, def, new_name)) - })); - - let (file_id, edit) = source_edit_from_def(sema, def, new_name)?; - source_change.insert_source_edit(file_id, edit); - Ok(source_change) -} - fn rename_to_self(sema: &Semantics, local: hir::Local) -> RenameResult { if never!(local.is_self(sema.db)) { bail!("rename_to_self invoked on self"); @@ -426,243 +252,6 @@ fn text_edit_from_self_param(self_param: &ast::SelfParam, new_name: &str) -> Opt Some(TextEdit::replace(self_param.syntax().text_range(), replacement_text)) } -fn source_edit_from_references( - references: &[FileReference], - def: Definition, - new_name: &str, -) -> TextEdit { - let mut edit = TextEdit::builder(); - for reference in references { - let (range, replacement) = match &reference.name { - // if the ranges differ then the node is inside a macro call, we can't really attempt - // to make special rewrites like shorthand syntax and such, so just rename the node in - // the macro input - ast::NameLike::NameRef(name_ref) - if name_ref.syntax().text_range() == reference.range => - { - source_edit_from_name_ref(name_ref, new_name, def) - } - ast::NameLike::Name(name) if name.syntax().text_range() == reference.range => { - source_edit_from_name(name, new_name) - } - _ => None, - } - .unwrap_or_else(|| (reference.range, new_name.to_string())); - edit.replace(range, replacement); - } - edit.finish() -} - -fn source_edit_from_name(name: &ast::Name, new_name: &str) -> Option<(TextRange, String)> { - if let Some(_) = ast::RecordPatField::for_field_name(name) { - if let Some(ident_pat) = name.syntax().parent().and_then(ast::IdentPat::cast) { - return Some(( - TextRange::empty(ident_pat.syntax().text_range().start()), - [new_name, ": "].concat(), - )); - } - } - None -} - -fn source_edit_from_name_ref( - name_ref: &ast::NameRef, - new_name: &str, - def: Definition, -) -> Option<(TextRange, String)> { - if let Some(record_field) = ast::RecordExprField::for_name_ref(name_ref) { - let rcf_name_ref = record_field.name_ref(); - let rcf_expr = record_field.expr(); - match (rcf_name_ref, rcf_expr.and_then(|it| it.name_ref())) { - // field: init-expr, check if we can use a field init shorthand - (Some(field_name), Some(init)) => { - if field_name == *name_ref { - if init.text() == new_name { - cov_mark::hit!(test_rename_field_put_init_shorthand); - // same names, we can use a shorthand here instead. - // we do not want to erase attributes hence this range start - let s = field_name.syntax().text_range().start(); - let e = record_field.syntax().text_range().end(); - return Some((TextRange::new(s, e), new_name.to_owned())); - } - } else if init == *name_ref { - if field_name.text() == new_name { - cov_mark::hit!(test_rename_local_put_init_shorthand); - // same names, we can use a shorthand here instead. - // we do not want to erase attributes hence this range start - let s = field_name.syntax().text_range().start(); - let e = record_field.syntax().text_range().end(); - return Some((TextRange::new(s, e), new_name.to_owned())); - } - } - None - } - // init shorthand - // FIXME: instead of splitting the shorthand, recursively trigger a rename of the - // other name https://github.com/rust-analyzer/rust-analyzer/issues/6547 - (None, Some(_)) if matches!(def, Definition::Field(_)) => { - cov_mark::hit!(test_rename_field_in_field_shorthand); - let s = name_ref.syntax().text_range().start(); - Some((TextRange::empty(s), format!("{}: ", new_name))) - } - (None, Some(_)) if matches!(def, Definition::Local(_)) => { - cov_mark::hit!(test_rename_local_in_field_shorthand); - let s = name_ref.syntax().text_range().end(); - Some((TextRange::empty(s), format!(": {}", new_name))) - } - _ => None, - } - } else if let Some(record_field) = ast::RecordPatField::for_field_name_ref(name_ref) { - let rcf_name_ref = record_field.name_ref(); - let rcf_pat = record_field.pat(); - match (rcf_name_ref, rcf_pat) { - // field: rename - (Some(field_name), Some(ast::Pat::IdentPat(pat))) if field_name == *name_ref => { - // field name is being renamed - if pat.name().map_or(false, |it| it.text() == new_name) { - cov_mark::hit!(test_rename_field_put_init_shorthand_pat); - // same names, we can use a shorthand here instead/ - // we do not want to erase attributes hence this range start - let s = field_name.syntax().text_range().start(); - let e = record_field.syntax().text_range().end(); - Some((TextRange::new(s, e), pat.to_string())) - } else { - None - } - } - _ => None, - } - } else { - None - } -} - -fn source_edit_from_def( - sema: &Semantics, - def: Definition, - new_name: &str, -) -> RenameResult<(FileId, TextEdit)> { - let frange: FileRange = def_name_range(sema, def) - .ok_or_else(|| format_err!("No identifier available to rename"))?; - - let mut replacement_text = String::new(); - let mut repl_range = frange.range; - if let Definition::Local(local) = def { - if let Either::Left(pat) = local.source(sema.db).value { - if matches!( - pat.syntax().parent().and_then(ast::RecordPatField::cast), - Some(pat_field) if pat_field.name_ref().is_none() - ) { - replacement_text.push_str(": "); - replacement_text.push_str(new_name); - repl_range = TextRange::new( - pat.syntax().text_range().end(), - pat.syntax().text_range().end(), - ); - } - } - } - if replacement_text.is_empty() { - replacement_text.push_str(new_name); - } - let edit = TextEdit::replace(repl_range, replacement_text); - Ok((frange.file_id, edit)) -} - -fn def_name_range(sema: &Semantics, def: Definition) -> Option { - // FIXME: the `original_file_range` calls here are wrong -- they never fail, - // and _fall back_ to the entirety of the macro call. Such fall back is - // incorrect for renames. The safe behavior would be to return an error for - // such cases. The correct behavior would be to return an auxiliary list of - // "can't rename these occurrences in macros" items, and then show some kind - // of a dialog to the user. - - let res = match def { - Definition::Macro(mac) => { - let src = mac.source(sema.db)?; - let name = match &src.value { - Either::Left(it) => it.name()?, - Either::Right(it) => it.name()?, - }; - src.with_value(name.syntax()).original_file_range(sema.db) - } - Definition::Field(field) => { - let src = field.source(sema.db)?; - - match &src.value { - FieldSource::Named(record_field) => { - let name = record_field.name()?; - src.with_value(name.syntax()).original_file_range(sema.db) - } - FieldSource::Pos(_) => { - return None; - } - } - } - Definition::ModuleDef(module_def) => match module_def { - hir::ModuleDef::Module(module) => { - let src = module.declaration_source(sema.db)?; - let name = src.value.name()?; - src.with_value(name.syntax()).original_file_range(sema.db) - } - hir::ModuleDef::Function(it) => name_range(it, sema)?, - hir::ModuleDef::Adt(adt) => match adt { - hir::Adt::Struct(it) => name_range(it, sema)?, - hir::Adt::Union(it) => name_range(it, sema)?, - hir::Adt::Enum(it) => name_range(it, sema)?, - }, - hir::ModuleDef::Variant(it) => name_range(it, sema)?, - hir::ModuleDef::Const(it) => name_range(it, sema)?, - hir::ModuleDef::Static(it) => name_range(it, sema)?, - hir::ModuleDef::Trait(it) => name_range(it, sema)?, - hir::ModuleDef::TypeAlias(it) => name_range(it, sema)?, - hir::ModuleDef::BuiltinType(_) => return None, - }, - Definition::SelfType(_) => return None, - Definition::Local(local) => { - let src = local.source(sema.db); - let name = match &src.value { - Either::Left(bind_pat) => bind_pat.name()?, - Either::Right(_) => return None, - }; - src.with_value(name.syntax()).original_file_range(sema.db) - } - Definition::GenericParam(generic_param) => match generic_param { - hir::GenericParam::TypeParam(type_param) => { - let src = type_param.source(sema.db)?; - let name = match &src.value { - Either::Left(_) => return None, - Either::Right(type_param) => type_param.name()?, - }; - src.with_value(name.syntax()).original_file_range(sema.db) - } - hir::GenericParam::LifetimeParam(lifetime_param) => { - let src = lifetime_param.source(sema.db)?; - let lifetime = src.value.lifetime()?; - src.with_value(lifetime.syntax()).original_file_range(sema.db) - } - hir::GenericParam::ConstParam(it) => name_range(it, sema)?, - }, - Definition::Label(label) => { - let src = label.source(sema.db); - let lifetime = src.value.lifetime()?; - src.with_value(lifetime.syntax()).original_file_range(sema.db) - } - }; - return Some(res); - - fn name_range(def: D, sema: &Semantics) -> Option - where - D: HasSource, - D::Ast: ast::NameOwner, - { - let src = def.source(sema.db)?; - let name = src.value.name()?; - let res = src.with_value(name.syntax()).original_file_range(sema.db); - Some(res) - } -} - #[cfg(test)] mod tests { use expect_test::{expect, Expect}; diff --git a/crates/ide_db/src/lib.rs b/crates/ide_db/src/lib.rs index 2ac215c066..7bbd08d6f1 100644 --- a/crates/ide_db/src/lib.rs +++ b/crates/ide_db/src/lib.rs @@ -8,7 +8,6 @@ pub mod label; pub mod line_index; pub mod symbol_index; pub mod defs; -pub mod search; pub mod items_locator; pub mod source_change; pub mod ty_filter; @@ -16,6 +15,9 @@ pub mod traits; pub mod call_info; pub mod helpers; +pub mod search; +pub mod rename; + use std::{fmt, sync::Arc}; use base_db::{ diff --git a/crates/ide_db/src/rename.rs b/crates/ide_db/src/rename.rs new file mode 100644 index 0000000000..877650df0f --- /dev/null +++ b/crates/ide_db/src/rename.rs @@ -0,0 +1,444 @@ +use std::fmt; + +use base_db::{AnchoredPathBuf, FileId, FileRange}; +use either::Either; +use hir::{AsAssocItem, FieldSource, HasSource, InFile, ModuleSource, Semantics}; +use stdx::never; +use syntax::{ + ast::{self, NameOwner}, + lex_single_syntax_kind, AstNode, SyntaxKind, TextRange, T, +}; +use text_edit::TextEdit; + +use crate::{ + defs::Definition, + search::FileReference, + source_change::{FileSystemEdit, SourceChange}, + RootDatabase, +}; + +pub type Result = std::result::Result; + +#[derive(Debug)] +pub struct RenameError(pub String); + +impl fmt::Display for RenameError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(&self.0, f) + } +} + +#[macro_export] +macro_rules! _format_err { + ($fmt:expr) => { RenameError(format!($fmt)) }; + ($fmt:expr, $($arg:tt)+) => { RenameError(format!($fmt, $($arg)+)) } +} +pub use _format_err as format_err; + +#[macro_export] +macro_rules! _bail { + ($($tokens:tt)*) => { return Err(format_err!($($tokens)*)) } +} +pub use _bail as bail; + +impl Definition { + pub fn rename(&self, sema: &Semantics, new_name: &str) -> Result { + match *self { + Definition::ModuleDef(hir::ModuleDef::Module(module)) => { + rename_mod(sema, module, new_name) + } + Definition::ModuleDef(hir::ModuleDef::BuiltinType(_)) => { + bail!("Cannot rename builtin type") + } + Definition::SelfType(_) => bail!("Cannot rename `Self`"), + def => rename_reference(sema, def, new_name), + } + } + + /// Textual range of the identifier which will change when renaming this + /// `Definition`. Note that some definitions, like buitin types, can't be + /// renamed. + pub fn rename_range(self, sema: &Semantics) -> Option { + // FIXME: the `original_file_range` calls here are wrong -- they never fail, + // and _fall back_ to the entirety of the macro call. Such fall back is + // incorrect for renames. The safe behavior would be to return an error for + // such cases. The correct behavior would be to return an auxiliary list of + // "can't rename these occurrences in macros" items, and then show some kind + // of a dialog to the user. + + let res = match self { + Definition::Macro(mac) => { + let src = mac.source(sema.db)?; + let name = match &src.value { + Either::Left(it) => it.name()?, + Either::Right(it) => it.name()?, + }; + src.with_value(name.syntax()).original_file_range(sema.db) + } + Definition::Field(field) => { + let src = field.source(sema.db)?; + + match &src.value { + FieldSource::Named(record_field) => { + let name = record_field.name()?; + src.with_value(name.syntax()).original_file_range(sema.db) + } + FieldSource::Pos(_) => { + return None; + } + } + } + Definition::ModuleDef(module_def) => match module_def { + hir::ModuleDef::Module(module) => { + let src = module.declaration_source(sema.db)?; + let name = src.value.name()?; + src.with_value(name.syntax()).original_file_range(sema.db) + } + hir::ModuleDef::Function(it) => name_range(it, sema)?, + hir::ModuleDef::Adt(adt) => match adt { + hir::Adt::Struct(it) => name_range(it, sema)?, + hir::Adt::Union(it) => name_range(it, sema)?, + hir::Adt::Enum(it) => name_range(it, sema)?, + }, + hir::ModuleDef::Variant(it) => name_range(it, sema)?, + hir::ModuleDef::Const(it) => name_range(it, sema)?, + hir::ModuleDef::Static(it) => name_range(it, sema)?, + hir::ModuleDef::Trait(it) => name_range(it, sema)?, + hir::ModuleDef::TypeAlias(it) => name_range(it, sema)?, + hir::ModuleDef::BuiltinType(_) => return None, + }, + Definition::SelfType(_) => return None, + Definition::Local(local) => { + let src = local.source(sema.db); + let name = match &src.value { + Either::Left(bind_pat) => bind_pat.name()?, + Either::Right(_) => return None, + }; + src.with_value(name.syntax()).original_file_range(sema.db) + } + Definition::GenericParam(generic_param) => match generic_param { + hir::GenericParam::TypeParam(type_param) => { + let src = type_param.source(sema.db)?; + let name = match &src.value { + Either::Left(_) => return None, + Either::Right(type_param) => type_param.name()?, + }; + src.with_value(name.syntax()).original_file_range(sema.db) + } + hir::GenericParam::LifetimeParam(lifetime_param) => { + let src = lifetime_param.source(sema.db)?; + let lifetime = src.value.lifetime()?; + src.with_value(lifetime.syntax()).original_file_range(sema.db) + } + hir::GenericParam::ConstParam(it) => name_range(it, sema)?, + }, + Definition::Label(label) => { + let src = label.source(sema.db); + let lifetime = src.value.lifetime()?; + src.with_value(lifetime.syntax()).original_file_range(sema.db) + } + }; + return Some(res); + + fn name_range(def: D, sema: &Semantics) -> Option + where + D: HasSource, + D::Ast: ast::NameOwner, + { + let src = def.source(sema.db)?; + let name = src.value.name()?; + let res = src.with_value(name.syntax()).original_file_range(sema.db); + Some(res) + } + } +} + +fn rename_mod( + sema: &Semantics, + module: hir::Module, + new_name: &str, +) -> Result { + if IdentifierKind::classify(new_name)? != IdentifierKind::Ident { + bail!("Invalid name `{0}`: cannot rename module to {0}", new_name); + } + + let mut source_change = SourceChange::default(); + + let InFile { file_id, value: def_source } = module.definition_source(sema.db); + let file_id = file_id.original_file(sema.db); + if let ModuleSource::SourceFile(..) = def_source { + // mod is defined in path/to/dir/mod.rs + let path = if module.is_mod_rs(sema.db) { + format!("../{}/mod.rs", new_name) + } else { + format!("{}.rs", new_name) + }; + let dst = AnchoredPathBuf { anchor: file_id, path }; + let move_file = FileSystemEdit::MoveFile { src: file_id, dst }; + source_change.push_file_system_edit(move_file); + } + + if let Some(InFile { file_id, value: decl_source }) = module.declaration_source(sema.db) { + let file_id = file_id.original_file(sema.db); + match decl_source.name() { + Some(name) => source_change.insert_source_edit( + file_id, + TextEdit::replace(name.syntax().text_range(), new_name.to_string()), + ), + _ => never!("Module source node is missing a name"), + } + } + let def = Definition::ModuleDef(hir::ModuleDef::Module(module)); + let usages = def.usages(sema).all(); + let ref_edits = usages.iter().map(|(&file_id, references)| { + (file_id, source_edit_from_references(references, def, new_name)) + }); + source_change.extend(ref_edits); + + Ok(source_change) +} + +fn rename_reference( + sema: &Semantics, + mut def: Definition, + new_name: &str, +) -> Result { + let ident_kind = IdentifierKind::classify(new_name)?; + + if matches!( + def, // is target a lifetime? + Definition::GenericParam(hir::GenericParam::LifetimeParam(_)) | Definition::Label(_) + ) { + match ident_kind { + IdentifierKind::Ident | IdentifierKind::Underscore => { + cov_mark::hit!(rename_not_a_lifetime_ident_ref); + bail!("Invalid name `{}`: not a lifetime identifier", new_name); + } + IdentifierKind::Lifetime => cov_mark::hit!(rename_lifetime), + } + } else { + match (ident_kind, def) { + (IdentifierKind::Lifetime, _) => { + cov_mark::hit!(rename_not_an_ident_ref); + bail!("Invalid name `{}`: not an identifier", new_name); + } + (IdentifierKind::Ident, _) => cov_mark::hit!(rename_non_local), + (IdentifierKind::Underscore, _) => (), + } + } + + def = match def { + // HACK: resolve trait impl items to the item def of the trait definition + // so that we properly resolve all trait item references + Definition::ModuleDef(mod_def) => mod_def + .as_assoc_item(sema.db) + .and_then(|it| it.containing_trait_impl(sema.db)) + .and_then(|it| { + it.items(sema.db).into_iter().find_map(|it| match (it, mod_def) { + (hir::AssocItem::Function(trait_func), hir::ModuleDef::Function(func)) + if trait_func.name(sema.db) == func.name(sema.db) => + { + Some(Definition::ModuleDef(hir::ModuleDef::Function(trait_func))) + } + (hir::AssocItem::Const(trait_konst), hir::ModuleDef::Const(konst)) + if trait_konst.name(sema.db) == konst.name(sema.db) => + { + Some(Definition::ModuleDef(hir::ModuleDef::Const(trait_konst))) + } + ( + hir::AssocItem::TypeAlias(trait_type_alias), + hir::ModuleDef::TypeAlias(type_alias), + ) if trait_type_alias.name(sema.db) == type_alias.name(sema.db) => { + Some(Definition::ModuleDef(hir::ModuleDef::TypeAlias(trait_type_alias))) + } + _ => None, + }) + }) + .unwrap_or(def), + _ => def, + }; + let usages = def.usages(sema).all(); + + if !usages.is_empty() && ident_kind == IdentifierKind::Underscore { + cov_mark::hit!(rename_underscore_multiple); + bail!("Cannot rename reference to `_` as it is being referenced multiple times"); + } + let mut source_change = SourceChange::default(); + source_change.extend(usages.iter().map(|(&file_id, references)| { + (file_id, source_edit_from_references(references, def, new_name)) + })); + + let (file_id, edit) = source_edit_from_def(sema, def, new_name)?; + source_change.insert_source_edit(file_id, edit); + Ok(source_change) +} + +pub fn source_edit_from_references( + references: &[FileReference], + def: Definition, + new_name: &str, +) -> TextEdit { + let mut edit = TextEdit::builder(); + for reference in references { + let (range, replacement) = match &reference.name { + // if the ranges differ then the node is inside a macro call, we can't really attempt + // to make special rewrites like shorthand syntax and such, so just rename the node in + // the macro input + ast::NameLike::NameRef(name_ref) + if name_ref.syntax().text_range() == reference.range => + { + source_edit_from_name_ref(name_ref, new_name, def) + } + ast::NameLike::Name(name) if name.syntax().text_range() == reference.range => { + source_edit_from_name(name, new_name) + } + _ => None, + } + .unwrap_or_else(|| (reference.range, new_name.to_string())); + edit.replace(range, replacement); + } + edit.finish() +} + +fn source_edit_from_name(name: &ast::Name, new_name: &str) -> Option<(TextRange, String)> { + if let Some(_) = ast::RecordPatField::for_field_name(name) { + if let Some(ident_pat) = name.syntax().parent().and_then(ast::IdentPat::cast) { + return Some(( + TextRange::empty(ident_pat.syntax().text_range().start()), + [new_name, ": "].concat(), + )); + } + } + None +} + +fn source_edit_from_name_ref( + name_ref: &ast::NameRef, + new_name: &str, + def: Definition, +) -> Option<(TextRange, String)> { + if let Some(record_field) = ast::RecordExprField::for_name_ref(name_ref) { + let rcf_name_ref = record_field.name_ref(); + let rcf_expr = record_field.expr(); + match (rcf_name_ref, rcf_expr.and_then(|it| it.name_ref())) { + // field: init-expr, check if we can use a field init shorthand + (Some(field_name), Some(init)) => { + if field_name == *name_ref { + if init.text() == new_name { + cov_mark::hit!(test_rename_field_put_init_shorthand); + // same names, we can use a shorthand here instead. + // we do not want to erase attributes hence this range start + let s = field_name.syntax().text_range().start(); + let e = record_field.syntax().text_range().end(); + return Some((TextRange::new(s, e), new_name.to_owned())); + } + } else if init == *name_ref { + if field_name.text() == new_name { + cov_mark::hit!(test_rename_local_put_init_shorthand); + // same names, we can use a shorthand here instead. + // we do not want to erase attributes hence this range start + let s = field_name.syntax().text_range().start(); + let e = record_field.syntax().text_range().end(); + return Some((TextRange::new(s, e), new_name.to_owned())); + } + } + None + } + // init shorthand + // FIXME: instead of splitting the shorthand, recursively trigger a rename of the + // other name https://github.com/rust-analyzer/rust-analyzer/issues/6547 + (None, Some(_)) if matches!(def, Definition::Field(_)) => { + cov_mark::hit!(test_rename_field_in_field_shorthand); + let s = name_ref.syntax().text_range().start(); + Some((TextRange::empty(s), format!("{}: ", new_name))) + } + (None, Some(_)) if matches!(def, Definition::Local(_)) => { + cov_mark::hit!(test_rename_local_in_field_shorthand); + let s = name_ref.syntax().text_range().end(); + Some((TextRange::empty(s), format!(": {}", new_name))) + } + _ => None, + } + } else if let Some(record_field) = ast::RecordPatField::for_field_name_ref(name_ref) { + let rcf_name_ref = record_field.name_ref(); + let rcf_pat = record_field.pat(); + match (rcf_name_ref, rcf_pat) { + // field: rename + (Some(field_name), Some(ast::Pat::IdentPat(pat))) if field_name == *name_ref => { + // field name is being renamed + if pat.name().map_or(false, |it| it.text() == new_name) { + cov_mark::hit!(test_rename_field_put_init_shorthand_pat); + // same names, we can use a shorthand here instead/ + // we do not want to erase attributes hence this range start + let s = field_name.syntax().text_range().start(); + let e = record_field.syntax().text_range().end(); + Some((TextRange::new(s, e), pat.to_string())) + } else { + None + } + } + _ => None, + } + } else { + None + } +} + +fn source_edit_from_def( + sema: &Semantics, + def: Definition, + new_name: &str, +) -> Result<(FileId, TextEdit)> { + let frange = + def.rename_range(sema).ok_or_else(|| format_err!("No identifier available to rename"))?; + + let mut replacement_text = String::new(); + let mut repl_range = frange.range; + if let Definition::Local(local) = def { + if let Either::Left(pat) = local.source(sema.db).value { + if matches!( + pat.syntax().parent().and_then(ast::RecordPatField::cast), + Some(pat_field) if pat_field.name_ref().is_none() + ) { + replacement_text.push_str(": "); + replacement_text.push_str(new_name); + repl_range = TextRange::new( + pat.syntax().text_range().end(), + pat.syntax().text_range().end(), + ); + } + } + } + if replacement_text.is_empty() { + replacement_text.push_str(new_name); + } + let edit = TextEdit::replace(repl_range, replacement_text); + Ok((frange.file_id, edit)) +} + +#[derive(Copy, Clone, Debug, PartialEq)] +pub enum IdentifierKind { + Ident, + Lifetime, + Underscore, +} + +impl IdentifierKind { + pub fn classify(new_name: &str) -> Result { + match lex_single_syntax_kind(new_name) { + Some(res) => match res { + (SyntaxKind::IDENT, _) => Ok(IdentifierKind::Ident), + (T![_], _) => Ok(IdentifierKind::Underscore), + (SyntaxKind::LIFETIME_IDENT, _) if new_name != "'static" && new_name != "'_" => { + Ok(IdentifierKind::Lifetime) + } + (SyntaxKind::LIFETIME_IDENT, _) => { + bail!("Invalid name `{}`: not a lifetime identifier", new_name) + } + (_, Some(syntax_error)) => bail!("Invalid name `{}`: {}", new_name, syntax_error), + (_, None) => bail!("Invalid name `{}`: not an identifier", new_name), + }, + None => bail!("Invalid name `{}`: not an identifier", new_name), + } + } +} diff --git a/crates/ide_diagnostics/src/incorrect_case.rs b/crates/ide_diagnostics/src/incorrect_case.rs index 8e1a93aa71..2cf232d564 100644 --- a/crates/ide_diagnostics/src/incorrect_case.rs +++ b/crates/ide_diagnostics/src/incorrect_case.rs @@ -1,5 +1,5 @@ use hir::{db::AstDatabase, InFile}; -use ide_db::{assists::Assist, base_db::FilePosition}; +use ide_db::{assists::Assist, defs::NameClass}; use syntax::AstNode; use crate::{ @@ -27,35 +27,26 @@ pub(super) fn incorrect_case(ctx: &DiagnosticsContext<'_>, d: &hir::IncorrectCas } fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::IncorrectCase) -> Option> { - if true { - return None; - } - let root = ctx.sema.db.parse_or_expand(d.file)?; let name_node = d.ident.to_node(&root); + let def = NameClass::classify(&ctx.sema, &name_node)?.defined(ctx.sema.db)?; let name_node = InFile::new(d.file, name_node.syntax()); let frange = name_node.original_file_range(ctx.sema.db); - let _file_position = FilePosition { file_id: frange.file_id, offset: frange.range.start() }; let label = format!("Rename to {}", d.suggested_text); - let res = unresolved_fix("change_case", &label, frange.range); + let mut res = unresolved_fix("change_case", &label, frange.range); if ctx.resolve.should_resolve(&res.id) { - //let source_change = rename_with_semantics(&ctx.sema, file_position, &d.suggested_text); - //res.source_change = Some(source_change.ok().unwrap_or_default()); - todo!() + let source_change = def.rename(&ctx.sema, &d.suggested_text); + res.source_change = Some(source_change.ok().unwrap_or_default()); } Some(vec![res]) } -#[cfg(TODO)] +#[cfg(test)] mod change_case { - use crate::{ - fixture, - tests::{check_diagnostics, check_fix}, - AssistResolveStrategy, DiagnosticsConfig, - }; + use crate::tests::{check_diagnostics, check_fix}; #[test] fn test_rename_incorrect_case() { @@ -123,7 +114,7 @@ fn some_fn() { check_diagnostics( r#" fn foo() { - const ANOTHER_ITEM$0: &str = "some_item"; + const ANOTHER_ITEM: &str = "some_item"; } "#, ); @@ -155,20 +146,13 @@ impl TestStruct { #[test] fn test_single_incorrect_case_diagnostic_in_function_name_issue_6970() { - let input = r#"fn FOO$0() {}"#; - let expected = r#"fn foo() {}"#; - - let (analysis, file_position) = fixture::position(input); - let diagnostics = analysis - .diagnostics( - &DiagnosticsConfig::default(), - AssistResolveStrategy::All, - file_position.file_id, - ) - .unwrap(); - assert_eq!(diagnostics.len(), 1); - - check_fix(input, expected); + check_diagnostics( + r#" +fn FOO() {} +// ^^^ Function `FOO` should have snake_case name, e.g. `foo` +"#, + ); + check_fix(r#"fn FOO$0() {}"#, r#"fn foo() {}"#); } #[test] From 9fb67e7477a43fc91946f17c00205b7e31db00d8 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 14 Jun 2021 19:08:12 +0300 Subject: [PATCH 6/9] internal: document rename challenges --- crates/ide/src/references/rename.rs | 18 ++++++++++++++++++ crates/ide_db/src/rename.rs | 25 ++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/crates/ide/src/references/rename.rs b/crates/ide/src/references/rename.rs index 88b6b12601..cec1d4552c 100644 --- a/crates/ide/src/references/rename.rs +++ b/crates/ide/src/references/rename.rs @@ -1767,4 +1767,22 @@ fn f() { <()>::BAR$0; }"#, res, ); } + + #[test] + fn macros_are_broken_lol() { + cov_mark::check!(macros_are_broken_lol); + check( + "lol", + r#" +macro_rules! m { () => { fn f() {} } } +m!(); +fn main() { f$0() } +"#, + r#" +macro_rules! m { () => { fn f() {} } } +lol +fn main() { lol() } +"#, + ) + } } diff --git a/crates/ide_db/src/rename.rs b/crates/ide_db/src/rename.rs index 877650df0f..d776024535 100644 --- a/crates/ide_db/src/rename.rs +++ b/crates/ide_db/src/rename.rs @@ -1,3 +1,25 @@ +//! Rename infrastructure for rust-analyzer. It is used primarily for the +//! literal "rename" in the ide (look for tests there), but it is also available +//! as a general-purpose service. For example, it is used by the fix for the +//! "incorrect case" diagnostic. +//! +//! It leverages the [`crate::search`] functionality to find what needs to be +//! renamed. The actual renames are tricky -- field shorthands need special +//! attention, and, when renaming modules, you also want to rename files on the +//! file system. +//! +//! Another can of worms are macros: +//! +//! ``` +//! macro_rules! m { () => { fn f() {} } } +//! m!(); +//! fn main() { +//! f() // <- rename me +//! } +//! ``` +//! +//! The correct behavior in such cases is probably to show a dialog to the user. +//! Our current behavior is ¯\_(ツ)_/¯. use std::fmt; use base_db::{AnchoredPathBuf, FileId, FileRange}; @@ -64,7 +86,8 @@ impl Definition { // incorrect for renames. The safe behavior would be to return an error for // such cases. The correct behavior would be to return an auxiliary list of // "can't rename these occurrences in macros" items, and then show some kind - // of a dialog to the user. + // of a dialog to the user. See: + cov_mark::hit!(macros_are_broken_lol); let res = match self { Definition::Macro(mac) => { From da534bdd074788e47b5dae76998b8f8535ea7257 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 14 Jun 2021 19:14:34 +0300 Subject: [PATCH 7/9] internal: flatten module hierarchy It seems that any crate can be made better by flattening the modules down to a single layer? --- crates/ide/src/lib.rs | 10 ++++++---- crates/ide/src/references.rs | 2 -- crates/ide/src/{references => }/rename.rs | 7 ++++--- 3 files changed, 10 insertions(+), 9 deletions(-) rename crates/ide/src/{references => }/rename.rs (99%) diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index 98d01f0ced..9db387d26d 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -39,6 +39,7 @@ mod matching_brace; mod move_item; mod parent_module; mod references; +mod rename; mod fn_references; mod runnables; mod ssr; @@ -79,7 +80,8 @@ pub use crate::{ markup::Markup, move_item::Direction, prime_caches::PrimeCachesProgress, - references::{rename::RenameError, ReferenceSearchResult}, + references::ReferenceSearchResult, + rename::RenameError, runnables::{Runnable, RunnableKind, TestId}, syntax_highlighting::{ tags::{Highlight, HlMod, HlMods, HlOperator, HlPunct, HlTag}, @@ -591,14 +593,14 @@ impl Analysis { position: FilePosition, new_name: &str, ) -> Cancellable> { - self.with_db(|db| references::rename::rename(db, position, new_name)) + self.with_db(|db| rename::rename(db, position, new_name)) } pub fn prepare_rename( &self, position: FilePosition, ) -> Cancellable, RenameError>> { - self.with_db(|db| references::rename::prepare_rename(db, position)) + self.with_db(|db| rename::prepare_rename(db, position)) } pub fn will_rename_file( @@ -606,7 +608,7 @@ impl Analysis { file_id: FileId, new_name_stem: &str, ) -> Cancellable> { - self.with_db(|db| references::rename::will_rename_file(db, file_id, new_name_stem)) + self.with_db(|db| rename::will_rename_file(db, file_id, new_name_stem)) } pub fn structural_search_replace( diff --git a/crates/ide/src/references.rs b/crates/ide/src/references.rs index a0fdead2c1..945c9b9e10 100644 --- a/crates/ide/src/references.rs +++ b/crates/ide/src/references.rs @@ -9,8 +9,6 @@ //! at the index that the match starts at and its tree parent is //! resolved to the search element definition, we get a reference. -pub(crate) mod rename; - use hir::{PathResolution, Semantics}; use ide_db::{ base_db::FileId, diff --git a/crates/ide/src/references/rename.rs b/crates/ide/src/rename.rs similarity index 99% rename from crates/ide/src/references/rename.rs rename to crates/ide/src/rename.rs index cec1d4552c..41689a939e 100644 --- a/crates/ide/src/references/rename.rs +++ b/crates/ide/src/rename.rs @@ -1,7 +1,8 @@ -//! Renaming functionality +//! Renaming functionality. //! -//! All reference and file rename requests go through here where the corresponding [`SourceChange`]s -//! will be calculated. +//! This is mostly front-end for [`ide_db::rename`], but it also includes the +//! tests. This module also implements a couple of magic tricks, like renaming +//! `self` and to `self` (to switch between associated function and method). use hir::{AsAssocItem, InFile, Semantics}; use ide_db::{ base_db::FileId, From 94f7b63522cb7464a853c74a8431587db6434b12 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 14 Jun 2021 19:15:53 +0300 Subject: [PATCH 8/9] minor: less ambiguous name --- crates/ide/src/rename.rs | 5 +++-- crates/ide_db/src/rename.rs | 7 ++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/crates/ide/src/rename.rs b/crates/ide/src/rename.rs index 41689a939e..8096dfa0ea 100644 --- a/crates/ide/src/rename.rs +++ b/crates/ide/src/rename.rs @@ -32,8 +32,9 @@ pub(crate) fn prepare_rename( let syntax = source_file.syntax(); let def = find_definition(&sema, syntax, position)?; - let frange = - def.rename_range(&sema).ok_or_else(|| format_err!("No references found at position"))?; + let frange = def + .range_for_rename(&sema) + .ok_or_else(|| format_err!("No references found at position"))?; Ok(RangeInfo::new(frange.range, ())) } diff --git a/crates/ide_db/src/rename.rs b/crates/ide_db/src/rename.rs index d776024535..82855725fa 100644 --- a/crates/ide_db/src/rename.rs +++ b/crates/ide_db/src/rename.rs @@ -80,7 +80,7 @@ impl Definition { /// Textual range of the identifier which will change when renaming this /// `Definition`. Note that some definitions, like buitin types, can't be /// renamed. - pub fn rename_range(self, sema: &Semantics) -> Option { + pub fn range_for_rename(self, sema: &Semantics) -> Option { // FIXME: the `original_file_range` calls here are wrong -- they never fail, // and _fall back_ to the entirety of the macro call. Such fall back is // incorrect for renames. The safe behavior would be to return an error for @@ -412,8 +412,9 @@ fn source_edit_from_def( def: Definition, new_name: &str, ) -> Result<(FileId, TextEdit)> { - let frange = - def.rename_range(sema).ok_or_else(|| format_err!("No identifier available to rename"))?; + let frange = def + .range_for_rename(sema) + .ok_or_else(|| format_err!("No identifier available to rename"))?; let mut replacement_text = String::new(); let mut repl_range = frange.range; From 4768e5fb23c058eba90f0a1dcd6e9d5c0ecdee1b Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 14 Jun 2021 19:32:39 +0300 Subject: [PATCH 9/9] internal: document diagnostics crate --- .../{ => handlers}/break_outside_of_loop.rs | 2 +- .../src/{ => handlers}/inactive_code.rs | 2 +- .../src/{ => handlers}/incorrect_case.rs | 2 +- .../src/{ => handlers}/macro_error.rs | 2 +- .../{ => handlers}/mismatched_arg_count.rs | 2 +- .../src/{ => handlers}/missing_fields.rs | 2 +- .../src/{ => handlers}/missing_match_arms.rs | 4 +- .../missing_ok_or_some_in_tail_expr.rs | 2 +- .../src/{ => handlers}/missing_unsafe.rs | 2 +- .../src/{ => handlers}/no_such_field.rs | 2 +- .../{ => handlers}/remove_this_semicolon.rs | 2 +- .../replace_filter_map_next_with_find_map.rs | 2 +- .../unimplemented_builtin_macro.rs | 2 +- .../src/{ => handlers}/unlinked_file.rs | 2 +- .../{ => handlers}/unresolved_extern_crate.rs | 2 +- .../src/{ => handlers}/unresolved_import.rs | 2 +- .../{ => handlers}/unresolved_macro_call.rs | 2 +- .../src/{ => handlers}/unresolved_module.rs | 2 +- .../{ => handlers}/unresolved_proc_macro.rs | 2 +- crates/ide_diagnostics/src/lib.rs | 108 +++++++++++------- 20 files changed, 85 insertions(+), 63 deletions(-) rename crates/ide_diagnostics/src/{ => handlers}/break_outside_of_loop.rs (94%) rename crates/ide_diagnostics/src/{ => handlers}/inactive_code.rs (99%) rename crates/ide_diagnostics/src/{ => handlers}/incorrect_case.rs (99%) rename crates/ide_diagnostics/src/{ => handlers}/macro_error.rs (98%) rename crates/ide_diagnostics/src/{ => handlers}/mismatched_arg_count.rs (99%) rename crates/ide_diagnostics/src/{ => handlers}/missing_fields.rs (99%) rename crates/ide_diagnostics/src/{ => handlers}/missing_match_arms.rs (99%) rename crates/ide_diagnostics/src/{ => handlers}/missing_ok_or_some_in_tail_expr.rs (99%) rename crates/ide_diagnostics/src/{ => handlers}/missing_unsafe.rs (97%) rename crates/ide_diagnostics/src/{ => handlers}/no_such_field.rs (99%) rename crates/ide_diagnostics/src/{ => handlers}/remove_this_semicolon.rs (97%) rename crates/ide_diagnostics/src/{ => handlers}/replace_filter_map_next_with_find_map.rs (98%) rename crates/ide_diagnostics/src/{ => handlers}/unimplemented_builtin_macro.rs (92%) rename crates/ide_diagnostics/src/{ => handlers}/unlinked_file.rs (99%) rename crates/ide_diagnostics/src/{ => handlers}/unresolved_extern_crate.rs (96%) rename crates/ide_diagnostics/src/{ => handlers}/unresolved_import.rs (98%) rename crates/ide_diagnostics/src/{ => handlers}/unresolved_macro_call.rs (98%) rename crates/ide_diagnostics/src/{ => handlers}/unresolved_module.rs (99%) rename crates/ide_diagnostics/src/{ => handlers}/unresolved_proc_macro.rs (97%) diff --git a/crates/ide_diagnostics/src/break_outside_of_loop.rs b/crates/ide_diagnostics/src/handlers/break_outside_of_loop.rs similarity index 94% rename from crates/ide_diagnostics/src/break_outside_of_loop.rs rename to crates/ide_diagnostics/src/handlers/break_outside_of_loop.rs index 79e8cea373..5ad0fbd1bb 100644 --- a/crates/ide_diagnostics/src/break_outside_of_loop.rs +++ b/crates/ide_diagnostics/src/handlers/break_outside_of_loop.rs @@ -3,7 +3,7 @@ use crate::{Diagnostic, DiagnosticsContext}; // Diagnostic: break-outside-of-loop // // This diagnostic is triggered if the `break` keyword is used outside of a loop. -pub(super) fn break_outside_of_loop( +pub(crate) fn break_outside_of_loop( ctx: &DiagnosticsContext<'_>, d: &hir::BreakOutsideOfLoop, ) -> Diagnostic { diff --git a/crates/ide_diagnostics/src/inactive_code.rs b/crates/ide_diagnostics/src/handlers/inactive_code.rs similarity index 99% rename from crates/ide_diagnostics/src/inactive_code.rs rename to crates/ide_diagnostics/src/handlers/inactive_code.rs index 34837cc0d0..4b722fd64b 100644 --- a/crates/ide_diagnostics/src/inactive_code.rs +++ b/crates/ide_diagnostics/src/handlers/inactive_code.rs @@ -6,7 +6,7 @@ use crate::{Diagnostic, DiagnosticsContext, Severity}; // Diagnostic: inactive-code // // This diagnostic is shown for code with inactive `#[cfg]` attributes. -pub(super) fn inactive_code( +pub(crate) fn inactive_code( ctx: &DiagnosticsContext<'_>, d: &hir::InactiveCode, ) -> Option { diff --git a/crates/ide_diagnostics/src/incorrect_case.rs b/crates/ide_diagnostics/src/handlers/incorrect_case.rs similarity index 99% rename from crates/ide_diagnostics/src/incorrect_case.rs rename to crates/ide_diagnostics/src/handlers/incorrect_case.rs index 2cf232d564..3a33029cf7 100644 --- a/crates/ide_diagnostics/src/incorrect_case.rs +++ b/crates/ide_diagnostics/src/handlers/incorrect_case.rs @@ -13,7 +13,7 @@ use crate::{ // Diagnostic: incorrect-ident-case // // This diagnostic is triggered if an item name doesn't follow https://doc.rust-lang.org/1.0.0/style/style/naming/README.html[Rust naming convention]. -pub(super) fn incorrect_case(ctx: &DiagnosticsContext<'_>, d: &hir::IncorrectCase) -> Diagnostic { +pub(crate) fn incorrect_case(ctx: &DiagnosticsContext<'_>, d: &hir::IncorrectCase) -> Diagnostic { Diagnostic::new( "incorrect-ident-case", format!( diff --git a/crates/ide_diagnostics/src/macro_error.rs b/crates/ide_diagnostics/src/handlers/macro_error.rs similarity index 98% rename from crates/ide_diagnostics/src/macro_error.rs rename to crates/ide_diagnostics/src/handlers/macro_error.rs index 180f297ebf..d4d928ad10 100644 --- a/crates/ide_diagnostics/src/macro_error.rs +++ b/crates/ide_diagnostics/src/handlers/macro_error.rs @@ -3,7 +3,7 @@ use crate::{Diagnostic, DiagnosticsContext}; // Diagnostic: macro-error // // This diagnostic is shown for macro expansion errors. -pub(super) fn macro_error(ctx: &DiagnosticsContext<'_>, d: &hir::MacroError) -> Diagnostic { +pub(crate) fn macro_error(ctx: &DiagnosticsContext<'_>, d: &hir::MacroError) -> Diagnostic { Diagnostic::new( "macro-error", d.message.clone(), diff --git a/crates/ide_diagnostics/src/mismatched_arg_count.rs b/crates/ide_diagnostics/src/handlers/mismatched_arg_count.rs similarity index 99% rename from crates/ide_diagnostics/src/mismatched_arg_count.rs rename to crates/ide_diagnostics/src/handlers/mismatched_arg_count.rs index c5749c8a6b..ce313b2cce 100644 --- a/crates/ide_diagnostics/src/mismatched_arg_count.rs +++ b/crates/ide_diagnostics/src/handlers/mismatched_arg_count.rs @@ -3,7 +3,7 @@ use crate::{Diagnostic, DiagnosticsContext}; // Diagnostic: mismatched-arg-count // // This diagnostic is triggered if a function is invoked with an incorrect amount of arguments. -pub(super) fn mismatched_arg_count( +pub(crate) fn mismatched_arg_count( ctx: &DiagnosticsContext<'_>, d: &hir::MismatchedArgCount, ) -> Diagnostic { diff --git a/crates/ide_diagnostics/src/missing_fields.rs b/crates/ide_diagnostics/src/handlers/missing_fields.rs similarity index 99% rename from crates/ide_diagnostics/src/missing_fields.rs rename to crates/ide_diagnostics/src/handlers/missing_fields.rs index 5af67f461c..bc82c0e4a0 100644 --- a/crates/ide_diagnostics/src/missing_fields.rs +++ b/crates/ide_diagnostics/src/handlers/missing_fields.rs @@ -18,7 +18,7 @@ use crate::{fix, Diagnostic, DiagnosticsContext}; // // let a = A { a: 10 }; // ``` -pub(super) fn missing_fields(ctx: &DiagnosticsContext<'_>, d: &hir::MissingFields) -> Diagnostic { +pub(crate) fn missing_fields(ctx: &DiagnosticsContext<'_>, d: &hir::MissingFields) -> Diagnostic { let mut message = String::from("Missing structure fields:\n"); for field in &d.missed_fields { format_to!(message, "- {}\n", field); diff --git a/crates/ide_diagnostics/src/missing_match_arms.rs b/crates/ide_diagnostics/src/handlers/missing_match_arms.rs similarity index 99% rename from crates/ide_diagnostics/src/missing_match_arms.rs rename to crates/ide_diagnostics/src/handlers/missing_match_arms.rs index c83155d2fe..9ea533d74f 100644 --- a/crates/ide_diagnostics/src/missing_match_arms.rs +++ b/crates/ide_diagnostics/src/handlers/missing_match_arms.rs @@ -5,7 +5,7 @@ use crate::{Diagnostic, DiagnosticsContext}; // Diagnostic: missing-match-arm // // This diagnostic is triggered if `match` block is missing one or more match arms. -pub(super) fn missing_match_arms( +pub(crate) fn missing_match_arms( ctx: &DiagnosticsContext<'_>, d: &hir::MissingMatchArms, ) -> Diagnostic { @@ -17,7 +17,7 @@ pub(super) fn missing_match_arms( } #[cfg(test)] -pub(super) mod tests { +mod tests { use crate::tests::check_diagnostics; fn check_diagnostics_no_bails(ra_fixture: &str) { diff --git a/crates/ide_diagnostics/src/missing_ok_or_some_in_tail_expr.rs b/crates/ide_diagnostics/src/handlers/missing_ok_or_some_in_tail_expr.rs similarity index 99% rename from crates/ide_diagnostics/src/missing_ok_or_some_in_tail_expr.rs rename to crates/ide_diagnostics/src/handlers/missing_ok_or_some_in_tail_expr.rs index 01c79b6f50..63de545707 100644 --- a/crates/ide_diagnostics/src/missing_ok_or_some_in_tail_expr.rs +++ b/crates/ide_diagnostics/src/handlers/missing_ok_or_some_in_tail_expr.rs @@ -17,7 +17,7 @@ use crate::{fix, Diagnostic, DiagnosticsContext}; // 10 // } // ``` -pub(super) fn missing_ok_or_some_in_tail_expr( +pub(crate) fn missing_ok_or_some_in_tail_expr( ctx: &DiagnosticsContext<'_>, d: &hir::MissingOkOrSomeInTailExpr, ) -> Diagnostic { diff --git a/crates/ide_diagnostics/src/missing_unsafe.rs b/crates/ide_diagnostics/src/handlers/missing_unsafe.rs similarity index 97% rename from crates/ide_diagnostics/src/missing_unsafe.rs rename to crates/ide_diagnostics/src/handlers/missing_unsafe.rs index f5f38a0d35..62d8687ba7 100644 --- a/crates/ide_diagnostics/src/missing_unsafe.rs +++ b/crates/ide_diagnostics/src/handlers/missing_unsafe.rs @@ -3,7 +3,7 @@ use crate::{Diagnostic, DiagnosticsContext}; // Diagnostic: missing-unsafe // // This diagnostic is triggered if an operation marked as `unsafe` is used outside of an `unsafe` function or block. -pub(super) fn missing_unsafe(ctx: &DiagnosticsContext<'_>, d: &hir::MissingUnsafe) -> Diagnostic { +pub(crate) fn missing_unsafe(ctx: &DiagnosticsContext<'_>, d: &hir::MissingUnsafe) -> Diagnostic { Diagnostic::new( "missing-unsafe", "this operation is unsafe and requires an unsafe function or block", diff --git a/crates/ide_diagnostics/src/no_such_field.rs b/crates/ide_diagnostics/src/handlers/no_such_field.rs similarity index 99% rename from crates/ide_diagnostics/src/no_such_field.rs rename to crates/ide_diagnostics/src/handlers/no_such_field.rs index c4fa387ca2..e4cc8a840d 100644 --- a/crates/ide_diagnostics/src/no_such_field.rs +++ b/crates/ide_diagnostics/src/handlers/no_such_field.rs @@ -11,7 +11,7 @@ use crate::{fix, Assist, Diagnostic, DiagnosticsContext}; // Diagnostic: no-such-field // // This diagnostic is triggered if created structure does not have field provided in record. -pub(super) fn no_such_field(ctx: &DiagnosticsContext<'_>, d: &hir::NoSuchField) -> Diagnostic { +pub(crate) fn no_such_field(ctx: &DiagnosticsContext<'_>, d: &hir::NoSuchField) -> Diagnostic { Diagnostic::new( "no-such-field", "no such field", diff --git a/crates/ide_diagnostics/src/remove_this_semicolon.rs b/crates/ide_diagnostics/src/handlers/remove_this_semicolon.rs similarity index 97% rename from crates/ide_diagnostics/src/remove_this_semicolon.rs rename to crates/ide_diagnostics/src/handlers/remove_this_semicolon.rs index dc6c9c0834..b52e4dc84f 100644 --- a/crates/ide_diagnostics/src/remove_this_semicolon.rs +++ b/crates/ide_diagnostics/src/handlers/remove_this_semicolon.rs @@ -8,7 +8,7 @@ use crate::{fix, Assist, Diagnostic, DiagnosticsContext}; // Diagnostic: remove-this-semicolon // // This diagnostic is triggered when there's an erroneous `;` at the end of the block. -pub(super) fn remove_this_semicolon( +pub(crate) fn remove_this_semicolon( ctx: &DiagnosticsContext<'_>, d: &hir::RemoveThisSemicolon, ) -> Diagnostic { diff --git a/crates/ide_diagnostics/src/replace_filter_map_next_with_find_map.rs b/crates/ide_diagnostics/src/handlers/replace_filter_map_next_with_find_map.rs similarity index 98% rename from crates/ide_diagnostics/src/replace_filter_map_next_with_find_map.rs rename to crates/ide_diagnostics/src/handlers/replace_filter_map_next_with_find_map.rs index 775c350d2a..10d5da15db 100644 --- a/crates/ide_diagnostics/src/replace_filter_map_next_with_find_map.rs +++ b/crates/ide_diagnostics/src/handlers/replace_filter_map_next_with_find_map.rs @@ -11,7 +11,7 @@ use crate::{fix, Assist, Diagnostic, DiagnosticsContext, Severity}; // Diagnostic: replace-filter-map-next-with-find-map // // This diagnostic is triggered when `.filter_map(..).next()` is used, rather than the more concise `.find_map(..)`. -pub(super) fn replace_filter_map_next_with_find_map( +pub(crate) fn replace_filter_map_next_with_find_map( ctx: &DiagnosticsContext<'_>, d: &hir::ReplaceFilterMapNextWithFindMap, ) -> Diagnostic { diff --git a/crates/ide_diagnostics/src/unimplemented_builtin_macro.rs b/crates/ide_diagnostics/src/handlers/unimplemented_builtin_macro.rs similarity index 92% rename from crates/ide_diagnostics/src/unimplemented_builtin_macro.rs rename to crates/ide_diagnostics/src/handlers/unimplemented_builtin_macro.rs index a600544f0e..e879de75cd 100644 --- a/crates/ide_diagnostics/src/unimplemented_builtin_macro.rs +++ b/crates/ide_diagnostics/src/handlers/unimplemented_builtin_macro.rs @@ -3,7 +3,7 @@ use crate::{Diagnostic, DiagnosticsContext, Severity}; // Diagnostic: unimplemented-builtin-macro // // This diagnostic is shown for builtin macros which are not yet implemented by rust-analyzer -pub(super) fn unimplemented_builtin_macro( +pub(crate) fn unimplemented_builtin_macro( ctx: &DiagnosticsContext<'_>, d: &hir::UnimplementedBuiltinMacro, ) -> Diagnostic { diff --git a/crates/ide_diagnostics/src/unlinked_file.rs b/crates/ide_diagnostics/src/handlers/unlinked_file.rs similarity index 99% rename from crates/ide_diagnostics/src/unlinked_file.rs rename to crates/ide_diagnostics/src/handlers/unlinked_file.rs index 424532e3a4..8921ddde25 100644 --- a/crates/ide_diagnostics/src/unlinked_file.rs +++ b/crates/ide_diagnostics/src/handlers/unlinked_file.rs @@ -23,7 +23,7 @@ pub(crate) struct UnlinkedFile { // // This diagnostic is shown for files that are not included in any crate, or files that are part of // crates rust-analyzer failed to discover. The file will not have IDE features available. -pub(super) fn unlinked_file(ctx: &DiagnosticsContext, d: &UnlinkedFile) -> Diagnostic { +pub(crate) fn unlinked_file(ctx: &DiagnosticsContext, d: &UnlinkedFile) -> Diagnostic { // Limit diagnostic to the first few characters in the file. This matches how VS Code // renders it with the full span, but on other editors, and is less invasive. let range = ctx.sema.db.parse(d.file).syntax_node().text_range(); diff --git a/crates/ide_diagnostics/src/unresolved_extern_crate.rs b/crates/ide_diagnostics/src/handlers/unresolved_extern_crate.rs similarity index 96% rename from crates/ide_diagnostics/src/unresolved_extern_crate.rs rename to crates/ide_diagnostics/src/handlers/unresolved_extern_crate.rs index 69f07d0b0a..f5313cc0c6 100644 --- a/crates/ide_diagnostics/src/unresolved_extern_crate.rs +++ b/crates/ide_diagnostics/src/handlers/unresolved_extern_crate.rs @@ -3,7 +3,7 @@ use crate::{Diagnostic, DiagnosticsContext}; // Diagnostic: unresolved-extern-crate // // This diagnostic is triggered if rust-analyzer is unable to discover referred extern crate. -pub(super) fn unresolved_extern_crate( +pub(crate) fn unresolved_extern_crate( ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedExternCrate, ) -> Diagnostic { diff --git a/crates/ide_diagnostics/src/unresolved_import.rs b/crates/ide_diagnostics/src/handlers/unresolved_import.rs similarity index 98% rename from crates/ide_diagnostics/src/unresolved_import.rs rename to crates/ide_diagnostics/src/handlers/unresolved_import.rs index 7779033d43..f30051c126 100644 --- a/crates/ide_diagnostics/src/unresolved_import.rs +++ b/crates/ide_diagnostics/src/handlers/unresolved_import.rs @@ -4,7 +4,7 @@ use crate::{Diagnostic, DiagnosticsContext}; // // This diagnostic is triggered if rust-analyzer is unable to resolve a path in // a `use` declaration. -pub(super) fn unresolved_import( +pub(crate) fn unresolved_import( ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedImport, ) -> Diagnostic { diff --git a/crates/ide_diagnostics/src/unresolved_macro_call.rs b/crates/ide_diagnostics/src/handlers/unresolved_macro_call.rs similarity index 98% rename from crates/ide_diagnostics/src/unresolved_macro_call.rs rename to crates/ide_diagnostics/src/handlers/unresolved_macro_call.rs index 88133d0f3b..4c3c1c19af 100644 --- a/crates/ide_diagnostics/src/unresolved_macro_call.rs +++ b/crates/ide_diagnostics/src/handlers/unresolved_macro_call.rs @@ -7,7 +7,7 @@ use crate::{Diagnostic, DiagnosticsContext}; // // This diagnostic is triggered if rust-analyzer is unable to resolve the path // to a macro in a macro invocation. -pub(super) fn unresolved_macro_call( +pub(crate) fn unresolved_macro_call( ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedMacroCall, ) -> Diagnostic { diff --git a/crates/ide_diagnostics/src/unresolved_module.rs b/crates/ide_diagnostics/src/handlers/unresolved_module.rs similarity index 99% rename from crates/ide_diagnostics/src/unresolved_module.rs rename to crates/ide_diagnostics/src/handlers/unresolved_module.rs index 5aa9dae179..17166a0c6c 100644 --- a/crates/ide_diagnostics/src/unresolved_module.rs +++ b/crates/ide_diagnostics/src/handlers/unresolved_module.rs @@ -7,7 +7,7 @@ use crate::{fix, Diagnostic, DiagnosticsContext}; // Diagnostic: unresolved-module // // This diagnostic is triggered if rust-analyzer is unable to discover referred module. -pub(super) fn unresolved_module( +pub(crate) fn unresolved_module( ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedModule, ) -> Diagnostic { diff --git a/crates/ide_diagnostics/src/unresolved_proc_macro.rs b/crates/ide_diagnostics/src/handlers/unresolved_proc_macro.rs similarity index 97% rename from crates/ide_diagnostics/src/unresolved_proc_macro.rs rename to crates/ide_diagnostics/src/handlers/unresolved_proc_macro.rs index 744cce5082..fde1d1323f 100644 --- a/crates/ide_diagnostics/src/unresolved_proc_macro.rs +++ b/crates/ide_diagnostics/src/handlers/unresolved_proc_macro.rs @@ -9,7 +9,7 @@ use crate::{Diagnostic, DiagnosticsContext, Severity}; // If you are seeing a lot of "proc macro not expanded" warnings, you can add this option to the // `rust-analyzer.diagnostics.disabled` list to prevent them from showing. Alternatively you can // enable support for procedural macros (see `rust-analyzer.procMacro.enable`). -pub(super) fn unresolved_proc_macro( +pub(crate) fn unresolved_proc_macro( ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedProcMacro, ) -> Diagnostic { diff --git a/crates/ide_diagnostics/src/lib.rs b/crates/ide_diagnostics/src/lib.rs index 2a16c73a8b..88037be5a9 100644 --- a/crates/ide_diagnostics/src/lib.rs +++ b/crates/ide_diagnostics/src/lib.rs @@ -1,28 +1,49 @@ -//! Collects diagnostics & fixits for a single file. +//! Diagnostics rendering and fixits. //! -//! The tricky bit here is that diagnostics are produced by hir in terms of -//! macro-expanded files, but we need to present them to the users in terms of -//! original files. So we need to map the ranges. +//! Most of the diagnostics originate from the dark depth of the compiler, and +//! are originally expressed in term of IR. When we emit the diagnostic, we are +//! usually not in the position to decide how to best "render" it in terms of +//! user-authored source code. We are especially not in the position to offer +//! fixits, as the compiler completely lacks the infrastructure to edit the +//! source code. +//! +//! Instead, we "bubble up" raw, structured diagnostics until the `hir` crate, +//! where we "cook" them so that each diagnostic is formulated in terms of `hir` +//! types. Well, at least that's the aspiration, the "cooking" is somewhat +//! ad-hoc at the moment. Anyways, we get a bunch of ide-friendly diagnostic +//! structs from hir, and we want to render them to unified serializable +//! representation (span, level, message) here. If we can, we also provide +//! fixits. By the way, that's why we want to keep diagnostics structured +//! internally -- so that we have all the info to make fixes. +//! +//! We have one "handler" module per diagnostic code. Such a module contains +//! rendering, optional fixes and tests. It's OK if some low-level compiler +//! functionality ends up being tested via a diagnostic. +//! +//! There are also a couple of ad-hoc diagnostics implemented directly here, we +//! don't yet have a great pattern for how to do them properly. -mod break_outside_of_loop; -mod inactive_code; -mod incorrect_case; -mod macro_error; -mod mismatched_arg_count; -mod missing_fields; -mod missing_match_arms; -mod missing_ok_or_some_in_tail_expr; -mod missing_unsafe; -mod no_such_field; -mod remove_this_semicolon; -mod replace_filter_map_next_with_find_map; -mod unimplemented_builtin_macro; -mod unlinked_file; -mod unresolved_extern_crate; -mod unresolved_import; -mod unresolved_macro_call; -mod unresolved_module; -mod unresolved_proc_macro; +mod handlers { + pub(crate) mod break_outside_of_loop; + pub(crate) mod inactive_code; + pub(crate) mod incorrect_case; + pub(crate) mod macro_error; + pub(crate) mod mismatched_arg_count; + pub(crate) mod missing_fields; + pub(crate) mod missing_match_arms; + pub(crate) mod missing_ok_or_some_in_tail_expr; + pub(crate) mod missing_unsafe; + pub(crate) mod no_such_field; + pub(crate) mod remove_this_semicolon; + pub(crate) mod replace_filter_map_next_with_find_map; + pub(crate) mod unimplemented_builtin_macro; + pub(crate) mod unlinked_file; + pub(crate) mod unresolved_extern_crate; + pub(crate) mod unresolved_import; + pub(crate) mod unresolved_macro_call; + pub(crate) mod unresolved_module; + pub(crate) mod unresolved_proc_macro; +} mod field_shorthand; @@ -41,7 +62,8 @@ use syntax::{ SyntaxNode, TextRange, }; use text_edit::TextEdit; -use unlinked_file::UnlinkedFile; + +use crate::handlers::unlinked_file::UnlinkedFile; #[derive(Copy, Clone, Debug, PartialEq)] pub struct DiagnosticCode(pub &'static str); @@ -148,32 +170,32 @@ pub fn diagnostics( let ctx = DiagnosticsContext { config, sema, resolve }; if module.is_none() { let d = UnlinkedFile { file: file_id }; - let d = unlinked_file::unlinked_file(&ctx, &d); + let d = handlers::unlinked_file::unlinked_file(&ctx, &d); res.push(d) } for diag in diags { #[rustfmt::skip] let d = match diag { - AnyDiagnostic::BreakOutsideOfLoop(d) => break_outside_of_loop::break_outside_of_loop(&ctx, &d), - AnyDiagnostic::IncorrectCase(d) => incorrect_case::incorrect_case(&ctx, &d), - AnyDiagnostic::MacroError(d) => macro_error::macro_error(&ctx, &d), - AnyDiagnostic::MismatchedArgCount(d) => mismatched_arg_count::mismatched_arg_count(&ctx, &d), - AnyDiagnostic::MissingFields(d) => missing_fields::missing_fields(&ctx, &d), - AnyDiagnostic::MissingMatchArms(d) => missing_match_arms::missing_match_arms(&ctx, &d), - AnyDiagnostic::MissingOkOrSomeInTailExpr(d) => missing_ok_or_some_in_tail_expr::missing_ok_or_some_in_tail_expr(&ctx, &d), - AnyDiagnostic::MissingUnsafe(d) => missing_unsafe::missing_unsafe(&ctx, &d), - AnyDiagnostic::NoSuchField(d) => no_such_field::no_such_field(&ctx, &d), - AnyDiagnostic::RemoveThisSemicolon(d) => remove_this_semicolon::remove_this_semicolon(&ctx, &d), - AnyDiagnostic::ReplaceFilterMapNextWithFindMap(d) => replace_filter_map_next_with_find_map::replace_filter_map_next_with_find_map(&ctx, &d), - AnyDiagnostic::UnimplementedBuiltinMacro(d) => unimplemented_builtin_macro::unimplemented_builtin_macro(&ctx, &d), - AnyDiagnostic::UnresolvedExternCrate(d) => unresolved_extern_crate::unresolved_extern_crate(&ctx, &d), - AnyDiagnostic::UnresolvedImport(d) => unresolved_import::unresolved_import(&ctx, &d), - AnyDiagnostic::UnresolvedMacroCall(d) => unresolved_macro_call::unresolved_macro_call(&ctx, &d), - AnyDiagnostic::UnresolvedModule(d) => unresolved_module::unresolved_module(&ctx, &d), - AnyDiagnostic::UnresolvedProcMacro(d) => unresolved_proc_macro::unresolved_proc_macro(&ctx, &d), + AnyDiagnostic::BreakOutsideOfLoop(d) => handlers::break_outside_of_loop::break_outside_of_loop(&ctx, &d), + AnyDiagnostic::IncorrectCase(d) => handlers::incorrect_case::incorrect_case(&ctx, &d), + AnyDiagnostic::MacroError(d) => handlers::macro_error::macro_error(&ctx, &d), + AnyDiagnostic::MismatchedArgCount(d) => handlers::mismatched_arg_count::mismatched_arg_count(&ctx, &d), + AnyDiagnostic::MissingFields(d) => handlers::missing_fields::missing_fields(&ctx, &d), + AnyDiagnostic::MissingMatchArms(d) => handlers::missing_match_arms::missing_match_arms(&ctx, &d), + AnyDiagnostic::MissingOkOrSomeInTailExpr(d) => handlers::missing_ok_or_some_in_tail_expr::missing_ok_or_some_in_tail_expr(&ctx, &d), + AnyDiagnostic::MissingUnsafe(d) => handlers::missing_unsafe::missing_unsafe(&ctx, &d), + AnyDiagnostic::NoSuchField(d) => handlers::no_such_field::no_such_field(&ctx, &d), + AnyDiagnostic::RemoveThisSemicolon(d) => handlers::remove_this_semicolon::remove_this_semicolon(&ctx, &d), + AnyDiagnostic::ReplaceFilterMapNextWithFindMap(d) => handlers::replace_filter_map_next_with_find_map::replace_filter_map_next_with_find_map(&ctx, &d), + AnyDiagnostic::UnimplementedBuiltinMacro(d) => handlers::unimplemented_builtin_macro::unimplemented_builtin_macro(&ctx, &d), + AnyDiagnostic::UnresolvedExternCrate(d) => handlers::unresolved_extern_crate::unresolved_extern_crate(&ctx, &d), + AnyDiagnostic::UnresolvedImport(d) => handlers::unresolved_import::unresolved_import(&ctx, &d), + AnyDiagnostic::UnresolvedMacroCall(d) => handlers::unresolved_macro_call::unresolved_macro_call(&ctx, &d), + AnyDiagnostic::UnresolvedModule(d) => handlers::unresolved_module::unresolved_module(&ctx, &d), + AnyDiagnostic::UnresolvedProcMacro(d) => handlers::unresolved_proc_macro::unresolved_proc_macro(&ctx, &d), - AnyDiagnostic::InactiveCode(d) => match inactive_code::inactive_code(&ctx, &d) { + AnyDiagnostic::InactiveCode(d) => match handlers::inactive_code::inactive_code(&ctx, &d) { Some(it) => it, None => continue, }