From f55441f7ed944fa0fd56a2a225fb72ee5a5044c0 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 4 Aug 2021 14:17:56 +0200 Subject: [PATCH 1/2] Document reference highlighting mod --- crates/ide/src/syntax_highlighting.rs | 3 ++- crates/ide/src/syntax_highlighting/tags.rs | 12 ++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/crates/ide/src/syntax_highlighting.rs b/crates/ide/src/syntax_highlighting.rs index b8e62a1a860..c6b8a77be73 100644 --- a/crates/ide/src/syntax_highlighting.rs +++ b/crates/ide/src/syntax_highlighting.rs @@ -138,8 +138,9 @@ pub struct HlRange { // injected:: Emitted for doc-string injected highlighting like rust source blocks in documentation. // intraDocLink:: Emitted for intra doc links in doc-strings. // library:: Emitted for items that are defined outside of the current crate. +// mutable:: Emitted for mutable locals and statics as well as functions taking `&mut self`. // public:: Emitted for items that are from the current crate and are `pub`. -// mutable:: Emitted for mutable locals and statics. +// reference: Emitted for locals behind a reference and functions taking `self` by reference. // static:: Emitted for "static" functions, also known as functions that do not take a `self` param, as well as statics and consts. // trait:: Emitted for associated trait items. // unsafe:: Emitted for unsafe operations, like unsafe function calls, as well as the `unsafe` token. diff --git a/crates/ide/src/syntax_highlighting/tags.rs b/crates/ide/src/syntax_highlighting/tags.rs index 95c6a3d5af8..dc5dfec1165 100644 --- a/crates/ide/src/syntax_highlighting/tags.rs +++ b/crates/ide/src/syntax_highlighting/tags.rs @@ -45,6 +45,8 @@ pub enum HlTag { pub enum HlMod { /// Used for items in traits and impls. Associated = 0, + /// Used with keywords like `async` and `await`. + Async, /// Used to differentiate individual elements within attributes. Attribute, /// Callable item or value. @@ -62,20 +64,18 @@ pub enum HlMod { Injected, /// Used for intra doc links in doc injection. IntraDocLink, + /// Used for items from other crates. + Library, /// Mutable binding. Mutable, + /// Used for public items. + Public, /// Immutable reference. Reference, /// Used for associated functions. Static, /// Used for items in traits and trait impls. Trait, - /// Used with keywords like `async` and `await`. - Async, - /// Used for items from other crates. - Library, - /// Used for public items. - Public, // Keep this last! /// Used for unsafe functions, unsafe traits, mutable statics, union accesses and unsafe operations. Unsafe, From 01413dd7d4d04ba3c1324bb149ad25b68d93dd98 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 4 Aug 2021 20:00:45 +0200 Subject: [PATCH 2/2] extract_function does not move locals defined outside of loops --- .../src/handlers/extract_function.rs | 132 ++++++++++++++++-- crates/test_utils/src/minicore.rs | 23 +++ 2 files changed, 141 insertions(+), 14 deletions(-) diff --git a/crates/ide_assists/src/handlers/extract_function.rs b/crates/ide_assists/src/handlers/extract_function.rs index 2ff1511f528..bae62908a19 100644 --- a/crates/ide_assists/src/handlers/extract_function.rs +++ b/crates/ide_assists/src/handlers/extract_function.rs @@ -96,7 +96,8 @@ pub(crate) fn extract_function(acc: &mut Assists, ctx: &AssistContext) -> Option return; } - let params = body.extracted_function_params(ctx, locals_used.iter().copied()); + let params = + body.extracted_function_params(ctx, &container_info, locals_used.iter().copied()); let fun = Function { name: "fun_name".to_string(), @@ -183,8 +184,8 @@ struct Function { struct Param { var: Local, ty: hir::Type, - has_usages_afterwards: bool, - has_mut_inside_body: bool, + move_local: bool, + requires_mut: bool, is_copy: bool, } @@ -226,6 +227,7 @@ struct ControlFlow { struct ContainerInfo { is_const: bool, is_in_tail: bool, + parent_loop: Option, /// The function's return type, const's type etc. ret_type: Option, } @@ -335,11 +337,11 @@ fn is_ref(&self) -> bool { impl Param { fn kind(&self) -> ParamKind { - match (self.has_usages_afterwards, self.has_mut_inside_body, self.is_copy) { - (true, true, _) => ParamKind::MutRef, - (true, false, false) => ParamKind::SharedRef, - (false, true, _) => ParamKind::MutValue, - (true, false, true) | (false, false, _) => ParamKind::Value, + match (self.move_local, self.requires_mut, self.is_copy) { + (false, true, _) => ParamKind::MutRef, + (false, false, false) => ParamKind::SharedRef, + (true, true, _) => ParamKind::MutValue, + (_, false, _) => ParamKind::Value, } } @@ -622,6 +624,15 @@ fn analyze( fn analyze_container(&self, sema: &Semantics) -> Option { let mut ancestors = self.parent()?.ancestors(); let infer_expr_opt = |expr| sema.type_of_expr(&expr?).map(TypeInfo::adjusted); + let mut parent_loop = None; + let mut set_parent_loop = |loop_: &dyn ast::LoopBodyOwner| { + if loop_ + .loop_body() + .map_or(false, |it| it.syntax().text_range().contains_range(self.text_range())) + { + parent_loop.get_or_insert(loop_.syntax().clone()); + } + }; let (is_const, expr, ty) = loop { let anc = ancestors.next()?; break match_ast! { @@ -658,6 +669,18 @@ fn analyze_container(&self, sema: &Semantics) -> Option return None, ast::Meta(__) => return None, + ast::LoopExpr(it) => { + set_parent_loop(&it); + continue; + }, + ast::ForExpr(it) => { + set_parent_loop(&it); + continue; + }, + ast::WhileExpr(it) => { + set_parent_loop(&it); + continue; + }, _ => continue, } }; @@ -670,7 +693,7 @@ fn analyze_container(&self, sema: &Semantics) -> Option Option { @@ -780,34 +803,38 @@ fn external_control_flow( Some(ControlFlow { kind, is_async, is_unsafe: _is_unsafe }) } + /// find variables that should be extracted as params /// /// Computes additional info that affects param type and mutability fn extracted_function_params( &self, ctx: &AssistContext, + container_info: &ContainerInfo, locals: impl Iterator, ) -> Vec { locals .map(|local| (local, local.source(ctx.db()))) .filter(|(_, src)| is_defined_outside_of_body(ctx, self, src)) .filter_map(|(local, src)| { - if src.value.is_left() { - Some(local) + if let Either::Left(src) = src.value { + Some((local, src)) } else { stdx::never!(false, "Local::is_self returned false, but source is SelfParam"); None } }) - .map(|var| { + .map(|(var, src)| { let usages = LocalUsages::find_local_usages(ctx, var); let ty = var.ty(ctx.db()); let is_copy = ty.is_copy(ctx.db()); Param { var, ty, - has_usages_afterwards: self.has_usages_after_body(&usages), - has_mut_inside_body: has_exclusive_usages(ctx, &usages, self), + move_local: container_info.parent_loop.as_ref().map_or(true, |it| { + it.text_range().contains_range(src.syntax().text_range()) + }) && !self.has_usages_after_body(&usages), + requires_mut: has_exclusive_usages(ctx, &usages, self), is_copy, } }) @@ -4009,6 +4036,83 @@ const fn $0fun_name() { const fn $0fun_name() { () } +"#, + ); + } + + #[test] + fn extract_does_not_move_outer_loop_vars() { + check_assist( + extract_function, + r#" +fn foo() { + let mut x = 5; + for _ in 0..10 { + $0x += 1;$0 + } +} +"#, + r#" +fn foo() { + let mut x = 5; + for _ in 0..10 { + fun_name(&mut x); + } +} + +fn $0fun_name(x: &mut i32) { + *x += 1; +} +"#, + ); + check_assist( + extract_function, + r#" +fn foo() { + for _ in 0..10 { + let mut x = 5; + $0x += 1;$0 + } +} +"#, + r#" +fn foo() { + for _ in 0..10 { + let mut x = 5; + fun_name(x); + } +} + +fn $0fun_name(mut x: i32) { + x += 1; +} +"#, + ); + check_assist( + extract_function, + r#" +fn foo() { + loop { + let mut x = 5; + for _ in 0..10 { + $0x += 1;$0 + } + } +} +"#, + r#" +fn foo() { + loop { + let mut x = 5; + for _ in 0..10 { + fun_name(&mut x); + } + } +} + +fn $0fun_name(x: &mut i32) { + *x += 1; +} "#, ); } diff --git a/crates/test_utils/src/minicore.rs b/crates/test_utils/src/minicore.rs index 8e212523fb6..2532738562b 100644 --- a/crates/test_utils/src/minicore.rs +++ b/crates/test_utils/src/minicore.rs @@ -17,6 +17,7 @@ //! deref_mut: deref //! index: sized //! fn: +//! try: //! pin: //! future: pin //! option: @@ -266,6 +267,28 @@ pub trait FnOnce { } pub use self::function::{Fn, FnMut, FnOnce}; // endregion:fn + // region:try + mod try_ { + pub enum ControlFlow { + Continue(C), + Break(B), + } + pub trait FromResidual { + #[lang = "from_residual"] + fn from_residual(residual: R) -> Self; + } + #[lang = "try"] + pub trait Try: FromResidual { + type Output; + type Residual; + #[lang = "from_output"] + fn from_output(output: Self::Output) -> Self; + #[lang = "branch"] + fn branch(self) -> ControlFlow; + } + } + pub use self::try_::{ControlFlow, FromResidual, Try}; + // endregion:try } // region:eq