From b5c330bf5a1884ff8ad196a23fa6e356c460e130 Mon Sep 17 00:00:00 2001 From: Jesse Bakker Date: Sat, 2 May 2020 18:10:23 +0200 Subject: Make `change_visibility` assist work for tuple struct field visibility --- crates/ra_assists/src/handlers/change_visibility.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) (limited to 'crates/ra_assists/src/handlers/change_visibility.rs') diff --git a/crates/ra_assists/src/handlers/change_visibility.rs b/crates/ra_assists/src/handlers/change_visibility.rs index 44f6a1dae..1cd532e80 100644 --- a/crates/ra_assists/src/handlers/change_visibility.rs +++ b/crates/ra_assists/src/handlers/change_visibility.rs @@ -47,8 +47,7 @@ fn add_vis(ctx: AssistCtx) -> Option { return None; } (vis_offset(&parent), keyword.text_range()) - } else { - let field_name: ast::Name = ctx.find_node_at_offset()?; + } else if let Some(field_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); @@ -58,6 +57,13 @@ fn add_vis(ctx: AssistCtx) -> Option { return None; } (vis_offset(field.syntax()), field_name.syntax().text_range()) + } else if let Some(field) = ctx.find_node_at_offset::() { + if field.visibility().is_some() { + return None; + } + (vis_offset(field.syntax()), field.syntax().text_range()) + } else { + return None; }; ctx.add_assist(AssistId("change_visibility"), "Change visibility to pub(crate)", |edit| { @@ -129,7 +135,8 @@ mod tests { change_visibility, r"struct S { <|>field: u32 }", r"struct S { <|>pub(crate) field: u32 }", - ) + ); + check_assist(change_visibility, r"struct S ( <|>u32 )", r"struct S ( <|>pub(crate) u32 )"); } #[test] -- cgit v1.2.3 From 25e6bbde01d4a9cd08fa79db5b8b7da6bbf1a623 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 6 May 2020 10:16:55 +0200 Subject: Merge assits::test_helpers and tests --- crates/ra_assists/src/handlers/change_visibility.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'crates/ra_assists/src/handlers/change_visibility.rs') diff --git a/crates/ra_assists/src/handlers/change_visibility.rs b/crates/ra_assists/src/handlers/change_visibility.rs index 1cd532e80..6ac1f8e69 100644 --- a/crates/ra_assists/src/handlers/change_visibility.rs +++ b/crates/ra_assists/src/handlers/change_visibility.rs @@ -110,7 +110,7 @@ fn change_vis(ctx: AssistCtx, vis: ast::Visibility) -> Option { mod tests { use test_utils::covers; - use crate::helpers::{check_assist, check_assist_not_applicable, check_assist_target}; + use crate::tests::{check_assist, check_assist_not_applicable, check_assist_target}; use super::*; -- cgit v1.2.3 From 233f01c9ba555e5d06f336cb0ff64e7a83e4a23a Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 6 May 2020 12:51:28 +0200 Subject: Move target to AssistLabel Target is used for assists sorting, so we need it before we compute the action. --- .../ra_assists/src/handlers/change_visibility.rs | 32 ++++++++++++++-------- 1 file changed, 21 insertions(+), 11 deletions(-) (limited to 'crates/ra_assists/src/handlers/change_visibility.rs') diff --git a/crates/ra_assists/src/handlers/change_visibility.rs b/crates/ra_assists/src/handlers/change_visibility.rs index 6ac1f8e69..489db83e6 100644 --- a/crates/ra_assists/src/handlers/change_visibility.rs +++ b/crates/ra_assists/src/handlers/change_visibility.rs @@ -66,11 +66,15 @@ fn add_vis(ctx: AssistCtx) -> Option { return None; }; - ctx.add_assist(AssistId("change_visibility"), "Change visibility to pub(crate)", |edit| { - edit.target(target); - edit.insert(offset, "pub(crate) "); - edit.set_cursor(offset); - }) + ctx.add_assist( + AssistId("change_visibility"), + "Change visibility to pub(crate)", + target, + |edit| { + edit.insert(offset, "pub(crate) "); + edit.set_cursor(offset); + }, + ) } fn vis_offset(node: &SyntaxNode) -> TextSize { @@ -86,22 +90,28 @@ fn vis_offset(node: &SyntaxNode) -> TextSize { fn change_vis(ctx: AssistCtx, vis: ast::Visibility) -> Option { if vis.syntax().text() == "pub" { + let target = vis.syntax().text_range(); return ctx.add_assist( AssistId("change_visibility"), "Change Visibility to pub(crate)", + target, |edit| { - edit.target(vis.syntax().text_range()); edit.replace(vis.syntax().text_range(), "pub(crate)"); edit.set_cursor(vis.syntax().text_range().start()) }, ); } if vis.syntax().text() == "pub(crate)" { - return ctx.add_assist(AssistId("change_visibility"), "Change visibility to pub", |edit| { - edit.target(vis.syntax().text_range()); - edit.replace(vis.syntax().text_range(), "pub"); - edit.set_cursor(vis.syntax().text_range().start()); - }); + let target = vis.syntax().text_range(); + return ctx.add_assist( + AssistId("change_visibility"), + "Change visibility to pub", + target, + |edit| { + edit.replace(vis.syntax().text_range(), "pub"); + edit.set_cursor(vis.syntax().text_range().start()); + }, + ); } None } -- cgit v1.2.3 From 4867968d22899395e6551f22641b3617e995140c Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 6 May 2020 18:45:35 +0200 Subject: Refactor assists API to be more convenient for adding new assists It now duplicates completion API in its shape. --- .../ra_assists/src/handlers/change_visibility.rs | 31 +++++++++------------- 1 file changed, 13 insertions(+), 18 deletions(-) (limited to 'crates/ra_assists/src/handlers/change_visibility.rs') diff --git a/crates/ra_assists/src/handlers/change_visibility.rs b/crates/ra_assists/src/handlers/change_visibility.rs index 489db83e6..e631766ef 100644 --- a/crates/ra_assists/src/handlers/change_visibility.rs +++ b/crates/ra_assists/src/handlers/change_visibility.rs @@ -7,10 +7,10 @@ use ra_syntax::{ }, SyntaxNode, TextSize, T, }; - -use crate::{Assist, AssistCtx, AssistId}; use test_utils::tested_by; +use crate::{AssistContext, AssistId, Assists}; + // Assist: change_visibility // // Adds or changes existing visibility specifier. @@ -22,14 +22,14 @@ use test_utils::tested_by; // ``` // pub(crate) fn frobnicate() {} // ``` -pub(crate) fn change_visibility(ctx: AssistCtx) -> Option { +pub(crate) fn change_visibility(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { if let Some(vis) = ctx.find_node_at_offset::() { - return change_vis(ctx, vis); + return change_vis(acc, vis); } - add_vis(ctx) + add_vis(acc, ctx) } -fn add_vis(ctx: AssistCtx) -> Option { +fn add_vis(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let item_keyword = ctx.token_at_offset().find(|leaf| match leaf.kind() { T![const] | T![fn] | T![mod] | T![struct] | T![enum] | T![trait] => true, _ => false, @@ -66,15 +66,10 @@ fn add_vis(ctx: AssistCtx) -> Option { return None; }; - ctx.add_assist( - AssistId("change_visibility"), - "Change visibility to pub(crate)", - target, - |edit| { - edit.insert(offset, "pub(crate) "); - edit.set_cursor(offset); - }, - ) + acc.add(AssistId("change_visibility"), "Change visibility to pub(crate)", target, |edit| { + edit.insert(offset, "pub(crate) "); + edit.set_cursor(offset); + }) } fn vis_offset(node: &SyntaxNode) -> TextSize { @@ -88,10 +83,10 @@ fn vis_offset(node: &SyntaxNode) -> TextSize { .unwrap_or_else(|| node.text_range().start()) } -fn change_vis(ctx: AssistCtx, vis: ast::Visibility) -> Option { +fn change_vis(acc: &mut Assists, vis: ast::Visibility) -> Option<()> { if vis.syntax().text() == "pub" { let target = vis.syntax().text_range(); - return ctx.add_assist( + return acc.add( AssistId("change_visibility"), "Change Visibility to pub(crate)", target, @@ -103,7 +98,7 @@ fn change_vis(ctx: AssistCtx, vis: ast::Visibility) -> Option { } if vis.syntax().text() == "pub(crate)" { let target = vis.syntax().text_range(); - return ctx.add_assist( + return acc.add( AssistId("change_visibility"), "Change visibility to pub", target, -- cgit v1.2.3 From 7568d0770934345be183670652e8064c06c05caf Mon Sep 17 00:00:00 2001 From: Timo Freiberg Date: Sun, 3 May 2020 01:01:59 +0200 Subject: Trigger change_visibility assist when on a path to an invisible def --- .../ra_assists/src/handlers/change_visibility.rs | 329 ++++++++++++++++++++- 1 file changed, 328 insertions(+), 1 deletion(-) (limited to 'crates/ra_assists/src/handlers/change_visibility.rs') diff --git a/crates/ra_assists/src/handlers/change_visibility.rs b/crates/ra_assists/src/handlers/change_visibility.rs index e631766ef..573f94a46 100644 --- a/crates/ra_assists/src/handlers/change_visibility.rs +++ b/crates/ra_assists/src/handlers/change_visibility.rs @@ -5,8 +5,10 @@ use ra_syntax::{ ATTR, COMMENT, CONST_DEF, ENUM_DEF, FN_DEF, MODULE, STRUCT_DEF, TRAIT_DEF, VISIBILITY, WHITESPACE, }, - SyntaxNode, TextSize, T, + SyntaxNode, TextRange, TextSize, T, }; + +use hir::{db::HirDatabase, HasSource, PathResolution}; use test_utils::tested_by; use crate::{AssistContext, AssistId, Assists}; @@ -72,6 +74,88 @@ fn add_vis(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { }) } +fn add_missing_vis(ctx: AssistCtx) -> Option { + let path: ast::Path = ctx.find_node_at_offset()?; + let path_res = dbg!(ctx.sema.resolve_path(&path))?; + let def = match path_res { + PathResolution::Def(def) => def, + _ => return None, + }; + dbg!(&def); + + let current_module = dbg!(ctx.sema.scope(&path.syntax()).module())?; + let target_module = dbg!(def.module(ctx.db))?; + + let vis = dbg!(target_module.visibility_of(ctx.db, &def))?; + if vis.is_visible_from(ctx.db, current_module.into()) { + return None; + }; + let target_name; + + let (offset, target) = match def { + hir::ModuleDef::Function(f) => { + target_name = Some(f.name(ctx.db)); + offset_and_target(ctx.db, f) + } + hir::ModuleDef::Adt(adt) => { + target_name = Some(adt.name(ctx.db)); + match adt { + hir::Adt::Struct(s) => offset_and_target(ctx.db, s), + hir::Adt::Union(u) => offset_and_target(ctx.db, u), + hir::Adt::Enum(e) => offset_and_target(ctx.db, e), + } + } + hir::ModuleDef::Const(c) => { + target_name = c.name(ctx.db); + offset_and_target(ctx.db, c) + } + hir::ModuleDef::Static(s) => { + target_name = s.name(ctx.db); + offset_and_target(ctx.db, s) + } + hir::ModuleDef::Trait(t) => { + target_name = Some(t.name(ctx.db)); + offset_and_target(ctx.db, t) + } + hir::ModuleDef::TypeAlias(t) => { + target_name = Some(t.name(ctx.db)); + offset_and_target(ctx.db, t) + } + hir::ModuleDef::Module(m) => { + target_name = m.name(ctx.db); + let source = dbg!(m.declaration_source(ctx.db))?.value; + let syntax = source.syntax(); + (vis_offset(syntax), syntax.text_range()) + } + // Enum variants can't be private, we can't modify builtin types + hir::ModuleDef::EnumVariant(_) | hir::ModuleDef::BuiltinType(_) => return None, + }; + + // FIXME if target is in another crate, add `pub` instead of `pub(crate)` + + let assist_label = match target_name { + None => "Change visibility to pub(crate)".to_string(), + Some(name) => format!("Change visibility of {} to pub(crate)", name), + }; + let target_file = target_module.definition_source(ctx.db).file_id.original_file(ctx.db); + + ctx.add_assist(AssistId("change_visibility"), assist_label, target, |edit| { + edit.set_file(target_file); + edit.insert(offset, "pub(crate) "); + edit.set_cursor(offset); + }) +} + +fn offset_and_target(db: &dyn HirDatabase, x: S) -> (TextSize, TextRange) +where + S: HasSource, + Ast: AstNode, +{ + let source = x.source(db); + let syntax = source.syntax().value; + (vis_offset(syntax), syntax.text_range()) +} + fn vis_offset(node: &SyntaxNode) -> TextSize { node.children_with_tokens() .skip_while(|it| match it.kind() { @@ -191,6 +275,249 @@ mod tests { ) } + #[test] + fn change_visibility_of_fn_via_path() { + check_assist( + change_visibility, + r"mod foo { fn foo() {} } + fn main() { foo::foo<|>() } ", + r"mod foo { <|>pub(crate) fn foo() {} } + fn main() { foo::foo() } ", + ); + check_assist_not_applicable( + change_visibility, + r"mod foo { pub fn foo() {} } + fn main() { foo::foo<|>() } ", + ) + } + + #[test] + fn change_visibility_of_adt_in_submodule_via_path() { + check_assist( + change_visibility, + r"mod foo { struct Foo; } + fn main() { foo::Foo<|> } ", + r"mod foo { <|>pub(crate) struct Foo; } + fn main() { foo::Foo } ", + ); + check_assist_not_applicable( + change_visibility, + r"mod foo { pub struct Foo; } + fn main() { foo::Foo<|> } ", + ); + check_assist( + change_visibility, + r"mod foo { enum Foo; } + fn main() { foo::Foo<|> } ", + r"mod foo { <|>pub(crate) enum Foo; } + fn main() { foo::Foo } ", + ); + check_assist_not_applicable( + change_visibility, + r"mod foo { pub enum Foo; } + fn main() { foo::Foo<|> } ", + ); + check_assist( + change_visibility, + r"mod foo { union Foo; } + fn main() { foo::Foo<|> } ", + r"mod foo { <|>pub(crate) union Foo; } + fn main() { foo::Foo } ", + ); + check_assist_not_applicable( + change_visibility, + r"mod foo { pub union Foo; } + fn main() { foo::Foo<|> } ", + ); + } + + #[test] + fn change_visibility_of_adt_in_other_file_via_path() { + check_assist( + change_visibility, + r" + //- /main.rs + mod foo; + fn main() { foo::Foo<|> } + + //- /foo.rs + struct Foo; + ", + r"<|>pub(crate) struct Foo; + +", + ); + } + + #[test] + // FIXME this requires a separate implementation, struct fields are not a ast::Path + fn change_visibility_of_struct_field_via_path() { + check_assist( + change_visibility, + r"mod foo { pub struct Foo { bar: (), } } + fn main() { foo::Foo { <|>bar: () }; } ", + r"mod foo { pub struct Foo { <|>pub(crate) bar: (), } } + fn main() { foo::Foo { bar: () }; } ", + ); + check_assist_not_applicable( + change_visibility, + r"mod foo { pub struct Foo { pub bar: (), } } + fn main() { foo::Foo { <|>bar: () }; } ", + ); + } + + #[test] + fn not_applicable_for_enum_variants() { + check_assist_not_applicable( + change_visibility, + r"mod foo { pub enum Foo {Foo1} } + fn main() { foo::Foo::Foo1<|> } ", + ); + } + + #[test] + fn change_visibility_of_const_via_path() { + check_assist( + change_visibility, + r"mod foo { const FOO: () = (); } + fn main() { foo::FOO<|> } ", + r"mod foo { <|>pub(crate) const FOO: () = (); } + fn main() { foo::FOO } ", + ); + check_assist_not_applicable( + change_visibility, + r"mod foo { pub const FOO: () = (); } + fn main() { foo::FOO<|> } ", + ); + } + + #[test] + fn change_visibility_of_static_via_path() { + check_assist( + change_visibility, + r"mod foo { static FOO: () = (); } + fn main() { foo::FOO<|> } ", + r"mod foo { <|>pub(crate) static FOO: () = (); } + fn main() { foo::FOO } ", + ); + check_assist_not_applicable( + change_visibility, + r"mod foo { pub static FOO: () = (); } + fn main() { foo::FOO<|> } ", + ); + } + + #[test] + fn change_visibility_of_trait_via_path() { + check_assist( + change_visibility, + r"mod foo { trait Foo { fn foo(&self) {} } } + fn main() { let x: &dyn foo::<|>Foo; } ", + r"mod foo { <|>pub(crate) trait Foo { fn foo(&self) {} } } + fn main() { let x: &dyn foo::Foo; } ", + ); + check_assist_not_applicable( + change_visibility, + r"mod foo { pub trait Foo { fn foo(&self) {} } } + fn main() { let x: &dyn foo::Foo<|>; } ", + ); + } + + #[test] + fn change_visibility_of_type_alias_via_path() { + check_assist( + change_visibility, + r"mod foo { type Foo = (); } + fn main() { let x: foo::Foo<|>; } ", + r"mod foo { <|>pub(crate) type Foo = (); } + fn main() { let x: foo::Foo; } ", + ); + check_assist_not_applicable( + change_visibility, + r"mod foo { pub type Foo = (); } + fn main() { let x: foo::Foo<|>; } ", + ); + } + + #[test] + fn change_visibility_of_module_via_path() { + check_assist( + change_visibility, + r"mod foo { mod bar { fn bar() {} } } + fn main() { foo::bar<|>::bar(); } ", + r"mod foo { <|>pub(crate) mod bar { fn bar() {} } } + fn main() { foo::bar::bar(); } ", + ); + + check_assist( + change_visibility, + r" + //- /main.rs + mod foo; + fn main() { foo::bar<|>::baz(); } + + //- /foo.rs + mod bar { + pub fn baz() {} + } + ", + r"<|>pub(crate) mod bar { + pub fn baz() {} +} + +", + ); + + check_assist_not_applicable( + change_visibility, + r"mod foo { pub mod bar { pub fn bar() {} } } + fn main() { foo::bar<|>::bar(); } ", + ); + } + + #[test] + fn change_visibility_of_inline_module_in_other_file_via_path() { + check_assist( + change_visibility, + r" + //- /main.rs + mod foo; + fn main() { foo::bar<|>::baz(); } + + //- /foo.rs + mod bar; + + //- /foo/bar.rs + pub fn baz() {} + } + ", + r"<|>pub(crate) mod bar; +", + ); + } + + #[test] + fn change_visibility_of_module_declaration_in_other_file_via_path() { + check_assist( + change_visibility, + r" + //- /main.rs + mod foo; + fn main() { foo::bar<|>>::baz(); } + + //- /foo.rs + mod bar { + pub fn baz() {} + } + ", + r"<|>pub(crate) mod bar { + pub fn baz() {} +} + +", + ); + } + #[test] fn change_visibility_target() { check_assist_target(change_visibility, "<|>fn foo() {}", "fn"); -- cgit v1.2.3 From 66db88d226704a75a2fa38782ee3b82fd7829d5e Mon Sep 17 00:00:00 2001 From: Timo Freiberg Date: Thu, 7 May 2020 00:10:56 +0200 Subject: Trigger change_visibility assist when on an invisible struct field Union fields apparently don't work :( --- .../ra_assists/src/handlers/change_visibility.rs | 309 +++++++++++++++++---- 1 file changed, 248 insertions(+), 61 deletions(-) (limited to 'crates/ra_assists/src/handlers/change_visibility.rs') diff --git a/crates/ra_assists/src/handlers/change_visibility.rs b/crates/ra_assists/src/handlers/change_visibility.rs index 573f94a46..40cf4b422 100644 --- a/crates/ra_assists/src/handlers/change_visibility.rs +++ b/crates/ra_assists/src/handlers/change_visibility.rs @@ -8,10 +8,11 @@ use ra_syntax::{ SyntaxNode, TextRange, TextSize, T, }; -use hir::{db::HirDatabase, HasSource, PathResolution}; +use hir::{db::HirDatabase, HasSource, HasVisibility, PathResolution}; use test_utils::tested_by; use crate::{AssistContext, AssistId, Assists}; +use ra_db::FileId; // Assist: change_visibility // @@ -29,6 +30,8 @@ pub(crate) fn change_visibility(acc: &mut Assists, ctx: &AssistContext) -> Optio return change_vis(acc, vis); } add_vis(acc, ctx) + .or_else(|| add_vis_to_referenced_module_def(acc, ctx)) + .or_else(|| add_vis_to_referenced_record_field(acc, ctx)) } fn add_vis(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { @@ -74,86 +77,141 @@ fn add_vis(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { }) } -fn add_missing_vis(ctx: AssistCtx) -> Option { +fn add_vis_to_referenced_module_def(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let path: ast::Path = ctx.find_node_at_offset()?; - let path_res = dbg!(ctx.sema.resolve_path(&path))?; + let path_res = ctx.sema.resolve_path(&path)?; let def = match path_res { PathResolution::Def(def) => def, _ => return None, }; - dbg!(&def); - let current_module = dbg!(ctx.sema.scope(&path.syntax()).module())?; - let target_module = dbg!(def.module(ctx.db))?; + let current_module = ctx.sema.scope(&path.syntax()).module()?; + let target_module = def.module(ctx.db)?; - let vis = dbg!(target_module.visibility_of(ctx.db, &def))?; + let vis = target_module.visibility_of(ctx.db, &def)?; if vis.is_visible_from(ctx.db, current_module.into()) { return None; }; - let target_name; - let (offset, target) = match def { + let (offset, target, target_file, target_name) = target_data_for_def(ctx.db, def)?; + + let missing_visibility = + if current_module.krate() == target_module.krate() { "pub(crate)" } else { "pub" }; + + let assist_label = match target_name { + None => format!("Change visibility to {}", missing_visibility), + Some(name) => format!("Change visibility of {} to {}", name, missing_visibility), + }; + + acc.add(AssistId("change_visibility"), assist_label, target, |edit| { + edit.set_file(target_file); + edit.insert(offset, format!("{} ", missing_visibility)); + edit.set_cursor(offset); + }) +} + +fn add_vis_to_referenced_record_field(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + let record_field: ast::RecordField = ctx.find_node_at_offset()?; + let (record_field_def, _) = ctx.sema.resolve_record_field(&record_field)?; + + let current_module = ctx.sema.scope(record_field.syntax()).module()?; + let visibility = record_field_def.visibility(ctx.db); + if visibility.is_visible_from(ctx.db, current_module.into()) { + return None; + } + + let parent = record_field_def.parent_def(ctx.db); + let parent_name = parent.name(ctx.db); + let target_module = parent.module(ctx.db); + + let in_file_source = record_field_def.source(ctx.db); + let (offset, target) = match in_file_source.value { + hir::FieldSource::Named(it) => { + let s = it.syntax(); + (vis_offset(s), s.text_range()) + } + hir::FieldSource::Pos(it) => { + let s = it.syntax(); + (vis_offset(s), s.text_range()) + } + }; + + let missing_visibility = + if current_module.krate() == target_module.krate() { "pub(crate)" } else { "pub" }; + let target_file = in_file_source.file_id.original_file(ctx.db); + + let target_name = record_field_def.name(ctx.db); + let assist_label = + format!("Change visibility of {}.{} to {}", parent_name, target_name, missing_visibility); + + acc.add(AssistId("change_visibility"), assist_label, target, |edit| { + edit.set_file(target_file); + edit.insert(offset, format!("{} ", missing_visibility)); + edit.set_cursor(offset) + }) +} + +fn target_data_for_def( + db: &dyn HirDatabase, + def: hir::ModuleDef, +) -> Option<(TextSize, TextRange, FileId, Option)> { + fn offset_target_and_file_id( + db: &dyn HirDatabase, + x: S, + ) -> (TextSize, TextRange, FileId) + where + S: HasSource, + Ast: AstNode, + { + let source = x.source(db); + let in_file_syntax = source.syntax(); + let file_id = in_file_syntax.file_id; + let syntax = in_file_syntax.value; + (vis_offset(syntax), syntax.text_range(), file_id.original_file(db.upcast())) + } + + let target_name; + let (offset, target, target_file) = match def { hir::ModuleDef::Function(f) => { - target_name = Some(f.name(ctx.db)); - offset_and_target(ctx.db, f) + target_name = Some(f.name(db)); + offset_target_and_file_id(db, f) } hir::ModuleDef::Adt(adt) => { - target_name = Some(adt.name(ctx.db)); + target_name = Some(adt.name(db)); match adt { - hir::Adt::Struct(s) => offset_and_target(ctx.db, s), - hir::Adt::Union(u) => offset_and_target(ctx.db, u), - hir::Adt::Enum(e) => offset_and_target(ctx.db, e), + hir::Adt::Struct(s) => offset_target_and_file_id(db, s), + hir::Adt::Union(u) => offset_target_and_file_id(db, u), + hir::Adt::Enum(e) => offset_target_and_file_id(db, e), } } hir::ModuleDef::Const(c) => { - target_name = c.name(ctx.db); - offset_and_target(ctx.db, c) + target_name = c.name(db); + offset_target_and_file_id(db, c) } hir::ModuleDef::Static(s) => { - target_name = s.name(ctx.db); - offset_and_target(ctx.db, s) + target_name = s.name(db); + offset_target_and_file_id(db, s) } hir::ModuleDef::Trait(t) => { - target_name = Some(t.name(ctx.db)); - offset_and_target(ctx.db, t) + target_name = Some(t.name(db)); + offset_target_and_file_id(db, t) } hir::ModuleDef::TypeAlias(t) => { - target_name = Some(t.name(ctx.db)); - offset_and_target(ctx.db, t) + target_name = Some(t.name(db)); + offset_target_and_file_id(db, t) } hir::ModuleDef::Module(m) => { - target_name = m.name(ctx.db); - let source = dbg!(m.declaration_source(ctx.db))?.value; - let syntax = source.syntax(); - (vis_offset(syntax), syntax.text_range()) + target_name = m.name(db); + let in_file_source = m.declaration_source(db)?; + let file_id = in_file_source.file_id.original_file(db.upcast()); + let syntax = in_file_source.value.syntax(); + (vis_offset(syntax), syntax.text_range(), file_id) } // Enum variants can't be private, we can't modify builtin types hir::ModuleDef::EnumVariant(_) | hir::ModuleDef::BuiltinType(_) => return None, }; - // FIXME if target is in another crate, add `pub` instead of `pub(crate)` - - let assist_label = match target_name { - None => "Change visibility to pub(crate)".to_string(), - Some(name) => format!("Change visibility of {} to pub(crate)", name), - }; - let target_file = target_module.definition_source(ctx.db).file_id.original_file(ctx.db); - - ctx.add_assist(AssistId("change_visibility"), assist_label, target, |edit| { - edit.set_file(target_file); - edit.insert(offset, "pub(crate) "); - edit.set_cursor(offset); - }) -} - -fn offset_and_target(db: &dyn HirDatabase, x: S) -> (TextSize, TextRange) -where - S: HasSource, - Ast: AstNode, -{ - let source = x.source(db); - let syntax = source.syntax().value; - (vis_offset(syntax), syntax.text_range()) + Some((offset, target, target_file, target_name)) } fn vis_offset(node: &SyntaxNode) -> TextSize { @@ -350,7 +408,6 @@ mod tests { } #[test] - // FIXME this requires a separate implementation, struct fields are not a ast::Path fn change_visibility_of_struct_field_via_path() { check_assist( change_visibility, @@ -359,11 +416,108 @@ mod tests { r"mod foo { pub struct Foo { <|>pub(crate) bar: (), } } fn main() { foo::Foo { bar: () }; } ", ); + check_assist( + change_visibility, + r"//- /lib.rs + mod foo; + fn main() { foo::Foo { <|>bar: () }; } + //- /foo.rs + pub struct Foo { bar: () } + ", + r"pub struct Foo { <|>pub(crate) bar: () } + +", + ); + check_assist_not_applicable( + change_visibility, + r"mod foo { pub struct Foo { pub bar: (), } } + fn main() { foo::Foo { <|>bar: () }; } ", + ); + check_assist_not_applicable( + change_visibility, + r"//- /lib.rs + mod foo; + fn main() { foo::Foo { <|>bar: () }; } + //- /foo.rs + pub struct Foo { pub bar: () } + ", + ); + } + + #[test] + fn change_visibility_of_enum_variant_field_via_path() { + check_assist( + change_visibility, + r"mod foo { pub enum Foo { Bar { bar: () } } } + fn main() { foo::Foo::Bar { <|>bar: () }; } ", + r"mod foo { pub enum Foo { Bar { <|>pub(crate) bar: () } } } + fn main() { foo::Foo::Bar { bar: () }; } ", + ); + check_assist( + change_visibility, + r"//- /lib.rs + mod foo; + fn main() { foo::Foo::Bar { <|>bar: () }; } + //- /foo.rs + pub enum Foo { Bar { bar: () } } + ", + r"pub enum Foo { Bar { <|>pub(crate) bar: () } } + +", + ); check_assist_not_applicable( change_visibility, r"mod foo { pub struct Foo { pub bar: (), } } fn main() { foo::Foo { <|>bar: () }; } ", ); + check_assist_not_applicable( + change_visibility, + r"//- /lib.rs + mod foo; + fn main() { foo::Foo { <|>bar: () }; } + //- /foo.rs + pub struct Foo { pub bar: () } + ", + ); + } + + #[test] + #[ignore] + // FIXME reenable this test when `Semantics::resolve_record_field` works with union fields + fn change_visibility_of_union_field_via_path() { + check_assist( + change_visibility, + r"mod foo { pub union Foo { bar: (), } } + fn main() { foo::Foo { <|>bar: () }; } ", + r"mod foo { pub union Foo { <|>pub(crate) bar: (), } } + fn main() { foo::Foo { bar: () }; } ", + ); + check_assist( + change_visibility, + r"//- /lib.rs + mod foo; + fn main() { foo::Foo { <|>bar: () }; } + //- /foo.rs + pub union Foo { bar: () } + ", + r"pub union Foo { <|>pub(crate) bar: () } + +", + ); + check_assist_not_applicable( + change_visibility, + r"mod foo { pub union Foo { pub bar: (), } } + fn main() { foo::Foo { <|>bar: () }; } ", + ); + check_assist_not_applicable( + change_visibility, + r"//- /lib.rs + mod foo; + fn main() { foo::Foo { <|>bar: () }; } + //- /foo.rs + pub union Foo { pub bar: () } + ", + ); } #[test] @@ -500,24 +654,57 @@ mod tests { fn change_visibility_of_module_declaration_in_other_file_via_path() { check_assist( change_visibility, - r" - //- /main.rs - mod foo; - fn main() { foo::bar<|>>::baz(); } + r"//- /main.rs + mod foo; + fn main() { foo::bar<|>>::baz(); } - //- /foo.rs - mod bar { - pub fn baz() {} - } - ", + //- /foo.rs + mod bar { + pub fn baz() {} + }", r"<|>pub(crate) mod bar { pub fn baz() {} } - ", ); } + #[test] + #[ignore] + // FIXME handle reexports properly + fn change_visibility_of_reexport() { + check_assist( + change_visibility, + r" + mod foo { + use bar::Baz; + mod bar { pub(super) struct Baz; } + } + foo::Baz<|> + ", + r" + mod foo { + <|>pub(crate) use bar::Baz; + mod bar { pub(super) struct Baz; } + } + foo::Baz + ", + ) + } + + #[test] + fn adds_pub_when_target_is_in_another_crate() { + check_assist( + change_visibility, + r"//- /main.rs crate:a deps:foo + foo::Bar<|> + //- /lib.rs crate:foo + struct Bar;", + r"<|>pub struct Bar; +", + ) + } + #[test] fn change_visibility_target() { check_assist_target(change_visibility, "<|>fn foo() {}", "fn"); -- cgit v1.2.3 From ecac5d7de2192873c24b7b06d4964d188d8abe6a Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 20 May 2020 12:59:20 +0200 Subject: Switch to new magic marks --- crates/ra_assists/src/handlers/change_visibility.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'crates/ra_assists/src/handlers/change_visibility.rs') diff --git a/crates/ra_assists/src/handlers/change_visibility.rs b/crates/ra_assists/src/handlers/change_visibility.rs index 40cf4b422..71d55e0c3 100644 --- a/crates/ra_assists/src/handlers/change_visibility.rs +++ b/crates/ra_assists/src/handlers/change_visibility.rs @@ -9,7 +9,7 @@ use ra_syntax::{ }; use hir::{db::HirDatabase, HasSource, HasVisibility, PathResolution}; -use test_utils::tested_by; +use test_utils::mark; use crate::{AssistContext, AssistId, Assists}; use ra_db::FileId; @@ -55,7 +55,7 @@ fn add_vis(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { } else if let Some(field_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); + mark::hit!(change_visibility_field_false_positive); return None; } if field.visibility().is_some() { @@ -255,7 +255,7 @@ fn change_vis(acc: &mut Assists, vis: ast::Visibility) -> Option<()> { #[cfg(test)] mod tests { - use test_utils::covers; + use test_utils::mark; use crate::tests::{check_assist, check_assist_not_applicable, check_assist_target}; @@ -288,7 +288,7 @@ mod tests { #[test] fn change_visibility_field_false_positive() { - covers!(change_visibility_field_false_positive); + mark::check!(change_visibility_field_false_positive); check_assist_not_applicable( change_visibility, r"struct S { field: [(); { let <|>x = ();}] }", -- cgit v1.2.3 From cec773926f08e2d46b05d923165f8e73c420aa8c Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 20 May 2020 13:33:13 +0200 Subject: Split change_ and fix_ visibility assists --- .../ra_assists/src/handlers/change_visibility.rs | 507 +-------------------- 1 file changed, 1 insertion(+), 506 deletions(-) (limited to 'crates/ra_assists/src/handlers/change_visibility.rs') diff --git a/crates/ra_assists/src/handlers/change_visibility.rs b/crates/ra_assists/src/handlers/change_visibility.rs index 71d55e0c3..1d9b8e645 100644 --- a/crates/ra_assists/src/handlers/change_visibility.rs +++ b/crates/ra_assists/src/handlers/change_visibility.rs @@ -5,14 +5,11 @@ use ra_syntax::{ ATTR, COMMENT, CONST_DEF, ENUM_DEF, FN_DEF, MODULE, STRUCT_DEF, TRAIT_DEF, VISIBILITY, WHITESPACE, }, - SyntaxNode, TextRange, TextSize, T, + SyntaxNode, TextSize, T, }; - -use hir::{db::HirDatabase, HasSource, HasVisibility, PathResolution}; use test_utils::mark; use crate::{AssistContext, AssistId, Assists}; -use ra_db::FileId; // Assist: change_visibility // @@ -30,8 +27,6 @@ pub(crate) fn change_visibility(acc: &mut Assists, ctx: &AssistContext) -> Optio return change_vis(acc, vis); } add_vis(acc, ctx) - .or_else(|| add_vis_to_referenced_module_def(acc, ctx)) - .or_else(|| add_vis_to_referenced_record_field(acc, ctx)) } fn add_vis(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { @@ -77,143 +72,6 @@ fn add_vis(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { }) } -fn add_vis_to_referenced_module_def(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { - let path: ast::Path = ctx.find_node_at_offset()?; - let path_res = ctx.sema.resolve_path(&path)?; - let def = match path_res { - PathResolution::Def(def) => def, - _ => return None, - }; - - let current_module = ctx.sema.scope(&path.syntax()).module()?; - let target_module = def.module(ctx.db)?; - - let vis = target_module.visibility_of(ctx.db, &def)?; - if vis.is_visible_from(ctx.db, current_module.into()) { - return None; - }; - - let (offset, target, target_file, target_name) = target_data_for_def(ctx.db, def)?; - - let missing_visibility = - if current_module.krate() == target_module.krate() { "pub(crate)" } else { "pub" }; - - let assist_label = match target_name { - None => format!("Change visibility to {}", missing_visibility), - Some(name) => format!("Change visibility of {} to {}", name, missing_visibility), - }; - - acc.add(AssistId("change_visibility"), assist_label, target, |edit| { - edit.set_file(target_file); - edit.insert(offset, format!("{} ", missing_visibility)); - edit.set_cursor(offset); - }) -} - -fn add_vis_to_referenced_record_field(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { - let record_field: ast::RecordField = ctx.find_node_at_offset()?; - let (record_field_def, _) = ctx.sema.resolve_record_field(&record_field)?; - - let current_module = ctx.sema.scope(record_field.syntax()).module()?; - let visibility = record_field_def.visibility(ctx.db); - if visibility.is_visible_from(ctx.db, current_module.into()) { - return None; - } - - let parent = record_field_def.parent_def(ctx.db); - let parent_name = parent.name(ctx.db); - let target_module = parent.module(ctx.db); - - let in_file_source = record_field_def.source(ctx.db); - let (offset, target) = match in_file_source.value { - hir::FieldSource::Named(it) => { - let s = it.syntax(); - (vis_offset(s), s.text_range()) - } - hir::FieldSource::Pos(it) => { - let s = it.syntax(); - (vis_offset(s), s.text_range()) - } - }; - - let missing_visibility = - if current_module.krate() == target_module.krate() { "pub(crate)" } else { "pub" }; - let target_file = in_file_source.file_id.original_file(ctx.db); - - let target_name = record_field_def.name(ctx.db); - let assist_label = - format!("Change visibility of {}.{} to {}", parent_name, target_name, missing_visibility); - - acc.add(AssistId("change_visibility"), assist_label, target, |edit| { - edit.set_file(target_file); - edit.insert(offset, format!("{} ", missing_visibility)); - edit.set_cursor(offset) - }) -} - -fn target_data_for_def( - db: &dyn HirDatabase, - def: hir::ModuleDef, -) -> Option<(TextSize, TextRange, FileId, Option)> { - fn offset_target_and_file_id( - db: &dyn HirDatabase, - x: S, - ) -> (TextSize, TextRange, FileId) - where - S: HasSource, - Ast: AstNode, - { - let source = x.source(db); - let in_file_syntax = source.syntax(); - let file_id = in_file_syntax.file_id; - let syntax = in_file_syntax.value; - (vis_offset(syntax), syntax.text_range(), file_id.original_file(db.upcast())) - } - - let target_name; - let (offset, target, target_file) = match def { - hir::ModuleDef::Function(f) => { - target_name = Some(f.name(db)); - offset_target_and_file_id(db, f) - } - hir::ModuleDef::Adt(adt) => { - target_name = Some(adt.name(db)); - match adt { - hir::Adt::Struct(s) => offset_target_and_file_id(db, s), - hir::Adt::Union(u) => offset_target_and_file_id(db, u), - hir::Adt::Enum(e) => offset_target_and_file_id(db, e), - } - } - hir::ModuleDef::Const(c) => { - target_name = c.name(db); - offset_target_and_file_id(db, c) - } - hir::ModuleDef::Static(s) => { - target_name = s.name(db); - offset_target_and_file_id(db, s) - } - hir::ModuleDef::Trait(t) => { - target_name = Some(t.name(db)); - offset_target_and_file_id(db, t) - } - hir::ModuleDef::TypeAlias(t) => { - target_name = Some(t.name(db)); - offset_target_and_file_id(db, t) - } - hir::ModuleDef::Module(m) => { - target_name = m.name(db); - let in_file_source = m.declaration_source(db)?; - let file_id = in_file_source.file_id.original_file(db.upcast()); - let syntax = in_file_source.value.syntax(); - (vis_offset(syntax), syntax.text_range(), file_id) - } - // Enum variants can't be private, we can't modify builtin types - hir::ModuleDef::EnumVariant(_) | hir::ModuleDef::BuiltinType(_) => return None, - }; - - Some((offset, target, target_file, target_name)) -} - fn vis_offset(node: &SyntaxNode) -> TextSize { node.children_with_tokens() .skip_while(|it| match it.kind() { @@ -333,193 +191,6 @@ mod tests { ) } - #[test] - fn change_visibility_of_fn_via_path() { - check_assist( - change_visibility, - r"mod foo { fn foo() {} } - fn main() { foo::foo<|>() } ", - r"mod foo { <|>pub(crate) fn foo() {} } - fn main() { foo::foo() } ", - ); - check_assist_not_applicable( - change_visibility, - r"mod foo { pub fn foo() {} } - fn main() { foo::foo<|>() } ", - ) - } - - #[test] - fn change_visibility_of_adt_in_submodule_via_path() { - check_assist( - change_visibility, - r"mod foo { struct Foo; } - fn main() { foo::Foo<|> } ", - r"mod foo { <|>pub(crate) struct Foo; } - fn main() { foo::Foo } ", - ); - check_assist_not_applicable( - change_visibility, - r"mod foo { pub struct Foo; } - fn main() { foo::Foo<|> } ", - ); - check_assist( - change_visibility, - r"mod foo { enum Foo; } - fn main() { foo::Foo<|> } ", - r"mod foo { <|>pub(crate) enum Foo; } - fn main() { foo::Foo } ", - ); - check_assist_not_applicable( - change_visibility, - r"mod foo { pub enum Foo; } - fn main() { foo::Foo<|> } ", - ); - check_assist( - change_visibility, - r"mod foo { union Foo; } - fn main() { foo::Foo<|> } ", - r"mod foo { <|>pub(crate) union Foo; } - fn main() { foo::Foo } ", - ); - check_assist_not_applicable( - change_visibility, - r"mod foo { pub union Foo; } - fn main() { foo::Foo<|> } ", - ); - } - - #[test] - fn change_visibility_of_adt_in_other_file_via_path() { - check_assist( - change_visibility, - r" - //- /main.rs - mod foo; - fn main() { foo::Foo<|> } - - //- /foo.rs - struct Foo; - ", - r"<|>pub(crate) struct Foo; - -", - ); - } - - #[test] - fn change_visibility_of_struct_field_via_path() { - check_assist( - change_visibility, - r"mod foo { pub struct Foo { bar: (), } } - fn main() { foo::Foo { <|>bar: () }; } ", - r"mod foo { pub struct Foo { <|>pub(crate) bar: (), } } - fn main() { foo::Foo { bar: () }; } ", - ); - check_assist( - change_visibility, - r"//- /lib.rs - mod foo; - fn main() { foo::Foo { <|>bar: () }; } - //- /foo.rs - pub struct Foo { bar: () } - ", - r"pub struct Foo { <|>pub(crate) bar: () } - -", - ); - check_assist_not_applicable( - change_visibility, - r"mod foo { pub struct Foo { pub bar: (), } } - fn main() { foo::Foo { <|>bar: () }; } ", - ); - check_assist_not_applicable( - change_visibility, - r"//- /lib.rs - mod foo; - fn main() { foo::Foo { <|>bar: () }; } - //- /foo.rs - pub struct Foo { pub bar: () } - ", - ); - } - - #[test] - fn change_visibility_of_enum_variant_field_via_path() { - check_assist( - change_visibility, - r"mod foo { pub enum Foo { Bar { bar: () } } } - fn main() { foo::Foo::Bar { <|>bar: () }; } ", - r"mod foo { pub enum Foo { Bar { <|>pub(crate) bar: () } } } - fn main() { foo::Foo::Bar { bar: () }; } ", - ); - check_assist( - change_visibility, - r"//- /lib.rs - mod foo; - fn main() { foo::Foo::Bar { <|>bar: () }; } - //- /foo.rs - pub enum Foo { Bar { bar: () } } - ", - r"pub enum Foo { Bar { <|>pub(crate) bar: () } } - -", - ); - check_assist_not_applicable( - change_visibility, - r"mod foo { pub struct Foo { pub bar: (), } } - fn main() { foo::Foo { <|>bar: () }; } ", - ); - check_assist_not_applicable( - change_visibility, - r"//- /lib.rs - mod foo; - fn main() { foo::Foo { <|>bar: () }; } - //- /foo.rs - pub struct Foo { pub bar: () } - ", - ); - } - - #[test] - #[ignore] - // FIXME reenable this test when `Semantics::resolve_record_field` works with union fields - fn change_visibility_of_union_field_via_path() { - check_assist( - change_visibility, - r"mod foo { pub union Foo { bar: (), } } - fn main() { foo::Foo { <|>bar: () }; } ", - r"mod foo { pub union Foo { <|>pub(crate) bar: (), } } - fn main() { foo::Foo { bar: () }; } ", - ); - check_assist( - change_visibility, - r"//- /lib.rs - mod foo; - fn main() { foo::Foo { <|>bar: () }; } - //- /foo.rs - pub union Foo { bar: () } - ", - r"pub union Foo { <|>pub(crate) bar: () } - -", - ); - check_assist_not_applicable( - change_visibility, - r"mod foo { pub union Foo { pub bar: (), } } - fn main() { foo::Foo { <|>bar: () }; } ", - ); - check_assist_not_applicable( - change_visibility, - r"//- /lib.rs - mod foo; - fn main() { foo::Foo { <|>bar: () }; } - //- /foo.rs - pub union Foo { pub bar: () } - ", - ); - } - #[test] fn not_applicable_for_enum_variants() { check_assist_not_applicable( @@ -529,182 +200,6 @@ mod tests { ); } - #[test] - fn change_visibility_of_const_via_path() { - check_assist( - change_visibility, - r"mod foo { const FOO: () = (); } - fn main() { foo::FOO<|> } ", - r"mod foo { <|>pub(crate) const FOO: () = (); } - fn main() { foo::FOO } ", - ); - check_assist_not_applicable( - change_visibility, - r"mod foo { pub const FOO: () = (); } - fn main() { foo::FOO<|> } ", - ); - } - - #[test] - fn change_visibility_of_static_via_path() { - check_assist( - change_visibility, - r"mod foo { static FOO: () = (); } - fn main() { foo::FOO<|> } ", - r"mod foo { <|>pub(crate) static FOO: () = (); } - fn main() { foo::FOO } ", - ); - check_assist_not_applicable( - change_visibility, - r"mod foo { pub static FOO: () = (); } - fn main() { foo::FOO<|> } ", - ); - } - - #[test] - fn change_visibility_of_trait_via_path() { - check_assist( - change_visibility, - r"mod foo { trait Foo { fn foo(&self) {} } } - fn main() { let x: &dyn foo::<|>Foo; } ", - r"mod foo { <|>pub(crate) trait Foo { fn foo(&self) {} } } - fn main() { let x: &dyn foo::Foo; } ", - ); - check_assist_not_applicable( - change_visibility, - r"mod foo { pub trait Foo { fn foo(&self) {} } } - fn main() { let x: &dyn foo::Foo<|>; } ", - ); - } - - #[test] - fn change_visibility_of_type_alias_via_path() { - check_assist( - change_visibility, - r"mod foo { type Foo = (); } - fn main() { let x: foo::Foo<|>; } ", - r"mod foo { <|>pub(crate) type Foo = (); } - fn main() { let x: foo::Foo; } ", - ); - check_assist_not_applicable( - change_visibility, - r"mod foo { pub type Foo = (); } - fn main() { let x: foo::Foo<|>; } ", - ); - } - - #[test] - fn change_visibility_of_module_via_path() { - check_assist( - change_visibility, - r"mod foo { mod bar { fn bar() {} } } - fn main() { foo::bar<|>::bar(); } ", - r"mod foo { <|>pub(crate) mod bar { fn bar() {} } } - fn main() { foo::bar::bar(); } ", - ); - - check_assist( - change_visibility, - r" - //- /main.rs - mod foo; - fn main() { foo::bar<|>::baz(); } - - //- /foo.rs - mod bar { - pub fn baz() {} - } - ", - r"<|>pub(crate) mod bar { - pub fn baz() {} -} - -", - ); - - check_assist_not_applicable( - change_visibility, - r"mod foo { pub mod bar { pub fn bar() {} } } - fn main() { foo::bar<|>::bar(); } ", - ); - } - - #[test] - fn change_visibility_of_inline_module_in_other_file_via_path() { - check_assist( - change_visibility, - r" - //- /main.rs - mod foo; - fn main() { foo::bar<|>::baz(); } - - //- /foo.rs - mod bar; - - //- /foo/bar.rs - pub fn baz() {} - } - ", - r"<|>pub(crate) mod bar; -", - ); - } - - #[test] - fn change_visibility_of_module_declaration_in_other_file_via_path() { - check_assist( - change_visibility, - r"//- /main.rs - mod foo; - fn main() { foo::bar<|>>::baz(); } - - //- /foo.rs - mod bar { - pub fn baz() {} - }", - r"<|>pub(crate) mod bar { - pub fn baz() {} -} -", - ); - } - - #[test] - #[ignore] - // FIXME handle reexports properly - fn change_visibility_of_reexport() { - check_assist( - change_visibility, - r" - mod foo { - use bar::Baz; - mod bar { pub(super) struct Baz; } - } - foo::Baz<|> - ", - r" - mod foo { - <|>pub(crate) use bar::Baz; - mod bar { pub(super) struct Baz; } - } - foo::Baz - ", - ) - } - - #[test] - fn adds_pub_when_target_is_in_another_crate() { - check_assist( - change_visibility, - r"//- /main.rs crate:a deps:foo - foo::Bar<|> - //- /lib.rs crate:foo - struct Bar;", - r"<|>pub struct Bar; -", - ) - } - #[test] fn change_visibility_target() { check_assist_target(change_visibility, "<|>fn foo() {}", "fn"); -- cgit v1.2.3 From a622b54ac029994328a99e30ee3169c21de498b7 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 20 May 2020 13:48:31 +0200 Subject: Don't set cursor in change_visibility --- crates/ra_assists/src/handlers/change_visibility.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'crates/ra_assists/src/handlers/change_visibility.rs') diff --git a/crates/ra_assists/src/handlers/change_visibility.rs b/crates/ra_assists/src/handlers/change_visibility.rs index 1d9b8e645..fbe459c9c 100644 --- a/crates/ra_assists/src/handlers/change_visibility.rs +++ b/crates/ra_assists/src/handlers/change_visibility.rs @@ -68,7 +68,6 @@ fn add_vis(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { acc.add(AssistId("change_visibility"), "Change visibility to pub(crate)", target, |edit| { edit.insert(offset, "pub(crate) "); - edit.set_cursor(offset); }) } @@ -92,7 +91,6 @@ fn change_vis(acc: &mut Assists, vis: ast::Visibility) -> Option<()> { target, |edit| { edit.replace(vis.syntax().text_range(), "pub(crate)"); - edit.set_cursor(vis.syntax().text_range().start()) }, ); } @@ -104,7 +102,6 @@ fn change_vis(acc: &mut Assists, vis: ast::Visibility) -> Option<()> { target, |edit| { edit.replace(vis.syntax().text_range(), "pub"); - edit.set_cursor(vis.syntax().text_range().start()); }, ); } @@ -122,15 +119,15 @@ mod tests { #[test] fn change_visibility_adds_pub_crate_to_items() { check_assist(change_visibility, "<|>fn foo() {}", "<|>pub(crate) fn foo() {}"); - check_assist(change_visibility, "f<|>n foo() {}", "<|>pub(crate) fn foo() {}"); + check_assist(change_visibility, "f<|>n foo() {}", "pub(crate) f<|>n foo() {}"); check_assist(change_visibility, "<|>struct Foo {}", "<|>pub(crate) struct Foo {}"); check_assist(change_visibility, "<|>mod foo {}", "<|>pub(crate) mod foo {}"); check_assist(change_visibility, "<|>trait Foo {}", "<|>pub(crate) trait Foo {}"); - check_assist(change_visibility, "m<|>od {}", "<|>pub(crate) mod {}"); + check_assist(change_visibility, "m<|>od {}", "pub(crate) m<|>od {}"); check_assist( change_visibility, "unsafe f<|>n foo() {}", - "<|>pub(crate) unsafe fn foo() {}", + "pub(crate) unsafe f<|>n foo() {}", ); } -- cgit v1.2.3 From 65fa5864105280267e6ccdaa61957cd9953e444e Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 20 May 2020 22:55:37 +0200 Subject: Relax cursor position tests in assists Those will be replaced with snippets anyway --- .../ra_assists/src/handlers/change_visibility.rs | 30 ++++++++++------------ 1 file changed, 13 insertions(+), 17 deletions(-) (limited to 'crates/ra_assists/src/handlers/change_visibility.rs') diff --git a/crates/ra_assists/src/handlers/change_visibility.rs b/crates/ra_assists/src/handlers/change_visibility.rs index fbe459c9c..c21d75be0 100644 --- a/crates/ra_assists/src/handlers/change_visibility.rs +++ b/crates/ra_assists/src/handlers/change_visibility.rs @@ -118,17 +118,13 @@ mod tests { #[test] fn change_visibility_adds_pub_crate_to_items() { - check_assist(change_visibility, "<|>fn foo() {}", "<|>pub(crate) fn foo() {}"); - check_assist(change_visibility, "f<|>n foo() {}", "pub(crate) f<|>n foo() {}"); - check_assist(change_visibility, "<|>struct Foo {}", "<|>pub(crate) struct Foo {}"); - check_assist(change_visibility, "<|>mod foo {}", "<|>pub(crate) mod foo {}"); - check_assist(change_visibility, "<|>trait Foo {}", "<|>pub(crate) trait Foo {}"); - check_assist(change_visibility, "m<|>od {}", "pub(crate) m<|>od {}"); - check_assist( - change_visibility, - "unsafe f<|>n foo() {}", - "pub(crate) unsafe f<|>n foo() {}", - ); + check_assist(change_visibility, "<|>fn foo() {}", "pub(crate) fn foo() {}"); + check_assist(change_visibility, "f<|>n foo() {}", "pub(crate) fn foo() {}"); + check_assist(change_visibility, "<|>struct Foo {}", "pub(crate) struct Foo {}"); + check_assist(change_visibility, "<|>mod foo {}", "pub(crate) mod foo {}"); + check_assist(change_visibility, "<|>trait Foo {}", "pub(crate) trait Foo {}"); + check_assist(change_visibility, "m<|>od {}", "pub(crate) mod {}"); + check_assist(change_visibility, "unsafe f<|>n foo() {}", "pub(crate) unsafe fn foo() {}"); } #[test] @@ -136,9 +132,9 @@ mod tests { check_assist( change_visibility, r"struct S { <|>field: u32 }", - r"struct S { <|>pub(crate) field: u32 }", + r"struct S { pub(crate) field: u32 }", ); - check_assist(change_visibility, r"struct S ( <|>u32 )", r"struct S ( <|>pub(crate) u32 )"); + check_assist(change_visibility, r"struct S ( <|>u32 )", r"struct S ( pub(crate) u32 )"); } #[test] @@ -152,17 +148,17 @@ mod tests { #[test] fn change_visibility_pub_to_pub_crate() { - check_assist(change_visibility, "<|>pub fn foo() {}", "<|>pub(crate) fn foo() {}") + check_assist(change_visibility, "<|>pub fn foo() {}", "pub(crate) fn foo() {}") } #[test] fn change_visibility_pub_crate_to_pub() { - check_assist(change_visibility, "<|>pub(crate) fn foo() {}", "<|>pub fn foo() {}") + check_assist(change_visibility, "<|>pub(crate) fn foo() {}", "pub fn foo() {}") } #[test] fn change_visibility_const() { - check_assist(change_visibility, "<|>const FOO = 3u8;", "<|>pub(crate) const FOO = 3u8;"); + check_assist(change_visibility, "<|>const FOO = 3u8;", "pub(crate) const FOO = 3u8;"); } #[test] @@ -183,7 +179,7 @@ mod tests { // comments #[derive(Debug)] - <|>pub(crate) struct Foo; + pub(crate) struct Foo; ", ) } -- cgit v1.2.3