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