Skip to content

Commit e92dc3f

Browse files
committed
Have inline_local_variable use precedence calculation for parentheses
1 parent 2c4ef38 commit e92dc3f

File tree

5 files changed

+44
-67
lines changed

5 files changed

+44
-67
lines changed

crates/ide-assists/src/handlers/inline_const_as_literal.rs

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -39,25 +39,10 @@ pub(crate) fn inline_const_as_literal(acc: &mut Assists, ctx: &AssistContext<'_>
3939
// FIXME: Add support to handle type aliases for builtin scalar types.
4040
validate_type_recursively(ctx, Some(&konst_ty), false, fuel)?;
4141

42-
let expr = konst.value(ctx.sema.db)?;
43-
44-
let value = match expr {
45-
ast::Expr::BlockExpr(_)
46-
| ast::Expr::Literal(_)
47-
| ast::Expr::RefExpr(_)
48-
| ast::Expr::ArrayExpr(_)
49-
| ast::Expr::TupleExpr(_)
50-
| ast::Expr::IfExpr(_)
51-
| ast::Expr::ParenExpr(_)
52-
| ast::Expr::MatchExpr(_)
53-
| ast::Expr::MacroExpr(_)
54-
| ast::Expr::BinExpr(_)
55-
| ast::Expr::CallExpr(_) => konst
56-
.eval(ctx.sema.db)
57-
.ok()?
58-
.render(ctx.sema.db, konst.krate(ctx.sema.db).edition(ctx.sema.db)),
59-
_ => return None,
60-
};
42+
let value = konst
43+
.eval(ctx.sema.db)
44+
.ok()?
45+
.render(ctx.sema.db, konst.krate(ctx.sema.db).edition(ctx.sema.db));
6146

6247
let id = AssistId("inline_const_as_literal", AssistKind::RefactorInline);
6348

crates/ide-assists/src/handlers/inline_local_variable.rs

Lines changed: 23 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,11 @@ use ide_db::{
55
EditionedFileId, RootDatabase,
66
};
77
use syntax::{
8-
ast::{self, AstNode, AstToken, HasName},
8+
ast::{
9+
self,
10+
prec::{precedence, ExprPrecedence},
11+
AstNode, AstToken, HasName,
12+
},
913
SyntaxElement, TextRange,
1014
};
1115

@@ -79,33 +83,16 @@ pub(crate) fn inline_local_variable(acc: &mut Assists, ctx: &AssistContext<'_>)
7983
Some(u) => u,
8084
None => return Some((range, name_ref, false)),
8185
};
82-
let initializer = matches!(
83-
initializer_expr,
84-
ast::Expr::CallExpr(_)
85-
| ast::Expr::IndexExpr(_)
86-
| ast::Expr::MethodCallExpr(_)
87-
| ast::Expr::FieldExpr(_)
88-
| ast::Expr::TryExpr(_)
89-
| ast::Expr::Literal(_)
90-
| ast::Expr::TupleExpr(_)
91-
| ast::Expr::ArrayExpr(_)
92-
| ast::Expr::ParenExpr(_)
93-
| ast::Expr::PathExpr(_)
94-
| ast::Expr::BlockExpr(_),
95-
);
96-
let parent = matches!(
97-
usage_parent,
98-
ast::Expr::TupleExpr(_)
99-
| ast::Expr::ArrayExpr(_)
100-
| ast::Expr::ParenExpr(_)
101-
| ast::Expr::ForExpr(_)
102-
| ast::Expr::WhileExpr(_)
103-
| ast::Expr::BreakExpr(_)
104-
| ast::Expr::ReturnExpr(_)
105-
| ast::Expr::MatchExpr(_)
106-
| ast::Expr::BlockExpr(_)
107-
);
108-
Some((range, name_ref, !(initializer || parent)))
86+
let initializer = precedence(&initializer_expr);
87+
let parent = precedence(&usage_parent);
88+
Some((
89+
range,
90+
name_ref,
91+
parent != ExprPrecedence::Unambiguous
92+
&& initializer < parent
93+
// initializer == ExprPrecedence::Prefix -> parent != ExprPrecedence::Jump
94+
&& (initializer != ExprPrecedence::Prefix || parent != ExprPrecedence::Jump),
95+
))
10996
})
11097
.collect::<Option<Vec<_>>>()?;
11198

