From aa598ecb75331068f4cacd443512991bc48937dc Mon Sep 17 00:00:00 2001 From: kjeremy Date: Mon, 13 Jul 2020 17:41:47 -0400 Subject: Filter assists --- crates/ra_assists/src/assist_context.rs | 38 +++++++++++++++++++++--- crates/ra_assists/src/lib.rs | 31 ++++++++++++++++++-- crates/ra_assists/src/tests.rs | 52 +++++++++++++++++++++++++++++---- 3 files changed, 108 insertions(+), 13 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/assist_context.rs b/crates/ra_assists/src/assist_context.rs index c33525363..9ca2cfe68 100644 --- a/crates/ra_assists/src/assist_context.rs +++ b/crates/ra_assists/src/assist_context.rs @@ -19,7 +19,7 @@ use ra_text_edit::TextEditBuilder; use crate::{ assist_config::{AssistConfig, SnippetCap}, - Assist, AssistId, GroupLabel, ResolvedAssist, + Assist, AssistId, AssistKind, GroupLabel, ResolvedAssist, }; /// `AssistContext` allows to apply an assist or check if it could be applied. @@ -57,6 +57,7 @@ pub(crate) struct AssistContext<'a> { pub(crate) sema: Semantics<'a, RootDatabase>, pub(crate) frange: FileRange, source_file: SourceFile, + allowed: Option>, } impl<'a> AssistContext<'a> { @@ -64,9 +65,10 @@ impl<'a> AssistContext<'a> { sema: Semantics<'a, RootDatabase>, config: &'a AssistConfig, frange: FileRange, + allowed: Option>, ) -> AssistContext<'a> { let source_file = sema.parse(frange.file_id); - AssistContext { config, sema, frange, source_file } + AssistContext { config, sema, frange, source_file, allowed } } pub(crate) fn db(&self) -> &RootDatabase { @@ -103,14 +105,26 @@ pub(crate) struct Assists { resolve: bool, file: FileId, buf: Vec<(Assist, Option)>, + allowed: Option>, } impl Assists { pub(crate) fn new_resolved(ctx: &AssistContext) -> Assists { - Assists { resolve: true, file: ctx.frange.file_id, buf: Vec::new() } + Assists { + resolve: true, + file: ctx.frange.file_id, + buf: Vec::new(), + allowed: ctx.allowed.clone(), + } } + pub(crate) fn new_unresolved(ctx: &AssistContext) -> Assists { - Assists { resolve: false, file: ctx.frange.file_id, buf: Vec::new() } + Assists { + resolve: false, + file: ctx.frange.file_id, + buf: Vec::new(), + allowed: ctx.allowed.clone(), + } } pub(crate) fn finish_unresolved(self) -> Vec { @@ -139,9 +153,13 @@ impl Assists { target: TextRange, f: impl FnOnce(&mut AssistBuilder), ) -> Option<()> { + if !self.is_allowed(&id) { + return None; + } let label = Assist::new(id, label.into(), None, target); self.add_impl(label, f) } + pub(crate) fn add_group( &mut self, group: &GroupLabel, @@ -150,9 +168,14 @@ impl Assists { target: TextRange, f: impl FnOnce(&mut AssistBuilder), ) -> Option<()> { + if !self.is_allowed(&id) { + return None; + } + let label = Assist::new(id, label.into(), Some(group.clone()), target); self.add_impl(label, f) } + fn add_impl(&mut self, label: Assist, f: impl FnOnce(&mut AssistBuilder)) -> Option<()> { let source_change = if self.resolve { let mut builder = AssistBuilder::new(self.file); @@ -170,6 +193,13 @@ impl Assists { self.buf.sort_by_key(|(label, _edit)| label.target.len()); self.buf } + + fn is_allowed(&self, id: &AssistId) -> bool { + match &self.allowed { + Some(allowed) => allowed.iter().any(|kind| kind.contains(id.1)), + None => true, + } + } } pub(crate) struct AssistBuilder { diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index 3d61fbded..13a283760 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -37,6 +37,25 @@ pub enum AssistKind { RefactorRewrite, } +impl AssistKind { + pub fn contains(self, other: AssistKind) -> bool { + if self == other { + return true; + } + + match self { + AssistKind::None | AssistKind::Generate => return true, + AssistKind::Refactor => match other { + AssistKind::RefactorExtract + | AssistKind::RefactorInline + | AssistKind::RefactorRewrite => return true, + _ => return false, + }, + _ => return false, + } + } +} + /// Unique identifier of the assist, should not be shown to the user /// directly. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -67,9 +86,14 @@ impl Assist { /// /// Assists are returned in the "unresolved" state, that is only labels are /// returned, without actual edits. - pub fn unresolved(db: &RootDatabase, config: &AssistConfig, range: FileRange) -> Vec { + pub fn unresolved( + db: &RootDatabase, + config: &AssistConfig, + range: FileRange, + allowed: Option>, + ) -> Vec { let sema = Semantics::new(db); - let ctx = AssistContext::new(sema, config, range); + let ctx = AssistContext::new(sema, config, range, allowed); let mut acc = Assists::new_unresolved(&ctx); handlers::all().iter().for_each(|handler| { handler(&mut acc, &ctx); @@ -85,9 +109,10 @@ impl Assist { db: &RootDatabase, config: &AssistConfig, range: FileRange, + allowed: Option>, ) -> Vec { let sema = Semantics::new(db); - let ctx = AssistContext::new(sema, config, range); + let ctx = AssistContext::new(sema, config, range, allowed); let mut acc = Assists::new_resolved(&ctx); handlers::all().iter().for_each(|handler| { handler(&mut acc, &ctx); diff --git a/crates/ra_assists/src/tests.rs b/crates/ra_assists/src/tests.rs index 858f5ca80..861622d86 100644 --- a/crates/ra_assists/src/tests.rs +++ b/crates/ra_assists/src/tests.rs @@ -6,7 +6,7 @@ use ra_ide_db::RootDatabase; use ra_syntax::TextRange; use test_utils::{assert_eq_text, extract_offset, extract_range}; -use crate::{handlers::Handler, Assist, AssistConfig, AssistContext, Assists}; +use crate::{handlers::Handler, Assist, AssistConfig, AssistContext, AssistKind, Assists}; use stdx::trim_indent; pub(crate) fn with_single_file(text: &str) -> (RootDatabase, FileId) { @@ -35,14 +35,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 mut assist = Assist::resolved(&db, &AssistConfig::default(), frange) + let mut assist = Assist::resolved(&db, &AssistConfig::default(), frange, None) .into_iter() .find(|assist| assist.assist.id.0 == assist_id) .unwrap_or_else(|| { panic!( "\n\nAssist is not applicable: {}\nAvailable assists: {}", assist_id, - Assist::resolved(&db, &AssistConfig::default(), frange) + Assist::resolved(&db, &AssistConfig::default(), frange, None) .into_iter() .map(|assist| assist.assist.id.0) .collect::>() @@ -73,7 +73,7 @@ fn check(handler: Handler, before: &str, expected: ExpectedResult) { let sema = Semantics::new(&db); let config = AssistConfig::default(); - let ctx = AssistContext::new(sema, &config, frange); + let ctx = AssistContext::new(sema, &config, frange, None); let mut acc = Assists::new_resolved(&ctx); handler(&mut acc, &ctx); let mut res = acc.finish_resolved(); @@ -105,7 +105,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::resolved(&db, &AssistConfig::default(), frange); + let assists = Assist::resolved(&db, &AssistConfig::default(), frange, None); let mut assists = assists.iter(); assert_eq!( @@ -128,9 +128,49 @@ fn assist_order_if_expr() { let (range, before) = extract_range(before); let (db, file_id) = with_single_file(&before); let frange = FileRange { file_id, range }; - let assists = Assist::resolved(&db, &AssistConfig::default(), frange); + let assists = Assist::resolved(&db, &AssistConfig::default(), frange, None); let mut assists = assists.iter(); assert_eq!(assists.next().expect("expected assist").assist.label, "Extract into variable"); assert_eq!(assists.next().expect("expected assist").assist.label, "Replace with match"); } + +#[test] +fn assist_filter_works() { + let before = " + pub fn test_some_range(a: int) -> bool { + if let 2..6 = <|>5<|> { + true + } else { + false + } + }"; + let (range, before) = extract_range(before); + let (db, file_id) = with_single_file(&before); + let frange = FileRange { file_id, range }; + + { + let allowed = Some(vec![AssistKind::Refactor]); + + let assists = Assist::resolved(&db, &AssistConfig::default(), frange, allowed); + let mut assists = assists.iter(); + + assert_eq!(assists.next().expect("expected assist").assist.label, "Extract into variable"); + assert_eq!(assists.next().expect("expected assist").assist.label, "Replace with match"); + } + + { + let allowed = Some(vec![AssistKind::RefactorExtract]); + let assists = Assist::resolved(&db, &AssistConfig::default(), frange, allowed); + assert_eq!(assists.len(), 1); + + let mut assists = assists.iter(); + assert_eq!(assists.next().expect("expected assist").assist.label, "Extract into variable"); + } + + { + let allowed = Some(vec![AssistKind::QuickFix]); + let assists = Assist::resolved(&db, &AssistConfig::default(), frange, allowed); + assert!(assists.is_empty(), "All asserts but quickfixes should be filtered out"); + } +} -- cgit v1.2.3 From 21c1504ca972d59307a065f72154e50bd32aa763 Mon Sep 17 00:00:00 2001 From: Jeremy Kolb Date: Wed, 15 Jul 2020 09:45:30 -0400 Subject: Move allow list into AssistConfig --- crates/ra_assists/src/assist_config.rs | 5 ++++- crates/ra_assists/src/assist_context.rs | 8 +++----- crates/ra_assists/src/lib.rs | 12 +++--------- crates/ra_assists/src/tests.rs | 25 ++++++++++++++----------- 4 files changed, 24 insertions(+), 26 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/assist_config.rs b/crates/ra_assists/src/assist_config.rs index c0a0226fb..cda2abfb9 100644 --- a/crates/ra_assists/src/assist_config.rs +++ b/crates/ra_assists/src/assist_config.rs @@ -4,9 +4,12 @@ //! module, and we use to statically check that we only produce snippet //! assists if we are allowed to. +use crate::AssistKind; + #[derive(Clone, Debug, PartialEq, Eq)] pub struct AssistConfig { pub snippet_cap: Option, + pub allowed: Option>, } impl AssistConfig { @@ -22,6 +25,6 @@ pub struct SnippetCap { impl Default for AssistConfig { fn default() -> Self { - AssistConfig { snippet_cap: Some(SnippetCap { _private: () }) } + AssistConfig { snippet_cap: Some(SnippetCap { _private: () }), allowed: None } } } diff --git a/crates/ra_assists/src/assist_context.rs b/crates/ra_assists/src/assist_context.rs index 9ca2cfe68..3407df856 100644 --- a/crates/ra_assists/src/assist_context.rs +++ b/crates/ra_assists/src/assist_context.rs @@ -57,7 +57,6 @@ pub(crate) struct AssistContext<'a> { pub(crate) sema: Semantics<'a, RootDatabase>, pub(crate) frange: FileRange, source_file: SourceFile, - allowed: Option>, } impl<'a> AssistContext<'a> { @@ -65,10 +64,9 @@ impl<'a> AssistContext<'a> { sema: Semantics<'a, RootDatabase>, config: &'a AssistConfig, frange: FileRange, - allowed: Option>, ) -> AssistContext<'a> { let source_file = sema.parse(frange.file_id); - AssistContext { config, sema, frange, source_file, allowed } + AssistContext { config, sema, frange, source_file } } pub(crate) fn db(&self) -> &RootDatabase { @@ -114,7 +112,7 @@ impl Assists { resolve: true, file: ctx.frange.file_id, buf: Vec::new(), - allowed: ctx.allowed.clone(), + allowed: ctx.config.allowed.clone(), } } @@ -123,7 +121,7 @@ impl Assists { resolve: false, file: ctx.frange.file_id, buf: Vec::new(), - allowed: ctx.allowed.clone(), + allowed: ctx.config.allowed.clone(), } } diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index 13a283760..465b90415 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -86,14 +86,9 @@ impl Assist { /// /// Assists are returned in the "unresolved" state, that is only labels are /// returned, without actual edits. - pub fn unresolved( - db: &RootDatabase, - config: &AssistConfig, - range: FileRange, - allowed: Option>, - ) -> Vec { + pub fn unresolved(db: &RootDatabase, config: &AssistConfig, range: FileRange) -> Vec { let sema = Semantics::new(db); - let ctx = AssistContext::new(sema, config, range, allowed); + let ctx = AssistContext::new(sema, config, range); let mut acc = Assists::new_unresolved(&ctx); handlers::all().iter().for_each(|handler| { handler(&mut acc, &ctx); @@ -109,10 +104,9 @@ impl Assist { db: &RootDatabase, config: &AssistConfig, range: FileRange, - allowed: Option>, ) -> Vec { let sema = Semantics::new(db); - let ctx = AssistContext::new(sema, config, range, allowed); + let ctx = AssistContext::new(sema, config, range); let mut acc = Assists::new_resolved(&ctx); handlers::all().iter().for_each(|handler| { handler(&mut acc, &ctx); diff --git a/crates/ra_assists/src/tests.rs b/crates/ra_assists/src/tests.rs index 861622d86..18fcb9049 100644 --- a/crates/ra_assists/src/tests.rs +++ b/crates/ra_assists/src/tests.rs @@ -35,14 +35,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 mut assist = Assist::resolved(&db, &AssistConfig::default(), frange, None) + let mut assist = Assist::resolved(&db, &AssistConfig::default(), frange) .into_iter() .find(|assist| assist.assist.id.0 == assist_id) .unwrap_or_else(|| { panic!( "\n\nAssist is not applicable: {}\nAvailable assists: {}", assist_id, - Assist::resolved(&db, &AssistConfig::default(), frange, None) + Assist::resolved(&db, &AssistConfig::default(), frange) .into_iter() .map(|assist| assist.assist.id.0) .collect::>() @@ -73,7 +73,7 @@ fn check(handler: Handler, before: &str, expected: ExpectedResult) { let sema = Semantics::new(&db); let config = AssistConfig::default(); - let ctx = AssistContext::new(sema, &config, frange, None); + let ctx = AssistContext::new(sema, &config, frange); let mut acc = Assists::new_resolved(&ctx); handler(&mut acc, &ctx); let mut res = acc.finish_resolved(); @@ -105,7 +105,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::resolved(&db, &AssistConfig::default(), frange, None); + let assists = Assist::resolved(&db, &AssistConfig::default(), frange); let mut assists = assists.iter(); assert_eq!( @@ -128,7 +128,7 @@ fn assist_order_if_expr() { let (range, before) = extract_range(before); let (db, file_id) = with_single_file(&before); let frange = FileRange { file_id, range }; - let assists = Assist::resolved(&db, &AssistConfig::default(), frange, None); + let assists = Assist::resolved(&db, &AssistConfig::default(), frange); let mut assists = assists.iter(); assert_eq!(assists.next().expect("expected assist").assist.label, "Extract into variable"); @@ -150,9 +150,10 @@ fn assist_filter_works() { let frange = FileRange { file_id, range }; { - let allowed = Some(vec![AssistKind::Refactor]); + let mut cfg = AssistConfig::default(); + cfg.allowed = Some(vec![AssistKind::Refactor]); - let assists = Assist::resolved(&db, &AssistConfig::default(), frange, allowed); + let assists = Assist::resolved(&db, &cfg, frange); let mut assists = assists.iter(); assert_eq!(assists.next().expect("expected assist").assist.label, "Extract into variable"); @@ -160,8 +161,9 @@ fn assist_filter_works() { } { - let allowed = Some(vec![AssistKind::RefactorExtract]); - let assists = Assist::resolved(&db, &AssistConfig::default(), frange, allowed); + let mut cfg = AssistConfig::default(); + cfg.allowed = Some(vec![AssistKind::RefactorExtract]); + let assists = Assist::resolved(&db, &cfg, frange); assert_eq!(assists.len(), 1); let mut assists = assists.iter(); @@ -169,8 +171,9 @@ fn assist_filter_works() { } { - let allowed = Some(vec![AssistKind::QuickFix]); - let assists = Assist::resolved(&db, &AssistConfig::default(), frange, allowed); + let mut cfg = AssistConfig::default(); + cfg.allowed = Some(vec![AssistKind::QuickFix]); + let assists = Assist::resolved(&db, &cfg, frange); assert!(assists.is_empty(), "All asserts but quickfixes should be filtered out"); } } -- cgit v1.2.3