Skip to content

Commit 432bb22

Browse files
VeykrilVeykril
committed
Simplify inline_local_variable assist
1 parent eb513c2 commit 432bb22

File tree

2 files changed

+93
-112
lines changed

2 files changed

+93
-112
lines changed

‎crates/ide_assists/src/assist_context.rs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,6 @@ impl<'a> AssistContext<'a> {
100100
pub(crate) fn covering_element(&self) -> SyntaxElement {
101101
self.source_file.syntax().covering_element(self.frange.range)
102102
}
103-
// FIXME: remove
104-
pub(crate) fn covering_node_for_range(&self, range: TextRange) -> SyntaxElement {
105-
self.source_file.syntax().covering_element(range)
106-
}
107103
}
108104

109105
pub(crate) struct Assists {

‎crates/ide_assists/src/handlers/inline_local_variable.rs‎

Lines changed: 93 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
use either::Either;
22
use hir::PathResolution;
3-
use ide_db::{base_db::FileId, defs::Definition, search::FileReference};
4-
use rustc_hash::FxHashMap;
3+
use ide_db::{
4+
defs::Definition,
5+
search::{FileReference, UsageSearchResult},
6+
};
57
use syntax::{
68
ast::{self, AstNode, AstToken, NameOwner},
7-
TextRange,
9+
SyntaxElement, TextRange,
810
};
911

1012
use crate::{
@@ -14,7 +16,7 @@ use crate::{
1416

1517
// Assist: inline_local_variable
1618
//
17-
// Inlines local variable.
19+
// Inlines a local variable.
1820
//
1921
// ```
2022
// fn main() {
@@ -29,76 +31,77 @@ use crate::{
2931
// }
3032
// ```
3133
pub(crate) fn inline_local_variable(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
32-
let InlineData { let_stmt, delete_let, replace_usages, target } =
34+
let InlineData { let_stmt, delete_let, references, target } =
3335
inline_let(ctx).or_else(|| inline_usage(ctx))?;
3436
let initializer_expr = let_stmt.initializer()?;
3537

36-
let delete_range = if delete_let {
38+
let delete_range = delete_let.then(|| {
3739
if let Some(whitespace) = let_stmt
3840
.syntax()
3941
.next_sibling_or_token()
40-
.and_then(|it| ast::Whitespace::cast(it.as_token()?.clone()))
42+
.and_then(SyntaxElement::into_token)
43+
.and_then(ast::Whitespace::cast)
4144
{
42-
Some(TextRange::new(
45+
TextRange::new(
4346
let_stmt.syntax().text_range().start(),
4447
whitespace.syntax().text_range().end(),
45-
))
48+
)
4649
} else {
47-
Some(let_stmt.syntax().text_range())
50+
let_stmt.syntax().text_range()
4851
}
49-
} else {
50-
None
51-
};
52+
});
5253

53-
let wrap_in_parens = replace_usages
54-
.iter()
55-
.map(|(&file_id, refs)| {
56-
refs.iter()
57-
.map(|&FileReference { range, .. }| {
58-
let usage_node = ctx
59-
.covering_node_for_range(range)
60-
.ancestors()
61-
.find_map(ast::PathExpr::cast)?;
62-
let usage_parent_option =
63-
usage_node.syntax().parent().and_then(ast::Expr::cast);
64-
let usage_parent = match usage_parent_option {
65-
Some(u) => u,
66-
None => return Some(false),
67-
};
68-
let initializer = matches!(
69-
initializer_expr,
70-
ast::Expr::CallExpr(_)
71-
| ast::Expr::IndexExpr(_)
72-
| ast::Expr::MethodCallExpr(_)
73-
| ast::Expr::FieldExpr(_)
74-
| ast::Expr::TryExpr(_)
75-
| ast::Expr::RefExpr(_)
76-
| ast::Expr::Literal(_)
77-
| ast::Expr::TupleExpr(_)
78-
| ast::Expr::ArrayExpr(_)
79-
| ast::Expr::ParenExpr(_)
80-
| ast::Expr::PathExpr(_)
81-
| ast::Expr::BlockExpr(_)
82-
| ast::Expr::EffectExpr(_),
83-
);
84-
let parent = matches!(
85-
usage_parent,
86-
ast::Expr::CallExpr(_)
87-
| ast::Expr::TupleExpr(_)
88-
| ast::Expr::ArrayExpr(_)
89-
| ast::Expr::ParenExpr(_)
90-
| ast::Expr::ForExpr(_)
91-
| ast::Expr::WhileExpr(_)
92-
| ast::Expr::BreakExpr(_)
93-
| ast::Expr::ReturnExpr(_)
94-
| ast::Expr::MatchExpr(_)
95-
);
96-
Some(!(initializer || parent))
97-
})
98-
.collect::<Option<_>>()
99-
.map(|b| (file_id, b))
54+
let wrap_in_parens = references
55+
.into_iter()
56+
.filter_map(|FileReference { range, name, .. }| match name {
57+
ast::NameLike::NameRef(name) => Some((range, name)),
58+
_ => None,
59+
})
60+
.map(|(range, name_ref)| {
61+
if range != name_ref.syntax().text_range() {
62+
// Do not rename inside macros
63+
// FIXME: This feels like a bad heuristic for macros
64+
return None;
65+
}
66+
let usage_node =
67+
name_ref.syntax().ancestors().find(|it| ast::PathExpr::can_cast(it.kind()));
68+
let usage_parent_option =
69+
usage_node.and_then(|it| it.parent()).and_then(ast::Expr::cast);
70+
let usage_parent = match usage_parent_option {
71+
Some(u) => u,
72+
None => return Some((range, name_ref, false)),
73+
};
74+
let initializer = matches!(
75+
initializer_expr,
76+
ast::Expr::CallExpr(_)
77+
| ast::Expr::IndexExpr(_)
78+
| ast::Expr::MethodCallExpr(_)
79+
| ast::Expr::FieldExpr(_)
80+
| ast::Expr::TryExpr(_)
81+
| ast::Expr::RefExpr(_)
82+
| ast::Expr::Literal(_)
83+
| ast::Expr::TupleExpr(_)
84+
| ast::Expr::ArrayExpr(_)
85+
| ast::Expr::ParenExpr(_)
86+
| ast::Expr::PathExpr(_)
87+
| ast::Expr::BlockExpr(_)
88+
| ast::Expr::EffectExpr(_),
89+
);
90+
let parent = matches!(
91+
usage_parent,
92+
ast::Expr::CallExpr(_)
93+
| ast::Expr::TupleExpr(_)
94+
| ast::Expr::ArrayExpr(_)
95+
| ast::Expr::ParenExpr(_)
96+
| ast::Expr::ForExpr(_)
97+
| ast::Expr::WhileExpr(_)
98+
| ast::Expr::BreakExpr(_)
99+
| ast::Expr::ReturnExpr(_)
100+
| ast::Expr::MatchExpr(_)
101+
);
102+
Some((range, name_ref, !(initializer || parent)))
100103
})
101-
.collect::<Option<FxHashMap<_, Vec<_>>>>()?;
104+
.collect::<Option<Vec<_>>>()?;
102105

103106
let init_str = initializer_expr.syntax().text().to_string();
104107
let init_in_paren = format!("({})", &init_str);
@@ -116,19 +119,13 @@ pub(crate) fn inline_local_variable(acc: &mut Assists, ctx: &AssistContext) -> O
116119
if let Some(range) = delete_range {
117120
builder.delete(range);
118121
}
119-
for (file_id, references) in replace_usages {
120-
for (&should_wrap, reference) in wrap_in_parens[&file_id].iter().zip(references) {
121-
let replacement =
122-
if should_wrap { init_in_paren.clone() } else { init_str.clone() };
123-
match reference.name.as_name_ref() {
124-
Some(name_ref)
125-
if ast::RecordExprField::for_field_name(name_ref).is_some() =>
126-
{
127-
cov_mark::hit!(inline_field_shorthand);
128-
builder.insert(reference.range.end(), format!(": {}", replacement));
129-
}
130-
_ => builder.replace(reference.range, replacement),
131-
}
122+
for (range, name, should_wrap) in wrap_in_parens {
123+
let replacement = if should_wrap { &init_in_paren } else { &init_str };
124+
if ast::RecordExprField::for_field_name(&name).is_some() {
125+
cov_mark::hit!(inline_field_shorthand);
126+
builder.insert(range.end(), format!(": {}", replacement));
127+
} else {
128+
builder.replace(range, replacement.clone())
132129
}
133130
}
134131
},
@@ -139,7 +136,7 @@ struct InlineData {
139136
let_stmt: ast::LetStmt,
140137
delete_let: bool,
141138
target: ast::NameOrNameRef,
142-
replace_usages: FxHashMap<FileId, Vec<FileReference>>,
139+
references: Vec<FileReference>,
143140
}
144141

145142
fn inline_let(ctx: &AssistContext) -> Option<InlineData> {
@@ -157,35 +154,32 @@ fn inline_let(ctx: &AssistContext) -> Option<InlineData> {
157154
return None;
158155
}
159156

160-
let def = ctx.sema.to_def(&bind_pat)?;
161-
let def = Definition::Local(def);
162-
let usages = def.usages(&ctx.sema).all();
163-
if usages.is_empty() {
164-
cov_mark::hit!(test_not_applicable_if_variable_unused);
165-
return None;
166-
};
167-
168-
Some(InlineData {
169-
let_stmt,
170-
delete_let: true,
171-
target: ast::NameOrNameRef::Name(bind_pat.name()?),
172-
replace_usages: usages.references,
173-
})
157+
let local = ctx.sema.to_def(&bind_pat)?;
158+
let UsageSearchResult { mut references } = Definition::Local(local).usages(&ctx.sema).all();
159+
match references.remove(&ctx.frange.file_id) {
160+
Some(references) => Some(InlineData {
161+
let_stmt,
162+
delete_let: true,
163+
target: ast::NameOrNameRef::Name(bind_pat.name()?),
164+
references,
165+
}),
166+
None => {
167+
cov_mark::hit!(test_not_applicable_if_variable_unused);
168+
None
169+
}
170+
}
174171
}
175172

176173
fn inline_usage(ctx: &AssistContext) -> Option<InlineData> {
177174
let path_expr = ctx.find_node_at_offset::<ast::PathExpr>()?;
178175
let path = path_expr.path()?;
179-
let name = match path.as_single_segment()?.kind()? {
180-
ast::PathSegmentKind::Name(name) => name,
181-
_ => return None,
182-
};
176+
let name = path.as_single_name_ref()?;
183177

184178
let local = match ctx.sema.resolve_path(&path)? {
185179
PathResolution::Local(local) => local,
186180
_ => return None,
187181
};
188-
if local.is_mut(ctx.sema.db) {
182+
if local.is_mut(ctx.db()) {
189183
cov_mark::hit!(test_not_inline_mut_variable_use);
190184
return None;
191185
}
@@ -197,21 +191,12 @@ fn inline_usage(ctx: &AssistContext) -> Option<InlineData> {
197191

198192
let let_stmt = ast::LetStmt::cast(bind_pat.syntax().parent()?)?;
199193

200-
let def = Definition::Local(local);
201-
let mut usages = def.usages(&ctx.sema).all();
202-
203-
let delete_let = usages.references.values().map(|v| v.len()).sum::<usize>() == 1;
204-
205-
for references in usages.references.values_mut() {
206-
references.retain(|reference| reference.name.as_name_ref() == Some(&name));
207-
}
194+
let UsageSearchResult { mut references } = Definition::Local(local).usages(&ctx.sema).all();
195+
let mut references = references.remove(&ctx.frange.file_id)?;
196+
let delete_let = references.len() == 1;
197+
references.retain(|fref| fref.name.as_name_ref() == Some(&name));
208198

209-
Some(InlineData {
210-
let_stmt,
211-
delete_let,
212-
target: ast::NameOrNameRef::NameRef(name),
213-
replace_usages: usages.references,
214-
})
199+
Some(InlineData { let_stmt, delete_let, target: ast::NameOrNameRef::NameRef(name), references })
215200
}
216201

217202
#[cfg(test)]

0 commit comments

Comments
 (0)