@@ -281,11 +268,11 @@ fn foo() {
281268
r"
282269
fn bar(a: usize) {}
283270
fn foo() {
284-
(1 + 1) + 1;
285-
if (1 + 1) > 10 {
271+
1 + 1 + 1;
272+
if 1 + 1 > 10 {
286273
}
287274
288-
while (1 + 1) > 10 {
275+
while 1 + 1 > 10 {
289276
290277
}
291278
let b = (1 + 1) * 10;
@@ -350,14 +337,14 @@ fn foo() {
350337
r"
351338
fn bar(a: usize) -> usize { a }
352339
fn foo() {
353-
(bar(1) as u64) + 1;
354-
if (bar(1) as u64) > 10 {
340+
bar(1) as u64 + 1;
341+
if bar(1) as u64 > 10 {
355342
}
356343
357-
while (bar(1) as u64) > 10 {
344+
while bar(1) as u64 > 10 {
358345
359346
}
360-
let b = (bar(1) as u64) * 10;
347+
let b = bar(1) as u64 * 10;
361348
bar(bar(1) as u64);
362349
}",
363350
);
@@ -574,7 +561,7 @@ fn foo() {
574561
r"
575562
fn foo() {
576563
let bar = 10;
577-
let b = (&bar) * 10;
564+
let b = &bar * 10;
578565
}",
579566
);
580567
}

crates/ide/src/inlay_hints/adjustment.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -259,10 +259,8 @@ fn needs_parens_for_adjustment_hints(expr: &ast::Expr, postfix: bool) -> (bool,
259259
let prec = expr.precedence();
260260
if postfix {
261261
// postfix ops have higher precedence than any other operator, so we need to wrap
262-
// any inner expression that is below (except for jumps if they don't have a value)
263-
let needs_inner_parens = prec < ExprPrecedence::Postfix && {
264-
prec != ExprPrecedence::Jump || !expr.is_ret_like_with_no_value()
265-
};
262+
// any inner expression that is below
263+
let needs_inner_parens = prec < ExprPrecedence::Postfix;
266264
// given we are the higher precedence, no parent expression will have stronger requirements
267265
let needs_outer_parens = false;
268266
(needs_outer_parens, needs_inner_parens)
@@ -275,13 +273,13 @@ fn needs_parens_for_adjustment_hints(expr: &ast::Expr, postfix: bool) -> (bool,
275273
.and_then(ast::Expr::cast)
276274
// if we are already wrapped, great, no need to wrap again
277275
.filter(|it| !matches!(it, ast::Expr::ParenExpr(_)))
278-
.map(|it| it.precedence());
276+
.map(|it| it.precedence())
277+
.filter(|&prec| prec != ExprPrecedence::Unambiguous);
279278

280279
// if we have no parent, we don't need outer parens to disambiguate
281280
// otherwise anything with higher precedence than what we insert needs to wrap us
282-
// that means only postfix ops
283281
let needs_outer_parens =
284-
parent.is_some_and(|parent_prec| parent_prec == ExprPrecedence::Postfix);
282+
parent.is_some_and(|parent_prec| parent_prec > ExprPrecedence::Prefix);
285283
(needs_outer_parens, needs_inner_parens)
286284
}
287285
}

crates/syntax/src/ast/prec.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::{
77

88
#[derive(Debug, Clone, Copy, PartialEq, PartialOrd)]
99
pub enum ExprPrecedence {
10-
// return, break, yield, closures
10+
// return val, break val, yield val, closures
1111
Jump,
1212
// = += -= *= /= %= &= |= ^= <<= >>=
1313
Assign,
@@ -58,12 +58,18 @@ pub fn precedence(expr: &ast::Expr) -> ExprPrecedence {
5858
Some(_) => ExprPrecedence::Unambiguous,
5959
},
6060

61+
Expr::BreakExpr(e) if e.expr().is_some() => ExprPrecedence::Jump,
62+
Expr::BecomeExpr(e) if e.expr().is_some() => ExprPrecedence::Jump,
63+
Expr::ReturnExpr(e) if e.expr().is_some() => ExprPrecedence::Jump,
64+
Expr::YeetExpr(e) if e.expr().is_some() => ExprPrecedence::Jump,
65+
Expr::YieldExpr(e) if e.expr().is_some() => ExprPrecedence::Jump,
66+
6167
Expr::BreakExpr(_)
6268
| Expr::BecomeExpr(_)
63-
| Expr::ContinueExpr(_)
6469
| Expr::ReturnExpr(_)
6570
| Expr::YeetExpr(_)
66-
| Expr::YieldExpr(_) => ExprPrecedence::Jump,
71+
| Expr::YieldExpr(_)
72+
| Expr::ContinueExpr(_) => ExprPrecedence::Unambiguous,
6773

6874
Expr::RangeExpr(..) => ExprPrecedence::Range,
6975

@@ -387,6 +393,7 @@ impl Expr {
387393
BreakExpr(e) => e.expr().is_none(),
388394
ContinueExpr(_) => true,
389395
YieldExpr(e) => e.expr().is_none(),
396+
BecomeExpr(e) => e.expr().is_none(),
390397
_ => false,
391398
}
392399
}

crates/syntax/src/ast/token_ext.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ impl ast::ByteString {
269269
}
270270
(Ok(c), true) => {
271271
buf.reserve_exact(text.len());
272-
buf.extend_from_slice(text[..prev_end].as_bytes());
272+
buf.extend_from_slice(&text.as_bytes()[..prev_end]);
273273
buf.push(c as u8);
274274
}
275275
(Err(e), _) => has_error = Some(e),
@@ -333,7 +333,7 @@ impl ast::CString {
333333
}
334334
(Ok(u), true) => {
335335
buf.reserve_exact(text.len());
336-
buf.extend(text[..prev_end].as_bytes());
336+
buf.extend(&text.as_bytes()[..prev_end]);
337337
extend_unit(&mut buf, u);
338338
}
339339
(Err(e), _) => has_error = Some(e),

0 commit comments

Comments
 (0)