From e5cdcb8b124f5b7d59950429787e760e46388f72 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 3 May 2021 17:08:09 +0300 Subject: Add a way to resolve certain assists --- crates/ide/src/diagnostics.rs | 43 +++++++++++++++------- crates/ide/src/diagnostics/fixes.rs | 51 +++++++++++++++++++++----- crates/ide/src/diagnostics/unlinked_file.rs | 7 +++- crates/ide/src/lib.rs | 35 ++++++++++-------- crates/ide/src/ssr.rs | 55 +++++++++++++---------------- 5 files changed, 123 insertions(+), 68 deletions(-) (limited to 'crates/ide/src') diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index 1c911a8b2..455f20c93 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -15,6 +15,7 @@ use hir::{ diagnostics::{Diagnostic as _, DiagnosticCode, DiagnosticSinkBuilder}, InFile, Semantics, }; +use ide_assists::AssistResolveStrategy; use ide_db::{base_db::SourceDatabase, RootDatabase}; use itertools::Itertools; use rustc_hash::FxHashSet; @@ -84,7 +85,7 @@ pub struct DiagnosticsConfig { pub(crate) fn diagnostics( db: &RootDatabase, config: &DiagnosticsConfig, - resolve: bool, + resolve: AssistResolveStrategy, file_id: FileId, ) -> Vec { let _p = profile::span("diagnostics"); @@ -212,7 +213,7 @@ pub(crate) fn diagnostics( fn diagnostic_with_fix( d: &D, sema: &Semantics, - resolve: bool, + resolve: AssistResolveStrategy, ) -> Diagnostic { Diagnostic::error(sema.diagnostics_display_range(d.display_source()).range, d.message()) .with_fix(d.fix(&sema, resolve)) @@ -222,7 +223,7 @@ fn diagnostic_with_fix( fn warning_with_fix( d: &D, sema: &Semantics, - resolve: bool, + resolve: AssistResolveStrategy, ) -> Diagnostic { Diagnostic::hint(sema.diagnostics_display_range(d.display_source()).range, d.message()) .with_fix(d.fix(&sema, resolve)) @@ -299,6 +300,7 @@ fn unresolved_fix(id: &'static str, label: &str, target: TextRange) -> Assist { #[cfg(test)] mod tests { use expect_test::{expect, Expect}; + use ide_assists::AssistResolveStrategy; use stdx::trim_indent; use test_utils::assert_eq_text; @@ -314,7 +316,11 @@ mod tests { let (analysis, file_position) = fixture::position(ra_fixture_before); let diagnostic = analysis - .diagnostics(&DiagnosticsConfig::default(), true, file_position.file_id) + .diagnostics( + &DiagnosticsConfig::default(), + AssistResolveStrategy::All, + file_position.file_id, + ) .unwrap() .pop() .unwrap(); @@ -343,7 +349,11 @@ mod tests { fn check_no_fix(ra_fixture: &str) { let (analysis, file_position) = fixture::position(ra_fixture); let diagnostic = analysis - .diagnostics(&DiagnosticsConfig::default(), true, file_position.file_id) + .diagnostics( + &DiagnosticsConfig::default(), + AssistResolveStrategy::All, + file_position.file_id, + ) .unwrap() .pop() .unwrap(); @@ -357,7 +367,9 @@ mod tests { let diagnostics = files .into_iter() .flat_map(|file_id| { - analysis.diagnostics(&DiagnosticsConfig::default(), true, file_id).unwrap() + analysis + .diagnostics(&DiagnosticsConfig::default(), AssistResolveStrategy::All, file_id) + .unwrap() }) .collect::>(); assert_eq!(diagnostics.len(), 0, "unexpected diagnostics:\n{:#?}", diagnostics); @@ -365,8 +377,9 @@ mod tests { fn check_expect(ra_fixture: &str, expect: Expect) { let (analysis, file_id) = fixture::file(ra_fixture); - let diagnostics = - analysis.diagnostics(&DiagnosticsConfig::default(), true, file_id).unwrap(); + let diagnostics = analysis + .diagnostics(&DiagnosticsConfig::default(), AssistResolveStrategy::All, file_id) + .unwrap(); expect.assert_debug_eq(&diagnostics) } @@ -911,11 +924,13 @@ struct Foo { let (analysis, file_id) = fixture::file(r#"mod foo;"#); - let diagnostics = analysis.diagnostics(&config, true, file_id).unwrap(); + let diagnostics = + analysis.diagnostics(&config, AssistResolveStrategy::All, file_id).unwrap(); assert!(diagnostics.is_empty()); - let diagnostics = - analysis.diagnostics(&DiagnosticsConfig::default(), true, file_id).unwrap(); + let diagnostics = analysis + .diagnostics(&DiagnosticsConfig::default(), AssistResolveStrategy::All, file_id) + .unwrap(); assert!(!diagnostics.is_empty()); } @@ -1022,7 +1037,11 @@ impl TestStruct { let (analysis, file_position) = fixture::position(input); let diagnostics = analysis - .diagnostics(&DiagnosticsConfig::default(), true, file_position.file_id) + .diagnostics( + &DiagnosticsConfig::default(), + AssistResolveStrategy::All, + file_position.file_id, + ) .unwrap(); assert_eq!(diagnostics.len(), 1); diff --git a/crates/ide/src/diagnostics/fixes.rs b/crates/ide/src/diagnostics/fixes.rs index 7be8b3459..f23064eac 100644 --- a/crates/ide/src/diagnostics/fixes.rs +++ b/crates/ide/src/diagnostics/fixes.rs @@ -8,6 +8,7 @@ use hir::{ }, HasSource, HirDisplay, InFile, Semantics, VariantDef, }; +use ide_assists::AssistResolveStrategy; use ide_db::{ base_db::{AnchoredPathBuf, FileId}, source_change::{FileSystemEdit, SourceChange}, @@ -35,11 +36,19 @@ pub(crate) trait DiagnosticWithFix: Diagnostic { /// /// If `resolve` is false, the edit will be computed later, on demand, and /// can be omitted. - fn fix(&self, sema: &Semantics, _resolve: bool) -> Option; + fn fix( + &self, + sema: &Semantics, + _resolve: AssistResolveStrategy, + ) -> Option; } impl DiagnosticWithFix for UnresolvedModule { - fn fix(&self, sema: &Semantics, _resolve: bool) -> Option { + fn fix( + &self, + sema: &Semantics, + _resolve: AssistResolveStrategy, + ) -> Option { let root = sema.db.parse_or_expand(self.file)?; let unresolved_module = self.decl.to_node(&root); Some(fix( @@ -59,7 +68,11 @@ impl DiagnosticWithFix for UnresolvedModule { } impl DiagnosticWithFix for NoSuchField { - fn fix(&self, sema: &Semantics, _resolve: bool) -> Option { + fn fix( + &self, + sema: &Semantics, + _resolve: AssistResolveStrategy, + ) -> Option { let root = sema.db.parse_or_expand(self.file)?; missing_record_expr_field_fix( &sema, @@ -70,7 +83,11 @@ impl DiagnosticWithFix for NoSuchField { } impl DiagnosticWithFix for MissingFields { - fn fix(&self, sema: &Semantics, _resolve: bool) -> Option { + fn fix( + &self, + sema: &Semantics, + _resolve: AssistResolveStrategy, + ) -> Option { // Note that although we could add a diagnostics to // fill the missing tuple field, e.g : // `struct A(usize);` @@ -106,7 +123,11 @@ impl DiagnosticWithFix for MissingFields { } impl DiagnosticWithFix for MissingOkOrSomeInTailExpr { - fn fix(&self, sema: &Semantics, _resolve: bool) -> Option { + fn fix( + &self, + sema: &Semantics, + _resolve: AssistResolveStrategy, + ) -> Option { let root = sema.db.parse_or_expand(self.file)?; let tail_expr = self.expr.to_node(&root); let tail_expr_range = tail_expr.syntax().text_range(); @@ -119,7 +140,11 @@ impl DiagnosticWithFix for MissingOkOrSomeInTailExpr { } impl DiagnosticWithFix for RemoveThisSemicolon { - fn fix(&self, sema: &Semantics, _resolve: bool) -> Option { + fn fix( + &self, + sema: &Semantics, + _resolve: AssistResolveStrategy, + ) -> Option { let root = sema.db.parse_or_expand(self.file)?; let semicolon = self @@ -139,7 +164,11 @@ impl DiagnosticWithFix for RemoveThisSemicolon { } impl DiagnosticWithFix for IncorrectCase { - fn fix(&self, sema: &Semantics, resolve: bool) -> Option { + fn fix( + &self, + sema: &Semantics, + resolve: AssistResolveStrategy, + ) -> Option { let root = sema.db.parse_or_expand(self.file)?; let name_node = self.ident.to_node(&root); @@ -149,7 +178,7 @@ impl DiagnosticWithFix for IncorrectCase { let label = format!("Rename to {}", self.suggested_text); let mut res = unresolved_fix("change_case", &label, frange.range); - if resolve { + if resolve.should_resolve(&res.id) { let source_change = rename_with_semantics(sema, file_position, &self.suggested_text); res.source_change = Some(source_change.ok().unwrap_or_default()); } @@ -159,7 +188,11 @@ impl DiagnosticWithFix for IncorrectCase { } impl DiagnosticWithFix for ReplaceFilterMapNextWithFindMap { - fn fix(&self, sema: &Semantics, _resolve: bool) -> Option { + fn fix( + &self, + sema: &Semantics, + _resolve: AssistResolveStrategy, + ) -> Option { let root = sema.db.parse_or_expand(self.file)?; let next_expr = self.next_expr.to_node(&root); let next_call = ast::MethodCallExpr::cast(next_expr.syntax().clone())?; diff --git a/crates/ide/src/diagnostics/unlinked_file.rs b/crates/ide/src/diagnostics/unlinked_file.rs index 7d39f4fbe..e48528bed 100644 --- a/crates/ide/src/diagnostics/unlinked_file.rs +++ b/crates/ide/src/diagnostics/unlinked_file.rs @@ -5,6 +5,7 @@ use hir::{ diagnostics::{Diagnostic, DiagnosticCode}, InFile, }; +use ide_assists::AssistResolveStrategy; use ide_db::{ base_db::{FileId, FileLoader, SourceDatabase, SourceDatabaseExt}, source_change::SourceChange, @@ -50,7 +51,11 @@ impl Diagnostic for UnlinkedFile { } impl DiagnosticWithFix for UnlinkedFile { - fn fix(&self, sema: &hir::Semantics, _resolve: bool) -> Option { + fn fix( + &self, + sema: &hir::Semantics, + _resolve: AssistResolveStrategy, + ) -> Option { // If there's an existing module that could add a `mod` item to include the unlinked file, // suggest that as a fix. diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index 99e45633e..6847b7e83 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -87,7 +87,7 @@ pub use crate::{ }, }; pub use hir::{Documentation, Semantics}; -pub use ide_assists::{Assist, AssistConfig, AssistId, AssistKind}; +pub use ide_assists::{Assist, AssistConfig, AssistId, AssistKind, AssistResolveStrategy}; pub use ide_completion::{ CompletionConfig, CompletionItem, CompletionItemKind, CompletionRelevance, ImportEdit, InsertTextFormat, @@ -518,12 +518,13 @@ impl Analysis { pub fn assists( &self, config: &AssistConfig, - resolve: bool, + resolve: AssistResolveStrategy, frange: FileRange, ) -> Cancelable> { self.with_db(|db| { + let ssr_assists = ssr::ssr_assists(db, resolve, frange); let mut acc = Assist::get(db, config, resolve, frange); - ssr::add_ssr_assist(db, &mut acc, resolve, frange); + acc.extend(ssr_assists.into_iter()); acc }) } @@ -532,7 +533,7 @@ impl Analysis { pub fn diagnostics( &self, config: &DiagnosticsConfig, - resolve: bool, + resolve: AssistResolveStrategy, file_id: FileId, ) -> Cancelable> { self.with_db(|db| diagnostics::diagnostics(db, config, resolve, file_id)) @@ -543,7 +544,7 @@ impl Analysis { &self, assist_config: &AssistConfig, diagnostics_config: &DiagnosticsConfig, - resolve: bool, + resolve: AssistResolveStrategy, frange: FileRange, ) -> Cancelable> { let include_fixes = match &assist_config.allowed { @@ -552,17 +553,21 @@ 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) + .into_iter() + .filter_map(|it| it.fix) + .filter(|it| it.target.intersect(frange.range).is_some()) + .collect() + } else { + Vec::new() + }; + let mut res = Assist::get(db, assist_config, resolve, frange); - ssr::add_ssr_assist(db, &mut res, resolve, frange); - - if include_fixes { - res.extend( - diagnostics::diagnostics(db, diagnostics_config, resolve, frange.file_id) - .into_iter() - .filter_map(|it| it.fix) - .filter(|it| it.target.intersect(frange.range).is_some()), - ); - } + res.extend(ssr_assists.into_iter()); + res.extend(diagnostic_assists.into_iter()); + res }) } diff --git a/crates/ide/src/ssr.rs b/crates/ide/src/ssr.rs index f3638d928..785ce3010 100644 --- a/crates/ide/src/ssr.rs +++ b/crates/ide/src/ssr.rs @@ -2,18 +2,23 @@ //! assist in ide_assists because that would require the ide_assists crate //! depend on the ide_ssr crate. -use ide_assists::{Assist, AssistId, AssistKind, GroupLabel}; +use ide_assists::{Assist, AssistId, AssistKind, AssistResolveStrategy, GroupLabel}; use ide_db::{base_db::FileRange, label::Label, source_change::SourceChange, RootDatabase}; -pub(crate) fn add_ssr_assist( +pub(crate) fn ssr_assists( db: &RootDatabase, - base: &mut Vec, - resolve: bool, + resolve: AssistResolveStrategy, frange: FileRange, -) -> Option<()> { - let (match_finder, comment_range) = ide_ssr::ssr_from_comment(db, frange)?; +) -> Vec { + let mut ssr_assists = Vec::with_capacity(2); - let (source_change_for_file, source_change_for_workspace) = if resolve { + let (match_finder, comment_range) = match ide_ssr::ssr_from_comment(db, frange) { + Some((match_finder, comment_range)) => (match_finder, comment_range), + None => return ssr_assists, + }; + let id = AssistId("ssr", AssistKind::RefactorRewrite); + + let (source_change_for_file, source_change_for_workspace) = if resolve.should_resolve(&id) { let edits = match_finder.edits(); let source_change_for_file = { @@ -35,16 +40,17 @@ pub(crate) fn add_ssr_assist( for (label, source_change) in assists.into_iter() { let assist = Assist { - id: AssistId("ssr", AssistKind::RefactorRewrite), + id, label: Label::new(label), group: Some(GroupLabel("Apply SSR".into())), target: comment_range, source_change, }; - base.push(assist); + ssr_assists.push(assist); } - Some(()) + + ssr_assists } #[cfg(test)] @@ -52,7 +58,7 @@ mod tests { use std::sync::Arc; use expect_test::expect; - use ide_assists::Assist; + use ide_assists::{Assist, AssistResolveStrategy}; use ide_db::{ base_db::{fixture::WithFixture, salsa::Durability, FileRange}, symbol_index::SymbolsDatabase, @@ -60,24 +66,14 @@ mod tests { }; use rustc_hash::FxHashSet; - use super::add_ssr_assist; + use super::ssr_assists; - fn get_assists(ra_fixture: &str, resolve: bool) -> Vec { + fn get_assists(ra_fixture: &str, resolve: AssistResolveStrategy) -> Vec { let (mut db, file_id, range_or_offset) = RootDatabase::with_range_or_offset(ra_fixture); let mut local_roots = FxHashSet::default(); local_roots.insert(ide_db::base_db::fixture::WORKSPACE); db.set_local_roots_with_durability(Arc::new(local_roots), Durability::HIGH); - - let mut assists = vec![]; - - add_ssr_assist( - &db, - &mut assists, - resolve, - FileRange { file_id, range: range_or_offset.into() }, - ); - - assists + ssr_assists(&db, resolve, FileRange { file_id, range: range_or_offset.into() }) } #[test] @@ -88,16 +84,14 @@ mod tests { // This is foo $0 fn foo() {} "#; - let resolve = true; - - let assists = get_assists(ra_fixture, resolve); + let assists = get_assists(ra_fixture, AssistResolveStrategy::All); assert_eq!(0, assists.len()); } + // TODO kb add partial resolve test #[test] fn resolve_edits_true() { - let resolve = true; let assists = get_assists( r#" //- /lib.rs @@ -109,7 +103,7 @@ mod tests { //- /bar.rs fn bar() { 2 } "#, - resolve, + AssistResolveStrategy::All, ); assert_eq!(2, assists.len()); @@ -200,7 +194,6 @@ mod tests { #[test] fn resolve_edits_false() { - let resolve = false; let assists = get_assists( r#" //- /lib.rs @@ -212,7 +205,7 @@ mod tests { //- /bar.rs fn bar() { 2 } "#, - resolve, + AssistResolveStrategy::None, ); assert_eq!(2, assists.len()); -- cgit v1.2.3