From b3665fccfb0a81752c35e56f6c41043133a949dd Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 25 Mar 2020 15:45:52 +0100 Subject: [PATCH 1/2] Preserve relative ordering of grouped assists --- .../rust-analyzer/src/main_loop/handlers.rs | 44 +++++++++++-------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/crates/rust-analyzer/src/main_loop/handlers.rs b/crates/rust-analyzer/src/main_loop/handlers.rs index 1cc2f65712..1033d6de94 100644 --- a/crates/rust-analyzer/src/main_loop/handlers.rs +++ b/crates/rust-analyzer/src/main_loop/handlers.rs @@ -734,19 +734,29 @@ pub fn handle_code_action( res.push(fix.action.clone()); } - let mut grouped_assists: FxHashMap> = FxHashMap::default(); + let mut grouped_assists: FxHashMap)> = FxHashMap::default(); for assist in world.analysis().assists(FileRange { file_id, range })?.into_iter() { match &assist.group_label { - Some(label) => grouped_assists.entry(label.to_owned()).or_default().push(assist), - None => res.push(create_single_code_action(assist, &world)?.into()), + Some(label) => grouped_assists + .entry(label.to_owned()) + .or_insert_with(|| { + let idx = res.len(); + let dummy = Command::new(String::new(), String::new(), None); + res.push(dummy.into()); + (idx, Vec::new()) + }) + .1 + .push(assist), + None => { + res.push(create_single_code_action(assist, &world)?.into()); + } } } - for (group_label, assists) in grouped_assists { + for (group_label, (idx, assists)) in grouped_assists { if assists.len() == 1 { - res.push( - create_single_code_action(assists.into_iter().next().unwrap(), &world)?.into(), - ); + res[idx] = + create_single_code_action(assists.into_iter().next().unwrap(), &world)?.into(); } else { let title = group_label; @@ -760,17 +770,15 @@ pub fn handle_code_action( command: "rust-analyzer.selectAndApplySourceChange".to_string(), arguments: Some(vec![serde_json::Value::Array(arguments)]), }); - res.push( - CodeAction { - title, - kind: None, - diagnostics: None, - edit: None, - command, - is_preferred: None, - } - .into(), - ); + res[idx] = CodeAction { + title, + kind: None, + diagnostics: None, + edit: None, + command, + is_preferred: None, + } + .into(); } } From 72c6fc3ff0dff472c93468d97b96230f64aefe69 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 25 Mar 2020 15:55:57 +0100 Subject: [PATCH 2/2] Fix add visibility false-positive --- .../src/handlers/change_visibility.rs | 40 +++++++++++++------ crates/ra_assists/src/marks.rs | 1 + 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/crates/ra_assists/src/handlers/change_visibility.rs b/crates/ra_assists/src/handlers/change_visibility.rs index 54e0a6c840..cd6d1ee6c3 100644 --- a/crates/ra_assists/src/handlers/change_visibility.rs +++ b/crates/ra_assists/src/handlers/change_visibility.rs @@ -2,13 +2,14 @@ use ra_syntax::{ ast::{self, NameOwner, VisibilityOwner}, AstNode, SyntaxKind::{ - ATTR, COMMENT, CONST_DEF, ENUM_DEF, FN_DEF, IDENT, MODULE, STRUCT_DEF, TRAIT_DEF, - VISIBILITY, WHITESPACE, + ATTR, COMMENT, CONST_DEF, ENUM_DEF, FN_DEF, MODULE, STRUCT_DEF, TRAIT_DEF, VISIBILITY, + WHITESPACE, }, SyntaxNode, TextUnit, T, }; use crate::{Assist, AssistCtx, AssistId}; +use test_utils::tested_by; // Assist: change_visibility // @@ -47,13 +48,16 @@ fn add_vis(ctx: AssistCtx) -> Option { } (vis_offset(&parent), keyword.text_range()) } else { - let ident = ctx.token_at_offset().find(|leaf| leaf.kind() == IDENT)?; - let field = ident.parent().ancestors().find_map(ast::RecordFieldDef::cast)?; - if field.name()?.syntax().text_range() != ident.text_range() && field.visibility().is_some() - { + let field_name: ast::Name = ctx.find_node_at_offset()?; + let field = field_name.syntax().ancestors().find_map(ast::RecordFieldDef::cast)?; + if field.name()? != field_name { + tested_by!(change_visibility_field_false_positive); return None; } - (vis_offset(field.syntax()), ident.text_range()) + if field.visibility().is_some() { + return None; + } + (vis_offset(field.syntax()), field_name.syntax().text_range()) }; ctx.add_assist(AssistId("change_visibility"), "Change visibility to pub(crate)", |edit| { @@ -98,8 +102,11 @@ fn change_vis(ctx: AssistCtx, vis: ast::Visibility) -> Option { #[cfg(test)] mod tests { + use test_utils::covers; + + use crate::helpers::{check_assist, check_assist_not_applicable, check_assist_target}; + use super::*; - use crate::helpers::{check_assist, check_assist_target}; #[test] fn change_visibility_adds_pub_crate_to_items() { @@ -120,8 +127,17 @@ mod tests { fn change_visibility_works_with_struct_fields() { check_assist( change_visibility, - "struct S { <|>field: u32 }", - "struct S { <|>pub(crate) field: u32 }", + r"struct S { <|>field: u32 }", + r"struct S { <|>pub(crate) field: u32 }", + ) + } + + #[test] + fn change_visibility_field_false_positive() { + covers!(change_visibility_field_false_positive); + check_assist_not_applicable( + change_visibility, + r"struct S { field: [(); { let <|>x = ();}] }", ) } @@ -144,7 +160,7 @@ mod tests { fn change_visibility_handles_comment_attrs() { check_assist( change_visibility, - " + r" /// docs // comments @@ -152,7 +168,7 @@ mod tests { #[derive(Debug)] <|>struct Foo; ", - " + r" /// docs // comments diff --git a/crates/ra_assists/src/marks.rs b/crates/ra_assists/src/marks.rs index 22404ee80a..6c2a2b8b6e 100644 --- a/crates/ra_assists/src/marks.rs +++ b/crates/ra_assists/src/marks.rs @@ -7,4 +7,5 @@ test_utils::marks![ not_applicable_outside_of_bind_pat test_not_inline_mut_variable test_not_applicable_if_variable_unused + change_visibility_field_false_positive ];