Unnamed repository; edit this file 'description' to name the repository.
Merge pull request #21262 from dfireBird/hir-locals-used
feat: Implementation of locals_used in HIR level
| -rw-r--r-- | crates/hir-expand/src/name.rs | 4 | ||||
| -rw-r--r-- | crates/hir/src/semantics.rs | 103 | ||||
| -rw-r--r-- | crates/hir/src/source_analyzer.rs | 2 | ||||
| -rw-r--r-- | crates/ide-assists/src/handlers/extract_function.rs | 267 | ||||
| -rw-r--r-- | crates/ide-db/src/search.rs | 2 |
5 files changed, 282 insertions, 96 deletions
diff --git a/crates/hir-expand/src/name.rs b/crates/hir-expand/src/name.rs index 217d991d11..1e5efb6e14 100644 --- a/crates/hir-expand/src/name.rs +++ b/crates/hir-expand/src/name.rs @@ -197,6 +197,10 @@ impl Name { pub fn symbol(&self) -> &Symbol { &self.symbol } + + pub fn is_generated(&self) -> bool { + self.as_str().starts_with("<ra@gennew>") + } } struct Display<'a> { diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs index b15e642daa..80fac3512e 100644 --- a/crates/hir/src/semantics.rs +++ b/crates/hir/src/semantics.rs @@ -10,13 +10,14 @@ use std::{ ops::{self, ControlFlow, Not}, }; +use base_db::FxIndexSet; use either::Either; use hir_def::{ DefWithBodyId, FunctionId, MacroId, StructId, TraitId, VariantId, expr_store::{Body, ExprOrPatSource, HygieneId, path::Path}, hir::{BindingId, Expr, ExprId, ExprOrPatId, Pat}, nameres::{ModuleOrigin, crate_def_map}, - resolver::{self, HasResolver, Resolver, TypeNs}, + resolver::{self, HasResolver, Resolver, TypeNs, ValueNs}, type_ref::Mutability, }; use hir_expand::{ @@ -2192,6 +2193,106 @@ impl<'db> SemanticsImpl<'db> { self.cache(adt_source.value.syntax().ancestors().last().unwrap(), adt_source.file_id); ToDef::to_def(self, adt_source.as_ref()) } + + pub fn locals_used( + &self, + element: Either<&ast::Expr, &ast::StmtList>, + text_range: TextRange, + ) -> Option<FxIndexSet<Local>> { + let sa = self.analyze(element.either(|e| e.syntax(), |s| s.syntax()))?; + let store = sa.store()?; + let mut resolver = sa.resolver.clone(); + let def = resolver.body_owner()?; + + let is_not_generated = |path: &Path| { + !path.mod_path().and_then(|path| path.as_ident()).is_some_and(Name::is_generated) + }; + + let exprs = element.either( + |e| vec![e.clone()], + |stmts| { + let mut exprs: Vec<_> = stmts + .statements() + .filter(|stmt| text_range.contains_range(stmt.syntax().text_range())) + .filter_map(|stmt| match stmt { + ast::Stmt::ExprStmt(expr_stmt) => expr_stmt.expr().map(|e| vec![e]), + ast::Stmt::Item(_) => None, + ast::Stmt::LetStmt(let_stmt) => { + let init = let_stmt.initializer(); + let let_else = let_stmt + .let_else() + .and_then(|le| le.block_expr()) + .map(ast::Expr::BlockExpr); + + match (init, let_else) { + (Some(i), Some(le)) => Some(vec![i, le]), + (Some(i), _) => Some(vec![i]), + (_, Some(le)) => Some(vec![le]), + _ => None, + } + } + }) + .flatten() + .collect(); + + if let Some(tail_expr) = stmts.tail_expr() + && text_range.contains_range(tail_expr.syntax().text_range()) + { + exprs.push(tail_expr); + } + exprs + }, + ); + let mut exprs: Vec<_> = + exprs.into_iter().filter_map(|e| sa.expr_id(e).and_then(|e| e.as_expr())).collect(); + + let mut locals: FxIndexSet<Local> = FxIndexSet::default(); + let mut add_to_locals_used = |id, parent_expr| { + let path = match id { + ExprOrPatId::ExprId(expr_id) => { + if let Expr::Path(path) = &store[expr_id] { + Some(path) + } else { + None + } + } + ExprOrPatId::PatId(pat_id) => { + if let Pat::Path(path) = &store[pat_id] { + Some(path) + } else { + None + } + } + }; + + if let Some(path) = path + && is_not_generated(path) + { + let _ = resolver.update_to_inner_scope(self.db, def, parent_expr); + let hygiene = store.expr_or_pat_path_hygiene(id); + resolver.resolve_path_in_value_ns_fully(self.db, path, hygiene).inspect(|value| { + if let ValueNs::LocalBinding(id) = value { + locals.insert((def, *id).into()); + } + }); + } + }; + + while let Some(expr_id) = exprs.pop() { + if let Expr::Assignment { target, .. } = store[expr_id] { + store.walk_pats(target, &mut |id| { + add_to_locals_used(ExprOrPatId::PatId(id), expr_id) + }); + }; + store.walk_child_exprs(expr_id, |id| { + exprs.push(id); + }); + + add_to_locals_used(ExprOrPatId::ExprId(expr_id), expr_id) + } + + Some(locals) + } } // FIXME This can't be the best way to do this diff --git a/crates/hir/src/source_analyzer.rs b/crates/hir/src/source_analyzer.rs index 901c9e1575..b90eb97d87 100644 --- a/crates/hir/src/source_analyzer.rs +++ b/crates/hir/src/source_analyzer.rs @@ -240,7 +240,7 @@ impl<'db> SourceAnalyzer<'db> { ) } - fn expr_id(&self, expr: ast::Expr) -> Option<ExprOrPatId> { + pub(crate) fn expr_id(&self, expr: ast::Expr) -> Option<ExprOrPatId> { let src = InFile { file_id: self.file_id, value: expr }; self.store_sm()?.node_expr(src.as_ref()) } diff --git a/crates/ide-assists/src/handlers/extract_function.rs b/crates/ide-assists/src/handlers/extract_function.rs index 4b7314be46..231df9b5b3 100644 --- a/crates/ide-assists/src/handlers/extract_function.rs +++ b/crates/ide-assists/src/handlers/extract_function.rs @@ -9,14 +9,14 @@ use hir::{ use ide_db::{ FxIndexSet, RootDatabase, assists::GroupLabel, - defs::{Definition, NameRefClass}, + defs::Definition, famous_defs::FamousDefs, helpers::mod_path_to_ast, imports::insert_use::{ImportScope, insert_use}, search::{FileReference, ReferenceCategory, SearchScope}, source_change::SourceChangeBuilder, syntax_helpers::node_ext::{ - for_each_tail_expr, preorder_expr, walk_expr, walk_pat, walk_patterns_in_expr, + for_each_tail_expr, preorder_expr, walk_pat, walk_patterns_in_expr, }, }; use itertools::Itertools; @@ -687,29 +687,6 @@ impl FunctionBody { } } - fn walk_expr(&self, cb: &mut dyn FnMut(ast::Expr)) { - match self { - FunctionBody::Expr(expr) => walk_expr(expr, cb), - FunctionBody::Span { parent, text_range, .. } => { - parent - .statements() - .filter(|stmt| text_range.contains_range(stmt.syntax().text_range())) - .filter_map(|stmt| match stmt { - ast::Stmt::ExprStmt(expr_stmt) => expr_stmt.expr(), - ast::Stmt::Item(_) => None, - ast::Stmt::LetStmt(stmt) => stmt.initializer(), - }) - .for_each(|expr| walk_expr(&expr, cb)); - if let Some(expr) = parent - .tail_expr() - .filter(|it| text_range.contains_range(it.syntax().text_range())) - { - walk_expr(&expr, cb); - } - } - } - } - fn preorder_expr(&self, cb: &mut dyn FnMut(WalkEvent<ast::Expr>) -> bool) { match self { FunctionBody::Expr(expr) => preorder_expr(expr, cb), @@ -718,10 +695,24 @@ impl FunctionBody { .statements() .filter(|stmt| text_range.contains_range(stmt.syntax().text_range())) .filter_map(|stmt| match stmt { - ast::Stmt::ExprStmt(expr_stmt) => expr_stmt.expr(), + ast::Stmt::ExprStmt(expr_stmt) => expr_stmt.expr().map(|e| vec![e]), ast::Stmt::Item(_) => None, - ast::Stmt::LetStmt(stmt) => stmt.initializer(), + ast::Stmt::LetStmt(stmt) => { + let init = stmt.initializer(); + let let_else = stmt + .let_else() + .and_then(|le| le.block_expr()) + .map(ast::Expr::BlockExpr); + + match (init, let_else) { + (Some(i), Some(le)) => Some(vec![i, le]), + (Some(i), _) => Some(vec![i]), + (_, Some(le)) => Some(vec![le]), + _ => None, + } + } }) + .flatten() .for_each(|expr| preorder_expr(&expr, cb)); if let Some(expr) = parent .tail_expr() @@ -799,22 +790,14 @@ impl FunctionBody { let mut self_param = None; let mut res = FxIndexSet::default(); - fn local_from_name_ref( - sema: &Semantics<'_, RootDatabase>, - name_ref: ast::NameRef, - ) -> Option<hir::Local> { - match NameRefClass::classify(sema, &name_ref) { - Some( - NameRefClass::Definition(Definition::Local(local_ref), _) - | NameRefClass::FieldShorthand { local_ref, field_ref: _, adt_subst: _ }, - ) => Some(local_ref), - _ => None, - } - } + let (text_range, element) = match self { + FunctionBody::Expr(expr) => (expr.syntax().text_range(), Either::Left(expr)), + FunctionBody::Span { parent, text_range, .. } => (*text_range, Either::Right(parent)), + }; let mut add_name_if_local = |local_ref: Local| { - let InFile { file_id, value } = local_ref.primary_source(sema.db).source; // locals defined inside macros are not relevant to us + let InFile { file_id, value } = local_ref.primary_source(sema.db).source; if !file_id.is_macro() { match value { Either::Right(it) => { @@ -826,59 +809,11 @@ impl FunctionBody { } } }; - self.walk_expr(&mut |expr| match expr { - ast::Expr::PathExpr(path_expr) => { - if let Some(local) = path_expr - .path() - .and_then(|it| it.as_single_name_ref()) - .and_then(|name_ref| local_from_name_ref(sema, name_ref)) - { - add_name_if_local(local); - } - } - ast::Expr::ClosureExpr(closure_expr) => { - if let Some(body) = closure_expr.body() { - body.syntax() - .descendants() - .filter_map(ast::NameRef::cast) - .filter_map(|name_ref| local_from_name_ref(sema, name_ref)) - .for_each(&mut add_name_if_local); - } - } - ast::Expr::MacroExpr(expr) => { - if let Some(tt) = expr.macro_call().and_then(|call| call.token_tree()) { - tt.syntax() - .descendants_with_tokens() - .filter_map(SyntaxElement::into_token) - .filter(|it| { - matches!(it.kind(), SyntaxKind::STRING | SyntaxKind::IDENT | T![self]) - }) - .for_each(|t| { - if ast::String::can_cast(t.kind()) { - if let Some(parts) = - ast::String::cast(t).and_then(|s| sema.as_format_args_parts(&s)) - { - parts - .into_iter() - .filter_map(|(_, value)| value.and_then(|it| it.left())) - .filter_map(|path| match path { - PathResolution::Local(local) => Some(local), - _ => None, - }) - .for_each(&mut add_name_if_local); - } - } else { - sema.descend_into_macros_exact(t) - .into_iter() - .filter_map(|t| t.parent().and_then(ast::NameRef::cast)) - .filter_map(|name_ref| local_from_name_ref(sema, name_ref)) - .for_each(&mut add_name_if_local); - } - }); - } - } - _ => (), - }); + + if let Some(locals) = sema.locals_used(element, text_range) { + locals.into_iter().for_each(&mut add_name_if_local); + } + (res, self_param) } @@ -6294,4 +6229,150 @@ fn $0fun_name(v: i32) { }"#, ); } + + #[test] + fn no_parameter_for_variable_used_only_let_else() { + check_assist( + extract_function, + r#" +fn foo() -> u32 { + let x = 5; + + $0let Some(y) = Some(1) else { + return x * 2; + };$0 + + y +}"#, + r#" +fn foo() -> u32 { + let x = 5; + + let y = match fun_name(x) { + Ok(value) => value, + Err(value) => return value, + }; + + y +} + +fn $0fun_name(x: u32) -> Result<_, u32> { + let Some(y) = Some(1) else { + return Err(x * 2); + }; + Ok(y) +}"#, + ); + } + + #[test] + fn deeply_nested_macros() { + check_assist( + extract_function, + r#" +macro_rules! m { + ($val:ident) => { $val }; +} + +macro_rules! n { + ($v1:ident, $v2:ident) => { m!($v1) + $v2 }; +} + +macro_rules! o { + ($v1:ident, $v2:ident, $v3:ident) => { n!($v1, $v2) + $v3 }; +} + +fn foo() -> u32 { + let v1 = 1; + let v2 = 2; + $0let v3 = 3; + o!(v1, v2, v3)$0 +}"#, + r#" +macro_rules! m { + ($val:ident) => { $val }; +} + +macro_rules! n { + ($v1:ident, $v2:ident) => { m!($v1) + $v2 }; +} + +macro_rules! o { + ($v1:ident, $v2:ident, $v3:ident) => { n!($v1, $v2) + $v3 }; +} + +fn foo() -> u32 { + let v1 = 1; + let v2 = 2; + fun_name(v1, v2) +} + +fn $0fun_name(v1: u32, v2: u32) -> u32 { + let v3 = 3; + o!(v1, v2, v3) +}"#, + ); + } + + #[test] + fn pattern_assignment() { + check_assist( + extract_function, + r#" +struct Point {x: u32, y: u32}; + +fn point() -> Point { + Point { x: 45, y: 50 }; +} + +fn foo() { + let mut a = 1; + let mut b = 3; + $0Point { x: a, y: b } = point();$0 +} +"#, + r#" +struct Point {x: u32, y: u32}; + +fn point() -> Point { + Point { x: 45, y: 50 }; +} + +fn foo() { + let mut a = 1; + let mut b = 3; + fun_name(a, b); +} + +fn $0fun_name(mut a: u32, mut b: u32) { + Point { x: a, y: b } = point(); +} +"#, + ); + } + + #[test] + fn tuple_assignment() { + check_assist( + extract_function, + r#" +fn foo() { + let mut a = 3; + let mut b = 4; + $0(a, b) = (b, a);$0 +} +"#, + r#" +fn foo() { + let mut a = 3; + let mut b = 4; + fun_name(a, b); +} + +fn $0fun_name(mut a: i32, mut b: i32) { + (a, b) = (b, a); +} +"#, + ); + } } diff --git a/crates/ide-db/src/search.rs b/crates/ide-db/src/search.rs index a48438cfa8..1d865892a2 100644 --- a/crates/ide-db/src/search.rs +++ b/crates/ide-db/src/search.rs @@ -1345,7 +1345,7 @@ impl ReferenceCategory { // If the variable or field ends on the LHS's end then it's a Write // (covers fields and locals). FIXME: This is not terribly accurate. if let Some(lhs) = expr.lhs() - && lhs.syntax().text_range().end() == r.syntax().text_range().end() { + && lhs.syntax().text_range().contains_range(r.syntax().text_range()) { return Some(ReferenceCategory::WRITE) } } |